How to speed up this IP sweep, Port scanner python I am looking for a way to speed up my basic IP scanner/port scanner in Python 3.8.
My IP portion works, the port scan work, I just want to speed it up, in a format that someone learning python would understand.
#testing scanner.py
import socket
import subprocess
import sys
from datetime import datetime
def scan():
    # Ask for the IP address to scan, for this its a single IP
    remoteServer = input("Enter a Starting IP: ")
    remoteServerend = input("Enter the Ending IP: ")
    print ("Starting Scan at: ",remoteServer)
    print ("Ending Scan at: ",remoteServerend)
    # change DNS to IPmail
    #remoteServerIP = socket.gethostbyname(remoteServer)
    #print ("remoteServerIP is ",remoteServerIP)
    # Ask for starting port
    startport = int(input("What port number do you want to start with? "))
    # Ask for ending port
    endport = int(input("What is the ending port number? "))
    # Print 50 *'s to make a top border
    print ("-" * 50)
    # print wait here is the IP we are scanning
    print ("Please wait, scanning remote host", remoteServer)
    # print the bottom border
    print ("-" * 50)
    # Check what time the scan started
    t1 = datetime.now()
    # lets try the try statment for some error handeling and see if it can help?
    try:
    # trying our 1st port to our last port and replying if it is open, we don't care about closed ports
        for port in range(startport,endport):
            sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
            result = sock.connect_ex((remoteServer, port))
            if result == 0:
                print ("Port {}:     Open".format(port))
            sock.close()
    # you got tied of waiting and quit
    except KeyboardInterrupt:
        print( "You pressed Ctrl+C")
        sys.exit()
    # failed DNS lookup
    except socket.gaierror:
        print( 'Hostname could not be resolved. Exiting')
        sys.exit()
    # no route to server?
    except socket.error:
        print( "Couldn't connect to server")
        sys.exit()
    # get ending time
    t2 = datetime.now()
    # calculate how long did the script run?
    total =  t2 - t1
    # Printing the information to screen
    print( 'Scanning Completed in: ', total)

scan()

IP range

for this its a single IP

It is? Then why do you ask for a range? Your 'end' is not used, so just delete it.
Fail-safe scanning
Consider rearranging this:
try:
    for port in range(startport,endport):

to:
for port in range(startport,endport):
    try:

In other words, some ports may fail and others may succeed.


Unit Test class from iOS I am learning to write test cases in iOS using Swift. I was stuck at testing static functions from my Utilities class, so I tried to mock that class and then test the cases. Please let me know if its the correct approach or else tell me how to do that. Here is my code:

import XCTest
@testable import ConnectAndSell

class UtilitiesMock{
    func getBuildNumber() -> String{
        return Utilities.getAppBuildNumber()
    }
    func getVersionNumber() -> String{
        return Utilities.getAppVersion()
    }
}

class UtilitiesTests: XCTestCase {
    //system under test
    var sut:UtilitiesMock!

    override func setUp() {
        super.setUp()
        // Put setup code here. This method is called before the invocation of each test method in the class.
        sut = UtilitiesMock()
    }

    override func tearDown() {
        // Put teardown code here. This method is called after the invocation of each test method in the class.
        super.tearDown()
        sut = nil
    }

    func testBuildNumber() {
        let buildNumber = Bundle.main.infoDictionary?["CFBundleVersion"] as! String
        XCTAssertEqual(buildNumber, sut.getBuildNumber())

    }

    func testVersionNumber() {
        let versionNumber = Bundle.main.infoDictionary?["CFBundleShortVersionString"] as! String
        XCTAssertEqual(versionNumber, sut.getVersionNumber())
    }

}

EDIT: Adding Utilities class so that you guys can see what am I trying to test. It's just simple functions to verify the current build number and version number. I have taken these function just for example. My main aim is to know is it the correct approach to test static methods.

import Foundation

class Utilities {

    static func getAppVersion() -> String {
        if let version = Bundle.main.infoDictionary?["CFBundleShortVersionString"] as? String {
            log.debug("App verison: \(version)")
            return version
        }
        return ""
    }
    static func getAppBuildNumber() -> String {
        if let number = Bundle.main.infoDictionary?["CFBundleVersion"] as? String {
            log.debug("Build number: \(number)")
            return number
        }
        return ""
    }

}

Unit Tests should not have a dependency on the application bundle at runtime which could not be injected on the init phase or by a method arguments

The given Utility class has a static methods which act like a shortcuts to longer syntax calls. This methods calls directly methods on Bundle class object which is cannot be mocked. The solution is to move Bundle -> infoDictionary outside of a method and set it as a static property. Now, in the app the dictionary can reference a dictionary from the app bundle and in test bundle will reference a mocked dictionary. The main purpose of a unit testing this methods will be to validate usage of given dictionary and keys to access demanded values.

Utility methods aka class methods are not dependency injection friendly and that makes them more complicated to test
UtilitiesMock is not a mock, it's more a proxy / facade / wrapper class

Solution:

protocol BundleInfoDictionaryShortcuts {
    static var bundleMainInfoDictionary: [String : Any]? { get set }
}

enum BundleInfoDictionary: String {
    case keyCFBundleShortVersionString = "CFBundleShortVersionString"
    case keyCFBundleVersion = "CFBundleVersion"
}

class Utilities: BundleInfoDictionaryShortcuts {

    static var bundleMainInfoDictionary: [String : Any]? = Bundle.main.infoDictionary

    static func getAppVersion() -> String {

        guard let version = bundleMainInfoDictionary?[BundleInfoDictionary.keyCFBundleShortVersionString.rawValue] as? String else {
            fatalError()
        }
        return version
    }

    static func getAppBuildNumber() -> String {
        guard let number = bundleMainInfoDictionary?[BundleInfoDictionary.keyCFBundleVersion.rawValue] as? String else {
            fatalError()
        }
        return number
    }
}

// File: BundleInfoDictionaryShortcutsTests.swift

class BundleInfoDictionaryShortcutsTests: XCTestCase {
        static let appVersion = "1.0.1"
        static let appBuildNumber = "120"

        override class func setUp() {
            super.setUp()
            Utilities.bundleMainInfoDictionary = [
                BundleInfoDictionary.keyCFBundleShortVersionString.rawValue: appVersion,
                BundleInfoDictionary.keyCFBundleVersion.rawValue: appBuildNumber
            ]
        }

        func testGetAppVersion() {
            XCTAssertEqual(Utilities.getAppVersion(), Self.appVersion)
        }
        func testGetAppBuildNumber() {
            XCTAssertEqual(Utilities.getAppBuildNumber(), Self.appBuildNumber)
        }
    }

// File: BundleInfoDictionaryShortcutsTests.swift

    class BundleInfoDictionaryTests: XCTestCase {
        func testKeys() {
            XCTAssertEqual(BundleInfoDictionary.keyCFBundleShortVersionString.rawValue,"CFBundleShortVersionString")
            XCTAssertEqual(BundleInfoDictionary.keyCFBundleVersion.rawValue,"CFBundleVersion")
        }
    }


Mutex and condition variable implementation using futex I have implemented mutex and conditional variables using the futex syscall. I believe that my implementation is correct, but would like it to be verified by someone else. Any suggestions for further improvements in the performance of the mutex and conditional variables would also be appreciated.

  #ifndef __SYNCHRONIZATION__
  #define __SYNCHRONIZATION__

  #include <unistd.h>
  #include <limits.h>
  #include <sys/syscall.h>
  #include <linux/futex.h>
  #include "types.h"
  #include "assembly.h"

  typedef UINT32 mutex;
  typedef struct condvar condvar;

  struct condvar    {
   mutex *m;
   int seq;
  };

  void mutex_init(mutex *m) {
   *m = 0;
  }

  void mutex_destroy(mutex *m)  {
   *m = 0;
  }

  void mutex_lock(mutex *m) {
   UINT32 c;
   if((c = __sync_val_compare_and_swap(m, 0, 1)) != 0)  {
    do  {
        if((c == 2) || __sync_val_compare_and_swap(m, 1, 2) != 0)
            syscall(SYS_futex, m, FUTEX_WAIT_PRIVATE, 2, NULL, NULL, 0);
    } while((c = __sync_val_compare_and_swap(m, 0, 2)) != 0);
   }
  }

  void mutex_unlock(mutex *m)   {
   if(__sync_fetch_and_sub(m, 1) != 1)  {
    *m = 0;
    syscall(SYS_futex, m, FUTEX_WAKE_PRIVATE, 1, NULL, NULL, 0);
   }
  }

  void cond_init(condvar *c, mutex *m)  {
   c->m = m;
   c->seq = 0;
  }

  void cond_destroy(condvar *c) {
   c->m = NULL;
   c->seq = 0;
  }

  void cond_signal(condvar *c)  {
   __sync_fetch_and_add(&(c->seq), 1);
   syscall(SYS_futex, &(c->seq), FUTEX_WAKE_PRIVATE, 1, NULL, NULL, 0);
  }

  void cond_broadcast(condvar *c)   {
   __sync_fetch_and_add(&(c->seq), 1);
   syscall(SYS_futex, &(c->seq), FUTEX_REQUEUE_PRIVATE, 1, (void *) INT_MAX, c->m, 0);
  }

  void cond_wait(condvar *c)    {
   UINT32 oldSeq = c->seq;
   mutex_unlock(c->m);
   syscall(SYS_futex, &(c->seq), FUTEX_WAIT_PRIVATE, oldSeq, NULL, NULL, 0);
   while (xchg32(c->m, 2))  {
    syscall(SYS_futex, c->m, FUTEX_WAIT_PRIVATE, 2, NULL, NULL, 0);
   }
  }

  #endif

I have no particular experience of Futex code, so take this with caution:

Looking at mutex_lock, I think some comments would have been useful.  As I
understand it, this is what it does:

Firstly let's define what your values 0, 1, 2 mean.  I understand them to
mean 'free', 'locked', 'locked and contested' respectively.  Some #defined
constants would have made that clear (assuming it is right).
The first compare-and-swap  tries to lock the mutex.

if((c = __sync_val_compare_and_swap(m, 0, 1)) != 0)  {

If this succeeds, the
function exits with the mutex locked (ie. its value is 1).  However if the
mutex was not unlocked (0) this fails and we enter the do...while loop.
At this point we know the mutex is (or was, to be precise - it might have
changed) either locked or contested.  If it was contested we want to make a
futex system call to wait for it to be unlocked.

    if((c == 2) || __sync_val_compare_and_swap(m, 1, 2) != 0)
        syscall(SYS_futex, m, FUTEX_WAIT_PRIVATE, 2, NULL, NULL, 0);

Hence we make the system
call either if we know the mutex was contested (c == 2) or if it was
only locked and we succeeded in setting it contested.  But because the mutex
might have changed since the first compare-and-swap, we have to do another
compare-and-swap to swap its locked 1 for a contested 2.  If the mutex
changed state and is no longer locked, this will fail and we don't make the
system call.  (Note that the system call itself has protection against the mutex changing state before it is entered, hence the '2' parameter which ensures that if *m is not 2 the kernel will not be entered.)
If we either didn't make the system call (because the mutex had changed) or
we made the call and it returned (again because the mutex state had changed)
we reach the while part of the loop.

} while((c = __sync_val_compare_and_swap(m, 0, 2)) != 0);

