Project Euler problem 92 - square digit chains

1. Bug

There's a bug in your program. This line

if pastChains[term] is not None:

should be

if pastChains[term] is None:

2. Piecewise improvement

In this section I'm going to show how you can speed up a program like this by making a series of small piecewise improvements. This doesn't often work for the Project Euler problems: normally you have to come up with new algorithmic ideas. But here you're pretty close to getting under the minute mark, and that makes piecewise improvement a plausible approach.

Note that I'm using Python 2.7 here, which is generally faster than Python 3.

2.1. Base case

I'm going to start by putting your code into a form which is more convenien for testing (also, where we can run it for smaller limits than ten million, which can be convenient when making frequent measurement). Also, I'll improve the coding style while I'm about it (add docstrings; put local variables in lower_case_with_underscores; use the more meaningful values 1 and 89 instead of the Booleans False and True).

def square_digits(n):
    """Return the sum of squares of the base-10 digits of n."""
    total = 0
    for digit in str(n):
        total += int(digit) ** 2
    return total

def problem92a(limit):
    """Return the count of starting numbers below limit that eventually arrive
    at 89, as a result of iterating the sum-of-squares-of-digits.

    """
    arrive = [None] * limit # Number eventually arrived at, or None if unknown.
    arrive[1], arrive[89] = 1, 89
    for n in range(2, limit):
        chain = [n]
        while arrive[chain[-1]] is None:
            chain.append(square_digits(chain[-1]))
        for term in chain:
            if arrive[term] is None:
                arrive[term] = arrive[chain[-1]]
    return arrive.count(89)

>>> from timeit import timeit
>>> timeit(lambda:problem92a(10**7), number=1)
185.13595986366272

That's a lot slower than your reported "80 seconds": clearly you have a much faster machine than I do!

2.2. Avoiding number/string conversion

Now, let's do some work on the square_digits function. As written, this function has to convert the integer n to a string and then convert each digit back to a number. We could avoid these conversions by working with numbers throughout:

def square_digits(n):
    """Return the sum of squares of the base-10 digits of n."""
    total = 0
    while n:
        total += (n % 10) ** 2
        n //= 10
    return total

>>> timeit(lambda:problem92a(10**7), number=1)
61.409788846969604

Nearly under the minute already!

2.3. Second version

Here are some obvious minor improvements:

Avoid the repeated lookup of chain[-1] by remembering the value in a local variable.
Reduce the size of chain by one (because the last element in the chain is remembered in the local variable).
The test arrive[term] is None is unnecessary: by this point in the code we know that only the last term in chain was found in arrive.

That yields the following code:

def problem92b(limit):
    """Return the count of starting numbers below limit that eventually arrive
    at 89, as a result of iterating the sum-of-squares-of-digits.

    """
    arrive = [None] * limit # Number eventually arrived at, or None if unknown.
    arrive[1], arrive[89] = 1, 89
    for n in range(2, limit):
        chain = []
        while not arrive[n]:
            chain.append(n)
            n = square_digits(n)
        dest = arrive[n]
        for term in chain:
            arrive[term] = dest
    return arrive.count(89)

When transforming code like this, it's always important to check that our transformations didn't break anything. Here's where the ability to run the program for small values of limit comes in handy:

>>> all(problem92a(i) == problem92b(i) for i in range(1000, 2000))
True

And this yields a further 12% speedup:

>>> timeit(lambda:problem92b(10**7), number=1)
53.771003007888794

2.3. Third version

The largest number under ten million is 9999999, whose sum of squares of digits is just 567. All other numbers in the range have even smaller sums of squares of digits. So for 568 and up, there is no need to follow the chain: we can just look up the answer directly. That suggests the following approach:

def problem92c(limit):
    """Return the count of starting numbers below limit that eventually arrive
    at 89, as a result of iterating the sum-of-squares-of-digits.

    """
    sum_limit = len(str(limit - 1)) * 9 ** 2 + 1
    arrive = [None] * sum_limit
    arrive[1], arrive[89] = 1, 89
    for n in range(2, sum_limit):
        chain = []
        while not arrive[n]:
            chain.append(n)
            n = square_digits(n)
        dest = arrive[n]
        for term in chain:
            arrive[term] = dest
    c = arrive.count(89)
    for n in range(sum_limit, limit):
        c += arrive[square_digits(n)] == 89
    return c

Again, we better check that we didn't break anything:

>>> all(problem92a(i) == problem92c(i) for i in range(1000, 2000))
True

And this yields a further 30% improvement:

>>> timeit(lambda:problem92c(10**7), number=1)
37.56105399131775

That's about an 80% speedup on the original version, just by making piecewise and localized improvements.

3. New algorithm

For more radical improvements to the runtime, we need to rethink the algorithm completely. Here's a sketch of the combinatorial approach:

For each number k up to 567 that arrives at 89, we work out all the ways to partition k into a sum of no more than seven squares from 12 to 92 (using the partitioning technique we used in our answer to problem 76, say). This gives us a multiset of digits, and we can use combinatorial techniques to count the number of ways this multiset can be realised as a number up to 9999999.

For example, the starting number 4 arrives at 89. 4 can be partitioned into 22 or into 12 + 12 + 12 + 12. In the first case, there are 7 starting numbers in the range (2, 20, 200, 2000, 20000, 200000 and 2000000). In the second case, writing C(n, k) for the number of combinations of k items out of n, there are C(3,3) + C(4,3) + C(5,3) + C(6,3) = 35 starting numbers in the range (1111, 10111, 11011, and so on).

And here's some code, which I hope is self-documenting! Ask if you don't understand anything.

It uses the @memoized decorator from the Python Decorator Library to avoid unnecessary re-computation.

from collections import Counter
from math import factorial
from memoize import memoized

@memoized
def partitions(n, k, v):
    """Return partitions of n into at most k items from v, with
    repetition. v must be a tuple sorted into numerical order. Each
    partition is returned as multiset in the form of a Counter object
    mapping items from v to the number of times they are used in the
    partition.

        >>> partitions(4, 7, (1, 4))
        [Counter({1: 4}), Counter({4: 1})]

    """
    if n == 0:
        # Base case: the empty partition.
        return [Counter()]
    if k == 0 or len(v) == 0 or n < v[0]:
        # No partitions possible here.
        return []
    pp = [p.copy() for p in partitions(n - v[0], k - 1, v)]
    for p in pp:
        p[v[0]] += 1
    return pp + partitions(n, k, v[1:])

@memoized
def multinomial(n, k):
    """Return the multinomial coefficient n! / k[0]! k[1]! ... k[m]!.

        >>> multinomial(6, (2, 2, 2))
        90

    """
    result = factorial(n)
    for i in k:
        result //= factorial(i)
    return result

@memoized
def number_count(digit_counts, min_digits, max_digits):
    """Return the count of numbers (with between min_digits and max_digits
    inclusive) whose distinct non-zero digits have counts given by the
    sequence digit_counts. For example if we have three identical
    non-zero digits and four digits in total:

        >>> number_count((3,), 4, 4)
        3

    because the possible numbers resemble 1011, 1101, and 1110.
    Similarly

        >>> number_count((1,1), 4, 4)
        6

    because the possible numbers resemble 1002, 1020, 1200, 2001,
    2010, and 2100.

    """
    nonzero_digits = sum(digit_counts)
    total = 0
    for digits in range(max(min_digits, nonzero_digits), max_digits + 1):
        for i, d in enumerate(digit_counts):
            counts = (digit_counts[:i] + (d - 1,) + digit_counts[i+1:]
                      + (digits - nonzero_digits,))
            total += multinomial(digits - 1, tuple(sorted(counts)))
    return total

def problem92d(limit):
    """Return the count of starting numbers below limit that eventually arrive
    at 89, as a result of iterating the sum-of-squares-of-digits.

    """
    max_digits = len(str(limit - 1))
    assert(limit == 10 ** max_digits) # algorithm works for powers of 10 only
    sum_limit = max_digits * 9 ** 2 + 1
    arrive = [None] * sum_limit
    arrive[1], arrive[89] = 1, 89
    for n in range(2, sum_limit):
        chain = []
        while not arrive[n]:
            chain.append(n)
            n = square_digits(n)
        dest = arrive[n]
        for term in chain:
            arrive[term] = dest
    total = 0
    squares = tuple(i ** 2 for i in range(1, 10))
    for n in range(2, sum_limit):
        if arrive[n] == 89:
            for p in partitions(n, max_digits, squares):
                total += number_count(tuple(sorted(p.values())), 1, max_digits)
    return total

As usual, we need to check that this radically new approach computes the right results:

>>> all(problem92c(10**i) == problem92d(10**i) for i in range(3, 7))
True