We can expect the mutex to be free
(0) at this point and that we can try to lock it.  But instead the code
tries to set it to contested (2).  If it succeeds the loop and function exit
with the mutex set to contested, not just locked.   This seems wrong - the
last compare-and-swap should be setting '1', not '2'.  If we replace the 2
with a 1, the loop exit condition is the same as the condition at the start
of the function and we can simplify to:

enum mutex_state {
    FREE = 0,
    LOCKED,
    CONTESTED
};

void mutex_lock(mutex *m)
{
    enum mutex_state c;
    while ((c = __sync_val_compare_and_swap(m, FREE, LOCKED)) != FREE)  // set locked
    {
        if ((c == CONTESTED) ||
            __sync_val_compare_and_swap(m, LOCKED, CONTESTED) != FREE)  // set contested
        {
            syscall(SYS_futex, m, FUTEX_WAIT_PRIVATE, CONTESTED, NULL, NULL, 0);
        }
    }
}


Performance in Hackerrank challenge "Sherlock and Queries" I translated the solution by the author to his challenge from C++ to Python.
It seems to be correct, but I get timeouts for the test cases 6, 9 and 12.

Here is the full challenge description:

  Watson gives to Sherlock an array: \$A_1, A_2, ..., A_N\$. He also gives to
  Sherlock two other arrays: \$B_1, B_2, ..., B_M\$ and \$C_1, C_2, ..., C_M\$.

  Then Watson asks Sherlock to perform the following program:

for i = 1 to M do
    for j = 1 to N do
        if j % B[i] == 0 then
            A[j] = A[j] * C[i]
        endif
    end do
end do

  Can you help Sherlock and tell him the resulting array A? You should
  print all the array elements modulo \$(10^9 + 7)\$.

  Input Format

  The first line contains two integer numbers \$N\$ and \$M\$. The next line
  contains \$N\$ integers, the elements of array A. The next two lines
  contains \$M\$ integers each, the elements of array B and C.

  Output Format

  Print \$N\$ integers, the elements of array A after performing the program
  modulo \$(10^9 + 7)\$.

  Constraints

  \$1 ≤ N, M ≤ 10^5\$
   \$1 ≤ B[i] ≤ N\$
   \$1 ≤ A[i], C[i] ≤ 10^5\$

  Sample Input

4 3
1 2 3 4
1 2 3
13 29 71

  Sample Output

13 754 2769 1508

And here is my code:

import fileinput
import collections

lines = [[int(x) for x in line.split()] for line in fileinput.input()]
[n, m], a, b, c = lines

def one():
    return 1

factors = collections.defaultdict(one)
for i in range(0, m):
    factors[b[i]] *= c[i] % 1000000007

for i, factor in factors.iteritems():
    for j in range(1, n/i + 1):
        idx = j*i-1
        a[idx] *= factor
        a[idx] %= 1000000007;

print ' '.join(map(str, a))

Any idea on how I could increase the performance of my solution?

This loop can be tuned in some ways:

for i, factor in factors.iteritems():
    for j in range(1, n/i + 1):
        idx = j*i-1
        a[idx] *= factor
        a[idx] %= 1000000007;

Use the third argument of range (step size).
Use xrange in Python 2.
The in-place operators may actually slow you down vs. a single expression.

Modified:

for i, factor in factors.iteritems():
    for idx in xrange(i-1, n, i):
        a[idx] = a[idx] * factor % 1000000007

The fileinput module offers convenience for reading multiple files, but adds some overhead. If you are reading from standard input, use for line in sys.stdin directly.

As noted in comments, the above changes do not make much difference. However, I now found something that actually matters. There is a subtle bug in the translation from C++ to Python here:

factors[b[i]] *= c[i] % 1000000007

Written like this the modulo operator applies to c[i] while it is supposed to apply to the result of the multiplication. The programs still gives the correct result because you apply the modulo in the later loop, but in the meanwhile Python has to multiply much larger numbers. I get a 100-fold speed increase by changing the line to

factors[b[i]] = factors[b[i]] * c[i] % 1000000007


Unit-testing account transaction code I'm playing with unit-testing in Ruby. There is a situation that I don't know if is good enough or if I am doing the wrong abstraction.

I have two classes: Transaction and Account. An Account could have 0..* Transactions. The only constraint is shall not have duplicated transactions for an Account.

it "add a duplicated transaction will fails" do
  transaction = double(:transaction, id: '124-acd-345')
  expect(transaction).to receive(:==).and_return true

  subject.add_transaction transaction
  subject.add_transaction transaction.clone

  expect(subject.transactions.count).to eq 1
end

Account#add_transaction piece:

def add_transaction(transaction)
  @transactions << transaction if transactions.select { |t| t == transaction }.empty?
end

Transaction#== piece:

def ==(other)
  other.id == self.id
end

The point is: Transaction#== is already unit tested in their test class. I know that transaction == works properly. So, my approach in Account#add_transaction was Test Behavior (with that expect(transaction) ...) and Test Value, checking if there is only one transaction into object.

IMO, I didn't like this solution, but I can't find anything better to do. Tips?

My first thought based on your comment (behavioral and state testing in same test) was that you should just make two different tests.

However, then I took a step back. The specification says that:

  add[ing] a duplicated transaction will fail

It doesn't say anything about how a duplicate is determined. Currently, you are using the naive implementation is to check if and of existing transactions equal the one being added. However, that is not the only way satisfy the requirement.

If there are a large number of transaction, it will be slow to iterate over each transaction. Instead, the transactions could be stored in a map that uses the id as a key. Now be can check if the transaction already exists by doing a single key lookup, \$O(1)\$ instead of \$O(n)\$.

The fact that Transaction#== is called is an implementation detail. Based on this, the behavioral assertion should be removed from the test. If there happens to be a different requirement, that was not presented here, that the duplication check must be done throw an equivalence check, then that should be a different test.


First non-repeating letter The problem:  find and return the first nonrepeating character in a string, if the string does not have a nonrepeating character return an empty string. For purposes of finding the nonrepeating character it's case insensitive so 't' and 'T' appearing in the string would imply that that 't' and 'T' are not candidates for a nonrepeating character.  When you return the character you must return it as its original case.

My Solution:

def non_repeat(_str):
    counts = {}
    for pos, letter in enumerate(_str.lower()):
        if letter not in counts:
                counts[letter] = (0, pos)
        incr_val = counts[letter][0] + 1
        counts[letter] = (incr_val, pos)
    for letter in _str.lower():
        if counts[letter][0] == 1:
            return _str[counts[letter][1]]
    return ''

How can I improve the readability of my solution?  In particular, I don't like:

counts[letter][0] because the 0 index is a bit ambiguous.
calculating the incremented value as another line, incr_val; it'd be nice to do the incrementing and updating of the dict in one line.

Anything else you could make more readable? I should add that I'm aware of collections.Counter; for this solution I'd like to avoid using it.

IMHO looping twice over the input string is not acceptable for multiple reasons

the string is of unknown size
you invalidate your code for generators

While this might not be necessary for your project you should learn to think like that. So a single pass algorithm should collect all necessary data to answer the question (e. g. the letter in original case).

import sys
assert sys.version_info >= (3, 6)

def non_repeat(s):
    repeated = set()
    candidates = dict()
    for original_case in s:
        lower_case = original_case.lower()
        if lower_case not in repeated:
            if lower_case not in candidates:
                candidates[lower_case] = original_case
            else:
                repeated.add(lower_case)
                del candidates[lower_case]

    if candidates:
        return next(iter(candidates.values()))
    else:
        return ''

This code makes use of the insertion order of a dict which is already implemented in 3.6 and guaranteed in 3.7.

Edit: generator example

Say you want to check a big file that does not fit into memory (for brevity I assume a line fits into memory). Yor write a little character generator and run your algorithm on the generator.

def char_gen(f):
    for line in f:
        for c in line.strip():
            yield c

with open('bigfile.txt') as f:
    print(non_repeat(char_gen(f)))

also you might use the algorithm on a generator expression

print(non_repeat(c for c in "aabbcd" if c != 'c'))


Creating a basic structure in tkinter I am making a basic program just for a local machine so I do not need to create anything fancy but I am finding that my code is going to become quite lengthy. Any tips on how to improve it so I am not repeating code? I have additional code to add in respect to client data etc but I am primarily concentrating on the foundation first.

from tkinter import *
from PIL import ImageTk, Image

root = Tk()
root.withdraw()

def login(event):
    if ent1.get() == 'admin' and ent2.get() == 'password':
        root.iconify()
        top.destroy()

def client_data(event):
    root.withdraw()
    top = Toplevel()
    top.title('Client Data')
    top.geometry('800x500')
    top.configure(background='grey')
    client1 = Message(top, text='Random skeleton', bg='grey', width=350)
    client1.pack()
    x = Button(top, text='Close', bg='red', command=top.destroy)
    root.iconify()
    x.pack()

image1 = ImageTk.PhotoImage(Image.open('ileye.png'))
top = Toplevel()
top.title('Login')
top.configure(background='grey')
photo = Label(top, image=image1)
photo.pack()
user = Label(top, text='User name', bg='grey')
user.pack()
ent1 = Entry(top, bg='grey')
ent1.pack()
pwd = Label(top, text='Password', bg='grey')
pwd.pack()
ent2 = Entry(top, show='*', bg='grey')
ent2.bind('<Return>', login)
ent2.pack()
ex = Button(top, text='EXIT', command=root.quit)
ex.pack()
check = Checkbutton(top, text='Remember me', bg='grey')
check.pack()

root.title('Main Screen')
root.attributes('-zoomed', True)
menu_drop = Menu(root)
root.config(menu=menu_drop)
file = Menu(menu_drop)
menu_drop.add_cascade(label='Options', menu=file)
file.add_command(label='New', command=root.quit)  ## New Function
client = Button(root, text='Client list')
file.add_separator()
file.add_command(label='Close App', command=root.quit)
client.bind('<Button-1>', client_data)
client.pack()
exi = Button(root, text='EXIT', command=root.quit)
exi.pack()

root.mainloop()

Any tips on how to improve it so I am not repeating code?

There isn't really a lot of repeated code, here. I think you're fine on that front.

Avoid globals

Starting with image1 onward, you have lots of globals. Try to move them into a class, for a few reasons: re-entrance (you could spawn multiple identical windows), general code organization, namespace cleanliness, etc.

Credential management

if ent1.get() == 'admin' and ent2.get() == 'password':

I know that you are

  making a basic program just for a local machine

but this is never a good habit to get into. Hard-coding passwords is not secure, and it isn't much more difficult to do the "right" thing. Import a library to be able to securely handle authentication and encrypted credential storage.


Schedule Global Task by using static System.Timers I'm using System.Timers to schedule tasks. I find this working, but I'm not quite sure if it's ok to do it this way.

using System.Web;
using System.Web.Security;
using System.Web.SessionState;
using System.Timers;
using System.IO;
using System.Threading;

namespace WebApplication2
{
    public class Global : System.Web.HttpApplication
    {
        public static System.Timers.Timer timer1 = new System.Timers.Timer();

        protected void Application_Start(object sender, EventArgs e)
        {
            timer1.Interval = 100;
            timer1.Elapsed += timer1_Elapsed;
        }

        void timer1_Elapsed(object sender, ElapsedEventArgs e)
        {
            timer1.Stop();
            // Example: Execute Task after 6 hours
            Thread.Sleep(6 * 60 * 60 * 1000);
            // Do something. Example:
            File.AppendAllText("C:\\log.txt", DateTime.Now + "\r\n");
        }
    }
}

By using this method, I can start the task anywhere in the application by calling this:

WebApplication2.Global.timer1.Start();

What do you think?

There's no reason to expose timer1 as a public member, I would just have a static method DoFoo() that calls timer1.Start().

There are a few issues with the way you implement you thread passing.  With the short interval and the immediate stop it seems that you are trying to just pass the job to another thread.  This can fail in a variety of ways:

Multiple calls to DoFoo() in quick succession will result in just one invocation of timer1_Elapsed
Under heavy load or immediately after startup, it may take more than 100 ms to reach time1.Stop().  This way one call to DoFoo() can result in multiple invocations of timer1_Elapsed.
Sleeping in a thread is the last thing you want to do.  You are occupying this thread for 6 hours for no good reason.  Further, your application may not be running in 6 hours.

So how do you fix it?  If you really need to sleep for 6 hours and the task is important, you need to save the job to an external store (like a database) in case your process terminates.  If it's just about scheduling work on another thread, that's what the ThreadPool is for.  If you do need the delay but it's not critical work, you can create a Timer with the long delay.  Make a new one for each call, and make sure you store a reference so it doesn't get GC'ed before it fires.


Downset calculation code I'm looking to port a Python script I have over to Swift, partly for some performance benefits, and partly to dip my toe into a new language.

My program computes what is called the downset of a permutation of \$[n] := \{1,2,\cdots,n\}\$. For me, a permutation of \$[n]\$ is a tuple containing each integer precisely once. Deleting the entry \$\pi(i)\$ from a permutation \$\pi = \pi(1) \pi(2) \cdots \pi(n)\$ produces a permutation \$\sigma = \pi(1) \pi(2) \cdots \pi(i-1) \pi(i+1) \cdots \pi(n)\$ with each entry larger than \$\pi(i)\$ lowered by \$1\$. The downset of a permutation is the set of all permutations which can be obtained by a sequence of such deletions. I'm calculating it so that the downset is sorted by rank: I'll return an array L such that L[m] is the set of permutations in the downset of length m.

Here's the working Python3 code I have:

def one_deletion(sigma,i):
    """ Deletes by value, not by index.
    """
    smaller_perm = tuple(val if val < i else val-1 for val in sigma if val != i)
    return smaller_perm

def downset(pi):
    L = []
    new_perms = {pi}
    L.append(new_perms)
    for _ in range(len(pi)):
        old_perms = new_perms
        new_perms = set()
        for sigma in old_perms:
            new_perms.update(one_deletion(sigma,i) for i in sigma)
        L.append(new_perms)
    return L[::-1]

if __name__ == "__main__":
    u3 = tuple([2,5,3,1,4])
    print(downset(u3))

In trying to port this to Swift code, I've learned a lot! Here's my code:

struct Permutation: Hashable {
    let perm : Array<Int>
    var hashValue: Int {
        return perm.description.hashValue
    }
    static func == (lhs: Permutation, rhs: Permutation) -> Bool {
        return lhs.perm == rhs.perm
    }
}

func one_deletion (sigma: Permutation, i: Int) -> Permutation {
    var A = [Int]() // Empty array of integers
    for val in sigma.perm {
        if val < i {
            A.append(val)
        } else if val > i {
            A.append(val-1)
        }
    }
    let B = Permutation(perm:A)
    return B
}

func downset (pi: Permutation) -> [Set<Permutation>] {
    var L = [Set<Permutation>]()
    var new_perms = Set([pi])
    L.append(new_perms)

    for _ in pi.perm {
        let old_perms = new_perms
        new_perms = []
        for sigma in old_perms {
            for val in sigma.perm {
                new_perms.insert(one_deletion(sigma:sigma, i:val))
            }
        }
        L.append(new_perms)
    }
    return L
}

let u3 = Permutation(perm:[2,5,3,1,4])
print(downset(pi: u3))

Now, some questions!

Is there a better way to make an array of integers hashable in Swift? Here, two Permutation objects are the same when they're literally the same tuple, so hashing by description works.
Are there any obvious things I'm doing wrong in Swift? In practice, this code is slower than my Python code.

Any general purpose comments are encouraged as well.

Naming

According to the Swift API Design Guidelines,

  Names of types and protocols are UpperCamelCase. Everything else is lowerCamelCase

therefore it should be func oneDeletion, oldPerms, newPerms etc.

Some variable names such as A, L are non-descriptive.

I would use a different name for the array inside struct Permutation,
for example

struct Permutation: Hashable {
    let sigma: [Int]
    // ...
}

so that perm can be used for a Permutation variable without
causing confusion. (This is of course opinion-based.)

Hashing

You don't need to implement

static func == (lhs: Permutation, rhs: Permutation) -> Bool

because that is automatically synthesized in Swift 4.1. A future version
of Swift will also synthesize the hashValue automatically, this
is currently discussed at

Let Optional, Dictionary and Array conditionally conform to Hashable in the Swift forum.

and implemented in https://github.com/apple/swift/pull/15382.

Until that is implemented, you have to provide a hashValue method.
I would probably not choose the description method because that
requires a conversion to strings. A good choice would be the
_combineHashValues function in Hashing.swift from the above pull
request, this is also what hash.hpp from the Boost library uses:

// From https://github.com/lorentey/swift/blob/e03cb1340ad89013bbaef1c4f802a72de06e9e1a/stdlib/public/core/Hashing.swift
func combineHashValues(_ firstValue: Int, _ secondValue: Int) -> Int {
    let magic = 0x9e3779b9 as UInt
    var x = UInt(bitPattern: firstValue)
    x ^= UInt(bitPattern: secondValue) &+ magic &+ (x &<< 6) &+ (x &>> 2)
    return Int(bitPattern: x)
}

struct Permutation: Hashable {
    let sigma: [Int]

    var hashValue: Int {
        return sigma.reduce(0, combineHashValues)
    }

    // ...
}

Use functional methods

The loop

var A = [Int]() // Empty array of integers
for val in sigma.perm {
    if val < i {
        A.append(val)
    } else if val > i {
        A.append(val-1)
    }
}

in func one_deletion can be simplified using compactMap:

let a = sigma.perm.compactMap { val in
    val < i ? val : val > i ? val - 1 : nil
}

and in func downset the inner loop

        for val in sigma.perm {
            new_perms.insert(one_deletion(sigma:sigma, i:val))
        }

can be simplified using map:

        new_perms.formUnion(sigma.perm.map { one_deletion(sigma: sigma, i: $0) } )

Define instance methods

Both func one_deletion and func downset operate on an instance of
struct Permutation, therefore I would replace the free functions by
instance methods:

struct Permutation: Hashable {
    // ...

    func deleting(i: Int) -> Permutation { /* ... */ }
    func downset() -> [Set<Permutation>] { /* ... */ }
}

which would then be used as

let reduced = perm.deleting(val: i)

or

let ds = perm.downset()

Further remarks

Array<Int> and [Int] are equivalent notations for an array of integers, but I would
suggest to use only one of them consistently.

The comment in

var A = [Int]() // Empty array of integers

is useless, and here

let B = Permutation(perm:A)
return B

the intermediate variable is not needed.

Instead of re-initializing the set with an empty set

    new_perms = []

it might be more efficient to call

    new_perms.removeAll(keepingCapacity: true)

to avoid subsequence reallocations of the element storage.

Putting it all together

// From https://github.com/lorentey/swift/blob/e03cb1340ad89013bbaef1c4f802a72de06e9e1a/stdlib/public/core/Hashing.swift
func combineHashValues(_ firstValue: Int, _ secondValue: Int) -> Int {
    let magic = 0x9e3779b9 as UInt
    var x = UInt(bitPattern: firstValue)
    x ^= UInt(bitPattern: secondValue) &+ magic &+ (x &<< 6) &+ (x &>> 2)
    return Int(bitPattern: x)
}

struct Permutation: Hashable {
    let sigma: [Int]

    var hashValue: Int {
        return sigma.reduce(0, combineHashValues)
    }

    func deleting(val: Int) -> Permutation {
        let newSigma = sigma.compactMap { j in
            j < val ? j : j > val ? j - 1 : nil
        }
        return Permutation(sigma: newSigma)
    }

    func downset() -> [Set<Permutation>] {
        var newPerms = Set([self])
        var result = [newPerms]

        for _ in 0..<sigma.count {
            let oldPerms = newPerms
            newPerms.removeAll(keepingCapacity: true)
            for perm in oldPerms {
                newPerms.formUnion(perm.sigma.map { perm.deleting(val: $0)} )
            }
            result.append(newPerms)
        }

        return result
    }
}

let u3 = Permutation(sigma: [2,5,3,1,4])
let ds = u3.downset()


Checking if an array is initialized and allocated in VB.NET I am creating an add-in for a piece of software our company uses, so I'm working pretty extensively with that program's API.  There are many COM object members which return or contain an array that I need to work with. Sometimes functions will return an uninitialized array (Null or "Nothing" return) or sometimes they may return an initialized array that is not allocated.  My goal is to eliminate redundant code in the form of If/Else blocks before using each array, or Try/Catch blocks to handle exceptions thrown by the array being un-initialized or un-allocated.

Note:  I'm not trying to validate any data here - that can be done in the calling code or in a function or whatever - I'm simply looking to determine if an array is in a usable state (e.g. initialized and allocated with at least one member)

I've created a member extension for arrays to check if the array is both initialized and allocated.

I'd like to get some input on whether or not this is a well designed solution or if it's total bupkis, if I'm missing anything else I should be checking for, and if improvements can be made in terms of robustness, efficiency, or general best practices; I would like to make it as fast as possible for dealing with large arrays.

Imports System.Runtime.CompilerServices

Module Module1
#Region "Extension Methods"
    <Extension()>
    Public Function IsAllocated(ByVal inArray As System.Array) As Boolean
        Dim FlagEx As Boolean = True
        Try
            If inArray Is Nothing Then
                FlagEx = False
            ElseIf inArray.Length <= 0 Then
                FlagEx = False
            ElseIf inArray(0) Is Nothing Then
                FlagEx = False
            End If
        Catch ex As Exception
            FlagEx = False
        End Try
        Return FlagEx
    End Function

#End Region
End Module

Option Strict

First thing first: Enable Option Strict please read : whats-an-option-strict-and-explicit

This will make your code less error prone. If you have enabled Option Strict you will see a warning on the line ElseIf inArray(0) Is Nothing Then that late binding is not allowed if using Option Strict On.

Regions

Please read are-regions-an-antipattern-or-code-smell

  Is there a good use for regions?

  No. There was a legacy use: generated code. Still, code generation
  tools just have to use partial classes instead. If C# has regions
  support, it's mostly because this legacy use, and because now that too
  many people used regions in their code, it would be impossible to
  remove them without breaking existent codebases.

  Think about it as about goto. The fact that the language or the IDE
  supports a feature doesn't mean that it should be used daily. StyleCop
  SA1124 rule is clear: you should not use regions. Never.