Note that since we are using memoization, for fair timing results we must reload the code before running the timer (otherwise we'll get misleadingly fast times due to some of the computation having previously been done and stored in the memoization caches). But with a fresh Python instance:

>>> timeit(lambda:problem92d(10**7), number=1)
0.7318389415740967

Less than a second! I hope that it's clear now how important the choice of algorithm is. With the piecewise improvement approach the code became about five times faster (which is pretty decent result). But with a better algorithm the code is 250 times faster.

4. Questions in comments

You asked some questions in the comments.

Obviously in Python 3 you'd use range, not xrange. And in fact, even in Python 2.7, substituting xrange for range makes hardly any difference to the runtime (a percent or so), so in my revised answer I've eschewed that.
In Python, small integer objects like 1 and 89 are shared and reused:

>>> x = 89
>>> x is 8900 / 100
True

If you look at the source code for longobject.c, you'll see that numbers from NSMALLNEGINTS (−5) up to NSMALLPOSINTS-1 (256) are preallocated in an array. So there is no memory penalty to using them.
Iteration works in Python by fetching successive values from an "iterator" object (here the object returned by range(2, limit)), not by incrementing the loop variable as you might do in languages like C. So there is no need to be scared about updating the loop variable.
Yes, that's right. Project Euler asks "How many starting numbers below ten million will arrive at 89?" (my emphasis). So it's convenient to use limit like this.
Probably a copy-paste mistake on my part. I've revised the answer and hopefully the sequence of improvements is clearer now.


Unnecessary storage

The current implementation basically processes the two input arrays in parallel, taking the next smaller element from the appropriate array, and appending to ar,
until either the end of an input array is reached,
or the counter i reaches half.

If the end of an input array was reached,
the remaining elements of the other array are appended to ar.
The size of ar becomes len.
If the counter i reaches half, then nothing is appended to ar.
At this point the size of ar is half.

Then, the median function doesn't actually pick the median of the array parameter,
but it computes a value based on the len parameter.

The ar array is unnecessary storage. In the end, only the half-th and possibly the half + 1-th values will be used.

The benefit of the ar storage is it makes it simple to compute the median in the case when the end of an input array was reached.
In this case, without ar, it's not trivial to find the correct index.

An alternative algorithm is possible without extra storage:
- pick values from the input arrays
- if we haven't reached yet either end, take the next smaller element
- otherwise, if we reached the end of the first array, take the next element from the second
- otherwise, if we reached the end of the second array, take the next element from the first
- before the above conditions, save the previously picked value in a variable

After counting until half elements, you can compute the value of the median based on len, prev and current.

Like this:

  for (let i = 0, r1 = 0, r2 = 0; i <= half; i++) {
    prev = current;
    if (r1 < nums1.length && r2 < nums2.length) {
      if (nums1[r1] < nums2[r2]) {
        current = nums1[r1++];
      } else {
        current = nums2[r2++];
      }
    } else if (r1 < nums1.length) {
      current = nums1[r1++];
    } else if (r2 < nums2.length) {
      current = nums2[r2++];
    }
  }

  if (prev === undefined) {
    return current;
  }

  if (len % 2 === 0) {
    return (prev + current) / 2;
  }
  return current;

Unnecessary computation

The median helper function recomputes half.
You could simply reuse the half variable that's already computed in the closure.

The loop condition i <= len is unnecessary.
It will always be true,
because the implementation in the loop body guarantees that i will never reach len.

Return value in case of empty input arrays

In case of empty input arrays,
the implementation returns NaN.
I'm not sure that's correct, or expected.
Unless the exercise specifies to return NaN in this case,
I think returning undefined makes more sense.


Checking the validation of a field through the Property mutator method `set`

No one has yet addressed the fact that you have chosen the wrong tool for the job.

What does Config represent? It looks like you should have chosen an enum where you chose an int.

public enum ConfigurationType
{
    None = 0,
    Type1 = 1,
    Type2 = 2,
    // etc.
}

You can get what ConfigurationType an int corresponds to quite easily:

ConfigurationType configType = (ConfigurationType)someInteger;

Then, you can validate it more fluidly:

public ConfigurationType Config
{
    get
    {
        return _config;
    }
    set
    {
        if (ConfigurationType.IsDefined(typeof(ConfigurationType), value))
        {
            _config = value;
        }
        else
        {
            throw new InvalidEnumArgumentException($"The specified value must be a valid value of the {nameof(ConfigurationType)} enumeration.");
        }
    }
}
private ConfigurationType _config; // the actual field that the property 'Config' refers to.

If you don't have C#6.0, your exception would be more along the lines of:

throw new InvalidEnumArgumentException(string.Format("The specified value must be a valid value of the {0} enumeration.", ConfigurationType.ToString()));

You should also consider a refactoring of the name Config, it's non-descriptive. There is likely a better name for such a property/field combination. (Had we more context we would likely be able to suggest better names.)


Validate URL with Regex and java.net.URL

Welcome to Code Review!

Bad naming

The method doesn't actually check that a URL is valid, as it allows values that are not URLs. Thus it should not be named as "isValidURL". It should be isValidAddress or similar, but not URL. URLs have a very well defined syntax.

Unnecessary variables

There is no need to store the return value of specialCharactersExists(url) to a variable. It only adds an unnecessary large if-statement. Instead, check the value and exit early. Also, the characters you check are not special, they're "illegal" or "invalid" in your implementation so change name to reflect that:

    if (illegalCharactersExist(url)) {
        return false;
    }

Missing final

Variables that are not supposed to change should be marked as final. If you decide to keep the containsSpecialCharacters variable, it should be marked as final (and named as decribed above).

final boolean containsIllegalCharacters = illegalCharactersExist(url);

Reuse Pattern

The Pattern class is thread safe. You should compile the pattern once into a static variable and reuse it in the matcher. Also, regex is not a good name. It tells what the variable is, not what it's purpose is. Naming should always reflect purpose.

    private static final Pattern ILLEGAL_CHAR_PATTERN = Pattern.compile("[^A-Za-z0-9.-]");


Subdividing a 4×4 array into four 2×2 arrays

Flatten and copy array values

You can flatten your fourByFour array and then populate the twoByTwo array:

private static object convertArray1()
{
    int[,] fourByFour = new int[,]{
                        {1,2,3,4 },
                        {12,13,14,15 },
                        {21,22,23,24 },
                        {31,32,33,34 } };

    int[] arrayElems = new int[fourByFour.GetLength(0) * fourByFour.GetLength(1)];

    Buffer.BlockCopy(fourByFour, 0, arrayElems, 0, arrayElems.Length * sizeof(int));

    int[][,] twoByTwo = new int[4][,];

    int elemIndex = 0;
    for (int i = 0; i < twoByTwo.GetLength(0); i++)
    {
        twoByTwo[i] = new int[2, 2];

        for (int j = 0; j < twoByTwo[i].GetLength(0); j++)
        {
            for (int k = 0; k < twoByTwo[i].GetLength(1); k++)
            {
                twoByTwo[i][j, k] = arrayElems[elemIndex];
                elemIndex++;
            }
        }
    }

    return twoByTwo;
}

This solution has a performance penalty - Buffer.BlockCopy copies the source array fourByFour an extra time (which will also require additional memory).

Dimension counter class

Use a helper class to keep track of and increment index positions in each dimension.

This solution is more generic.

private static object convertArray2()
{
    int[,] fourByFour = new int[,]{
                        {1,2,3,4 },
                        {12,13,14,15 },
                        {21,22,23,24 },
                        {31,32,33,34 } };

    DimensionCounter dim1 = new DimensionCounter(4, 4);
    DimensionCounter dim2 = new DimensionCounter(4, 2, 2);

    int[][,] twoByTwo = new int[4][,];

    for (int i = 0; i < dim1.Capacity; i++)
    {
        int val = (int)fourByFour.GetValue(dim1.Current);

        if (twoByTwo[dim2.Current[0]] == null)
        {
            twoByTwo[dim2.Current[0]] = new int[dim2.DimensionSizes[1], dim2.DimensionSizes[2]];
        }

        twoByTwo[dim2.Current[0]][dim2.Current[1], dim2.Current[2]] = val;

        if (i + 1 < dim1.Capacity)
        {
            dim1.Next();
            dim2.Next();
        }
    }

    return twoByTwo;
}

private class DimensionCounter
{
    private readonly int[] dimSizes;
    private readonly int capacity;

    private int[] dimIndexes;
    private int position;

    public int[] DimensionSizes { get { return dimSizes; } }
    public int[] Current
    {
        get { return dimIndexes; }
    }
    public int Position { get { return position; } }
    public int Capacity { get { return capacity; } }
    public DimensionCounter(params int[] dimSizes)
    {
        this.dimSizes = dimSizes;
        this.capacity = dimSizes.Aggregate(1, (a, b) => a * b);
        this.dimIndexes = new int[this.dimSizes.Length];
    }

    public int[] Next()
    {
        for (int i = dimIndexes.Length - 1; i >= 0; i--)
        {
            if (dimIndexes[i] < dimSizes[i] - 1)
            {
                dimIndexes[i]++;
                break;
            }
            else
            {
                for (int j = i; j < dimIndexes.Length; j++)
                {
                    dimIndexes[j] = 0;
                }

                if (i == 0)
                {
                    throw new IndexOutOfRangeException("Cannot increment indexes any more!");
                }
            }
        }
        position++;
        return dimIndexes;
    }

    public void Reset()
    {
        dimIndexes = new int[dimIndexes.Length];
        position = 0;
    }
}


C++ algorithm to implement multiple operators in one

Make use of the standard library

This looks like a job for std::transform(), which in many other languages would be the equivalent of the map function. It works on iterators, so it would help if your class provides begin() and end() functions, or if internally you store values in a std::vector. However, the data pointer can be used as well.

Then, it's possible to write your operators like so:

#include <algorithm>

IntMatrix IntMatrix::operator<(int num) const {
    IntMatrix result(width(), height()); // assuming this creates a matrix with the same dimensions
    const auto count = width() * height();
    std::transform(data, data + count, result.data,
                   [num](int value){return value < num;});
    return result;
}

With this, there is no need to write your own filter() function.


Program that requests two floating-point numbers and prints the value of their difference divided by their product

For a newbie, your solution is ok. You can simplify it quite a bit though by using the function fabs. It's a fairly simple function: fabs(x) returns x if x >= 0, and returns -x if x < 0. So all of your if checks can be replaced by the line:

different = fabs(num1 - num2);

Note that you need to #include <math.h> for this.

Also, what happens if one of your numbers is 0? Then your product will also be 0 and your program will crash. It's always a good idea to add some basic checks. Unfortunately, with floating point numbers this can be a bit tricky, so simply doing something like:

if(product == 0.0) { ... }

may not actually work! Fixing this might be a bit tricky for someone who is just learning, but it's something to always keep in mind.

Finally, main should return an int - generally, 0 is what is returned if everything works as it should - so down the bottom should be a return 0;.


Circular Queue Implementation Using Array in java

Since this is in Java and for academic purposes, my guess would be that you're using Eclipse. Hit CTRL+SHIFT+F to format your code, and move any lines that it chops up back into one where appropriate. The key binding is similar for Netbeans and IntelliJ's IDEA.
Too many useless variables. You have a bad habit of creating variables for one off-uses, e.g. in elementOrElements(). Just return what send equals. If you want to describe what the code does, then use comments (which you don't use at all in your implementation btw).
If the functionality isn't obvious, then use comments. Most programmers can discern what generic methods and operations look like, but when in doubt comment. Remember that you're writing software primarily for other humans. This is critical in a professional environment.
Delete elementOrElements(). In the single print call in which it's used, just say, The contents of the Queue are:.
Consistent variable naming is required. You have tmpfront, which should be tempFront according to Java conventions. Not a huge deal since you seem to adhere to this throughout the program. I add the extra e out of personal preference (and the fact that it helps other humans ever so slightly).
Use proper Queue naming conventions, i.e. use enqueue instead of enQueue. Creating a couple of helper methods like isEmpty() and isFull() would help your implementation in a couple of places.
Move CircularQueue to CircularQueue.java and add some public modifiers to your methods. This is mainly for convention training.


Finding unique values in an array with possible nested arrays

When we use recursion, we are building an algorithm based on an implicit1 stack, the function call stack. We are pushing and popping elements, essentially every time we recurse down and back up. If we make the stack explicit in our code, we can make the method iterative instead, and skip the recursive calls.

A pseudocode version of the algorithm would look like:

function findOccurences(array):
    ensure array is actually an array

    set occurrences to empty object

    create a stack
    push all elements in array onto the stack
    while the stack is not empty:
        if pop returns an array:
            push all of the elements in the popped array onto the stack
            continue while

        process the popped element

    return occurrences

A nice feature in Javascript arrays is that they have stack semantics2, so we can just use a built in array as our stack. We can actually get a little more clever than that, though. Since we are putting every element from one array into our stack, which is also an array, we could just as effectively clone the original array and use that as our stack3. We can then just translate the pseudocode into Javascript.

function findOccurrences(array) {
    if(!Array.isArray(array)) {
        return {};
    }

    var occurrences = {};

    // JS arrays don't have a native clone method so
    // Array#slice with a start point of 0 creates
    // a copy of our array. If handling in the same
    // order is important, tack a .reverse() at the
    // end.
    var stack = array.slice(0);

    while(stack.length !== 0) {
        var nextElement = stack.pop();
        if(Array.isArray(nextElement)) {
            // We use some fanciness here so we don't have
            // to write out an explicit loop
            [].push.apply(stack, nextElement);
            continue;
        }

        occurrences[nextElement] = occurrences[nextElement] + 1 || 1;
    }

    return occurrences;
}

1 Implicit to our algorithm, explicit to our runtime.

2 Array#push, Array#pop

3 We could just use the original array, but since it is passed by reference, we are going to corrupt the array in the process and that could lead to unwanted side effects.


Ruby on rails complex select statement...there has to be a better way!

First of all be very careful when building queries from request data. When building your where string, you're using parametrized queries, so that's fine. But in your order string, you're building actual SQL code from data you directly take out of params. Do NOT do that. This is a big risk of SQL injection.

What you should do instead is take the order string apart (splitting at the comma), then check that each element is a column name optionally followed by ASC or DESC, then put the string back together accordingly.

A method for this could look something like this:

def safe_order_from_param(collection, order_string)
  tokens = order_string.split(/ *, */)
  tokens.each do |token|
    column, direction = token.match(/^(\w+)(? +(\w+))?$/).captures
    if Patient.column_names.include? column
      # if direction is nil, it will just be turned into the empty
      # string, so no problem there
      collection = collection.order("#{column} #{direction}")
    else
      raise ArgumentError, "No such column: #{column}"
    end
  end
end

Second of all defined?(params[:foo]) does is check that params is defined. It does not check that params is a hash and has the key :foo. Since params will always exist, there really is no need for the check. Checking params[:foo].blank? is entirely sufficient.

Now to the main part of your question: the ugly building of the query string:

I assume from the fact that you're using the joins method that this is rails 3. In rails 3 options like :include, :conditions and :order are deprecated in favor of the corresponding query methods. The beauty of those methods is that they're chainable. So instead of building a query string using concatenation, it is much cleaner to just chain together multiple wheres.

if is_admin?
  @patients = Patient.joins(:user).includes(:user).order('users.last_name ASC')

  unless params[:username].blank?
    @patients = @patients.where("users.username LIKE ?", params[:username])
  end

  # same for :last_name

  unless params[:active].blank?
    @patients = @patients.where("users.active" => params[:active])
  end

  # same for disabled

  unless params[:order_by].blank?
    @patients = safe_order_from_param(@patients, params[:order_by])
  end

  # same for order

  per_page = params[:per_page].blank? ? 20 : params[:per_page].to_i
  @patients = @patients.paginate(:page => params[:page], :per_page => per_page)
end


or should i write session initialization code every time when i want to execute query?

Sessions are meant to be re-used. You should not initialize a new method every time. I believe HibernateUtil.getSessionFactory().getCurrentSession(); is OK to use as that re-uses an existing session.

As for the rest of your code, I have a couple of things to say.

public static Query getHqlQueryObject(String hql) throws Exception
public static List<Object> getHqlQueryData(Query query) throws Exception

Neither of these methods actually throw an Exception. Because you have this code:

} catch (Exception e) {
    System.out.println("Exception in Helper::...");
    e.printStackTrace();
}

THIS IS BAD! A caller of your method has absolutely no guarantee that nothing went wrong. And is therefore unable to tell any potential user of your application that something did went wrong.

Simply printing the stacktrace accomplishes nothing for robustness!

Additionally, the only exception that should be able to occur is HibernateException, there is no need to catch all the other ones.

Additionally, there is no need for Transaction transaction = session.beginTransaction(); in your getHqlQueryObject. The transaction is only needed when the Query is performed, not when it is created.

Likewise, your getHqlQueryData are not guaranteed that a transaction is active.

I also have to question the usefulness of your code. Hibernate is already an abstraction and does a great job in simplifying things. What is it exactly you want to make easier with these methods?

Your current code:

Query query = Helper.getHqlQueryObject("from BookDetails where pkBookId = :pkBookId");
query.setParameter("pkBookId", new Long(1));
List dataList = Helper.getHqlQueryData(query);

How I would do it:

try {
    Query query = session.createQuery("from BookDetails where pkBookId = :pkBookId");
    query.setParameter("pkBookId", 1L);
    Transaction transaction = session.getTransaction();
    List<?> dataList = query.list();
    transaction.commit();
}
catch (HibernateException ex) {
    if (transaction.isActive()) {
         transaction.rollback();
    }
    // either re-throw the exception, wrap in another exception, or return a value indicating error.
}

You do have a session variable available here already, I hope? If not, you should remember the principle Tell, don't ask. That is, if your code needs some information, you should pass it there. I generally don't recommend calling a public static method or similar to get access to it.

By the way, I am not so sure that query.list(); needs an active transaction.

Also, when a transaction fails (an exception is thrown), you should call transaction.rollback();

And the last thing I'd like to say, with your HQL being "from BookDetails where pkBookId = :pkBookId" there is no need to return a list, you can instead call the method uniqueResult


"""Gives the class it's core functionality."""
def __call__(self, *args):
    if args not in self._memos:
        self._memos[args] = self._function(*args)
    return self._memos[args]

It's more idiomatic (and faster) to "ask for forgivness rather than ask for permission", e.g.:

def __call__(self, *args):
    try:
        return self._memos[args]
    except KeyError:
        value = self._function(*args)
        self._memos[args] = value
        return value


Functional K-way merge (Merge k sorted sequences)

I've been thinking about this question literally for weeks.

It always seemed like this implementation suffers from "hammer syndrome" - if you have a really good hammer, you want to think every problem is a nail. Functional coding is a great tool for many things, but in this case, it has a problem.

The reason why algorithms use mutable rather than immutable heaps is asymptotic performance. In a heap sort, a mutable heap gives you log n time per element, but an immutable heap would have n time per element (because you have to copy the entire heap of size n in addition to doing the re-heap with time log n).

Similarly, a k-way merge implemented with immutable heaps will run in nk time, rather than n log k time. That's fine if k is small but even with modest sized k, will become a significant performance hit.

It would have been helpful if test cases had been included so that reviewers could take a look at performance numbers with various possible changes.

The code itself looks fine to me. The fact that nobody wrote a review could mean that most people who read the question came to the same conclusion. As an academic exercise, I say, good job. In the long run, I doubt such an algorithm is genuinely useful except in truly unusual circumstances


Python generator function that yields combinations of elements in a sequence sorted by subset order

Rather converting from tuple to list and back again, construct a new tuple by adding to it.

def combinations_by_subset(seq, r):
    if r:
        for i in xrange(r - 1, len(seq)):
            for cl in combinations_by_subset(seq[:i], r - 1):
                yield cl + (seq[i],)
    else:
        yield tuple()


Deleting a node from Binary Tree in OCaml

Syntax

There are no hard rules on this, but I would change a couple things in your syntax:

avoid ' in identifiers. Especially in that case, you can use tree instead of tree' (there is no collision with the type name).
usually there's a space before constructor arguments, like Node (left, nodeValue, right) (really up to convention, but I'd say it's more common).
parentheses around function application are not necessary, so it's more idiomatic to write Node (deleteNode left value, nodeValue, right). You can also extract a binding: let newLeft = deleteNode left value in Node (newLeft, nodeValue, right)
camel case is rare is ocaml (I know, that's weird!). For example the standard library uses names like print_endline etc. So, I'd use node_value, etc.

Replace conditionals with guards

Every time a if is in a pattern matching branch, you can extract it into a guard. The syntax is | pattern when condition ->.

By applying this to the first ifs, we can arrive to this state:

let rec deleteNode tree' value =
  match tree' with
  | Empty -> Empty
  | Node (left, nodeValue, right) when value < nodeValue ->
        Node((deleteNode left value), nodeValue, right)
  | Node (left, nodeValue, right) when value > nodeValue ->
        Node(left, nodeValue, (deleteNode right value))
  | Node (left, nodeValue, right) ->
      if left = Empty && right = Empty then
        Empty
      else if left = Empty then
        right
      else if right = Empty then
        left
      else
        let newValue = minValue right in
        Node(left, newValue, (deleteNode right newValue))

Use deep pattern matching

You can replace the x = Empty tests by pattern matching. In other words, patterns can contain patterns. By applying this to all the conditionals, we get:

let rec deleteNode tree' value =
  match tree' with
  | Empty -> Empty
  | Node (left, nodeValue, right) when value < nodeValue ->
        Node((deleteNode left value), nodeValue, right)
  | Node (left, nodeValue, right) when value > nodeValue ->
        Node(left, nodeValue, (deleteNode right value))
  | Node (Empty, nodeValue, Empty) ->
        Empty
  | Node (Empty, nodeValue, right) ->
        right
  | Node (left, nodeValue, Empty) ->
        left
  | Node (left, nodeValue, right) ->
        let newValue = minValue right in
        Node(left, newValue, (deleteNode right newValue))

 Remove redundant cases

That's more obvious with pattern matching, but the Node (Empty, nodeValue, right) cases also applies when right = Empty, so we can delete the more specific Node (Empty, nodeValue, Empty) case.

That's about it! Have a nice journey exploring OCaml.


Servlet responses to upload/download requests

if (info.isUpload()) {
        if (result.isValid()) {
            wrap = new InfoWrapperImpl1(info, result);
        } else {
            wrap = new InfoWrapperImpl2(info, result);
        }
    } else {
        wrap = new InfoWrapperImpl1(info, result);
    }

Can be written as:

   if (!info.isUpload() || result.isValid()) {
        wrap = new InfoWrapperImpl1(info, result);
    } else {
        wrap = new InfoWrapperImpl2(info, result);
    }

Also InfoWrapperImpl1 and InfoWrapperImpl2 are not very good names, as they are not expressive and look quite similar to each other.


I suspect one bottleneck is the grouping in the subqueries. For each row in the ianc_fons table the rows are being duplicated - one for each join the chile table ianc_unicat. And then they are being grouped again.

I would try a different approach and move the summing into a joined subquery :

SELECT f.codi_arxiu,
       f.nom_arxiu,
       f.codi_grup,
       f.codi_fons_refe,
       f.codi_fons,
       f.nom_fons,
       f.any_ini,
       f.any_fi,
       f.cronologia,
       child.num_uni_imatges,
       child.unitats_text
 FROM ianc_fons f
 LEFT JOIN
      ( SELECT codi_arxiu, codi_fons,
               SUM (CASE WHEN u.num_imatges > 0 THEN 1 ELSE 0 END) AS num_uni_imatges,
               COUNT (u.codi_unitat) AS unitats_text
      FROM ianc_unicat u
      WHERE u.anc_unicat_id IN
                    (SELECT u.anc_unicat_id
                       FROM ianc_unicat u
                      WHERE     1 = 1
                            AND u.codi_arxiu = '1'
                            AND u.codi_grup = 'ANC'
                            AND u.codi_fons = 1)
      GROUP BY codi_arxiu, codi_fons
      ) AS child ON f.codi_arxiu = child.codi_arxiu AND f.codi_fons = child.codi_fons

Note I have removed the DISTINCT from the Select from ianc_unicat. As this is just used in the filtering, the DISTINCT is not necessary. Using DISTINCT is always an overhead as the query engine needs to sort the records and then compare them all one by one to remove duplicates. Avoid as much as possible.

Each side of the UNION is selecting from the same table, and then summing different child records.

As the summing is now in a separate joined subquery, you can get rid of the union, and just join in the other table in a similar fashion. The Select will now need to sum up the totals from both joined subqueries.

SELECT f.codi_arxiu,
       f.nom_arxiu,
       f.codi_grup,
       f.codi_fons_refe,
       f.codi_fons,
       f.nom_fons,
       f.any_ini,
       f.any_fi,
       f.cronologia,
       COALESCE(child.num_uni_imatges, 0) + COALESCE(child2.num_uni_imatges, 0) AS num_uni_imatges,
       child.unitats_text,
       child2.unitats_notext

Note the use of COALESCE. As you are left joining these totals now, there is a chance that the values may be Null. Null is not the same as 0. Null + 4 would not equal 4, the result would be Null. So the COALESCE is necessary to make Null values 0.

Now you no longer need the outer select as this was being used to just sum the rows within the union. You can get rid of this all together. The row joined in to the outer query can just be joined into our main query. We no longer the the extra grouping here either. This gives us a query similar to the following :

SELECT f.codi_arxiu,
       f.nom_arxiu,
       f.codi_grup,
       f.codi_fons_refe,
       f.codi_fons,
       f.nom_fons,
       f.any_ini,
       f.any_fi,
       f.cronologia,
       COALESCE(child.num_uni_imatges, 0) + COALESCE(child2.num_uni_imatges, 0) AS num_uni_imatges,
       child.unitats_text,
       child2.unitats_notext
 FROM ianc_fons f
 LEFT JOIN
      ( SELECT codi_arxiu, codi_fons,
               SUM (CASE WHEN u.num_imatges > 0 THEN 1 ELSE 0 END) AS num_uni_imatges,
               COUNT (u.codi_unitat) AS unitats_text
      FROM ianc_unicat u
      WHERE u.anc_unicat_id IN
                    (SELECT u.anc_unicat_id
                       FROM ianc_unicat u
                      WHERE     1 = 1
                            AND u.codi_arxiu = '1'
                            AND u.codi_grup = 'ANC'
                            AND u.codi_fons = 1)
      GROUP BY codi_arxiu, codi_fons
      ) AS child ON f.codi_arxiu = child.codi_arxiu AND f.codi_fons = child.codi_fons

 LEFT JOIN
      ( SELECT u.nt1_codi_arxiu, u.codi_fons,
               SUM (CASE WHEN u.num_imatges > 0 THEN 1 ELSE 0 END) AS num_uni_imatges,
               COUNT (u.nt1_unitat) AS unitats_notext
        FROM ianci_unicats u
       WHERE     1 = 1
             AND u.nt1_id IN
                    (SELECT u.nt1_id
                       FROM ianci_unicats u
                        WHERE     1 = 1
                            AND u.nt1_codi_arxiu = '1'
                            AND u.codi_grup = 'ANC'
                            AND u.nt1_codi_fons = 1)
      ) AS child2 ON f.codi_arxiu = child2.nt1_codi_arxiu AND f.codi_fons = child2.nt1_codi_fons

  LEFT JOIN v_ianc_arxius_order v ON ( f.codi_arxiu=v.codi_arxiu )
  ORDER BY v.order_ANC,v.order_ANC2,f.nom_arxiu, f.nom_fons


I think the best way to implement fftshift is like it is done in OpenCv itself:

//The body is inspired from OpenCV, the fct_fftshift is mine
void fct_fftshift(cv::Mat& src)
{
    int cx = src.cols/2;
    int cy = src.rows/2;

    cv::Mat q0(src, cv::Rect(0, 0, cx, cy));
    cv::Mat q1(src, cv::Rect(cx, 0, cx, cy));
    cv::Mat q2(src, cv::Rect(0, cy, cx, cy));
    cv::Mat q3(src, cv::Rect(cx, cy, cx, cy));

    cv::Mat tmp;
    q0.copyTo(tmp);
    q3.copyTo(q0);
    tmp.copyTo(q3);

    q1.copyTo(tmp);
    q2.copyTo(q1);
    tmp.copyTo(q2);
}

int main(int argc, char* argv[])
{
  cv::Mat src=cv::imread(argv[1], CV_8UC1);
  fct_fftshift(src); //it will change your original image
}


Getting data from database and then outputing it to the DataGridView

Even if you are stuck with the old/wrong technology (all the cool kids use ORMs these days), you can still dance around that and simplify the act of interfacing with it with clean code of your own. I like having a direct link to the database because I can do whatever the hell I want with it. If the author of an ORM library and I had different goals and priorities, then this library will actually hinder my productivity. Disclaimer: I have little experience with ORMs.

Anyhow, try using lots of smaller functions.
Try passing everything a method needs in - that way you can move this code into a separate class.
Use exception handling to signal that something went wrong. Do not use the MessageBox / return null combination. You might be running this code as a windows service where showing Message Boxes is not allowed. It also does not feel right. The error handling / display effort needs to be well thought out, not copy /pasted into every method.
Features are assets; code is liability, hence less code is more.
Try using descriptive variable/method/etc. names.

public DataTable RunQueryExpectOneTable(
    string queryText,
    string[] expectedColumnNames = null)
{
    return RunQueryExpectOneTable(
    connectionString: this.ConnectionString,
    queryText: queryText,
    expectedColumnNames: expectedColumnNames);
}

public static DataTable RunQueryExpectOneTable(
    string connectionString,
    string queryText,
    string[] expectedColumnNames = null)
{
    // Even the body of this using method is too long, it can be further split up into methods.
    using(var connection = new OleDbConnection(connectionString))
    {
        // Opens it and makes sure that it is open.
        this.OpenAndCheckConnection();
        var adapter = new OleDbDataAdapter(query, conn);
        DataSet ds = new DataSet();
        adapter.Fill(ds);

        // if more than one table, then throw an exception.
        if (expectedColumnNames != null)
        {
            // Also check the number and the names of the columns
        }

        return ds.Tables[0];
    }
}


Bash function to create the next SVN release tag

Here is a small improvement.

svnnextrelease() {
  v=$(svn ls ^/tags/release | tail -n 1)
  svn cp ^/trunk ^/tags/release/$((${v%/} + 1 )) -m "$@"
}

You can use $(( )) as a replacement for expr in bash. (depends on your taste.)
bash can cheaply get rid of / for you.


Curiously Recurring Template Pattern (CRTP) implementation

As far I can tell this doesn't generate any warnings:

class IntValue implements Value<IntValue> {

    Integer value;

    public IntValue(Integer value) {
        this.value = value;
    }

    public Integer getValue() {
        return value;
    }

    @Override
    public IntValue deepCopy() {
        return new IntValue(value);
    }
}

class ArrayValue<T extends Value<T>> implements Value<ArrayValue<T>> {

    List<T> values;

    public ArrayValue() {
        this(new ArrayList<>());
    }

    public ArrayValue(List<T> values) {
        this.values = values;
    }

    @Override
    public ArrayValue<T> deepCopy() {
        List<T> copyValues = new ArrayList<>();
        values.forEach(value -> copyValues.add(value.deepCopy()));
        return new ArrayValue<>(copyValues);
    }
}

But it makes more sense to do it without CRTP:

public interface Value<T> {
    Value<T> deepCopy();
    T getValue();
}

class IntValue implements Value<Integer> {

    Integer value;

    public IntValue(Integer value) {
        this.value = value;
    }

    @Override
    public Integer getValue() {
        return value;
    }

    @Override
    public IntValue deepCopy() {
        return new IntValue(value);
    }
}

class ArrayValue<T> implements Value<List<Value<T>>> {

    List<Value<T>> values;

    public ArrayValue() {
        this(new ArrayList<>());
    }

    public ArrayValue(List<Value<T>> values) {
        this.values = values;
    }

    @Override
    public ArrayValue<T> deepCopy() {
        List<Value<T>> copyValues = new ArrayList<>();
        values.forEach(value -> copyValues.add(value.deepCopy()));
        return new ArrayValue<>(copyValues);
    }

    @Override
    public List<Value<T>> getValue() {
        return values;
    }
}

class X {

    public static Value sub(Value<Integer> a, Value<Integer> b) {
         return new IntValue(b.getValue() - b.getValue());
    }
}


A simple Pygame that has a square move and jump

Few areas to improve:

self.vel. When I first saw this field/variable I thought: "What is that?".I guess many other people may have the same first feeling.I would rename it to an explicit and clear name: self.velocity
self.isJump and self.jumpCount violate Python naming conventions (instance variable names should be all lower case, words in an instance variable name should be separated by an underscore). Therefore, rename them to:

...
self.is_jump
self.jump_count`

run = True flag. That flag for just controlling the while loop flow is redundant.We're just starting the loop with while True: and breaking it with break.So it becomes:

def run(self):
    while True:
        pygame.time.delay(30)
        keys = pygame.key.get_pressed()

        # handling pressed keys
        if keys ...

        win.fill((0, 0, 0))
        self.draw()

        for event in pygame.event.get():
            if event.type == pygame.QUIT:
                break
    pygame.quit()


Use .Where extension method to filter the records you need:

foreach(ModelClass item in listObj.Where(x => x.id < 0))
{
    // code to create a new Obj in the database
}


Your code

(define (enumerate-integers start end)
  (if (>= start end)
      (list end)
      (cons start (enumerate-integers (+ 1 start) end))))

(define (unique-pairs n)
  (flat-map (lambda (i)
              (map (lambda (j) (list i j))
                   (enumerate-integers 1 (- i 1))))
            (enumerate-integers 2 n)))

looks fine to me. If you want to massage some details, you could rewrite enumerate-integers e.g. to:

(define (enumerate-integers start end)
  (if (> start end) '()
      (cons start (enumerate-integers (+ 1 start) end))))

which is cleaner because you don't have (list end) as a special case, and you can correctly produce an empty list of integers if start > end.

If you want to be even cleaner, you can do:

(define (enumerate-integers start end)
  (define (iter n)
    (if (> n end) '()
        (cons n (iter (+ n 1)))))
  (iter start))

This is a good pattern in case of more complex procedures.

Your flat-map is more complex than needed, your code:

(define (flat-map f seq)
  (accumulate append
              null
              (map (lambda (x) (f x)) seq)))

can be replaced with:

(define (flat-map f seq)
  (accumulate append
              null
              (map f seq)))

because (lambda (x) (f x)) is equal to f.


Scheduling Algorithm : First Come First Serve

int process;

  public:
    Scheduling() : process(0) {}
    Scheduling(int n) : process(n) { /* ... */ }

When initializing constant values for member data, prefer in-class member initializers.

Use reasonably descriptive names. For new users, is process a count? An id? They have to dig through the code to understand that it represents the number of processes stored.  Do you need process? Can you get by with the size of burst_time?

    std::size_t process_count {0};

  public:
    Scheduling() = default;

    Scheduling(std::size_t number_of_processes)
    : process_count (number_of_processes)
    { /* ... */ }

    std::vector<int> burst_time;
    std::vector<int> waiting_time;

burst_time and waiting_time would benefit from stronger typing.  Prefer duration types over int.

        for (int i = 0; i < n; i++)
        {
            std::cout << "Enter Burst time for process " << i+1 << " : ";
            int val;
            std::cin >> val;
            burst_time.push_back(val);
        }

Compiling with warnings would have caught the signed/unsigned comparison mismatch.

You should check to see if reading from std::cin was successful.

    Scheduling(const Scheduling &other)
    {
        process = other.process;
        int size = other.burst_time.size();

        for (int i = 0; i < size; i++)
        {
            burst_time.push_back(other.burst_time[i]);
        }
    }

When you need to initialize a member variable with a non-constant value in a constructor, prefer initialization instead of assignment.

    Scheduling(const Scheduling &other)
    : process{other.process}
    , burst_time{other.burst_time}
    , waiting_time{other.waiting_time}
    {}

Since each member variable has a well-defined copy behavior, we can use the compiler-generated copy constructor.

    Scheduling(const Scheduling &) = default;

    Scheduling &operator=(const Scheduling &other)
    {
        process = other.process;
        int size = other.burst_time.size();

        for (int i = 0; i < size; i++)
        {
            burst_time.push_back(other.burst_time[i]);
        }

        return *this;
    }

Use const to define objects with values that do not change after it has been constructed.

        const int size = other.burst_time.size();

Your copy assignment is actually a copy append.  You forgot to clear the existing list before copying.

Obey the rule of five/zero.  In C++, there are six special member functions.  Ignoring the default constructor, the rules cover the following member functions:

Destructor
Copy Constructor
Move Constructor
Copy Assignment Operator
Move Assignment Operator

The rule of five is simple.  If you define, =default, or =delete any of these five special member functions, you should consider how the other special member functions operate in the class.

The rule of five defaults goes a step further.  If you define, =default, or =delete any of the five special member functions, then explicitly define, =default, or =delete them all.

    class Scheduling {
        ...
      public:
        Scheduling() = default;
        Scheduling(int);

        ~Scheduling() = default;
        Scheduling(Scheduling const&) { ... }
        Scheduling(Scheduling &&) = default;
        Scheduling& operator=(Scheduling const&) { ... }
        Scheduling& operator=(Scheduling &&) = default;

        void print_table() { ... }
        std::vector<int> cal_waiting_time(std::vector<int>) { ... }
    };

The rule of zero is just as simple.  If you can avoid defining all of the special members, then do so.  As it turns out, the compiler generated behavior of the copy, move, and destruct operations are well-defined for your member variables and consistent in their use. You do not need to define these operations yourself.

    class Scheduling {
        ...
      public:
        Scheduling() = default;
        Scheduling(int) { ... };

        void print_table() const { ... }
        std::vector<int> cal_waiting_time(std::vector<int>) { ... }
    };

    std::vector<int> cal_waiting_time(std::vector<int>& burst_time)
    {
        waiting_time.push_back(0);

        for (int i = 0; i < burst_time.size() - 1; i++)
        {
            waiting_time.push_back(waiting_time[i] + burst_time[i]);
        }
        return waiting_time;
    }

Know your algorithms.  See std::partial_sum.

On multiple calls, the new results do not overwrite the old results.  Instead, the new results are appended to the old results.

Did you intend for this function to be a part of the public api for this class?  Have you considered returning a cached waiting_time rather than recalculating the partial sum on each call?

    void print_table()
    {
        waiting_time = cal_waiting_time(burst_time);
        std::cout << "Process\t Burst Time\t Waiting Time\n";
        for (int i = 0; i < burst_time.size(); i++)
        {
            std::cout << i+1 << "\t\t " << burst_time[i] << "\t\t " << waiting_time[i] << "\n";
        }
    }

This function isn't being truthful in what it does.  It recalculates the waiting_time on every call which means its doing more than "print".  Consider an alternative design, such as calculating the partial sum on construction.


Lots of click handlers that hide one element and show another

Extract the duplicated code to a function:

function showOneHideOther(elToShow, elToHide) {
    $(elToShow).show();
    $(elToHide).hide();
}

You can reuse the function anyplace now:

$('#Back').on('click', function() {
    showOneHideOther('#TableRow', '#FormRow')
});
$('#AddLanguageShow').on('click', function() {
    showOneHideOther('#FormRow', '#TableRow')
});

$('table').on('click', 'button.edit', function (evt) {
    showOneHideOther('#FormRow', '#TableRow')
    languageManager.load(getRowId($(this)));
});


Is this a practical algorithm for pattern matching in a text?

Are you trying to reinvent the wheel?
String has .indexOf for exactly this purpose.
These statements will print the same things, for example:

// both prints 2
System.out.println(match("hello", "x hello, there"));
System.out.println("x hello, there".indexOf("hello"));

// both prints -1
System.out.println(match("hello", "x, there"));
System.out.println("x, there".indexOf("hello"));

  Assume text can be very large, is there anything I can do to speed up the performance?

In this example, using String.indexOf is probably the fastest you can get.
If you have a more complex pattern,
like a regular expression,
then using Pattern will be more efficient than hand-coded solutions.

If you're reinventing the wheel intentionally, then here are some tips to improve your implementation:

Rename the method to indexOf(text, pattern), as "match" doesn't imply that it actually returns the starting index of the matched pattern. indexOf will also look familiar to other developers
The continue statement is unnecessary when it's the last statement in a loop
The int ret = -1 initialization before the loop is pointless: the value is reassigned in every iteration
It's better to declare variables in the smallest possible scope. So ret should be declared inside the loop.

The same goes for i, it would be better to convert that outer while loop to a for loop: for (int i = 0; i < N; ) {


In Python, stylistically better way to remove a potentially nonexistent key from a dictionary?

Option 1 looks perfect to me. It does the job in one line and is easy to read.


Structure

Currently, there is no reason to have prepare or close, they are just delegating to the public field. A user of your class may just as well use that, and may be confused about the difference.

There is also no need for open() to be public, as there is no need to ever reopen the connection.

If you think you do need a wrapper around mysqli, it should: 1) actually add new functionality or make the existing functionality considerably easier to use 2) be an extensive wrapper, so that there is no need to make CONN public so that it can be accessed directly.

Also, don't die in a class, it makes it difficult to (re-)use. What if you want to fall back on something else? Or show a proper error message? Throw an exception instead.

Also, it would be a good idea to pass the credentials to the class, so it is more flexible (you may eg want to load them from a config file or at least a file only containing the credentials, so you can exclude them from versioning, etc).

Style

Your indentation is off, and you have quite a bit of unnecessary vertical whitespace.

Variable names should all follow a common naming scheme, for example camelCase. I see no apparent reason why $CONN is all uppercase.

Class names should start with an upper-case character.

Comments

Your comments just repeat what the code already told me (close() closes, prepare() prepares, and so on). Comments like these will lead me to believe that all your comments will not be useful to me, which leads to me missing comments that actually are important.

Result

If you follow all my advise, your code may look like this:

class DBConnection {

    public $connection;

    public function __construct($dbServername, $dbUsername, $dbPassword, $dbNam) {
        $this->connection = new mysqli($dbServername, $dbUsername, $dbPassword, $dbName);
        if ($this->connection->connect_error) {
            throw Exception("Connection failed: " . $connection->connect_error);
        }
    }
}

Wrapping mysqli like this may still give you some flexibility later on, but personally, I'm not sure that it is worth it. I would either use mysqli directly, or wrap it in such a way that no element of mysqli is actually required outside of the class.


PDO connection / prep and execute in their own functions

Here are a few things I noticed.

If you are going to catch a possible exception when you connect to your MySQL DB you want to include the PDO instantiation in the try block.

try {
    $db = new PDO('mysql:host=' . $config['db']['host'] . ';dbname=' . $config['db'}['dbname'], $config['db']['username'], $config['db']['password']);
    $db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
    $db->exec("SET CHARACTER SET utf8");
}  catch(PDOException $e) {
    print "Error!: " . $ex->getMessage() . "<br/>";
    die();
}

The arguments $vlue and $response don't do anything in your update function. You probably want to get rid of those.
You can use a quicker syntax for binding parameters by defining an array that holds the values and passing it to PDOStatement::execute(). This has the benefit of less function calls. Doing it this way will default all parameter value datatypes to be PDO::PARAM_STR, which matches what you are doing currently.

$parameters = array(
    ':fname'  => $fn,
    ':lname'  => $ln,
    ':email'  => $email,
    ':signup' => $offers
);
$stmt->execute($parameters);

The point of exceptions is to be able to handle, on your own terms, exceptional events. If a PDO exception occurs in your update function, more than likely it won't/shouldn't know how to handle it. It makes more sense for the code that calls update to take care of that situation. So I would suggest moving the try/catch block to wherever the update function is called.

   try {
       update($db, $fn, $ln, $email, $offers, $vlue, $response)
   } catch (PDOException $e) {
       echo "no update there is a slight problem " . $e->getMessage();
   }


There is a lot to review here, so I'll just cover this handler:

private void FileButton_Click(object sender, EventArgs e) {
    saveFileDialog1 = new SaveFileDialog();
    saveFileDialog1.Filter = "txt files (*.txt)|*.txt|All files (*.*)|*.*";
    saveFileDialog1.FilterIndex = 2;
    saveFileDialog1.RestoreDirectory = true;

    if (times.Count > -1) {
        if (saveFileDialog1.ShowDialog() == DialogResult.OK) {
            string path = saveFileDialog1.FileName;
            System.IO.File.WriteAllLines(path + ".txt", times);
            times.Clear();
        }
    }
}

It's very easy to cram all of a functionality's logic inside an event handler in WinForms; it's a little bit harder to handle a Click event and not have the Single Responsibility Principle take a beating. This handler:

Configures and displays a SaveFileDialog.
Writes to the specified file.
Clears the times list.

These are 3 distinct operations, and yet they all happen in the FileButton's Click handler.

One of my favorite refactorings, is the Extract Method functionality - in Visual Studio, there's a convenient keyboard shortcut for it: just select the code you want to extract, and then Ctrl+R, M.

I would select the first part, extract it into its own function, and get this:

private string PromptSaveAsFileName()
{
    var dialog = new SaveFileDialog
        {
            Filter = "txt files (*.txt)|*.txt|All files (*.*)|*.*",
            FilterIndex = 2,
            RestoreDirectory = true
        };

    if (dialog.ShowDialog() == DialogResult.OK)
    {
        return dialog.FileName;
    }

    return null;
}

This would leave the handler, with a few little more changes, like this:

private void FileButton_Click(object sender, EventArgs e) {

    if (times.Count == -1)
    {
        return;
    }

    var fileName = PromptSaveAsFileName();
    if (fileName == null)
    {
        return;
    }

    System.IO.File.WriteAllLines(fileName + ".txt", times);
    times.Clear();
}

It's still more than I'd like to see, but it's already much better. A few question marks before going any further:

Under what circumstances would times.Count be -1?

   Never. The Count property of an empty list will return 0.

Since we have a function whose role is to return our SaveAsFileName, who's responsibility is it to ensure the file has a .txt extension?

   The function should return the full file name, ready to be used, including its extension.

So. I'd do whatever I have to do, to make the handler look like this:

private void FileButton_Click(object sender, EventArgs e)
{
    if (!times.Any())
    {
        return;
    }

    var fileName = PromptSaveAsFileName();
    if (fileName == null)
    {
        return;
    }

    WriteResultsToFile(fileName);
    times.Clear();
}

Notice a couple more points:

{ scope opening brace is placed on the next line. This is C# convention; opening the scope on the same line is Java convention. I like that your brace style is consistent though, so that's just a minor point.
Return early, and don't work for nothing: the above code will not set up a new SaveFileDialog and then return if there's nothing to save.
Avoid extraneous nesting: it's often possible to reduce nesting by simply reversing a condition.


Printing the number of ways a message can be decoded

Your code performs one recursion per character and recursion depth is limited. For long inputs, your code will raise RecursionError: maximum recursion depth exceeded. Replacing recursion with an explicit loop solves the issue.

Also, if you start the calculation at k = 1 with memo[-1] and memo[0] properly initialized and then work upwards to k = len(s), the code will become significantly simpler. When calculating memo[k], you only ever access memo[k-2] and memo[k-1], so you can store only those. I renamed them old and new. Here is how it could look like:

def num_ways_dp(s):
    old = new = 1
    for i in range(len(s)-1):
        if int(s[i:i+2]) > 26: old = 0 # no two-digit solutions
        if int(s[i+1]) == '0': new = 0 # no one-digit solutions
        (old, new) = (new, old + new)
    return new

if __name__ == '__main__':
    number = input()
    while number != '0':
        print(num_ways_dp(number))
        number = input()


Using fflush is pretty much required as output would otherwise be buffered until a newline character is printed.

You could use counter as an index to a character array instead of a big clunky repetitive switch. If you have a char *spinner = "|/-\\";, spinner[0] will be "|", spinner[1] will be "/", etc.

char *spinner = "|/-\\"; printf("\b%c", spinner[counter]); is going to print the common backspace character and the correct "spinner symbol" for each value of counter.  The spinner variable does not need to be declared because it's a string literal: printf("%c", "|/-\\"[counter]); can be used instead.

Apart from the unnecessary switch, I'd say you have an excessive amount of blank lines. If they are used more scarcely, their relative effect will be greater. Readability becomes more important as the program gets bigger.

/*
 * Separated groups of code:
 * If there were empty lines between all statements, it would be
 * difficult to tell the groups apart.
 */

foo(bar->baz);
foo(bar->quux);

counter++;
counter %= 4;

Here's a modified version:

#ifdef _WIN32
    #include <windows.h>
    #define sleepThr(x) Sleep(x)
#else
    #include <unistd.h>
    #define sleepThr(x) usleep((x) * 1000)
#endif

#include <stdio.h>

#define INTERVAL 200

int main()
{
    int counter;
    while (1)
    {
        for (counter = 0; counter < 4; counter++)
        {
            printf("\b%c", "|/-\\"[counter]);
            fflush(stdout);
            sleepThr(INTERVAL);
        }
    }

    return 0;
}


How can I condense several IBAction methods in a class with a delegate?

You're right.  There is a simpler way.

First of all, instead of giving every interactive element its own method, let's give them all the same method.

- (IBAction)buttonPressed:(id)sender;

If they're not all buttons, a different method name is in line, but for this example, I'll assume them all to be buttons.

Now then, whether you're creating these UI elements programmatically or via interface builder, the next important step is to give each element its own unique tag.  In the Interface Builder, this is done under the "Attributes Inspector" (the icon that looks like a shield).  The "tag" field is the second field under the "View" heading.  It doesn't matter what tag you give the button.  It only matters that the tags are unique.

Setting the tag programmatically is as simple as this:

myButton.tag = 100;

Now, instead of a different method for each button (or other UI element), we can simply use a single button:

- (IBAction)buttonPressed:(id)sender {
    [self.delegate someRequiredMethod:sender.tag];
}

Now we've condensed your code.  But we can do even better.

Let's create an enum so that we're not just sending arbitrary numbers, but instead we send values that the delegate can make more sense of (and if using a switch, Xcode will help the delegate be sure he accounts for every case).

typedef NS_ENUM(NSInteger, MyExampleButtonEnum) {
    ButtonIndexUnknown = 0,
    ButtonIndexTopLeft = 100,
    ButtonIndexTopRight = 101,
    ButtonIndexBottomLeft = 102,
    ButtonIndexBottomRight = 103
};

This should be placed in the .h and before our @protocol.  Then we can change our protocol to look like this instead:

@protocol MyExampleProtocol <NSObject>
@required
- (void)someRequiredMethod:(MyExampleButtonEnum)button;
@end

Now we just have to set our button tags to match these enum values.  The good news is, if we're doing it programmatically, we can use the enum itself to set the values.  For example...

UIButton *topLeftButton = [[UIButton alloc] initWithFrame:someFrame];
topLeftButton.tag = ButtonIndexTopLeft;
[topLeftButton addTarget:self action:@selector(buttonPressed:)
    forControlEvents:UIControlEventTouchUpInside]

And now we slightly modify our buttonPressed: method as such:

- (IBAction)buttonPressed:(id)sender {
    if (sender) {
        [self.delegate someRequiredMethod:sender.tag];
    } else {
        [self.delegate someRequiredMethod:ButtonIndexUnknown];
    }
}

And our delegate can implement the delegate method in some fashion like this:

- (void)someRequiredMethod:(MyExampleButtonEnum)button {
    switch(button) {
        case ButtonIndexTopLeft:
            // do stuff
            break;
        case ButtonIndexTopRight:
            // do stuff
            break;
        case ButtonIndexBottomLeft:
            // do stuff
            break;
        case ButtonIndexBottomRight:
            // do stuff
            break;
        case ButtonIndexUnknown:
        default:
            // do stuff
            break;
    }
}


how to allow user to input results in Array using java?

You have a lot of lines that do the same thing and which with a little rearranging can be consolidated:

int entrants = 0;
String[][] runners = null; //no need to create an array that you'll be throwing away
boolean valid = false; //this is for the initial number of runners question - it will stay false and keep us in the loop until a number between 2-7 is entered
System.out.println("Please enter the number of runners for this race (2 to 7 entrants permitted)");

do {
  //read the input
  entrants = input.nextInt();
  input.nextLine();

  //validate (we'll keep your messages)
  if(entrants < 2) {
    System.out.println("You need at least 2 runners for a race, please re-enter the number of runners");
  } else if (entrants > 7) {
    System.out.println("You have entered too many runners for this race (max runners is 7). Please re-enter the number of runners");
  } else {
    System.out.printf("You have entered %d runners\n", entrants);

    //create the array and set the exit condition
    runners = new String[entrants][2];
    valid = true;
  }
} while (!valid);

Note the use of a do-while which is like a while-loop but evaluates the condition after an iteration, i.e. the loop body is run at least once.

Further improvements could include putting the constraints into constants (i.e. min = 2 and max = 7) instead of having them in all over your code.


There are a few things you can do differently

Animations are most often accomplished by getting the difference between start and end values, and animating a factor from zero to 1. Multiply that with the difference, and you get an offset.
Instead of relying on an interval (which may be slow, since JS is single-threaded), most animation libraries compare the start time to the current time, to see how far along the animation should be.
Instead of waiting for the animation to stop and a boolean to flip, you can clear the animation's timer, and forcibly stop the animation, before starting a new one. That way the user's clicks aren't ignored, but override previous clicks instead.

Here's an alternative implementation using that approach and other good stuff:

var smoothScrollTo = (function () {
  var timer, start, factor;

  return function (target, duration) {
    var offset = window.pageYOffset,
        delta  = target - window.pageYOffset; // Y-offset difference
    duration = duration || 1000;              // default 1 sec animation
    start = Date.now();                       // get start time
    factor = 0;

    if( timer ) {
      clearInterval(timer); // stop any running animation
    }

    function step() {
      var y;
      factor = (Date.now() - start) / duration; // get interpolation factor
      if( factor >= 1 ) {
        clearInterval(timer); // stop animation
        factor = 1;           // clip to max 1.0
      }
      y = factor * delta + offset;
      window.scrollBy(0, y - window.pageYOffset);
    }

    timer = setInterval(step, 10);
    return timer; // return the interval timer, so you can clear it elsewhere
  };
}());

Here's a demo

Another advantage of always animating a factor between zero and 1 is that you can run it through other expressions, and (for instance) animated the scroll with a sine wave.


This complete script works for me with PyQt on Windows, displays a small window and waits for me to close it.

from PyQt4 import QtGui

app = QtGui.QApplication([])

scene = QtGui.QGraphicsScene()
scene.addText("Hello, world!")
view = QtGui.QGraphicsView(scene)
view.show()

app.exec_()

A QApplication object must be created before any other GUI objects.


user_choice is your main loop, and so it would be better described as main. Whilst fairly undescriptive on what it does you can add a docstring to add information on what it performs.
It is more idiomatic to use loops in Python rather than recursion. This is partly due to the recursion limit, and partially that the iterator pattern has a lot of support in Python.
I've rarely seen '\r' in Python. I don't see why you would need it and so have just removed them.
I like the way that you got a users email. The use of a while loop here is pretty clean.
I would change the prompted text when getting an email as it's different to the rest of the prompts.
I would DRY your do while loop. There's not really a point to having two different input prompts with the above suggestion.
Recently I was creating an account using passwd and multiple times I kept messing up the first password entry. It was rather vexing as I knew I messed up and I just had to hope I pressed backspace the correct amount of times to fix my mess up. Unfortunately I wasn't skilled enough to fix my mistake.

However with your solution I would have to quit out of your entire program, removing the data I have already to fix my mistake. You should instead get again the password and the confirmation.
There is no point in opening login_data until the user input has been confirmed to be correct. You should try to keep with statements as small as necessary.
I would prefer to build a dictionary or an abstract datatype for the user information.
You can remove one of your login_data.writes by passing i + '\n'.
"Login_Data.txt" seems ok, but it's not great. You can use JSON via import json and make a much easier and standard file type to work with.
Your interactions with your globals seem really poor. If you want a global, than I suggest you only have one. This is easy when using dictionaries.

I would also suggest changing your code to have no globals.
I would change login_account to align with the above, changed do-while loops, no globals and using JSON.
Since we are using JSON we can easily change the format to display the first and last name easier.
Your solution is in no way safe, it's prone to over the shoulder attacks by not using getpass and you're storing passwords in plain-text. This is not safe. Do not use this for anything more than a toy project that will only ever have you entering a password unique to this toy.

import json

def main():
    """Create an account or log in."""
    while True:
        print('Hello user, create an account or login to an existing one.')
        choice = input('Insert "1" if you wish to create an account or "2" if you wish to login: ')
        print()
        if choice == '1':
            create_account()
        else:
            login_account()

def input_user_email(emails):
    """Get an unowned email."""
    while True:
        email = input('Email: ')
        if email not in emails:
            return email
        print('That email is already registered.')

def input_user_password():
    """Get a user's email."""
    while True:
        password = input('Password: ')
        confirmation = input('Confirm password: ')
        if password == confirmation:
            return password
        print('The passwords do not match.')

def get_user_information(emails):
    """Get a user's information."""
    return {
        'first_name': input('First Name: '),
        'last_name': input('Last Name: '),
        'email': input_user_email(emails),
        'password': input_user_password(),
    }

def create_account():
    """Create user account."""
    with open('login_data.json', 'w+') as f:
        users = json.load(f)
        emails = {user['email'] for user in users}
        user = get_user_information(emails)
        users.append(user)
        json.dump(users, f)
    print('Nice! Your account was registered.\n')

def login_account():
    """Log into a user account."""
    with open('login_data.json') as f:
        users = json.load(f)
    by_email = {user['email']: user for user in users}

    while True:
        email = input('Email: ')
        if email in by_email:
            user = by_email[email]
            break
        print('Invalid Email')

    while True:
        password = input('Password: ')
        if password == user['password']:
            break
        print('Invalid password')

    print('Hello {0.first_name} {0.last_name}, welcome back!\n'.format(user))

if __name__ == '__main__':
    main()


Matching PEP8 style with long one line for loop

How about breaking the lines like this:

patient = [doctor['patient']
           for doctor in doctors
           for section in sections if doctor == section['doctor']]

I find this more readable and it's PEP8 compliant.
I also removed the unnecessary (...) from around the expression,
and renamed doctor to the more intuitive doctors,
and used more meaningful loop variable names.

But the truth is I find it difficult to follow PEP8 always,
even though I really like clean code.
So I usually cheat,
and write a wrapper shell script to check my project and hide some PEP8 violations that I don't really want to follow.
Something like this:

#!/bin/bash

cd $(dirname "$0")

# E121 continuation line indentation is not a multiple of four
# E123 closing bracket does not match indentation of opening bracket's line
# E126 continuation line over-indented for hanging indent
# E128 continuation line under-indented for visual indent
# E501 line too long > 79 characters
pep8 . | grep -v \
    -e E121 \
    -e E123 \
    -e E126 \
    -e E128 \
    -e E501


Rails service for a series of calculations

The real threat to thread safety is actually in your RecordBaseAmountService class. This takes two arbitrary objects, a record and sub-record. This logic really should be moved into the Record class to ensure you are modifying the objects you think you are. Consider this case:

record1 = Record.find 3
record2 = Record.find 8
subrecord = record1.subrecord
update_record = RecordBaseAmountService.build.call record2, subrecord

Do you see any bugs here? Any thread safety issues? We pass in record2 but pass in the sub record for record1 -- oops!

Moving this logic into the Record class itself encapsulates this behavior and limits it to the proper objects. This also limits the ability of code outside that Record object to modify data outside its thread.

class Record < ActiveRecord::Base
  has_many :subrecords
  validates :base_amount, presence: true
  around_update :cache_in_subrecord

  def calculate_base_amount
    if subrecord.type == 0
      base_amount = 10
    elsif subrecord.type == 1
      base_amount = 20
    else
      base_amount = 30
    end

    update_audit_field "base amount calculated"
    adjust_base_amount
    pro_rate_record if subrecord.is_prorated?

    base_amount
  end

  private

  def adjust_base_amount
    base_amount = base_amount * 30
    update_audit_field "base amount adjusted"
  end

  def pro_rate_record
    pro_rated_amount = base_amount * subrecord.pro_rate
    base_amount = base_amount - pro_rated_amount
    update_audit_field "prorated"
  end

  def cache_in_subrecord
    subrecord = self.subrecord
    update_audit_field "subrecord cached " + Time.now.utc.to_date
    subrecord.cached_attribute_field = (subrecord.cached_attribute_field || {}).merge(get_cached_attributes)

    yield

    subrecord.save
  end

  def get_cached_attributes
    {
      record_id: self.id,
      base_amount: self.base_amount,
      audit_field: self.audit_field,
      time: Time.now.utc.to_date
    }
  end

  def update_audit_field(message)
    audit_field += "=>" + message
  end

end

The RecordBaseAmountService is entirely unneeded.

The MonthlyRecordWorker is still useful, but could use a simple change:

class MonthlyRecordWorker
  include Celluloid

  def update_record(record)
    record.calculate_base_amount
    record.save
  end

end

Nothing else needs to change. Just encapsulate the base amount calculation to limit the chances that objects get modified outside the current thread.

Some nitpicks:

Use 2 spaces for indentation. I see 3 and 4 spaces.
Put spaces around all operators:

record.base_amount = record.base_amount*30

Becomes:

record.base_amount = record.base_amount * 30

No need to convert a date object to a string when updating the audit message. The string concatenation operation will do that automatically

self.audit_field += "=>subrecord cached " + Time.now.utc.to_date.to_s

No need for a build method if it just delegates to new

worker = MonthlyRecordWorker.new
worker.call

Or:

MonthlyRecordWorker.new.call

Suggestions:

Create an update_audit_field method instead of concatenating strings and prefixing it with => constantly (in Record):

def update_audit_field(message)
  audit_field += "=>" + message
end


How can this code for applying filters to data by date and size be improved?

The comments seem redundant, I'm assuming they're just for this review, if not, I would remove them.  The basically just duplicate what the code says.

Your function names seem misleading.  filterBySize seems to be filtering by data usage.  filterByDate seems to be filtering by the day of the week.  It would be better if they said what they did.

Your filtration functions contain two branches, both of which return.  Generally you  would only wrap the if side.  Everything that doesn't trigger the if would be the else...

CustomerData customerData = new CustomerData(Customer);
if (customerData.getUsage() > USAGE_THRESHOLD) {
    return true;
}
// The else is implicit... so just noise
return false;

For me, there's an awful lot of logging across these methods.  Maybe that's what's required by your processes, but it seems excessive.  We don't have the context for how the code is going to be called, however I would expect that 'USAGE_THRESHOLD' is the same for every invocation, do you really need to log it for each custom to say if they are above/below it?  The current day of the week is the same for each invocation, again do you really need to output it for every customer that's selected?   Each of your filters logs the specific reason that the customer has been rejected  do you need to log the generic message at the top level saying they are being skipped.  If you're looking for reuse, then maybe your filterByDate method shouldn't mention the import, that way it could be reused to filter other things (export perhaps) in the future...

As an aside, I'm not sure about this:

CustomerData customerData = new CustomerData(Customer);

It makes me wonder what the constructor for CustomerData is doing... If the information needed for the filtration isn't in the Customer, then is it reading that extra data in the the constructor for CustomerData?  That seems wrong...

Another small issue is naming.... variables should start with lower case in java.... CustomerUnit Customer should be CustomerUnit customer, you can see it's confusing the code preview...

It would also be interesting to see how invokeRequestByCustomer is being called... It looks a lot like it could fit into a stream something like:

allCustomers.stream()
    .filter(CustomerFilters::byUsage)
    .filter(CustomerFilters::byDay)
    .foreach(c->invoke(c));


Broken
If you call enterLock() with id 0 followed by enterLock() with id 1 (on another thread perhaps), both calls will succeed.  Since your function doesn't change the state of the mutex, it isn't surprising.  Perhaps you meant to leave lock->waiting[id] at 1 instead of 0?  Note, even if you did that, it wouldn't be enough to fix the mutex.
Also, what is the turn variable used for?
Spinlock, not mutex
Technically, what you are doing is a spinlock.  A real mutex should efficiently wait for its turn instead of spinning in a loop.


Convert an array of doubles to a concatenated Hex string

string is immutable type, whenever you modify a string, it actually creates a new string.

if you convert it to StringBuilder such as :

var sb = new StringBuilder(string.Empty);

foreach (var value in list)
    sb.Append(DoubleToHex(value.lat)).Append(DoubleToHex(value.lon));

You'll gain more performance.

However, the DoubleToHex() also can simplified further to :

public static string DoubleToHex(double input)
{
    return BitConverter.ToString(BitConverter.GetBytes(input)).Replace("-", "");
}

this will boost the performance further.


DRY (Don't Repeat Yourself)

You have a list of else if statements that all do the same thing. JavaScript has the .toUpperCase method so hardcoding character codes to perform capitalization is unnecessary. You can do newString += str[i].toUpperCase().

Use variables for constant values

Instead of constant values directly in your code, use a variable to store them. You know you want to capitalize vowels, so you can create an array var vowels = ['a', 'e', 'i', 'o', 'u'] to store your constants. This makes it easier to modify in the future - like if the requirements changed and now y is a vowel.

Putting these two together, your capitalization code can be pretty concise:

// vowels.indexOf(str[i]) !== -1 would work, too
if(vowels.includes(str[i]){
  newString += str[i].toUpperCase();
}


Optimizing and improving a username regex

I would very strongly recommend against using a regular expression for this. There is no clear mapping between the list of requirements you posted and the code.

Imagine another developer looking at this. Are they able to deduce the list of requirements? Given the list of requirements, are they able to verify that the regular expression is correct? How long would it take to convince them that it's correct?

In fact, it's not correct. The character class \d is not the same as [0-9] (unless you specify ECMAScript-compliant behaviour), and your regexp matches the username j1vad-a٠miry (that's an ARABIC-INDIC DIGIT ZERO). (It doesn't match on the link you posted, but that's a Ruby, not .NET, regexp tester.)

Now suppose one of your requirements changes -- this code is going to be tricky to maintain.

Instead, I would recommend writing a series of tests, each corresponding to one of your requirements. Here is the code I came up with. It still uses regular expressions where appropriate, but very simple ones.

public static bool IsValidUsername(string username)
{
    if (username == null)
    {
        return false;
    }

    var length = username.Length;
    if (length < 3 || length > 32)
    {
        return false;
    }

    if (!IsLowerAlpha(username[0]))
    {
        return false;
    }

    if (!IsLowerAlphanumeric(username[length - 1]))
    {
        return false;
    }

    if (!Regex.IsMatch(username, "^[a-z0-9._-]*$"))
    {
        return false;
    }

    if (Regex.IsMatch(username, "[0-9]{5,}"))
    {
        return false;
    }

    // Each username can contain only one of '.', '_', '-'.
    var punctuation = new [] { '.', '_', '-' };
    if (punctuation.Count(c => username.Contains(c)) > 1)
    {
        return false;
    }

    // Each '.', '_', and '-' should be followed by an alpha-numeric.
    for (var i = 0; i < length - 1; i++)
    {
        if (punctuation.Contains(username[i]) && !IsLowerAlphanumeric(username[i + 1]))
        {
            return false;
        }
    }

    return true;
}

private static bool IsLowerAlpha(char c)
{
    return c >= 'a' && c <= 'z';
}

private static bool IsLowerAlphanumeric(char c)
{
    return IsLowerAlpha(c) || (c >= '0' && c <= '9');
}

Now, IsValidUsername is getting a bit long. We could split out each check into a separate static method, for instance,

private static bool IsNotNull(string username)
{
    return username != null;
}

private static bool IsInLengthRange(string username)
{
    var length = username.Length;
    return length >= 3 && length <= 32;
}

And then just check our rules like this:

private static readonly Predicate<string>[] Rules = new Predicate<string>[]
{
    IsNotNull,
    IsInLengthRange,
    StartsWithLowerAlpha,
    ...
};

public static bool IsValidUsername(string username)
{
    return Rules.All(rule => rule(username));
}


Simple Python module to test dictionaries

Just reviewing get_conf.

Support for Python 2.7 is planned to end in 2020, so it is not a good time to be starting new projects that run on Python 2.7 only.
The line:

PyDict_SetItemString(dict, "num_foo", PyLong_FromLong(42));

has three problems! First, PyLong_FromLong can fail (returning NULL), but your code does not check for this. Second, PyLong_FromLong returns a "new reference" which you never decrement, so this object will leak. Third, PyDict_SetItemString can also fail (returning -1), but your code does not check for this.

So, inconvenient as it may seem, you must write something like:

PyObject *num_foo = PyLong_FromLong(42);
if (num_foo == NULL) {
    /* handle failure somehow */
}
if (PyDict_SetItemString(dict, "num_foo", num_foo) != 0) {
    /* handle failure somehow */
}
Py_DECREF(num_foo);

One approach to writing a function that has many error cases is to have an error-handling section which is responsible for decrementing all the references that were acquired by the code. The error handlers should be arranged in reverse order, like this:

static PyObject *get_conf(PyObject *self, PyObject *args)
{
    PyObject *dict = PyDict_New(); /* (1) */
    if (dict == NULL) {
        goto fail_dict;
    }
    PyObject *enabled = g_enabled ? Py_True : Py_False;
    if (PyDict_SetItemString(dict, "enabled", enabled) != 0) {
        goto fail_set_enabled;
    }
    PyObject *num_foo = PyLong_FromLong(42);
    if (num_foo == NULL) {
        goto fail_num_foo;
    }
    if (PyDict_SetItemString(dict, "num_foo", num_foo) != 0) {
        goto fail_set_num_foo;
    }
    Py_DECREF(num_foo);
    return dict; /* (2) */

fail_set_num_foo:
    Py_DECREF(num_foo);
fail_num_foo:
fail_set_enabled:
    Py_DECREF(dict); /* (3) */
fail_dict:
    return NULL;
}

You can then check the reference-counting behaviour for each object. For example, the dict object gets one reference at (1), which is either returned to the caller at (2), or else decremented at (3). So this object does not leak.
The general inconvenience and delicacy of reference counting in C extension modules suggests that you might be better off with a hybrid approach where you implement a minimal hardware interface layer in C, and then package this up in Python to provide a friendlier interface.

The way this would work is that in C you would make a module _myfoo which contains functions like this:

static PyObject *get_enabled(PyObject *self, PyObject *args)
{
    return PyBool_FromLong(g_enabled);
}

static PyObject *get_num_foo(PyObject *self, PyObject *args)
{
    return PyLong_FromLong(g_val);
}

which are so simple that there is little risk of getting the reference counting wrong, and then in Python you would write a module myfoo containing:

import _myfoo

def get_conf():
    """Return hardware configuration dictionary with keys:

    enabled: bool -- [documentation here]
    num_foo: int -- [documentation here]
    """
    return dict(enabled=_myfoo.get_enabled(),
                num_foo=_myfoo.get_num_foo())

which you can see is much simpler than trying to allocate a dictionary in C.


First, I thought I'd mention that your code is stylistically really good - variable and function names make sense, indentation is great (with one small exception noted below), and the functions are focused and readable.  Just one minor point:

The code under each Case in a Select Case is typically indented one more level:

Select Case adviser
    Case JonHussey
        AddNameVariations coll, "Jon", "Hussey"
    Case MartinCotter
        '...

As far as your specific question goes, this actually looks like it would be quite a chore to maintain.  Adding a new name to an existing advisor is easy with your set-up.  For example, if John Cusins decided to change his name to Jim Nasium, all you would have to do is add AddNameVariations coll, "Jim", "Nasium" under his Case in NameStringsFromAdviser and change his Case in CurrentNameStringFromAdviser to NameString = "Nasium, Jim".

But, this points to the next issue - your Enum isn't really serving much use as much other than a place-holder.  On the one hand you could change the JohnCusins element to JimNasium everywhere in your code, but this kind of defeats the purpose of having a named identifier.  On the other hand you could leave it as JohnCusins, but this now means that it isn't appropriately named - why should the Enum element associated with a person be different from their name? If I was only looking at the CurrentNameStringFromAdviser function and not the entire module, I'd be wondering "why is the SarahCotter case returning Oluwole, Sarah?"

This also points to the main issue in maintainability and extensibility - to add an advisor, you need to change the code in 3 different functions and add a value to the enumeration.  At the same time, you need to make sure that those changes are all consistent with each other. The urge to copy and paste will be very hard to resist...

...which leads to the last point - you're repeating code all over the place. This gives me a whiff of code smell. Wouldn't it be nice if you didn't have to type If AdviserNameInString(FooBar, str) Then again and again and again?  It would be much better to approach this problem with the mantra Don't Repeat Yourself in mind.  This is a problem begging for an OO solution.

You have 2 distinct things you care about - advisors and names. So make an advisor class and a name class.  Make them responsible for the things that advisors and names care about.  I.e.:

Name.cls

Option Explicit

Private mFirst As String
Private mLast As String

Public Property Let FirstName(first As String)
    mFirst = first
End Property

Public Property Get FirstName() As String
    FirstName = mFirst
End Property

Public Property Let LastName(last As String)
    mLast = last
End Property

Public Property Get LastName() As String
    LastName = mLast
End Property

Public Property Get NameString() As String
    NameString = mLast & ", " & mFirst
End Property

Public Function Matches(test As String) As Boolean
    Matches = InStr(1, test, mFirst) Or InStr(1, test, mLast)
End Function

Advisor.cls:

Option Explicit

Private mNames As New Collection
Private mPreferred As Name

Public Sub AddName(inValue As Name)
    mNames.Add inValue
End Sub

Public Property Set PreferredName(inValue As Name)
    Set mPreferred = inValue
    mNames.Add inValue
End Property

Public Property Get PreferredName() As Name
    Set PreferredName = mPreferred
End Property

Public Function NameMatches(test As String) As Boolean
    Dim candidate As Variant
    For Each candidate In mNames
        If candidate.Matches(test) Then
            NameMatches = True
            Exit For
        End If
    Next candidate
End Function

Note, it isn't clear from your question if the numbers in the Enum have meaning outside of the code that you posted. If they are, just make them a property of Advisor. Throw in a couple factory functions to easily generate objects (curse you VBA for not having constructors)...

'Name factory.
Public Function NewName(first As String, last As String) As Name
    Set NewName = New Name
    NewName.FirstName = first
    NewName.LastName = last
End Function

'Advisor factory
Public Function NewAdvisor(preferred As Name, _
                           ParamArray names() As Variant) As Advisor
    Dim variation As Variant

    Set NewAdvisor = New Advisor
    Set NewAdvisor.PreferredName = preferred

    For Each variation In names
        Dim cast As Name
        Set cast = variation
        NewAdvisor.AddName cast
    Next variation
End Function

...and you get something to maintain that looks like this:

Option Explicit

Private Advisors As Collection

Public Sub InitializeAdvisors()
    Set Advisors = New Collection

    Advisors.Add NewAdvisor(NewName("Jon", "Hussey"))
    Advisors.Add NewAdvisor(NewName("Martin", "Cotter"))
    Advisors.Add NewAdvisor(NewName("Jeremy", "Smith"))
    Advisors.Add NewAdvisor(NewName("Jonathan", "Blair"))
    Advisors.Add NewAdvisor(NewName("Sarah", "Oluwole"), _
                            NewName("Sarah", "Cotter"))
    Advisors.Add NewAdvisor(NewName("John", "Cusins"))
    Advisors.Add NewAdvisor(NewName("Micky", "Mahbubani"))
End Sub

Public Function FindAdvisor(inValue As String) As Advisor
    Dim candidate As Variant
    For Each candidate In Advisors
        If candidate.NameMatches(inValue) Then
            Set FindAdvisor = candidate
            Exit Function
        End If
    Next candidate
End Function

Example usage:

Private Sub ExampleUsage()
    InitializeAdvisors
    Debug.Print FindAdvisor("Someone named Sarah").PreferredName().NameString
End Sub

Add an advisor? One line of code. Add an alias? Change one line of code. Remove an advisor? Delete one line of code. Change from "Last, First" format to "First Last" format? Change one line of code. An advisor has 42 aliases? No problem - just add them all in one place. Now that is easy to maintain.