Hungarian notation

The parameter name inArray is a sign, that you are misusing the original intention. Please read this: what-is-the-benefit-of-not-using-hungarian-notation

And don't forget to read the reffered links.

That beeing said let us check how we could enhance this method by grouping the conditions together.

By using OrElse or AndAlsowe can short circuit conditions, which means that if a conditionA is true the conditionB won't be evaluated. like

If conditionA OrElse conditionB Then

End If

If we invert the conditions we can simplify the whole method to

Module Module2

    <Extension()>
    Public Function IsAllocated(ByVal inArray As System.Array) As Boolean

        Return Not ((inArray Is Nothing) OrElse (inArray.Length = 0) OrElse (inArray.GetValue(0) Is Nothing))

    End Function

End Module


Trie with insert, search, and startsWith methods for lowercase strings I wrote the following prefix tree data structure for this Leet code challenge.

  Implement a trie with insert, search, and startsWith methods.

  Note:
  You may assume that all inputs are consist of lowercase letters a-z.

However it keeps timing out on the long input. As far as I know the insert, search and startsWith functions have \$\mathcal{O}(n)\$ time complexity, where \$n\$ is the length of the input string. Can someone review my code and see where I could improve the speed of the code?

class TrieNode(object):
    def __init__(self):
        self.next = [['!',None],['!',None],['!',None],['!',None],['!',None],['!',None],['!',None],['!',None],['!',None],
                     ['!',None],['!',None],['!',None],['!',None],['!',None],['!',None],['!',None],['!',None],['!',None],
                     ['!',None],['!',None],['!',None],['!',None],['!',None],['!',None],['!',None],['!',None]]
        self.isWord = False

class Trie(object):
    def __init__(self):
        self.root = TrieNode()

    def insert(self, word):
        """
        Inserts a word into the trie.
        :type word: str
        :rtype: void
        """
        i = 0
        wordLength = len(word)
        node = self.root
        while i < wordLength:
            idx = ord(word[i]) - ord('a')
            if node.next[idx][0] == '!':
                node.next[idx][0] = word[i]
            if i < wordLength-1 and node.next[idx][1] == None:
                node.next[idx][1] = TrieNode()
            elif i == wordLength-1:
                node.isWord = True
                return
            i += 1
            node = node.next[idx][1]

    def search(self, word):
        """
        Returns if the word is in the trie.
        :type word: str
        :rtype: bool
        """
        i = 0
        wordLength = len(word)
        node = self.root
        while i < wordLength and node != None:
            idx = ord(word[i]) - ord('a')
            if node.next[idx][0] == '!':
                return False
            if i == wordLength - 1 and node.next[idx][1] != None:
                return node.isWord
            node = node.next[idx][1]
            i += 1
        if node == None and i < wordLength:
            return False
        return True

    def startsWith(self, prefix):
        """
        Returns if there is any word in the trie
        that starts with the given prefix.
        :type prefix: str
        :rtype: bool
        """
        i = 0
        wordLength = len(prefix)
        node = self.root
        while i < wordLength and node != None:
            idx = ord(prefix[i]) - ord('a')
            if node.next[idx][0] == '!':
                return False
            node = node.next[idx][1]
            i+=1
        if node == None and i < wordLength:
            return False
        return True

Assuming the next attribute of TrieNode is just a lookup for letters of the alphabet, you might be better off replacing it with a dictionary that maps letters of the alphabet to their next value. Something like:

class TrieNode(object):
    def __init__(self):
        self.next = {letter: ['!', None] for letter in string.ascii_lowercase}
        self.isWord = False

and now, rather than mucking around with ord() in your insert() and search() methods, you can use the characters directly. That won’t fundamentally change the complexity, but will make a small performance saving.

I think insert and search of a trie is O(key_length), so this is probably the sort of saving you need. You can’t change the complexity.

A few other small suggestions:

Comparisons to None should use foo is None and foo is not None, not direct equality.
Once you’re using the characters as keys in the next attribute, you’d be better of replacing those while i < wordLength loops with something like for i, char in enumerate(word).
In general, PEP 8 convention is that Python variables are lowercase with underscores (e.g. is_word, starts_with), and only classes are camel case. (Although as @Graipher notes in the comments, camel case is specified for startsWith().)


Collecting form errors to be displayed I have a piece of code that get errors from form and display to view:

$errors = $form->getMessages();
$msg = '';
foreach ($errors as $error)
{
    foreach ($error as $err)
    {
        $msg .= $err . '<br/>';
    }
}
$this->view->error_message = $msg;

It look so ugly, but now I don't have any ideas to improve it. Thank for help.

You can use array_merge to reduce the array from 2-dim to 1-dim, then implode the array.

$this->view->error_message =
    implode(array_reduce($form->getMessages(),
                         "array_merge",
                         array()),
            "<br/>");

http://php.net/manual/en/function.array-merge.php

http://www.php.net/manual/en/function.array-reduce.php


Open database and hande errors How to make this code better?

if (Program.Data.DataBase.TryOpenDataBase())
{
 bool result;
 DataBase= new Program.Data.DataBase(out result);
 if (!result)
 {
    if (Program.DataBase!= null)
       Program.DataBase.Dispose();
     Program.DataBase= null;
     Log.WriteERROR("Can not open database");
     Application.Exit();
     return;
 }
 else
 {
    Log.WriteERROR("Can not open database");
    Application.Exit();
    return;
 }

I would remove the out parameter from the constructor. It is really confusing to read. Maybe create a function that throws an exception instead. If the database implements IDisposable you can use a using block instead.

Edit - You have some duplicate code as well:

Log.WriteERROR("Can not open database");
Application.Exit();
return;


Get Minimum of four and maximum up to a limit Sita shortlists 4 gifts priced at r1, r2, r3 and r4
  respectively. She wants to buy 3 of these gifts such that it doesn't
  exceed her budget of x.

  Input and Output Format:

  Input consists of 5 integers that correspond
  to r1, r2, r3, r4 and x. Output consists of 2 integers. The first
  integer corresponds to the minimal amount she would spend for buying 3
  gifts. The second integer corresponds to the maximal amount she would
  spend for buying 3 gifts, staying within the budget.

  Sample Input

20 30 10 25 80

  Sample Output

55 75

  #include<stdio.h>

 int main()
 {
int x,a[4],i,j,n,temp,big,small,sum=0;

// scanf("%d",&r1);
// scanf("%d",&r2);
// scanf("%d",&r3);
// scanf("%d",&r4);
// scanf("%d",&x);
big = 0;
n=4;
for(i=0;i<n;i++)
{
    scanf("%d",&a[i]);
}
scanf("%d",&x);
for(i=0;i<n;i++)
{
    for(j=i;j<(n);j++)
    {
        if(a[i]>a[j]){
            temp = a[i];
            a[i] = a[j];
            a[j] = temp;
        }
    }
    sum+=a[i];
}
//sum = a[0]+a[]
small = sum-a[3];

if((sum-a[0])<=x)
big = sum-a[0];
else if((sum-a[1])<=x)
big = sum-a[1];
else if((sum-a[2])<=x)
big = sum-a[2];
else
big = small;

if(small<=x){
    printf("%d",small);
}

if(big == small){
    return 0;
}

if(big<=x){
    printf("\n%d",big);
}
return 0;
}

First impressions

Obviously, the code needs to be indented properly.  (Is that really how you wrote the code, or an artifact of the posting process?  The easiest way to post code on Stack Exchange is to paste it, select it all, and hit CtrlK on the entire block.)  In addition, you need to pick a consistent brace style.  (Your if-statements use K&R braces, but the function definition and the for-loops use a different convention.)

Don't omit "optional" braces:

if((sum-a[0])<=x)
big = sum-a[0];
else if((sum-a[1])<=x)
big = sum-a[1];
else if((sum-a[2])<=x)
big = sum-a[2];
else
big = small;

By omitting braces, someday, you will contribute to a coding mishap, and it will be your fault.  (If you use K&R braces, the braces will hardly take up any extra space.)

In modern C, you should avoid overwhelming declarations like this:

int x,a[4],i,j,n,temp,big,small,sum=0;

I have no idea what these variables are, or where they will be significant.  Since the challenge used terminology r and x, I would use those names instead of making up new ones.

Validation and output

Near the end, you wrote:

if(big == small){
    return 0;
}

That check doesn't make much sense to me.  First of all, the specification calls for two numbers to be output.  You shouldn't decide to omit one of them.  (Note that if big is over budget, then so is small.)  In the absence of any guidance on how to handle these special cases, I wouldn't bother checking for these conditions, except perhaps to write a comment or assertion.

Algorithm

The messiest part of the program is this bubble-sort-and-sum loop:

for(i=0;i<n;i++)
{
    for(j=i;j<(n);j++)
    {
        if(a[i]>a[j]){
            temp = a[i];
            a[i] = a[j];
            a[j] = temp;
        }
    }
    sum+=a[i];
}

I don't think that sorting is necessary to solve this challenge, but if it were, I would recommend writing a function to do sorting.

Suggested solution

#include <stdio.h>

int sum(int a[], size_t len) {
    …
}

int max(int a[], size_t len) {
    …
}

int main() {
    // Read input r1, r2, r3, r4, and x.  Note that r[0] holds r1.
    int r[4], x;
    for (int i = 0; i < sizeof(r) / sizeof(r[0]); i++) {
        if (!scanf("%d", &r[i])) return 1;
    }
    if (!scanf("%d", &x)) return 1;

    // Sum of all four
    int total = sum(r, sizeof(r) / sizeof(r[0]));

    // Sum of three smallest r (assumed to be within budget)
    int least = total - max(r, sizeof(r) / sizeof(r[0]));

    int most = least;
    for (int i = 0; i < 4; i++) {
        int other_sum = total - r[i];
        if (most < other_sum && other_sum <= x) {
            most = other_sum;
        }
    }
    printf("%d %d\n", least, most);
}


TextBox rendered as Label I have created a custom web server control that will have a bool property 'RenderAsLabel' so that I can convert TextBoxes to Labels, for read-only forms. I was wondering if there is any reason why this code should not be safe, or work as intended. Upon initial testing it seems fine, but I just want to make sure I'm not doing something that will end up causing issues.

namespace OrmControlLibrary
{
    [DefaultProperty("Text")]
    [ToolboxData("<{0}:OrmTextBox ID='' runat=server ></{0}:OrmTextBox>")]
    public class OrmTextBox : TextBox
    {
        private Label lbl;
        public virtual bool RenderAsLabel
        {
            get
            {
                if (ViewState["OrmTextBox"] == null)
                {
                    return false;
                }
                else
                {
                    return (bool)ViewState["OrmTextBox"];
                }
            }
            set
            {
                ViewState["OrmTextBox"] = value;
            }
        }

        protected override void Render(HtmlTextWriter w)
        {
            if (RenderAsLabel)
            {
                SetLabelProperties();
                lbl.RenderControl(w);
            }
            else
            {
                base.Render(w);
            }
        }

        private void SetLabelProperties()
        {
            lbl = new Label();
            lbl.ID = this.ID;
            lbl.CssClass = this.CssClass;
            lbl.Text = this.Text;
        }
    }
}

I would probably have a custom control that wrapped instances of the controls instead of directly inheriting from one of two possible controls, if that makes sense.

This approach leaves your code more open in the future and allows you to add additional features as part of your control (like validation controls for example) that conditionally become applied when the mode is set to a TextBox.

Here is what I came up with (this should be a run-able example):

using System.ComponentModel;
using System.Web.UI;
using System.Web.UI.WebControls;

namespace CustomControls.Web {

    [DesignTimeVisible(true)]
    [ToolboxData("<{0}:TextBoxLabel ID='' runat='server' />")]
    [DefaultProperty("Text")]
    public class TextBoxLabel : WebControl {

        #region --Instance Variables--

        private Label _ctlLabel;
        private TextBox _ctlTextBox;
        private TextBoxLabelModes _mode;
        private WebControl _renderedControl;
        private string _text;

        #endregion

        #region --Constructors--

        /// <summary>
        /// Creates a new instance of the TextBoxLabel class.
        /// </summary>
        public TextBoxLabel()
            : base() {
                _ctlLabel = new Label();
                _ctlTextBox = new TextBox();
                _mode = TextBoxLabelModes.Default;
                _renderedControl = null;
                _text = string.Empty;
        } // end constructor

        #endregion

        #region --Methods--

        protected override void CreateChildControls() {
            base.CreateChildControls();
            if (this._mode == TextBoxLabelModes.TextBox) {
                _renderedControl = this._ctlTextBox;
            } else {
                _renderedControl = this._ctlLabel;
            }
            _renderedControl.ID = this.ID;
        } // end method CreateChildControls

        public override void RenderControl(HtmlTextWriter writer) {
            _renderedControl.RenderControl(writer);
        } // end method RenderControl

        #endregion

        #region --Properties--

        [Category("Appearance")]
        public virtual TextBoxLabelModes ControlMode {
            get {
                return _mode;
            } set {
                _mode = value;
            }
        } // end property ControlMode

        [Category("Data")]
        public virtual string Text {
            get {
                return _text;
            } set {
                if (!string.IsNullOrWhiteSpace(value)) {
                    _text = value;
                    _ctlLabel.Text = value;
                    _ctlTextBox.Text = value;
                } // end if
            }
        } // end property Text

        #endregion

    } // end class TextBoxLabel

    public enum TextBoxLabelModes : int {

        Default = 0,

        Label = 1,

        TextBox = 2
    }

} // end namespace


HTML Pagination Generator I have created a function that helps me generate pagination really easily but I am concerned about its performance mainly.

The function accepts 2 parameters:

A list of items (has meaning in the framework I'm using) which contains information about the state of pagination itself and state of the storage layer - more precisely:

page the current page
perpage how many items are being shown per page
total the total amount of items in storage.

Parameter name - allows to customize the query string parameter key, which is by default named page - http://example.com/news?page=1.

public function paginationControls(BaseList $list, $parameter = 'page') {

    if (!isset($list->pagination)) {
        return 'Error: List has not been supplied with pagination details.';
    }

    // if total results are less than the results displayed on each page
    // then do not render controls
    if ($list->pagination->perpage >= $list->pagination->total) {
        return null;
    }

    // Keep track of current query string parameters
    $query = trim(preg_replace('/' . $parameter . '\=\d+\&?/', '', $_SERVER['QUERY_STRING']), '&');
    if (strlen($query)) {
        $query .= '&';
    }
    $query .= $parameter . '=';

    $makeanchor = function($page, $current = false) use ($query) {
        if ($current) {
            $url = '#';
            $class = 'pagination anchor current';
        } else {
            $url = '?' . $query . $page;
            $class = 'pagination anchor';
        }

        return $this->a($url, $page + 1, null, ['class' => $class]);
    };

    $page = $list->pagination->page;
    $pages = ceil($list->pagination->total / $list->pagination->perpage);

    // If page is greater than 3 then display link to first page
    $output = $page > 3 ? ($makeanchor(0) . ' ... ') : '';

    // Add controls for 3 pages before current page
    $tmp = '';
    for ($i = 1; $i <= 3 && $page - $i >= 0; $i++) {
        $tmp = $makeanchor($page - $i) . $tmp;
    }

    // Add current page indicator
    $output .= $tmp . $makeanchor($page, true);

    // Add controls for 3 pages after current page
    for ($i = 1; $i <= 3 && $page + $i + 1 <= $pages; $i++) {
        $output .= $makeanchor($page + $i);
    }

    // If there are more than 3 pages after the current page
    // Then add control for last page
    if ($page + $i < $pages) {
        $output .= ' ... ' . $makeanchor($pages - 1);
    }

    return $output;
}

And here are the respective helper functions

public function a($url, $text = null, $target = null, array $attributes = array()) {

    $attributes['href'] = $url;

    if ($target !== null) {
        $attributes['target'] = $target;
    }

    if ($text === null) {
        $text = $attributes['href'];
    }

    return '<a' . $this->parseAttributes($attributes) . '>' . html($text) . '</a>';
}

private function parseAttributes($attributes) {
    $string = '';

    foreach ($attributes as $attribute => $value) {
        $string .= ' ' . html($attribute) . '="' . html($value) . '"';
    }

    return $string;
}

In a class far, far away

function html($string, $flag = ENT_QUOTES, $encoding = 'UTF-8') {
    return htmlspecialchars($string, $flag, $encoding);
}

I'm mainly interested in optimizing this function in terms of performance, it would be very much appreciated if someone more experienced than me can have a say on this.

Your code looks clean. The suggested changes below mainly focuses on reducing it a bit, so IMO improving its readability.
But I'm not quite sure it'll also improve performance as you said you're interested to: I only hope some light gain could come from having suppressed your $makeanchor() function.

Changes I propose affect only your main paginationControl() function:

public function paginationControls(BaseList $list, $parameter = 'page') {

    if (!isset($list->pagination)) {
        return 'Error: List has not been supplied with pagination details.';
    }

    // if total results are less than the results displayed on each page
    // then do not render controls
    if ($list->pagination->perpage >= $list->pagination->total) {
        return null;
    }

    // Keep track of current query string parameters
    $query = preg_replace('/' . $parameter . '\=\d+\&?/', '', $_SERVER['QUERY_STRING']);
    if ($query) {
        $query .= '&';
    }
    $query .= $parameter . '=';

    $page = $list->pagination->page;
    $pages = ceil($list->pagination->total / $list->pagination->perpage);

    // Prepare link to first page, if needed
    if ($page > 3) {
      $pageSet['1...'] = FALSE;
    }
    // Prepare link to 3 pages before + current + 3 pages after
    for ($i = $page - 3; $i <= $page + 3; $i++) {
      if ($i AND $i <= $pages) {
        $pageSet[$i] = ($i == $page);
      }
    }
    // Prepare link to last page, if needed
    if ($i < $pages) {
      $pageSet['...' . $pages] = FALSE;
    }

    // Build output
    $output = NULL;
    $classes = 'pagination anchor';
    foreach ($pageSet as $pageNo => $current) {
      $output .= $this->a(
        ($current ? '#' : '?' . $query . $pageNo),
        $pageNo,
        NULL,
        ($classes . ($current ? 'current' : NULL))
      );
    }

    return $output;
}

Apart from the way to build pagination, you'll notice that:

I dropped trim(..., '&') from your $query = ..., since "&" has already suppressed by preg_replace(); otherwise it sounds like you suspect the whole query to abusively include an ending "&"
at the just next line, I dropped strlen() from if ($query), which is enough (and may be slightly faster, but it is only cosmetic)


Phone number format and data entry restriction I created a form that formats phone numbers, allows only allowable keys to be pressed, and converts a lettered entry into its corresponding phone keypad number. Is there some cleanup or better coding that can be done? There is no business need for international phone numbers as it is a local business.

function pageLoad() {
    $(".PhoneSelection").keypress(function (e) {
        // Only allow number entry
        var regex = new RegExp("^[0-9]+$");
        var key = String.fromCharCode(!e.charCode ? e.which : e.charCode);
        if (!regex.test(key)) {
            // If not a number, is it a letter?
            var regex2 = new RegExp("^[A-Za-z]+$");
            if (regex2.test(key)) {
                // Convert the letter typed to the number on the phone keypad
                $(this).val($(this).val() + letterPos(key));
                return false;
            }
            else { return false; }
        }
    });
    $(".PhoneNum").text(function (i, text) {
        // returns (999) 555-1234 from 9995551234
        return text.replace(/(\d{3})?(\d{3})(\d{4})$/,
            function ($0, $1, $2, $3) {
                var formatted = ""; if ($1) { formatted += "(" + $1 + ") "; } if ($2) { formatted += $2 + "-"; } if ($3) { formatted += $3; } return formatted;
            }
        );
    });
}
function letterPos(val) {
    var alphaMap = { 'ABC': '2', 'DEF': '3', 'GHI': '4', 'JKL': '5', 'MNO': '6', 'PQRS': '7', 'TUV': '8', 'WXYZ': '9' };
    var number = "";
    $.each(alphaMap, function (key, value) {
        if (key.toLowerCase().indexOf(val.toLowerCase()) > -1)
        { number = value; }
    });
    return number;
}

Here is how it is consumed in the view form:

9995551234

<input id="ContactPhone" class="PhoneSelection nextField" type="text" placeholder="Phone" value="9995551234"></input>

Results on display:

  (999) 555-1234

Here are some points regarding your code:

Decide on a style regarding the if-statements, and stick to it. Don't collapse else statements like the } else { return false; }
Avoid multiple statements on one line, like the var formatted = ...
When both branches does the same, extract the return outside of it
You can inline regexp's when they are simple, (see Detect numbers or letters with jquery/javascript)
Additionaly when using regexp against a single key you don't need to use anchors or multiple match options. Instead of ^[0-9]+$, simply use [0-9]
When replacing text you can enter the replacement text directly, instead of using a function. And the function you use is somewhat complicated
Edit: Switched to a simpler letterToNumber() using the lowercase character code (with the code of a subtracted) to index into a numbered variant of the alphabet.

In this jsfiddle I've corrected for my comments:

$(".PhoneSelection").keypress(function (e) {
    // Only allow number entry
    var key = String.fromCharCode(!e.charCode ? e.which : e.charCode);

    if (!/[0-9]/.test(key)) {
        // If not a number, is it a letter?
        if (/[A-za-z]/.test(key)) {
            // Convert the letter typed to the number on the phone keypad
            $(this).val($(this).val() + letterToNumber(key));
        }
        return false;
    }
});

$(".PhoneNum").text(function (i, text) {
    // returns (999) 555-1234 from 9995551234
    return text.replace(/(\d{3})?(\d{3})(\d{4})$/, "($1) $2-$3")
        .replace(/\(\) /, "");
});

function letterToNumber(val) {
   charIndex = val.toLowerCase().charCodeAt(0) - 97;
   return "22233344455566677778889999"[charIndex];
}

However there are still some comments to be made:

To remove illegal characters, i.e. non-letters, and transforming letters to numbers, whilst you are typing is frowned upon in some camps. It would be better to present a normalised number next to the input field, or after it has been submitted
The transformation of the input also removes the visual feedback of whether the user has entered the correct telephone number
Your code as it stands does not connect the input field and the output field, which might be by design, but it threw me of whilst testing it for a while
You are bypassing backspacing and/or movement of cursor. As an example start entering a number using letters, i.e. xxjjjmmmm, and then you realise that you forgot the first x, so you move the cursor over there and enter it x. Now the code will add that at the end, bummer...

Edit: In original research I found Code golf: Letters in phone numbers and How to convert an alphanumeric phone number to digits), where the shortest, not very readable version of alternateLetterToNumber() is (with addition of lowercasing the character):

function alternateLetterToNumber(y) {
    y = y.toLowerCase().charCodeAt(0) - 91;
    return y > 27 ? 9 : y > 24 ? 8 : y > 20 ? 7 : ~~ (y / 3);
}


change html text based on date......JS let date= new Date()

let day = date.getDay()

let weekday = new Array(7);
    weekday[0] = "Sunday";
    weekday[1] = "Monday";
    weekday[2] = "Tuesday";
    weekday[3] = "Wednesday";
    weekday[4] = "Thursday";
    weekday[5] = "Friday";
    weekday[6] = "Saturday";

changeText = () => {

 if(day === 1){
   firstDay.textContent = weekday[2]
   secondDay.textContent = weekday[3]
   thirdDay.textContent = weekday[4]
   fourthDay.textContent = weekday[5]
   fifthDay.textContent = weekday[6]

 }else if(day === 2){
    firstDay.textContent = weekday[3]
    secondDay.textContent = weekday[4]
    thirdDay.textContent = weekday[5]
    fourthDay.textContent = weekday[6]
    fifthDay.textContent = weekday[0]
}
}
 changeText();

this is for a weather app. I need to change the html text.content based on the current day so that the extended forecast stays accurate

This code repeats for each day of the week it works but seems very inefficient. the purpose is to change the text in my html so the extended weather forecast is consistent

You can do:

changeText = () => {
    firstDay.textContent = weekday[(day + 1) % 7]
    secondDay.textContent = weekday[(day + 2) % 7]
    thirdDay.textContent = weekday[(day + 3) % 7]
    fourthDay.textContent = weekday[(day + 4) % 7]
    fifthDay.textContent = weekday[(day + 5) % 7]
}

Also I'd recommend passing day as a parameter. Just so that it's more clear.


Armstrong numbers in a given range using Java 8 (Follow Up) Previous Question: Armstrong numbers in a given range using Java 8

Updated program for getting all Armstrong Numbers between 1 and 10_000_000 as per the suggestions in this answer.

public class ArmstrongNumbers {

    public static void main(String[] args) {
        IntStream.range(1, 10_000_000)
                .filter((n) -> {
                    int size = Integer.toString(n).length();
                    return Integer.toString(n)
                            .chars()
                            .map(d -> d - '0')
                            .mapToDouble(v -> Math.pow(v, size))
                            .sum() == n;
                }).forEach(System.out::println);
    }

}

Output:

1
2
3
4
5
6
7
8
9
153
370
371
407
1634
8208
9474
54748
92727
93084
548834
1741725
4210818
9800817
9926315

Comments:

Was able to replace the while loop with a stream.
Code isn't shorter but is more readable.

Can this program be made even shorter? Any other suggestions also welcome.

Looking at the code for Integer.toString, it seems to be a lot of work to convert an integer to a string. At the moment, you're doing the conversion twice.

However, getting the length of a string, via String.length, is a simple getter. As such, you'd probably be better off with storing the string representation in a variable, and calling .length() on that.

public static void main(String[] args) {
    IntStream.range(1, 10_000_000)
            .filter((n) -> {
                final String number = Integer.toString(n);
                return number.chars()
                        .map(d -> d - '0')
                        .mapToDouble(v -> Math.pow(v, number.length()))
                        .sum() == n;
            }).forEach(System.out::println);
}

Also, good job coming up with the idea of substracting '0' from each character to cast it back to a number: that's a far better idea than what I had in mind (using complicated parse functions).

You could, in theory, perform this step at .mapToDouble, via Math.pow((v - '0'), number.length()) but I'm not sure whether that helps the readability. It does make things a lot shorter, I guess.

A potential performance upgrade after this would be to consider storing the result of Math.pow in a lookup table; whenever you surpass the current digit length, multiply each value of the lookup table by its index and continue. Right now, this doesn't seem very relevant to me, but later, this might be an issue.


Limit acceptable values for function parameter in VBA I wrote a function in VBA for Excel that finds the highest row and highest column number in a range, which may include multiple, non-contiguous areas.

The function contains a parameter I call rowOrColumnOptionand I want to only allow values r or c (to return either a row or column number).

I was able to do this with an enumerated variable type:

Public Enum rowOrColumnOption
    ' Custom variable type, used to limit allowed values in a
    ' function parameter to either r (row) or "c" (column).
    r = 0: Row = 0
    c = 1: Column = 1
End Enum

I then assigned this variable type to a parameter in my function:

Public Function GetHighestRowOrCol(ByVal targetRange As Range, ByVal rowOrColumn As rowOrColumnOption) As Long

...some code...

If rowOrColumn = 0 Then
    ' Return the highest row number.
    '  0 corresponds to "Row" in the custom variable type "rowOrColumnOption".
    GetHighestRowOrCol = maxRow
ElseIf rowOrColumn = 1 Then
    ' Return the highest column number.
    '  1 corresponds to "Column" in the custom variable type "rowOrColumnOption".
    GetHighestRowOrCol = maxCol
Else
    ' Return an error value of zero.
    GetHighestRowOrCol = 0
End If

End Function

While this does work, I wonder if there is a better, simpler, faster, clearer, smarter way to do this, without using Enum. I am also open to any general critique/advice about the code overall.

I see several issues with this.

First is the naming. Row and Column both hide global Excel variable (although whether this is good or bad is debatable...). This otherwise valid code becomes a compile error "Expected Array" everywhere in the project that contains the enumeration.

Public Sub Test()
    Debug.Print Row(1).Address
End Sub

I would personally change the Enum name to something like LimitDimension (Pascal case for namespace identifiers) and the two options to LimitRow and LimitColumn. This will be much less likely to create collisions. Note that r and c are also really common local loop counter variables.  If they're public enumeration member, they pollute the global namespace and can create bizarre\misleading compile errors:

Option Explicit

Public Sub Test1()
    Dim c As Long
    Debug.Print c   'This works just fine, prints 0.
End Sub

Public Sub Test2()
    Debug.Print c
    Dim c As Long   'Compile error.  Duplicate declaration in same scope.
End Sub

Public Sub Test3()
    For c = 1 To 5  'Compile error.  Variable required - can't assign to this expression.
        Debug.Print c
    Next
End Sub

Public Sub Test4()
    Dim c As Long
    For c = 1 To 5  'Works fine.
        Debug.Print c
    Next
End Sub

Second, enumerations should only have one member for each distinct value. The only real point to using them in VBA is to make your code more readable (there's no strict type enforcement), and it isn't immediately clear when you see something like this...

Select Case foo
    Case rowOrColumnOption.r
        'Do something.
    Case rowOrColumnOption.Row
        'Do something else.
End Select

...that it's a coding error.  This is immediately obvious:

Select Case foo
    Case rowOrColumnOption.Row
        'Do something.
    Case rowOrColumnOption.Row
        'Do something else.
End Select

Third (on a related note), you should use the Enum explicitly when you're checking for it's members.  This...

If rowOrColumn = 0 Then
    '...
ElseIf rowOrColumn = 1 Then
    '...
Else
    '...
End If

...is much clearer as:

If rowOrColumn = rowOrColumnOption.Row Then
    '...
ElseIf rowOrColumn = rowOrColumnOption.Column Then
    '...
Else
    '...
End If

Not only is the much clearer as to the intent of the code, it also prevents it from breaking if you decide to or need to change the values of the members. It also makes the code document itself, so you don't have to sprinkle comments like this around:

'  0 corresponds to "Row" in the custom variable type "rowOrColumnOption".

Fourth, IMHO the statement delimiter : is if not pure evil, mostly evil. It's much more natural to read code with the statements arranged vertically than it is if statements are arranged horizontally.  This is much easier to process at a glance:

Public Enum rowOrColumnOption
    r = 0
    Row = 0
    c = 1
    Column = 1
End Enum

While the intent may have been to group aliased members, you can make this explicit by just assigning aliases within the declaration itself:

Public Enum rowOrColumnOption
    Row = 0
    r = Row
    Column = 1
    c = Column
End Enum

Finally, just as an aside - I have no idea what ...some code... is doing in GetHighestRowOrCol, but from the If block it appears that ...some code... is calculating both maxRow and maxCol when it only ever returns one of them.  I'm guessing this would be a more efficient structure:

If rowOrColumn = rowOrColumnOption.Row Then
    '...some code to determine the highest row number.
    GetHighestRowOrCol = maxRow
ElseIf rowOrColumn = rowOrColumnOption.Column Then
    '...some code to determine the highest column number.
    GetHighestRowOrCol = maxCol
Else
    GetHighestRowOrCol = 0
End If


Adding grocery items I have full working code, but when I run it, it shows these warnings:

given is experimental at grocery.pl line 38.
when is experimental at grocery.pl line 40.
when is experimental at grocery.pl line 62.
when is experimental at grocery.pl line 89.
when is experimental at grocery.pl line 100.

How can I fix these?

use strict;
use warnings;
use List::Util qw(max);
use feature "switch";
my $b;
my $c;
my $p;
my $highest;

print "____________________________STORE THE ITEM_____________________\n";
my %grocery=
(
    "soap"=>{"lux"=>13.00,"enriche"=>11.00},
    "detergent"=>{"surf"=>18.00},
    "cleaner"=>{"domex"=>75.00}
);
foreach my $c(keys %grocery)
{
print "\n";
    print"$c\n";
    foreach my $b(keys %{$grocery{$c}})
    {
        print "$b:$grocery{$c}{$b}\n";
    }
}
my $ch;
do
{
    print "________________MENU_________________\n";
    print "1.ADD ITEM\n";
    print "2.SEARCH\n";
    print "3.DISPLAY\n";
    print "4.FIND THE MAX PRICE\n";
    print "5.EXIT\n";
    print "enter your choice \n";
    $ch=<STDIN>;
    chomp($ch);
    given($ch)
    {
        when(1)
        {
            print "Enter the category you want to add";
            $c=<STDIN>;
            chomp($c);
            print "Enter brand\n";
            $b=<STDIN>;
            chomp($b);
            print "Enter price\n";
            $p=<STDIN>;
            chomp($p);
            if(exists($grocery{$c}))
            {
                $grocery{$c}{$b} = $p;
                print "\n";
            }
            else
            {
                $grocery{$c}={$b,$p};
                print "\n";
            }
        }
        when(2)
        {
            print "Enter the item that you want to search\n";
            $c=<STDIN>;
            chomp($c);
            if(exists($grocery{$c}))
            {
                print "category $c exists\n\n";
                print "Enter brand\n";
                $b=<STDIN>;
                chomp($b);
                if(exists($grocery{$c}{$b}))
                {
                    print "brand $b of category $c exists\n\n";
                    print "-----$c-----\n";
                    print "$b: $grocery{$c}{$b}\n"
                }
                else
                {
                    print "brand $b does not exists\n";
                }
            }
            else
            {
                print "category $c does not exists\n";
            }
        }
        when(3)
        {
            foreach $c (keys %grocery)
            {
                print "$c:\n";
                foreach $b(keys %{$grocery{$c}})
                {
                    print "$b:$grocery{$c}{$b}\n";
                }
            }
        }
        when(4)
        {
            my @all_prices;
            foreach my $c (keys %grocery) {
            push @all_prices, values %{ $grocery{$c} };
        }
        $highest = max @all_prices;
        print "$highest\n";
    }
    }
}while($ch!=5);

The warning

The warning you are seeing is because the switch feature that gives you the given and when keywords is experimental. That means it's there in this specific Perl version, and possible future ones, but that is not clear. Sometimes experimental features make it into the core part of the language itself, but sometimes they get removed. With switch it's not clear yet which one it's going to be.

If you are use you are only going to use your specific Perl version, you can simply turn off the warnings.

use feature 'switch';
no warnings 'experimental';

Now you don't see any warnings any more. But if your code will be run on an older version that doesn't have the feature yet, or on a later version in the future where it has might have been changed or removed, your code will break.

I would not use experimental features in long-living production code. Instead, I would use either of the following two approaches, based on how complex your program is.

Using a bunch of ifs and elsifs

The most simple approach is to just replace the given/when with a couple of if statements. Your do { ... } while ( $ch != 5 ) approach is good, so we keep it. But because it's not really a loop, but rather a block with a postfix condition, we cannot use next to jump out, so we need to use an elsif construct.

my $ch;
do {
    $ch = <>;
    chomp $ch;

    if ( $ch == 1 ) {
        # new item ...
    }
    elsif ( $ch == 2 ) {
        # search ...
    }
    elsif ( $ch == 3 ) {
        # display ...
        next;
    }
    elsif ( $ch == 4 ) {
        # max price ...
    }
} while ( $ch != 5 );

We don't even need to use an elsif because we can just use next to skip to the next iteration.

Using a dispatch table

This is way more advanced, and a bit overkill for your situation, but it's very nicely extensible. A dispatch table is essentially a hash with code references that you call depending on your input.

my %dispatch = (
    1 => sub {
       # new item ...
    },
    2 => sub {
       # search ...
    },
    3 => sub {
       # display ...
    },
    4 => sub {
       # max price ...
    },
);

my $ch;
do {
    # read input ...

    if ( exists $dispatch{$ch} ) {
        $dispatch{$ch}->();
    }
} while ( $ch != 5 );

But as I said, this is an advanced technique and possibly a bit overkill for this program.

Telling the user they gave wrong input

If you enter another number than 1 to 5, your program does not complain. It just reiterates the same output. If you enter a letter, it will issue a warning.

Input validation up front

You could check the input first, issue a warning and then redo the loop.

do {
    # read input ...

    if ( $ch !~ m/^[1-5]$/ ) {
        print "Invalid input.\n";
    }
    elsif ( $ch == 1 ) {

    # ...
} while ( $ch != 5 );

Don't use numerical operators

Or you could replace all your == and != with eq and ne, which work on strings as well as numbers, and move the warning to the end of the loop.

my $ch;
do {
    $ch = <>;
    chomp $ch;

    if ( $ch eq 1 ) {
        # new item ...
    }
    elsif ( $ch eq 2 ) {
        # search ...
    }
    elsif ( $ch eq 3 ) {
        # display ...
        next;
    }
    elsif ( $ch eq 4 ) {
        # max price ...
    }
    else {
         print "Invalid input.\n";
    }
} while ( $ch ne 5 );

Both of those approaches work.

Self-explaining variable names

Your variable names $b, $c, $p and $ch do not speak for themselves. The program is very hard to read because of that. Instead of on-letter names, name them for what they represent.

$ch could be $selection, $c could be $category, $b the $brand and $p could be $price.

Smaller scope

You are already using strict and lexical variables with my, which is very good. But you should not declare your vars at the top like in a C program. In Perl you should always use the smallest possible scope.

For example, your $c is declared outside the loop (I'll continue to say loop, although it's not a loop structure), but you use it in multiple places where you assign new values in different branches of your code.

Instead, you should declare a new, lexical version in each of those branches, which are individual from each other. That helps to protect from bugs. Right now you could easily introduce a bug, if you for example accidentally put == instead of =. Because the variable is already declared in a higher scope, Perl won't tell you that that's possibly wrong. But if you instead declare it freshly, it will complain.

print "Enter the category you want to add";
my $category = <STDIN>;
chomp $category; # no need for parenthesis here
if ( exists( $grocery{$category} ) ) {
    print "Enter brand\n";
    my $brand = <STDIN>;
    chomp $brand;
    print "Enter price\n";
    my $price = <STDIN>;
    chomp $price ;
    $grocery{$category} = { $brand => $price };

    print "\n";
}
# ...

You should do that throughout the code.

Autovivification

When you create a new entry, you check if the category exists, and then branch the code. You don't need to do that. Right now you have duplicated all the code for getting the brand and price from the user, but you only need to write that once.

elsif ( $selection == 1 ) {
    print "Enter the category you want to add\n"; # you forgot a \n here
    my $category = <>; # <> is the same as <STDIN>

    print "Enter brand\n";
    my $brand = <STDIN>;

    print "Enter price\n";
    my $price = <STDIN>;
    chomp( $category, $brand, $price );
    $grocery{$category}->{$brand} = $price;
}

The above code sums both cases up into one. The assignment $grocery{$category}->{$brand} = $price works because Perl will automatically create a hash reference inside $grocery{$category} if it sees the ->{$brand} part. That's called autovivification and is explained in the perlglossary.

I also moved the chomp into one line so make the code more concise, as chomp can take a list of variables. In that case you need the parenthesis so it knows it's a list.

Use say instead of print "...\n"

You can enable the say builtin that works like print, but includes a newline at the end of each invocation. It will make your code more concise. To enable it, you need to

use feature 'say';

at the top of your program. You can then use it throughout the code.

if ( $selection !~ m/^[1-5]$/ ) {
    say "Invalid input.";
}

Use a HERE doc for blocks of output

HERE docs are useful for multi-line output. You can use one for the main menu that lists your options.

do {
    print <<'EOTEXT';
________________MENU_________________
1.ADD ITEM
2.SEARCH
3.DISPLAY
4.FIND THE MAX PRICE
5.EXIT
enter your choice
EOTEXT

    $selection = <>;

The delimiter EOTEXT can be chosen freely. It should be all-caps and I like to start it with EO, which means end of. The semicolon goes at the end of that same line. The single quotes '' mean that there is no interpolation. If you want to include variables in the text, use "EOTEXT" instead. The delimiter has to be placed in the line after the text, and needs to be preceded by an empty line. As of Perl 5.24, HERE docs cannot be indented. Perl 5.26 will include a new feature that allows this.


Ordering cell rows and columns How can I make this sort faster and better?

function OrderBy(a,b) {
    if (a < b) return -1;
    if (a > b) return 1;
    return 0;
}

SortBy = function($th) {
    var $tbody = $th.closest('tbody');
    var $table = $tbody.parent();
    $tbody.detach();
    $('#Processing').show('fast',function () {
        var column = $th.index();
        var rows = $tbody.find('tr').get();
        rows.sort(function(rowA,rowB) {
            var keyA = $(rowA).children('td').eq(column).text();
            var keyB = $(rowB).children('td').eq(column).text();
            return OrderBy(keyA,keyB);
        });
        $.each(rows, function(index,row) {
            $tbody.append(row);
        });
        $table.append($tbody);
    });
};

You could read the cell texts before sorting, so that it doesn't have to happen repeatedly in the order function:

    var rowsWithText = $tbody.find('tr').map(function() {
      return {
        row: this,
        text: $(this).children('td').eq(column).text()
      };
    });
    rowsWithText.sort(function(rowA, rowB) {
        return OrderBy(rowA.text, rowB.text);
    });
    $.each(rowsWithText, function(index,row) {
        $tbody.append(row.row);
    });


Can I use mapWriterT instead of mapM/mapM_ (Haskell)? First off, I'm just learning Haskell and I'm not proficient in that language yet.

I wrote this Haskell module to look for files in a Linux filesystem. It can use regular expressions as search patterns. I wrote two variants: one of them sends the search results to stdout, the other (shown here) gives a list with the search results.

For example (in GHCi): searchToList "conf" "." >>= return . take 1 (being "." my home directory) would give [".config"]

I made the function searchWriterT recursive using mapM_, but I know there is mapWriterT in the module Control.Monad.Writer.

Could anyone give me a hint about what I shall do to replace mapM_ with mapWriterT (if it's possible)?

Thanks in advance.

module FileSearch where

import System.Directory (getDirectoryContents, doesDirectoryExist)
import Control.Monad (filterM, unless)
import Data.List ((\\))
import Text.Regex.Posix ((=~))
import Control.Monad.Writer

data Dir = Dir { path :: FilePath -- path of the current directory
               , files :: [FilePath] -- files in the directory
               , subdirs :: [FilePath] } -- subdirectories
               deriving (Show)

dontSearch :: [FilePath]
dontSearch = ["/proc", "/sys"]

-- listDir takes the name of a directory and gives a Dir with
-- that name, a list of the files in the directory and a list of
-- its subdirectories, all prefixed with the path of the current
-- directory.
-- listDir will list neither /proc nor /sys
listDir :: FilePath -> IO Dir
listDir f
    | f `elem` dontSearch = return Dir { path = f
                                       , files = []
                                       , subdirs = [] }
    | otherwise = do
        c' <- getDirectoryContents f
        let c = map (appendPath f) (deleteDot c')
        d <- filterM doesDirectoryExist c
        return Dir { path = f
                   , files = (c \\ d)
                   , subdirs = d }

appendPath :: FilePath -> FilePath -> FilePath
-- if the path is "/", do not add another "/"
appendPath "/" y = concat ["/", y]
appendPath x y = concat [x, "/", y]

-- remove "." and ".." entries in directories
deleteDot :: [FilePath] -> [FilePath]
deleteDot = filter (`notElem` [".", ".."])

searchWriterT :: String -> FilePath -> WriterT [FilePath] IO ()
searchWriterT s f = do
    d <- liftIO $ listDir f
    let found = filter (=~ s) (files d)
    tell found
    unless (null (subdirs d)) $
        mapM_ (searchWriterT s) (subdirs d)

searchToList :: String -> FilePath -> IO [FilePath]
searchToList s f = execWriterT (searchWriterT s f)

mapWriterT is for transforming one WriterT into another. Its type is painfully generic, but allows you to transform pretty much everything about a WriterT.

mapWriterT :: (m (a, w) -> n (b, w')) -> WriterT w m a -> WriterT w' n b

So if you start with a WriterT w m a, and you want to end up with WriterT w' n b (in other words, you can transform all 3 type parameters), then you have to provide a function of the form
m (a, w) -> n (b, w').

Contrast this with mapM_ :: (a -> m b) -> [a] -> m (). The purpose of this mapping is to take a list and perform monadic operations parameterized by the list's contents in sequential order.

While there may be a way to write your function in terms of mapWriterT, it is a completely different kind of mapping than mapM_, so your request to " replace mapM_ with mapWriterT" doesn't really make much sense to me. Nevertheless, let's give it a spin.

newtype MatchedFiles = MatchedFiles { unwrapFiles :: [FilePath] } deriving Monoid
newtype SubDirs = SubDirs { unwrapDirs :: [FilePath] }

filesInDir :: String -> FilePath -> WriterT MatchedFiles IO SubDirs
filesInDir s f = do
  d <- liftIO $ listDir f
  let found = MatchedFiles $ filter (=~ s) (files d)
      subdirs = SubDirs $ subdirs d
  tell found
  return subdirs

OK, so here we've got a non-recursive WriterT that will simply tell the files that match the string at the given directory, and returns the subdirectories in that directory. I've used newtypes to help me not get the "found files" and "subdirectories" mixed up.

Now, what is the transformation we want to perform on this?

searchWriterT :: String -> FilePath -> WriterT MatchedFiles IO ()
searchWriterT s f = mapWriterT mapper (filesInDir s f)
  where
    mapper :: IO (SubDirs, MatchedFiles) -> IO ((), MatchedFiles)
    mapper action = undefined

Assuming we want to write searchWriterT in terms of mapWriterT and filesInDir, I just followed the types and this is what we've got. Now, the question is, how to write mapper?

mapper action = do
  (SubDirs subdirs, files) <- action
  if null subdirs
    then return ((), files)
    else undefined

Clearly, if there are no "subdirs", then we are done, and we simply regurgitate the files. But what if there are subdirs? Well, you've already figured out what to do: use mapM_ on the subdirs!

else runWriterT $ tell files >> mapM_ (searchWriterT s) subdirs

In fact, mapM_ will take care of the null case for us: it won't do anything in the event of an empty list.

mapper action = do
  (SubDirs subdirs, files) <- action
  runWriterT $ tell files >> mapM_ (searchWriterT s) subdirs

Conclusion: mapWriterT felt kind of awkward for this problem, especially since inside the mapper we turned right around and used runWriterT. We didn't even get rid of mapM_, because mapM_ embodies exactly what needs to be done for this problem.

searchWriterT s f = filesInDir s f >>= mapM_ (searchWriterT s) . unwrapDirs


A thread-safe stop watch utility class I am writing a stop watch utility class in Java that I want to be thread-safe (concurrent).  Here is what I have right now:

public class Stopwatch {

    private volatile long beginTime;
    private final AtomicLong lastLap = new AtomicLong();
    private final AtomicReference<List<Long>> laps = new AtomicReference<List<Long>>();

    public synchronized void start() {
        reset();
        beginTime = System.nanoTime();
        lastLap.set(beginTime);
    }

    public long timeElapsed() {
        return toMilliseconds(timeElapsed(beginTime));
    }

    public void lap() {
        long lap = timeElapsed(lastLap.longValue());
        lastLap.addAndGet(lap);
        getLaps().add(toMilliseconds(lap));
    }

    public synchronized void reset() {
        beginTime = 0;
        lastLap.set(0);
        if (laps.get() != null) {
            laps.get().clear();
        }
    }

    public List<Long> getLaps() {
        laps.compareAndSet(null, Collections.synchronizedList(new ArrayList<Long>()));
        return laps.get();
    }

    private long toMilliseconds(long nano) {
        return nano / 1000000;
    }

    private long timeElapsed(long start) {
        return System.nanoTime() - start;
    }
}

A few notes, the compareAndSet call in getLaps is used to lazily initialize the list. I added the synchronized modifiers to the start and reset methods to prevent the scenario of multiple threads calling those methods simultaneously, which could possibly result in some fields being reset and some being initialized, giving odd behavior. I did not add the synchronized modifiers to the other methods because I anticipate them to operate as expected even when invoked from multiple threads, though, quite frankly, I'm not entirely sure if that is the case.

I would like this implementation to avoid locking as much as possible because the locking and waiting could give slightly inaccurate results for certain methods, like timeElapsed.

Do I need to make any modifications to make this class thread-safe?

This class is... problematic. It is not thread safe at all. It has a mix of 3 different concurrency tools, and each of them has problems.

Volatile

volatile is almost always a bug. If you think you should be using volatile, you are wrong (almost always). Textbooks like to cover volatile, but "in the wild" it is about as common as the Sasquatch (clever pun there...).

In your code, for example, beginTime is only ever accessed from inside synchronized methods, so why is it volatile?

Atomics

You use AtomicLong to keep the lap time, but you have a race condition:

public void lap() {
    long lap = timeElapsed(lastLap.longValue());
    lastLap.addAndGet(lap);
    getLaps().add(toMilliseconds(lap));
}

In here, two threads may call lap() at about the same time. The first thread may get lastLap.LongValue() and convert it to lap. It may then pause there. The next thread may get the lap too, and call addAndGet(lap). The first thread may then overwrite the previous thread's value with the older lap. Then, both threads may get the laps List, and try to simultaneously add their lap values to the same synchronized ArrayList... the order they are added is unpredictable, but, for sure, you may end up with two really long laps, instead of a long one and a short one.

Atomics are atomic... they do not prevent race conditions (they just give a way to identify and handle them).

Synchronization

Here you have done a half-OK job. You are synchronizing on two methods, but you should be synchronizing on more.

I would recommend you throw away all the other concurrency tricks, and use only synchronization.

Believe it or not (you will believe it if you test it), synchronization blocks can be really really fast, and as a result, concurrency is still PDQ (Pretty Damn Quick).

So, with the following code, let me point out some things:

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;

public class StopWatch {

    private long beginTime = 0;
    private long[] laps = new long[1 << 4];
    private int lapCount = 0;

    private final Object sync = new Object();

    public void start() {
        synchronized(sync) {
            if (beginTime != 0) {
                // already started
                return;
            }
            beginTime = System.nanoTime();
            laps[lapCount++] = beginTime;
        }
    }

    public long timeElapsed() {
        synchronized(sync) {
            if (beginTime == 0) {
                return 0;
            }
            return toMilliseconds(System.nanoTime() - beginTime);
        }
    }

    public void lap() {
        synchronized(sync) {
            if (beginTime == 0) {
                return;
            }
            if (lapCount == laps.length) {
                laps = Arrays.copyOf(laps, lapCount + (lapcount >> 1) + 8);
            }
            laps[lapCount++] = System.nanoTime();
        }

    }

    public void reset() {
        synchronized(sync) {
            beginTime = 0;
            lapCount = 0;
        }
    }

    public List<Long> getLaps() {
        long[] times;
        synchronized(sync) {
            times = Arrays.copyOf(laps, lapCount);
        }
        List<Long> data = new ArrayList<>(times.length);
        for (int i = 1; i < times.length; i++) {
            data.add(toMilliseconds(times[i] - times[i - 1]));
        }
        return data;
    }

    private long toMilliseconds(long nano) {
        return nano / 1000000;
    }

}

I use a dedicated object sync to lock on to. This prevents someone from using my instance as a lock for their synchronization needs, and breaking my class. E.g., with your code, if someone did:

Stopwatch sw = new Stopwatch();

.....

synchronized(sw) {
    Thread.sleep(10000);
}

That code would make your methods block in other threads...
I have no volatiles or atomics... only synchronized.
I use primitives, and arrays to store the data. This is fast, and reduces memory usage.
I don't do any math in the time-sensitive methods. All I do is store away the System.nanoTime(). The math is all done in the get methods, and outside of the synchronized blocks.

Conclusion

Mixing concurrency strategies is bad.
synchronization is normally more than enough for most needs
atomics are atomic, not thread safe. Atomics provide mechanisms to identify race conditions, not avoid them.


Quick Sort C++ Implementation I'm learning C++ and algorithms myself, and as training I had to write quicksort by myself.All feedback on coding style/clarity and concepts are welcomed!

int QuickSort(int *Array, int start, int end)//Recursive function that sorts array
{
    int Wall = start;//Variable that will hold pivot position before recursive calls and tracks indexes
    int Temp = 0;   //Variable used for swaping array members
    if (start < end)//Exit condition
    {
        for (int Index = start; Index < end; Index++)
        {
            if (Array[Index] <= Array[end])
            {
                Temp = Array[Wall];
                Array[Wall] = Array[Index];
                Array[Index] = Temp;
                Wall++;
            }
        }
        Temp = Array[end];
        Array[end] = Array[Wall];
        Array[Wall] = Temp;
    QuickSort(Array, start, --Wall);//Calling racursion for left of the pivot
    QuickSort(Array, ++Wall, end);//Calling recursion for right of the pivot
    }
 return 0;
}

I think this is fairly impressive. I don't know if I would have done this in-place if I sat down to do this today. That said, I do see a few things you could improve.
Naming
I think your variable names could use some work. Why did you choose Wall? That name just doesn't strike me as useful in this context.
I notice that you're mixing capitals and lowercase letters for the first letter of your variables. Typically variables start with a lowercase letter and type names start with an uppercase letter. You don't necessarily have to follow that convention, but you should be consistent.
C++
The only line of code in this that differentiates it from C is the print statement, which I assume is just for debugging. If you #include <utility> you get some very useful functions. (You'll need to #include <algorithm> if you're using C++ that pre-dates C++11.) For example, std::swap() will swap 2 values for you without the need to write out 4 lines of code:
std::swap(Array[Wall], Array[index]);

does the same thing as:
int Temp = 0;   //Variable used for swaping array members
...
            Temp = Array[Wall];
            Array[Wall] = Array[Index];
            Array[Index] = Temp;

Also, you don't need std::endl where you've used it. That will flush the buffer which will slow things down. You can simply use:
std::cout << Wall << "\n";

And if you're really paranoid, you can add a single call to:
std::cout << std::endl;

after QuickSort returns;
Recursion
As you found, when you have a deeply recursive function, it can eventually eat up the entire stack if you're not careful. Luckily, you can eliminate recursion in many cases. I modified yours to remove the recursion by adding a simple std::queue and putting the ranges in the queue instead of calling back into the function. First, I made a simple struct to hold a range of indexes to sort:
struct Range {
    int start;
    int end;

    Range(int newStart, int newEnd) : start(newStart), end(newEnd) {}
};

Then I updated the function to use a std::queue and the new Range type:
void QuickSort(int* array, int start, int end)
{
    std::queue<Range>  queue;
    queue.push(Range(start, end));
    while(!queue.empty())
    {
        Range next    = queue.front();
        queue.pop();

        int start = next.start;
        int end = next.end;
        int Wall = start;
        if (start < end)
        {
            for (int Index = start; Index < end; Index++)
            {
                if (array[Index] <= array[end])
                {
                    std::swap(array[Wall], array[Index]);
                    Wall++;
                }
            }
            std::swap(array[end], array[Wall]);
            queue.push(Range(start, --Wall));
            queue.push(Range(++Wall, end));
        }
    }
}

I kept most of your code, but used std::swap and removed the recursion.
To be honest, I didn't think the performance of your version was bad at all. It seemed to work plenty fast for me.