Sorts and data structures: a bucket sort I am currently going through and implementing basic algorithms and data structures as I feel I didn't learn enough about this during my Data Structures and Algorithms unit. So far I've done the following algorithms, yet to get to data structures.
Sorting

Bucket

import random
#toSort = [round(random.uniform(0.10, 0.99), 2) for _ in range(20)]
toSort = [round(random.uniform(0.10, 0.99), 2) for i in range(10, 0, -1)]
print(toSort)

def bucket_sort(array):
    buckets = [[] for i in range(len(array))]
    sorted_buckets = []

    for index in range(len(array)):
        bucket_num = len(array) * array[index]
        print(bucket_num)
        buckets[int(bucket_num)].append(array[index])

    for bucket in buckets:
        insertion_sort(bucket)

    for bucket in buckets:

        if len(bucket) == 0:
            continue

        elif len(bucket) > 1:
            for num in bucket:
                sorted_buckets.append(num)
        else:
            sorted_buckets.append(bucket[0])

    return sorted_buckets

def insertion_sort(array):

    for unsorted_val in range(1, len(array)):
        val = array[unsorted_val]
        val_index = unsorted_val

        while val_index > 0 and array[val_index - 1] > val:
            array[val_index] = array[val_index - 1]
            val_index -= 1
        array[val_index] = val

toSort = bucket_sort(toSort)

Insertion
Merge
Quick
Selection

Searching

Binary
Linear

Merging

ArrayMerge

I was planning to include the code for the rest of the algorithms I've implemented but I have found code reviews for those already so I do not want to double up on the questions.
However, I did want to ask this on the meta page but I didn't have enough reputation to post. If I were to go through and update my code with the suggestions in the other code reviews, would it be okay for me to post here with my implementation and ask for further review?

You have implemented an unusual bucket sort. First, the logic to compute the
bucket number makes assumptions about the values themselves and will fail on
many types of numbers (for example, positive integers). And second, if N is the size
of the input list, you are creating N buckets. Typically, bucket sort uses a
number of buckets that is smaller than N. A common approach is to make an initial pass
over the values to find their min and max. Then each bucket will have a
span of (MAX - MIN) / K, where K is the number of buckets
(which might be set either by the caller or by the code based on N).
For any x value, I think its bucket index would be
min(K - 1, int((x - MIN) / SPAN)) (you should double check that).
My other comments relate to code readability and simplicity.
Use convenience variables to eliminate repeated calculations, such as
len(array). If you need it multiple times, create a variable and lighten the
visual weight of your code.
Organize your code into commented "paragraphs" -- one paragraph per
small step in the logic of your algorithm (shown below).
If you need to iterate over values in a collection, do it directly, not
indirectly via indexes. Use for x in xs not for i in range(len(xs). If the
algorithm requires both values an indexes, use enumerate(). Only iterate
over indexes if you don't actually need the values or if the algorithm's
readability is simpler that way (for example, in your
insertion_sort() function).
Your code to reassemble the sorted buckets it overly complicated --
specifically, the size of the buckets is not important. The work can be done
either with a list comprehension (as shown) or the equivalent use of 2 for
loops.
Consider using a naming convention that I learned from functional programming:
xs for a collection of things and x for one thing.
Its extendable (ys and y, zs and z, etc) and
it works quite nicely in
generic situations like this where we know nothing about the substantive
meaning of the values. This also lightens up the code weight -- enhancing
readability without any loss of understandability.
The variable naming in insertion_sort() is backwards. You iterate over the
indexes but call each index an unsorted_val. If it's an index, just call it
index or, even better, i (a convention everyone understands). Then if you
also need the value, get it with xs[i]. Again, notice how these short
variable naming conventions can often enhance readability -- especially if the
scope is small and well defined.
Finally, it is unusual to modify an index value during an
iteration over indexes, as you do in insertion_sort(). It forces
your reader to puzzle things over. I've seen more intuitive insertion
sort implementations. For comparison, see the this pseudo-code.
Note how the use of "swap" in that alternative implementation really
helps the reader understand what's going on. Either adjust your
code or add some guidance to your reader.
Here's an edit focused on the readability and simplicity issues only:
def bucket_sort(xs):
    # Convenience variables.
    N = len(xs)

    # Put values into buckets.
    buckets = [[] for _ in range(N)]
    for x in xs:
        i = int(N * x)
        buckets[i].append(x)

    # Sort each bucket.
    #
    # To keep hammering the point, `b` is a better variable
    # name than `bucket` within this tiny, well-defined context.
    for b in buckets:
        insertion_sort(b)

    # Return the sorted values.
    return [
        x
        for b in buckets
        for x in b
    ]

def insertion_sort(xs):
    # Only stylistic edits here.
    for i in range(1, len(xs)):
        x = xs[i]
        while i > 0 and xs[i - 1] > x:
            xs[i] = xs[i - 1]
            i -= 1
        xs[i] = x


Php: how to lower complexity of method I have this method rated B on Scrutinizer (direct link: https://scrutinizer-ci.com/g/sineverba/domotic-panel/inspections/76996c9f-543f-43b4-9475-c64fe810a278/code-structure/operation/App%5CHttp%5CControllers%5CApi%5CPublicIpController%3A%3Aupdate)

public function update()
{
    try {
        $data = array();
        $update = false;
        $string_previous_public_ip = null;
        $current_public_ip = $this->getGateway()->fetchPublicIp($this->getIp());
        $previous_public_ip = $this->getGateway()->getLastRecord();
        $data[ 'ip_address' ] = $current_public_ip;
        if (isset($previous_public_ip->ip_address)) {
            $string_previous_public_ip = $previous_public_ip->ip_address;
            $data[ 'id' ] = $previous_public_ip->id;
        }
        if ($current_public_ip != $string_previous_public_ip) {
            $update = $this->updateOrCreate($data);
        }
        return response()->json([
            'updated' => $update
        ], 200);
    } catch (ConnectionError $e) {
        // Will return error 500
    } catch (ServiceError $e) {
        // Will return error 500
    } catch (\Exception $e) {
        // Will return error 500
    }
    return response()->json([
        'updated' => $update
    ], 500);
}

Can I lower the cyclomatic complexity? I did move yet a if/else for the update (updateOrCreate method I think it is clear his work), but it is not sufficient.

Thank you!

Your three catch blocks and two returns do the same thing as each other.  You only need one of each.

$update is referenced outside of try, should be defined there too.

$current_public_ip is just another name for $data[ 'ip_address' ].  It's not any shorter or clearer and doesn't resemble its other name very closely.

$previous_public_ip isn't an IP address, it's an object that contains an IP address and other stuff.  It would be clearer to simply name it $previous, which lets you drop the redundant string_ from $string_previous_public_ip.  _public can go too.

public function update()
{
    $update = false;
    $status = 500; // default
    try {
        $data = array();
        $previous_ip = null;
        $data[ 'ip_address' ] = $this->getGateway()->fetchPublicIp($this->getIp());
        $previous = $this->getGateway()->getLastRecord();
        if ( isset($previous->ip_address) ) {
            $previous_ip = $previous->ip_address;
            $data[ 'id' ] = $previous->id;
        }
        if ( $data[ 'ip_address' ] != $previous_ip ) {
            $update = $this->updateOrCreate($data);
        }
        $status = 200;
    } catch ( \Exception $e ) {
        // return default status (500)
    }
    return response()->json([ 'updated' => $update ], $status);
}


Finds a pattern in a string by returning index of where the pattern starts in the string in Java This is the challenge I tried in Codefights: Avoid using built-in functions to solve this challenge. Implement them yourself, since this is what you would be asked to do during a real interview. Implement a function that takes two strings, s and x, as arguments and finds the first occurrence of the string x in s. The function should return an integer indicating the index in s of the first occurrence of x. If there are no occurrences of x in s, return -1. How can I make it faster? Thanks in advance. Here's my code, which is still slow for larger input cases(string of around 10^6 characters):

int findFirstSubstringOccurrence(String s, String x) {
   int j=0;
   for (int i=0; i<=(s.length()-x.length()); i++) {
       j=0;
       while ((j<x.length()) && s.charAt(i+j)==x.charAt(j)){
           j++;
       }
       if (j == x.length())
           return i;
       if(s.charAt(i+j)!=s.charAt(i))
           i += j;
   }
   return -1;
}

Naming

findFirstSubstringOccurrence is quite a long name. substringIndex is shorter and still very explicit about what it does.

Your parameters are not named very well. When calling your method, how do you know which of the strings is the substring and which one the containing string? I suggest the method header int substringIndex(string containing, string candidate).

Style

Your use of white space is very unconventional. Usually you would have spaces around operators for example.

Using braces around one liners makes your code prettier and more readible, and also prevents bugs when you add a line without noticing the missing braces. Instead of this:

if(s.charAt(i+j)!=s.charAt(i))
    i += j;

write this:

if (s.charAt(i + j) != s.charAt(i)) {
    i += j;
}

Your initialization of j is redundant, since you assign 0 at the start of the loop anyway. So just declare it before the loop, then assign it in the loop.

Performance

Expecting the likely case that your string does not contain the substring candidate and returning early can improve your performance in many cases, so check before looping whether the candidate is longer than the containing string.

After finding the first occurence of the first letter of the candidate in the containing string, the next improvement would be to check whether the last letter of the candidate is at position firstLetterPosition + candidateLength - 1. That allows you to quickly find situations, where only a part of the substring is contained. Then you can check the middle part.


Makefile to build and debug a C++ console app I had to create this makefile to build and debug a C++ console app. I just need some hints and tips on how I can organize my makefile.

CC=g++
CFLAGS=-c -Wall
LDFLAGS=
SOURCES=helloWorld.cpp
OBJECTS=$(SOURCES:.cpp=.o)
EXECUTABLE=helloWorld

all: $(SOURCES) $(EXECUTABLE)

debug: CXXFLAGS += -DDEBUG -g
debug: CCFLAGS += -DDEBUG -g
debug: helloWorld

clean:
    rm *o helloWorld

$(EXECUTABLE): $(OBJECTS)
    $(CC) $(LDFLAGS) $(OBJECTS) -o $@

.cpp.o:
    $(CC) $(CFLAGS) $< -o $@

Making sources?

The line

all: $(SOURCES) $(EXECUTABLE)

asks make to build $(SOURCES), in this case helloWorld.cpp. Is it possible to build it? Strictly speaking there are situations when you do want to build the source file (e.g. fetch it from git/cvs/sccs) but it is not applicable here: no rule is provided. Generally you don't want to build something which doesn't depend on anything. In any case, let make deduce; this is what it is good for.

all: $(EXECUTABLE)

is what you want.
Be consistent

all depends on $(EXECUTABLE), but debug depends on helloWorld. Once you defined a macro, use it everywhere.
Synonymous targets

Consider the scenario: make; ./helloWold; something goes wrong and you want to debug; make debug: everything is up to date. To have a debug build you must intervene with make clean. A good practice is to separate debug and release builds into different directories.
Automatic dependencies

In your example the .o file depends only on a corresponding .cpp. In real life the .cpp has some #includes - and the .o must depend on them all. Otherwise you will end up with an inconsistent build. Listing the .h dependencies manually is tedious and error prone. The standard practice is to let the compiler generate them automatically. For example, g++ has -MM, -MT, etc options just for this purpose:

DEPS := $(SOURCES:.cpp=.d)
.cpp.d:
    $(CC) $(CXXFLAGS) -MM -MT -o $@ $<
-include $(DEPS)

-c doesn't belong to CFLAGS

-c is typically not listed in a CFLAGS: you may want to generate various outputs (e.g. preprocessed source, assembly source, dependencies, documentation, etc) with the same set of flags. The way to achieve this is to specify -c or -MM or -S or whatever separately from other flags, e.g.

.cpp.o:
    $(CC) -c $(CFLAGS) ....

.cpp.s:
    $(CC) -S $(CFLAGS) ....

etc.
CC

Traditionally a c++ compiler is referred as CXX and uses CXXFLAGS. The CC and CFLAGS are reserved for plain c.


Counting Divisors of a number with restrictions on the divisors Given a fraction p/q, I want to count the number of unit fraction solutions

$$\frac{p}{q} = \frac{1}{u} + \frac{1}{v}$$

with some contraints

$$lower \le u \le upper$$

By manipulating the first equation and completing a square you arrive at a formula

$$(pu - q)(pv - q) = q^2$$

So the number of solutions is related to the divisors of q squared. If we only look at the smaller of two divisors (which we will call a), it is a solution if it produces a valid unit fraction (1 / u).

$$a = pu - q \implies u = \frac{a + q}{p}$$

We are interested in a divisor a iff

$$p*lower - q \le a \le p * upper - q, \space a \equiv -q \mod p$$

To compute the answer I first get the prime factorisation of q, then I  double each power to get the prime factorisation of q squared. Using this I enumerate over all the possible divisors, computing each one by trying all possible combination of powers. When I generate one I check if it matches the above requirements.

def factor_m(p, q, lower, upper):
    num = q
    factors = dict()
    count = 0
    while num % 2 == 0:
        num //= 2
        count += 1
    if count:
        factors[2] = count

    current = 3
    while num > 1:
        count = 0
        while num % current == 0:
            num //= current
            count += 1
        if count:
            factors[current] = count
        current += 2

    start, end = p * lower - q, p * upper - q
    mod = -q % p
    total = 0
    for powers in product(*(range((2 * v) + 1) for v in factors.values())):
        number = 1
        for b, _power in zip(factors, powers):
            number *= b ** _power

        if start <= number <= end and number % p == mod:
            total += 1

    return total

I'm calling this function a lot with reasonably big values of q (around 10**14 currently) so anything that saves time here is a massive boost.

Examples:

>>> factor_m(2, 7, 0, 50)  # 3
>>> factor_m(5, 1775025265104, 355005053021, 710010106041)  # 4101
>>> factor_m(737, 1046035200, 1926400, 2838630)  # 1
>>> factor_m(105467, 1231689911361, 11678439, 23356877)  # 0

EDIT To guarantee all solutions are distinct I only call the function with values that satisfy the constraint

$$lower \lt upper \le q$$

EDIT2 I ran the full program through cProfile along with the program with modifications suggested by Josay. Unfortunately it is slower, I suspect because it almost doubles the number of function calls.

current_code - top 3 functions by time

672574 function calls (614969 primitive calls) in 19.526 seconds

 ncalls  tottime  percall  cumtime  percall filename:lineno(function)
  53273   18.818    0.000   18.893    0.000 fast.py:40(factor_m)
   2957    0.295    0.000    0.298    0.000 fast.py:8(factor_p1)
57606/1    0.149    0.000   19.525   19.525 fast.py:75(f)

With Josay's changes - top 3 functions by time

1163833 function calls (1106228 primitive calls) in 19.835 seconds

ncalls  tottime  percall  cumtime  percall filename:lineno(function)
53273   18.427    0.000   19.160    0.000 fast.py:61(factor_m)
53273    0.500    0.000    0.650    0.000 fast.py:42(get_factors)
 2957    0.297    0.000    0.301    0.000 fast.py:8(factor_v2)

Code organisation

In order to improve the code performances, if could be a good idea to make the easier to understand and easier to change.

You could:

split your code into smaller functions
add documentation
add tests
reuse existing data structures such as collections.Counter.

We'd get something along the lines of:

from itertools import product
from collections import Counter
import time

def get_factors(n):
    """Get factor for integer n

    Return the factorisation of n as a dictionnary
    mapping prime factors to their exponents."""
    factors = Counter()
    d = 2
    while n % d == 0:
        n //= d
        factors[d] += 1
    d = 3
    while n > 1:
        while n % d == 0:
            n //= d
            factors[d] += 1
        d += 2
    return factors

def factor_m(p, q, lower, upper):
    """Return the number of unit fractions solutions to:
       p   1   1
       - = - + - with lower <= u <= upper
       q   u   v

    The equation is equivalent to ..."""
    factors = get_factors(q)
    start, end = p * lower - q, p * upper - q
    mod = -q % p
    total = 0
    for powers in product(*(range((2 * v) + 1) for v in factors.values())):
        number = 1
        for b, _power in zip(factors, powers):
            number *= b ** _power

        if start <= number <= end and number % p == mod:
            total += 1

    return total

t = time.time()
assert factor_m(2, 7, 0, 50) == 3
assert factor_m(5, 1775025265104, 355005053021, 710010106041) == 4101
assert factor_m(737, 1046035200, 1926400, 2838630) == 1
assert factor_m(105467, 1231689911361, 11678439, 23356877) == 0
print(time.time() - t)

Simple mathematical optimisation

A trivial optimisation can be added to the logic performing the prime decomposition: for all divisors d, d * d <= n. Thus we can stop the search earlier (and the remaining part is known to be a prime).

while n >= d * d:
    while n % d == 0:
        n //= d
        factors[d] += 1
    d += 2
if n >= 1:    # to avoid 1 as a factor
    factors[n] += 1

This makes the code only very slightly faster as the bottleneck is not in that part of the code.

Limit the number of operations performed/limit accesses to factors

When iterating over factors, you get the values to create range of exponents (0, 1, 2, ...) and then, in the loop, you zip this to the keys to generate the prime**exponent value.

You could feed into the product function, the values prime**exponent already computed so that you limit the number of operations (** mostly) and you access factors in a single place.

This would give something like:

def get_divisors_for_square_of(n):
    """"Generate all divisors for n^2.

    Use the factorisation of n and double the exponents."""
    factors = get_factors(q)
    powers_list = [[p**v for v in range(2 * v + 1)] for p, v in factors.items()]
    for powers in product(*powers_list):
       number = 1
       for p in powers:
           number *= p
       yield number

On my setup, this halves the computing time.

Final code

import itertools
import collections

def get_factors(n):
    """Generate factors for integer n."""
    d = 2
    while n % d == 0:
        n //= d
        yield d
    d = 3
    while n >= d * d:
        while n % d == 0:
            n //= d
            yield d
        d += 2
    if n >= 1:    # to avoid 1 as a factor
        yield n

def get_divisors_for_square_of(n):
    """"Generate all divisors for n^2.

    Use the factorisation of n and double the exponents."""
    factors = collections.Counter(get_factors(n))
    powers_list = [[p**v for v in range(2 * v + 1)] for p, v in factors.items()]
    for powers in itertools.product(*powers_list):
       number = 1
       for p in powers:
           number *= p
       yield number

def factor_m(p, q, lower, upper):
    """Return the number of unit fractions solutions to:
       p   1   1
       - = - + - with lower <= u <= upper
       q   u   v

    The equation is equivalent to:
        puv = q(u + v)
    or
        (pu-q) (pv-q) = q^2
    which boils down to finding divisors of q.
    Let a = pu - q, then u = (a + q) / p when it gives an integer
    which is when (a + q ) mod p == 0.
    Also, we want p * lower - q <= a <= p * upper - q.
    ....
    Solution for 2/7 are (u, v) = [(4, 28), (7, 7), (28, 4)].
    """
    start, end = p * lower - q, p * upper - q
    return sum(1
               for a in get_divisors_for_square_of(q)
               if start <= a <= end and (a + q) % p == 0)

if __name__ == '__main__':
    import time
    t = time.time()
    for i in range(5):
        assert factor_m(2, 7, 0, 50) == 3
        assert factor_m(2, 21, 0, 500) == 9
        assert factor_m(5, 1775025265104, 355005053021, 710010106041) == 4101
        assert factor_m(737, 1046035200, 1926400, 2838630) == 1
        assert factor_m(105467, 1231689911361, 11678439, 23356877) == 0
    print(time.time() - t)

I'll update the solution if I find other things.


Syntax Checker C++ Data Structure - Stack Syntax Checker is a very common question of c++ programming using the data structure stack. However, this is the advanced level base on this question I found in a reference book. Here is the question:

Base on the question checking the string parentheses balance, the output is to print “Success” if all the brackets [] {} () are matched. Otherwise, print
"fail"

Sample Input:
{[()]()}[] //Output:Success
{}([[] //Output:3
{}([[]}] //Output:7
{}{(]} //Output:5
{}{](} //Output:4
( [43]( i++ ; ) ) { lol = 3 ; }  //Output:Success

My code as follow:

#include<iostream>
#include<stack>
#include<string>

bool arePair(char start, char end)
{
    if (start == '(' && end == ')') return true;
    else if (start == '{' && end == '}') return true;
    else if (start == '[' && end == ']') return true;
    return false;
}
bool areParanthesesBalanced(std::string exp)
{
    std::stack<char>  S;
    for (int i = 0; i < exp.size(); i++)
    {
        char c = exp.at(i);
        if (c == '(' || c == '{' || c == '[')
            S.push(exp.at(i));
        else if (c == ')' || c == '}' || c == ']')
        {
            if (S.empty()|| !arePair(S.top(), c)) {
                return false;
            }
            else {
                S.pop();
            }
        }
    }
    return S.empty() ? true : false;
}
int main(){
    std::string input;
    while (std::getline(std::cin, input)) {
        if (areParanthesesBalanced(input))
            std::cout << "Success"<< std::endl;
        else
            std::cout << "Fail" << std::endl;
    }
    return 0;
}

Ok, first things first, there are a couple of formatting points that would make your code a little nicer to read:

Be consistent with your curly braces {}. In a couple of places, you have the opening brace on the same line as the thing before it whereas you don't in others.
Be consistent with your naming too. I would name your stack s because it fits better with the camelCase used elsewhere.
Spaces on your #include <header>s too please. It's just nitpicking but it's what most other programmers will be doing / used to.

That aside, lets get into the body of the code itself:

bool arePair(char, char) is very verbose in it's definition. You could turn the entire function into a single boolean expression of the form:

return (start == '(' && end == ')')
    || (start == '{' && end == '}')
    || (start == '[' && end == ']');

An alternative would be to use a switch statement:

switch (start) {

case '(': return end == ')';
case '{': return end == '}';
case '[': return end == ']';

default:
    return false;
}

What you choose is a matter of preference. I would probably go for the switch approach, however, because to me it is clearer what is intended.

areParanthesesBalanced is badly named. There is a typo in "parentheses" and parentheses refers specifically to () not {}, [] or <>. areBracketsBalanced would be more appropriate.
One idea would be to pass const std::string& exp to the function rather than std::string. This prevents the program having to make a copy of the entire string in case you modify it.
Your for loop for (int i = 0; i < exp.size(); i++) { ... } never actually makes use of the value of i. Instead you could use a range for like this for (char c : exp) which just gives you each character in turn.
Your else is unnecessary because you've already returned execution from the function at that point.
The line return s.empty() ? true : false; is bad because stack<>::empty() returns a boolean! Therefore, just use return s.empty();.

Overall this gives:

bool arePair(char start, char end)
{
    switch (start) {

    case '(': return end == ')';
    case '{': return end == '}';
    case '[': return end == ']';

    default:
        return false;
    }
}

bool areBracketsBalanced(const std::string& exp)
{
    std::stack<char>  s;
    for (char c : exp)
    {
        if (c == '(' || c == '{' || c == '[')
            s.push(c);
        else if (c == ')' || c == '}' || c == ']')
        {
            if (s.empty() || !arePair(s.top(), c))
                return false;
            s.pop();
        }
    }
    return s.empty();
}

But...

That covers a lot of smaller points but having constants defining your pairs of brackets in multiple places seems messy. Here's one possible solution but there might well be a nicer one I haven't thought of:

int isBracket(char c)
{
    switch (c) {

    case '(': return  1;
    case ')': return -1;

    case '{': return  2;
    case '}': return -2;

    case '[': return  3;
    case ']': return -3;

    default:
        return 0;
    }
}

bool areBracketsBalanced(const std::string& exp)
{
    std::stack<char>  s;
    for (char c : exp)
    {
        if (isBracket(c) > 0) // Opening brackets
            s.push(c);
        else if (isBracket(c) < 0) // Closing brackets
        {
            if (s.empty() || isBracket(s.top()) + isBracket(c))
                return false;
            s.pop();
        }
    }
    return s.empty();
}

Instead of printing fail...

You want to know the position of the failing bracket? Try changing the return type of areBracketsBalanced to int. You're already determining at what position it fails implicitly by returning false.


activerecord, refactoring the source code I have three tables

class User < AR::Base
  has_many :lexemes, :dependent => :destroy
end

class Lexeme < AR::Base
  belongs_to :user
  belongs_to :word
end

class Word < AR::Base
  has_many :lexemes
end

This code is storing the word to Words table and assign it to the user.

word = Word.where(:word=>token).first_or_create
lexeme = @user.lexemes.where(:word_id=>word.id).first_or_create

I don't like my code, so is it possible to improve it? Thanks

class User < AR::Base
  has_many :lexemes, :dependent => :destroy
  has_many  :words, :through => :lexemes
end

class Lexeme < AR::Base
  belongs_to :user
  belongs_to :word
end

class Word < AR::Base
  has_many :lexemes
  has_many :users, :through => :lexemes
end

You should normally be able to create word by following code but it seems there is a bug in Rails.

word = @user.words.find_or_create_by_word(token)

Therefore I have changed the model to following. You may remove it when the bug is fixed.

class User < ActiveRecord::Base
  has_many :lexemes
  has_many :words, :through => :lexemes do
        def find_or_create_by_word(token)
            find_by_word(token) || create(:word => token)
        end
  end
end


A method for events callback with the correct "this" What do you think of the following "utility" :

/**
 * Contextualize a method for callback
 *
 * @param    methodName      String Method name
 * @param    context         Object Context (this)
 * @param    mixed  OPTIONAL Argument to place at first
 * @param    mixed  OPTIONAL Argument to place at second (etc...)
 */
contextualize: function(methodName, context) {
    var otherArgs;
    if (
            (typeof context[methodName] === "undefined") ||
            !!!(context[methodName].constructor &&
                    context[methodName].call &&
                    context[methodName].apply)) {
        error("No " + methodName + " function defined");
    }
    otherArgs = $.makeArray(arguments).slice(2);
    return (function(obj, otherArgs) {
        return (function() {
            var args = [];
            $.merge(args, otherArgs);
            $.merge(args, $.makeArray(arguments));
            return (obj[methodName].apply(obj, args));
        });
    }(context, otherArgs));
},

For example, I use it to use the "fooBar" method of current object as callback of a click event, forcing a true as first argument :

$('#button').click(Utils.contextualize('fooBar', this, true));

That will use correctly the following method :

//....
fooBar: function(fromButton, event) {
    event.preventTarget();
    if (fromButton) {
      this.anotherMethod();
    }
    ...
},
//....

First of all, since you're using jQuery already, why not use $.proxy() which does the same thing?

$('#button').click($.proxy(this, 'fooBar', true));

That's equivalent to your contextualize example, as far as I can tell. And $.proxy has the added benefit of working with functions themselves, not just their names. That means you can use it to wrap a function from some other object in a given context. I.e.

$('#button').click($.proxy(this.fooBar, someOtherContext, true));

Meanwhile, your contextualize can't similarly "re-contextualize" a function; it only works for functions present in the context argument.

There's also the built-in Function.prototype.bind which will return a function bound to a given context, like $.proxy does above:

$('#button').click(this.fooBar.bind(this, true));

It's available in all modern browsers, but if you want more compatibility, use $.proxy.

Second: !!!(...). Just use 1 negation, please. It'll coerce a value to a boolean and negate it just fine; there's absolutely no reason to negate it 2 more times. Besides, since the expression in the parentheses is boolean logic, it's already is a boolean value to begin with, so it really is straight-up negation; no coercion involved. Yes, using !! can be a handy way of coercing a value to a boolean without negating it, but even that is almost never needed since whatever you're coercing is thruthy or falsy all on its own (if(!!something) is equivalent to just if(something))

I'd also skip the "is this a function" check entirely. JS will complain all by itself, if you let it. Of course, it won't call your error function when it complains, but I'd rather have JS complain loudly than go through a custom error function. Sure, your function checks for the function's existence right away, rather than failing when trying to call it, but you can always remove the function from the context object afterward, so the upfront check guarantees nothing.

var boundFunction = Utils.contextualize(contextObj, 'fooBar');
delete contextObj['fooBar']; // boundFunction will now fail when called

Or you might want to add the function after calling contextualize, but you can't do that right now without error being called.

Third: You're returning a function by invoking a function that returns a function. Much like the triple-negation above, that's an unnecessary, round-about way of doing things. This will do the same

return function() {
    var args = [];
    $.merge(args, otherArgs);
    $.merge(args, $.makeArray(arguments));
    return (context[methodName].apply(context, args));
};

otherArgs and context are already available as closures, so there's no reason to pass them to a function, just to create new closures there.

So, yeah, your function will work, but it's unnecessary and overly complex, while being less versatile than the alternatives you already have at hand. I wouldn't recommend using it, to be honest. Sorry.


Pythonic way to flatten lists or arbitrary depth using assignment expressions I'm working through the classic "99 problems in prolog" collection. But trying to do them Pythonically. Flatten a list is problem 07.
I am flattening a list of lists of arbitrary depth. I have two solutions below. Which is more Pythonic?
# helper function
def isList( x ): return isinstance( x, list )

def flatten3_h( *xs ):
  if ( len(xs) == 0 ): return [] # base case, empty list/tuple
  hd, tl = xs[ 0 ], xs[ 1: ]
  if ( not(isList( hd ))): return [ hd ] + flatten3_h( *tl )
  return flatten3_h( *hd ) + flatten3_h( *tl )

def flatten4( *xs ):
  acc = []
  [ acc := acc + [x] if not(isList( x )) else acc + flatten4( *x ) for x in xs ]
  return acc

Here is example usage:
flatten4([ [] , [1, [2, [3, [], 4]], 5]] ) # => [1, 2, 3, 4, 5]

I'm guessing flatten4 is more Pythonic as it uses list comprehensions.
But I'm not sure since it uses an assignment expression which I just learned about. As it's a new feature in python3.8.
Any feedback is appreciated.

Welcome to Code Review.

Follow the PEP8 guide for naming conventions. Functions and variables would be snake_case.

When checking for empty list, you don't need to find the length. Empty lists/strings/sets are all falsy values in python.

Use better/fuller names. hd/tl -> head/tail.

There are a few inbuilt utilities in python, which might help you:

itertools.chain
itertools.chain.from_iterable
functools.reduce


A classy wine collection - Construction vs. Initialization The following is for a chapter exercise in C++ Primer Plus, 6/e by Stephen Prata, for the first part of Chapter 14 which deals with code reuse through composition. The problem is to implement a Wine class (for modelling a wine collection) which contains a std::string (the wine name), an int and a Pair of std::valarrays to hold the vintages and bottles held of each. (Pair is a template class provided by the author.)

My question is whether I've got a good solution for the Wine constructors, or if I should be setting up my Pair in the initializer list (and what that would look like.)

Here's my header…

/**
* @file     winec.h
* @brief    Wine class with containment
*/

#ifndef WINEC_H_
#define WINEC_H_

#include <string>
#include <valarray>
#include "pair.h"

typedef std::valarray<int> ArrayInt;
typedef Pair<ArrayInt, ArrayInt> PairArray;

class Wine
{
private:
    std::string mLabel;
    PairArray holdings;
    int yearsHeld;

public:
    Wine();
    Wine(const char *l, int y);
    Wine(const char *l, int y, const int yrs[], const int bot[]);

    void getBottles();
    std::string & label();
    int sum() const;
    void show() const;
};

#endif

… and my implementation…

/**
* @file     winec.cpp
* @brief    Wine class with containment
*/

#include "winec.h"

Wine::Wine()
    : mLabel("Unnamed"), yearsHeld(0)
{
    holdings.first() = ArrayInt(0);
    holdings.second() = ArrayInt(0);
};

Wine::Wine(const char *l, int y)
    : mLabel(l), yearsHeld(y)
{
    holdings.first() = ArrayInt(0, y);
    holdings.second() = ArrayInt(0, y);
}

Wine::Wine(const char *l, int y, const int yrs[], const int bot[])
    : mLabel(l), yearsHeld(y)
{
    holdings.first() = ArrayInt(y);
    holdings.second() = ArrayInt(y);

    for (int i = 0; i < yearsHeld; i++)
    {
        holdings.first()[i] = yrs[i];
        holdings.second()[i] = bot[i];
    }
}

std::string & Wine::label()
{
    return mLabel;
}

void Wine::show() const
{
    using std::cout;
    using std::endl;

    cout << "Wine: " << mLabel << endl;
    cout << "\tYear\tBottles\n";
    for (int i = 0; i < yearsHeld; i++)
    {
        cout << "\t" << holdings.first()[i];
        cout << "\t" << holdings.second()[i];
        cout << endl;
    }
}

void Wine::getBottles()
{
    using std::cin;
    using std::cout;

    cout << "Enter holdings for " << yearsHeld << " years.\n";
    for (int i = 0; i < yearsHeld; i++)
    {
        cout << "Year: ";
        cin >> holdings.first()[i];
        cout << "Bottles: ";
        cin >> holdings.second()[i];
    }
}

int Wine::sum() const
{
    int sum = 0;
    for (int i = 0; i < yearsHeld; i++)
    {
        sum += holdings.second()[i];
    }
    return sum;
}

… along with the author's Pair<> template:

/**
* @file       pair.h
* @brief  Defining & using a Pair template
*/

#ifndef PAIR_H_
#define PAIR_H_

#include <iostream>
#include <string>

template <class T1, class T2>
class Pair
{
private:
  T1 a;
  T2 b;

public:
  T1 & first();
  T2 & second();
  T1 first() const { return a; }
  T2 second() const { return b; }
  Pair(const T1 &aval, const T2 &bval) : a(aval), b(bval) {}
  Pair() {}
};

template <class T1, class T2>
T1 & Pair<T1, T2>::first()
{
  return a;
}

template <class T1, class T2>
T2 & Pair<T1, T2>::second()
{
  return b;
}

#endif

Code Review

Just wondering if there is an EC library inside windows.
Best to be descriptive and long winded here.

#ifndef WINEC_H_
#define WINEC_H_

Also you may want to put all your code inside a namespace.

In this scheme were you access the year and count you have to perform two array lookups. Not a big deal. But to me the logic seems the wrong way around.

typedef std::valarray<int> ArrayInt;
typedef Pair<ArrayInt, ArrayInt> PairArray;

I would have done this:

typedef Pair<int, int>         WineInfo;
typedef std::vector<WineInfo>  PairArray;

Now when you have the year (vintage) you have the count in the same object (better spacial locality).

Is this not simply the size of holdings?

    int yearsHeld;

I would not have a default constructor.
Is there a need to create an special "unnamed" object?

Sure you can pass by C-String

Wine::Wine(const char *l, int y)

But I usually make this a std::string const& then it can be used with strings and with C-Strings (the string is dynamically constructed in place). If you are worried about the copy you can always add a constructor that takes the parameter by r-value reference so it is moved into the object.

I don;t like passing y the number of years held. Then just creating a set of unitialized values. The year 0 is not really a valid value.

Wine::Wine(const char *l, int y)

What you could do is allow a list initializer. This takes a list of WineInfo that allows you to initialize the object.

Wine  myWine("Flore de Sole", {{2014,15},{2013,8},{2012,3},
                                {2011,1},{2010,1},{2009,1}}
            );

OK. So you try. But this is a very dangerous interface. There is no way to guarantee that the yrs array and the bot array are at least y size.

Wine::Wine(const char *l, int y, const int yrs[], const int bot[])
    : mLabel(l), yearsHeld(y)

You should pass references to objects of known size. You could pass a reference to an array of length y

template<int y>
Wine::Wine(const char *l, const int (yrs&)[y], const int (bot&)[y])
    : mLabel(l), yearsHeld(y)

Always hat getters. They break encapsulation.

std::string & Wine::label()
{
    return mLabel;
}

Also note you are returning a non cost reference. So the caller can alter the label dynamically.

w.label() = "Plop";

Sure. Always good to have a print function. But rather than fix it with std::cout pass the stream as a parameter. The default can always be std::cout

void Wine::show() const

I would write as

void Wine::show(std::ostream& out = std::cout) const

Always prefer \n over std::endl

    cout << "Wine: " << mLabel << endl;

The difference is that std::endl will also flush the buffer. This makes using streams very in-effecient. You practically never need to flush the buffer manually.

Rather than using a manual loop. Prefer to use an algorithm

    for (int i = 0; i < yearsHeld; i++)
    {
        sum += holdings.second()[i];
    }

Simpler to write:

    return std::accumulate(std::begin(holdings), std::end(holdings));

No need to copy if you just need a reference.

  T1 first() const { return a; }
  T2 second() const { return b; }

Just return by const reference. Then it can be used without having a copy made.

  T1 const& first() const { return a; }
  T2 const& second() const { return b; }


Exact probability for coin flip streaks In another question the user was determining the probability of having a streak of 6 heads or 6 tails in 100 coin flips. To find the probability they would generate 100 random coin flips and determine if there was a streak. They would test 10,000 such sequences of 100 flips to find that there was about an 80% chance of there being a streak in 100 coin flips.
I decided to compute the exact probability. For 100 flips there are \$2^{100}\$ possible outcomes. To determine the percentage I compute how many of them have a streak, and then divide by \$2^{100}\$.
My naive solution gets me the number for 20 flips in few seconds:
from itertools import product

def naive(flips, streak):
    return sum('h' * streak in ''.join(p) or
               't' * streak in ''.join(p)
               for p in product('ht', repeat=flips))

Result:
>>> naive(20, 6)
248384

My fast solution gets me the number for 100 flips instantly:
from collections import Counter

def fast(flips, streak):
    needles = 'h' * streak, 't' * streak
    groups = {'-' * streak: 1}
    total = 0
    for i in range(flips):
        next_groups = Counter()
        for ending, count in groups.items():
            for coin in 'ht':
                new_ending = ending[1:] + coin
                if new_ending in needles:
                    total += count * 2**(flips - 1 - i)
                else:
                    next_groups[new_ending] += count
        groups = next_groups
    return total

The idea is to have a pool of still ongoing games, but grouped by the last six flips, and counts for how often that group has appeared. Then do the 100 flips one at a time, updating the groups and their counts. Any group that at some point ends with a streak doesn't continue playing, instead I add it to the total result. The group occurred count times, there are flips - 1 - i flips left, and they can be anything, so multiply count with 2flips - 1 - i.
Results (note that the result for 20 flips is the same as with the naive solution):
>>> fast(20, 6)
248384
>>> fast(100, 6)
1022766552856718355261682015984

And dividing by 2100 gives me the percentage similar to those of the linked-to experiments:
>>> 100 * fast(100, 6) / 2**100
80.68205487163246

Any comments, suggestions for improvement?

Your code looks good. It's a little hard to read, but given the context that is ok!
We can also see that if new_ending is never in needles then your code looks like it will run in \$O(f2^s)\$ time, where \$f\$ is flips and \$s\$ is streak.
Whilst I can see the code in if new_ending in needles: will reduce the the time your code takes to run.
For example when streak=2 it will allow your code to run in linear time, it's not going to help much on bigger numbers - the code will still tend to \$O(f2^s)\$.
We can see how you're performing this optimization in the following.
Since you are not searching the descendent of HH, TT, HTT, THH, etc. it cuts down how big the tree will get.

We can clearly see that tails is just the inverse of heads.
If we focus on heads and split the 'base' and 'tail' (the repeating results) we get the following:
     HH 1/2^2
H    TT 1/2^3
HT   HH 1/2^4
HTH  TT 1/2^5
HTHT HH 1/2^6

Whilst it's cool it runs in linear time, it's not really that interesting.
And so when streak=2 the total chance for \$f\$ flips is:
$$\Sigma_{n=2}^f \frac{2}{2^n}$$
However when we look at streak=3 we can see the start of a distinguishing pattern.
     HHH 1/2^3
H    TTT 1/2^4
HH   TTT 1/2^5
HT   HHH 1/2^5
HHT  HHH 1/2^6
HTH  TTT 1/2^6
HTT  HHH 1/2^6
HHTH TTT 1/2^7
HHTT HHH 1/2^7
HTHH TTT 1/2^7
HTHT HHH 1/2^7
HTTH TTT 1/2^7

If we take the count of each size then we get:
3: 1
4: 1
5: 2
6: 3
7: 5

This is cool because it's the start of the Fibonacci numbers.
I have verified that the first 30 values are the same.
And so we now can assume we have an equation for streak=3:
$$\Sigma_{n=3}^f \frac{2F(n-2)}{2^n}$$
Doing the same thing for streak=4,5,6,10 give the following sequences:

4 - Tribonacci
5 - Tetranacci
6 - Pentanacci
10 - Fibonacci 9-step

In all this is a pretty compelling pattern.
And so we can write an algorithm to run in \$O(fs)\$ time where \$f\$ is flips and \$s\$ is streaks.
import collections
import itertools
from fractions import Fraction

def fibonacci_nth(size):
    store = collections.deque([0] * size, size)
    store.append(1)
    while True:
        yield store[-1]
        store.append(sum(store))

def coin_chance(flips, streak):
    if streak <= 0 or streak % 1:
        raise ValueError("streak must be a positive integer")
    if flips < 0 or flips % 1:
        raise ValueError("flips must be a non-negative integer")
    if streak == 1:
        return Fraction(flips != 0, 1)
    sequence = (
        Fraction(2 * numerator, 2 ** exponent)
        for exponent, numerator in enumerate(fibonacci_nth(streak - 1), streak)
    )
    return sum(itertools.islice(sequence, flips - streak + 1))

# Code to get OEIS sequences
def funky_finder(depth, size):
    desired = (['H'] * size, ['T'] * size)
    stack = [iter("HT")]
    stack_value = []
    while stack:
        try:
            coin = next(stack[-1])
        except StopIteration:
            stack.pop()
            if stack_value:
                stack_value.pop()
            continue
        _stack_value = stack_value + [coin]
        if _stack_value[-size:] in desired:
            yield ''.join(_stack_value)
        elif len(stack) < depth:
            stack_value.append(coin)
            stack.append(iter('HT'))

# I know, I know. But I was using this in a REPL!
size = 3; [i // 2 for i in sorted(collections.Counter(len(i) - size for i in funky_finder(20 + size, size)).values())]

>>> 100 * fast(20, 6) / 2**20
23.687744140625
>>> 100 * float(coin_chance(20, 6))
23.687744140625

>>> 100 * fast(100, 6) / 2**100
80.68205487163246
>>> 100 * float(coin_chance(100, 6))
80.68205487163246


Schedule talks in tracks Consider my simple business requirement as follows:
There is a list of talks and they need to be scheduled in tracks. Each track has a morning session starting at 9am and an afternoon session starting at 1pm ending by 4pm and a lunch break from 12pm to 1pm.
Given: A list of talks - Talk.txt

Python session 90min
Java session 75min
Blender session 120min

When: I call by business implementation
Then: It gives the list of scheduled track

Track 1:
9:00am Python session
10:30am Java session
12:00pm Lunch
1:00pm Blender session

Now to approach this problem I want to design a contract that expect to return the final result. Besides I'm thinking some of the future (or possible) business requirement changes.

Presentation of the schedule has changed to "Java session at 10:30am"

In above expected output there is a gap of 15 mins between lunch and the previous talk

Perhaps the morning session's start time has changed to 10am

The simplest way to designing the contract that I have in my mind is:
public interface IScheduleTalk
{
    List<string> GetScheduledTalk(List<string> usrInput)
}

But it does not help me with the above problems, and then I have change the contract if any the requirement changes in above mentioned way. Besides it does not address the gap.
I went to redesign my contract and have come up with below two approaches:
1st being:
public class Track
{
    public int TrackNumber { get; set; }
    public Dictionary<Slot, string> ScheduledTalks { get; set; }
}

public class Slot
{
    public string ScheduledTime { get; set; }
    public int DurationInMin { get; set; }
}

public interface IScheduleTalk
{
    List<Track> GetScheduledTalk(List<string> talks);
}

And the 2nd is:
public class Track
{
    public int TrackNumber { get; set; }
    public Dictionary<string, Talk> ScheduledTalks { get; set; } //Dictionary TKey represents the schedule time such as "9:00am"
}

public class Talk
{
    public int DurationInMin {get; set;}
    public string TalkName {get; set;}
}

public interface IScheduleTalk
{
    List<Track> GetScheduledTalk(List<string> talks);
}

I need a suggestion which one should I use as the contract. Or if there is/are better way to do it.

I see what you are trying to achieve, while your question does not fully meet the Code Review rules, but I guess some inputs won't hurt specially in my free time ;).

In my opinion,

You should use DateTime instead of string and int.
You should combine Talk and Slot, as it should be only one class that hold the meeting details (subject, time, and place ..etc).
You can use DurationInMin, but I would rather use DateTime and based on the business requirements, I would make a way to make the user input only integer while using DateTime in the back-end, (e.g. in the front-end, I make a field that only accepts int, and in the back-end, I call DateTime.AddMinutes()) this way, if the business logic required (in the future) to add meetings for future reference (such as making a meeting schedule for the next day or week ..etc), it'll be easy to adjust without being a break change.
Dictionary<Slot, string> should be Dictionary<int, Slot> the int will represent the TrackNumber, which will remove the need of TrackNumber.
GetScheduledTalk takes List<string> and return List<Track>, doesn't make any sense to me! using string list in this situation will be a real pain. Even if is it an input that you get from another resource (such as CVS). if there is any serialization process in this part, I would suggest moving it to a handler class, and create a model for it, then change the string to that model.

Finally, Naming Convention, you should keep your objects names clearer by naming each object to its rule. if the object is related to another object, then include the name of that object as well.

Here is a proposal that might help you in your implementation.

public enum ScheduleSessionName
{
    Python,
    Java,
    Blender
}

public class ScheduleSessionSlot
{
    public ScheduleSessionName SessionName { get; set; }

    public DateTime SessionTimeStart { get; set; }

    public DateTime SessionTimeEnd { get; set; }

    //you can use this method if you want to make SessionTimeEnd readonly property, so it will be automatically set with def
    public DateTime SetDefaultSessionTime(DateTime sessionTimeStart)
    {
        switch (SessionName)
        {
            case ScheduleSessionName.Python:
                return sessionTimeStart.AddMinutes(90);
            case ScheduleSessionName.Java:
                return sessionTimeStart.AddMinutes(75);
            case ScheduleSessionName.Blender:
                return sessionTimeStart.AddMinutes(120);
            default:
                return sessionTimeStart.AddMinutes(60);
        }

    }

}

public interface ISchedule : IEnumerable
{
    int Count { get; }

    void Add(int trackNumber, ScheduleSessionSlot sessionSlot);

    void Remove(int trackNumber);

    void Clear();

    new IEnumerator GetEnumerator();

}

public class Schedule : ISchedule
{

    private readonly Dictionary<int, ScheduleSessionSlot> _schedulesStore = new Dictionary<int, ScheduleSessionSlot>();

    public int Count => _schedulesStore.Count;

    public ICollection<int> TrackNumbers => _schedulesStore.Keys;

    public ICollection<ScheduleSessionSlot> SessionSlots => _schedulesStore.Values;

    public ScheduleSessionSlot this[int trackNumber]
    {
        get => _schedulesStore.ContainsKey(trackNumber) ? _schedulesStore[trackNumber] : null;
        set => _schedulesStore[trackNumber] = value;
    }

    public void Add(int trackNumber, ScheduleSessionSlot sessionSlot) => _schedulesStore.Add(trackNumber, sessionSlot);

    public void Remove(int trackNumber) => _schedulesStore.Remove(trackNumber);

    public bool Contains(int trackNumber) => _schedulesStore.ContainsKey(trackNumber);

    public bool Contains(ScheduleSessionSlot sessionSlot) => _schedulesStore.ContainsValue(sessionSlot);

    public bool Contains(KeyValuePair<int, ScheduleSessionSlot> schedule) => _schedulesStore.Contains(schedule);

    public void Clear() => _schedulesStore.Clear();

    public IEnumerator GetEnumerator() => _schedulesStore.GetEnumerator();

}

The rest, will require your action ;).


Page loader function This is the function which I wrote and am currently using in my project.  I want to know if there is a better way to write it:

function pageLoader(pageIndex){

            $(".ServicesSectionWrapper,.ServicesSectionWrapper .Selector,.ServicesSection,.JournalSectionWrapper,.JournalSectionWrapper .Selector,.JournalSection,.AboutSectionWrapper,.AboutSectionWrapper .Selector,.AboutSection").hide();

        switch(pageIndex){

        case 1:
        $(".AboutSectionWrapper").fadeIn(400,function(){$("#AboutWrapper").fadeIn(400,function(){$("#ManagerWrapper").fadeIn(400,function(){$("#DeveloperWrapper").fadeIn(400,function(){$("#DesignerWrapper").fadeIn(400,function(){$(".AboutSection").fadeIn(400,function(){$(".AboutSection").addClass("PreLoadRotate")})})})})})});
        break;

        case 2:
        $(".JournalSectionWrapper").fadeIn(400,function(){$("#DateOne").fadeIn(400,function(){$("#DateTwo").fadeIn(400,function(){$("#DateThree").fadeIn(400,function(){$("#DateFour").fadeIn(400,function(){$("#DateFive").fadeIn(400,function(){$("#DateSix").fadeIn(400,function(){$("#DateSeven").fadeIn(400,function(){$("#DateEight").fadeIn(400,function(){$(".JournalSection").fadeIn(400,function(){$(".JournalSection").addClass("PreLoadRotate")})})})})})})})})})});
        break;

        case 3:
        $(".ServicesSectionWrapper").fadeIn(400,function(){$("#AppsWrapper").fadeIn(400,function(){$("#ResponsiveWrapper").fadeIn(400,function(){$("#DigitalWrapper").fadeIn(400,function(){$("#PTRWrapper").fadeIn(400,function(){$(".ServicesSection").fadeIn(400,function(){$(".ServicesSection").addClass("PreLoadRotate")})})})})})});
        break;

        }
    }

And here is the HTML:

This is a menu (AboutSectionWrapper) which has selectors as you can see.  I want to load the menu first and then sequentially load the selectors and finally fade in the menu indicator and add a CSS class which has a transition in it.

It is the same for other cases but for example in CASE 2, there are more selectors!

display:none instead of .hide()

Instead of hiding them like this:

 $(".ServicesSectionWrapper,.ServicesSectionWrapper .Selector,.ServicesS...hide()

Add a class common to each (on the HTML) and with that class, use display:none.

.commonClass{display:none}

Sequential Animation

Well, I haven't found anything elegant for sequential jQuery animation (or haven't looked that hard). However, I have made this which executes functions sequentially. It isn't a generic solution for animation, only for this situation.

function animateQueue(q){

    // Get the next item
    var next = q.shift();

    // Remove from the array the selector (or jquery object) and function name
    var obj = $(next.shift());
    var functionName = next.shift();

    // Add in the callback that runs after the animation
    next.push(function(){
       if(!q.length) return; // When done, don't continue
       animateQueue(q);      // otherwise, pass on the queue
    });

    // Run the animation
    jQuery.fn[functionName].apply(obj,next);
}

All you need to do is write the queue like so:

[selector OR jQuery object, animation function, params except the callback]
animateQueue([
    ['#red','fadeIn',400],
    ['#blue','fadeIn',400],
    ['#green','fadeIn',400],
    [$('#red'),'fadeOut',400],
    [$('#blue'),'fadeOut',400],
    [$('#green'),'fadeOut',400]
]);

Added for a jQuery object since...

Cache objects

to avoid fetching them from the DOM everytime

var red = $('#red');
var blue = $('#blue');
var green = $('#green');

animateQueue([
    [red,'fadeIn',400],
    [blue,'fadeIn',400],
    [green,'fadeIn',400]
]);

Taking it further

Since the only vital function for the pageLoader with respect to this functionality is selecting what to animate, we can move out everything but the selection. Further, we can store the animation queues into an array, and have pageIndex select them instead. pageIndex should be 0-indexed, so we subtract 1:

var queues = [
  first : [...],
  second : [...],
  third : [...]
];

function animateQueue(q){...}

function pageLoader(pageIndex) {
  animateQueue(queues[pageIndex-1]);
}


Truth table generator for an arbitrary function Summary: This function generates a truth table for a boolean function of variable number of arguments. The name of the function passed and its arguments are deduced outside of the computable function scope. The returned value is a logical matrix with columns corresponding to function arguments and result.

library(R.utils)
library(stringr)

truthTable <- function(func, valuesOnly = F) {
  numArguments <- length(formals(func))
  if(valuesOnly) {
    values <- vector(length = 2^numArguments)
    for (i in 1:(2^numArguments)) {
      arguments <- rev(as.logical(intToBits(i-1)))[-(1:(32-numArguments))]
      values[i] <- do.call(func, as.list(arguments))
    }
    return(values)
  }
  result <- matrix(nrow = 2^numArguments, ncol = numArguments + 1)
  colnames(result) <- c(names(formals(func)), as.character(substitute(func)))
  for (i in 1:(2^numArguments)) {
    arguments <- rev(as.logical(intToBits(i-1)))[-(1:(32-numArguments))]
    result[i, ] <- c(arguments, do.call(func, as.list(arguments)))
  }
  return(result)
}

Example:

majority <- function(a, b, c) {
  return ((a & b) | (b & c) | (a & c))
}

truthTable(majority)

Result:

         a     b     c majority
[1,] FALSE FALSE FALSE    FALSE
[2,] FALSE FALSE  TRUE    FALSE
[3,] FALSE  TRUE FALSE    FALSE
[4,] FALSE  TRUE  TRUE     TRUE
[5,]  TRUE FALSE FALSE    FALSE
[6,]  TRUE FALSE  TRUE     TRUE
[7,]  TRUE  TRUE FALSE     TRUE
[8,]  TRUE  TRUE  TRUE     TRUE

Possible improvements:

Faster conversion of numeric iterator to logical vector
Usage of apply instead of iterative computation

What could be improved?

Any advice would be appreciated.

The main flaw that can be observed in your function is the presence of code duplication: expressions such as 2^numArguments and arguments <- rev(as.logical(intToBits(i-1)))[-(1:(32-numArguments))] appear multiple times. Code duplication is generally bad, you could refactor so that each of them appears only one time.

Other little things:

R.utils and stringr are loaded but never used.
It's better to use FALSE instead of F.

Here is an alternative solution using expand.grid:

truthTable2 <- function(func, valuesOnly = FALSE) {
  args <- formals(func)
  L <- setNames(rep(list(c(TRUE, FALSE)), length(args)), names(args))
  df <- expand.grid(L)
  result <- sapply(1:nrow(df), function(i) do.call(func, lapply(df, `[`, i)))
  # or result <- do.call(func, df) if func is vectorized
  if (valuesOnly) {
    unname(result)
  } else {
    df[[substitute(func)]] <- result
    as.matrix(df)
  }
}

truthTable(majority)
#          a     b     c majority
# [1,]  TRUE  TRUE  TRUE     TRUE
# [2,] FALSE  TRUE  TRUE     TRUE
# [3,]  TRUE FALSE  TRUE     TRUE
# [4,] FALSE FALSE  TRUE    FALSE
# [5,]  TRUE  TRUE FALSE     TRUE
# [6,] FALSE  TRUE FALSE    FALSE
# [7,]  TRUE FALSE FALSE    FALSE
# [8,] FALSE FALSE FALSE    FALSE

Benchmark:

bench::mark(
  truthTable(majority),
  truthTable2(majority),
  check = FALSE
)
# # A tibble: 2 x 13
# expression                min  median `itr/sec` mem_alloc `gc/sec` n_itr  n_gc total_time result memory time
# <bch:expr>            <bch:t> <bch:t>     <dbl> <bch:byt>    <dbl> <int> <dbl>   <bch:tm> <list> <list> <lis>
# 1 truthTable(majority)   89.5us  97.4us     9634.    1.44KB     6.22  4650     3      483ms <lgl[~ <df[,~ <bch~
# 2 truthTable2(majority) 189.6us 204.7us     4292.        0B     4.07  2110     2      492ms <lgl[~ <df[,~ <bch~
# # ... with 1 more variable: gc <list>

bigf <- function(a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q) TRUE

bench::mark(
  truthTable(bigf),
  truthTable2(bigf),
  check = FALSE
)
# # A tibble: 2 x 13
#   expression             min   median `itr/sec` mem_alloc `gc/sec` n_itr  n_gc total_time result               memory            time    gc
#   <bch:expr>        <bch:tm> <bch:tm>     <dbl> <bch:byt>    <dbl> <int> <dbl>   <bch:tm> <list>               <list>            <list>  <list>
# 1 truthTable(bigf)     2.13s    2.13s     0.469      64MB     5.63     1    12      2.13s <lgl[,18] [131,072 ~ <df[,3] [262,351~ <bch:t~ <tibble [1 x~
# 2 truthTable2(bigf)    2.43s    2.43s     0.412    76.5MB     5.77     1    14      2.43s <lgl[,18] [131,072 ~ <df[,3] [262,218~ <bch:t~ <tibble [1 x~
# Warning message:
# Some expressions had a GC in every iteration; so filtering is disabled.


Process a line in chunks via Regular Expressions with Ruby I'm writing a Ruby script to reverse engineering message log files. They come from an external system with the following characteristics:

Each line of the log file has at least one message.
Each line of the log file can have multiple messages.
Each message consists of a set of numbers separated by spaces (e.g. 30 0 -1 1 2 1).
Each message can have one of many different templates (e.g. some contain five numbers, others contains six).

The approach I'm using is to process each line, one at a time, via a method that takes a string to work on as an argument. It saves a copy of the initial input (for later comparison) then tries to match known patterns. When a pattern is matched, the string that made it up is removed. If there is nothing left, or if no more matches are found, the method exits. Otherwise, it calls itself with the remainder of the string to process. Here's the code I camp up with along with an example.

#!/usr/bin/evn ruby

def parse_line remainder_of_line

  puts "Processing: #{remainder_of_line}"

  # Save a copy of the initial input for later comparison
  initial_snapshot = remainder_of_line.dup

  # Look for known pattern matches, removing them if found
  if remainder_of_line.gsub!(/^(\d+) 0 -1 1 (\d+) \d+\s*/, '')
    puts " - Matched format 1 - found: #{$1} - #{$2}\n\n"
  elsif remainder_of_line.gsub!(/^\d+ 0 -1 2 (\d+) \d+\s*/, '')
    puts " - Matched format 2 - found: #{$1}\n\n"

  ### More patterns here.
  end

  # If noting changed, then no matches were found.
  if initial_snapshot.eql? remainder_of_line
    puts " - Line still has data but no matches found. (Left with: #{remainder_of_line}\n\n"
  # Keep going if there is anything left.
  elsif !remainder_of_line.empty?
    parse_line remainder_of_line
  end

end

line = "11 0 -1 2 13560 2 11 0 -1 2 13564 2 11 0 -1 1 36880 106 91 0 -1 1 36881 106 36881 106 91 1 13556 2 36880 106 36880 106 11 1 734 11 0 -1 1 36884 106 91 0 -1 1 36885 106 36885 106 91 1 13556 2 36884 106 36884 106 11 1 735 13556 2 31 18 799 13556 2 31 25 799 "

parse_line line

This works but I'm wondering if there is a better way.

Because you're using the "bang" version of gsub, parse_line modifies the string you pass to it, which is generally a not a good idea. I wouldn't expect a parsing method to "eat" my input.
Since there's only one line and your regexes are anchored to the start of it, there's little point in using gsub (i.e. global substitution), since you'll only ever match 1 occurrence of the pattern.
Don't bother with all the newline literals. puts will automatically add one, and if you want an extra one, you should be able to just say puts with no argument in a strategic location (i.e. after having tried all the patterns).

This seems like a good fit for Ruby's case statement (aka switch) since you can match against regexes directly. And Ruby also sets other magic variables besides $1 and $2 whenever you match a regex. There's no reason to make the method recursive, though. A simple loop would do nicely too.

For instance:

def parse_line(line)
  puts "Processing: #{line}"

  # Loop until the string's empty (or we hit the return below)
  until line.empty?
    # Try matching the line
    case line
    when /^(\d+) 0 -1 1 (\d+) \d+\s*/
      puts " - Matched format 1 - found: #{$1} - #{$2}"
    when /^\d+ 0 -1 2 (\d+) \d+\s*/
      puts " - Matched format 2 - found: #{$1}"

    # more patterns...

    else # no match
      puts " - Line still has data but no matches found. (Left with: #{line})"
      return # stop here
    end
    line = $' # set line to the *unmatched* part, i.e. the remainder
    puts "" # output an extra blank line
  end

  puts "Entire line matched, yay"
end


Return a rank for string of letters based on alphabetical permutations So, first off, a little bit of unrelated information: I am preparing for a technical interview. I was reading this article, How to Prepare for (and Ace) the Technical Interview, by Rich Hein and I decided to try to tackle the problem he uses as an example.

From the article:

  We are looking for a solution that is compact and runs quickly. What quickly means to us depends on the language you use, but it should take no longer than 500 milliseconds to run. Please use what language you are most comfortable with. However, please know that we do not have Visual Studio so C++ solutions should have the ability to compile with the standard G++.

  Problem:
  Consider a word as any sequence of capital letters A-Z, not limited to just dictionary words. For any word with at least two different letters there are other words composed of the same letters but in a different order. For instance, stationarily and antiroyalist, which happen to both be dictionary words. For our purpose alianrostily is also a word composed of the same letters as these two. We can then assign a number to every word based on where it falls within an alphabetically sorted list of all words made up of the same set of letters.

  One way to do this would be to generate the entire list of words and then find the desired ones, but this would be slow if the word is long. Write a program that takes a word as a command line argument and prints to standard output its number. Do not use the method above of generating the entire list. Your program should be able to accept any word 25 letters or less in length possibly with some letters repeated. It should use no more than 1 gig of memory and take no more than 500 milliseconds to run.

emphasis mine

So, this is my summarized understanding of the problem:

  Create a function that accepts a capitalized string of letters. You are tasked with finding its alphabetical ranking among the possible permutations for that given input. Your method should return the integer ranking. Do not store a list of the permutations.

What I first did was open up Excel and using the following example dacb come up with an equation of how to do this:

Then, I wrote the program as a C# Console Application:

using System;
using System.Collections.Generic;

namespace WordAlphabeticalRank
{
    class Program
    {
        static void Main(string[] args)
        {
            var testArray = new []{ "DACB", "CA", "FLCR", "RTZX" };

            foreach (var test in testArray)
            {
                Console.WriteLine("Alphabetical rank of \"{0}\": {1}", test, AlphabeticalRank(test));
            }

            Console.Read();
        }

        public static int AlphabeticalRank(string s)
        {
            var rankingArray = new List<char>(s.ToCharArray());
            rankingArray.Sort();
            var score = 1; //this is the lowest possible rank

            for(int i = 0; i < s.Length -1; i++){

                var rank = rankingArray.IndexOf(s[i]);
                score += Factorial(rank) * rank;

                rankingArray.RemoveAt(rank);
            }

            return score;
        }

        public static int Factorial(int n)
        {
            var product = 1;
            for (int i = n; i > 0; i--)
            {
                product *= i;
            }

            return product;
        }
    }
}

The above program, when run outputs:

  Alphabetical rank of "DACB": 20
  Alphabetical rank of "CA": 2
  Alphabetical rank of "FLCR": 3
  Alphabetical rank of "RTZX": 2

So, what points can I improve on? Did I understand the problem correctly? (I wasn't able to ask questions, but I definitely would have because I have some questions about the wording and examples). Was Excel a good idea? Is my solution easy to understand? And, last but not least, does it meet the requirements?

Your program should be able to accept any word 25 letters or less in length

The largest possible answer then is \$25! = 15511210043330985984000000\$. This value won't fit in an int, a long, or even a ulong. I would recommend using a BigInteger.


Multiply two 4x4 Matrices fn_mat4 fn_multMat4(fn_mat4 a,fn_mat4 b)
  {

  fn_mat4 ret;

  for (j = 0;j < 4;j++)
  {

  #pragma omp simd
  for (i = 0;i < 4;i++)
     ret.m[i+j*4]= a.m[j*4]*b.m[i] + a.m[j*4 + 1]*b.m[i+4] + a.m[j*4 + 2]*b.m[i+8] + a.m[j*4 + 3]*b.m[i+12];

  }
  return ret;
  }

If anybody knows how to reduce this to one loop instead of nested loops it would be greatly appreciated.


Wrapping the classList API to add/remove array of classes I've written a function that wraps the classList API, allowing you to pass an array of classes to add/remove, or the variable arguments already supported by classList.

The methods are the following:

var flatten = function(array) {
    return Array.prototype.concat.apply([], array);
};

var classMethod = function(method) {
    return function(element /*, classes... */) {
        return flatten(Array.prototype.slice.call(arguments, 1)).forEach(function(klass) {
            element.classList[method](klass);
        });
    }
};

var addClass = classMethod('add');
var removeClass = classMethod('remove');

Usage:

var elem = document.querySelector('.box');

addClass(elem,     'class1');
addClass(elem,     'class2', 'class3');
addClass(elem,    ['class2', 'class3']);

removeClass(elem,  'class1');
removeClass(elem,  'class2', 'class3');
removeClass(elem, ['class2', 'class3']);

I'm wondering if there's some better way to implement this function, say without using the forEach call and instead using Function.prototype.apply, but I get a TypeError: Illegal invocation when trying to refactor this into, say:

var classMethod = function(method) {
    return function(element /*, classes... */) {
        element.classList[method].apply(element, flatten(Array.prototype.slice.call(arguments, 1)));
    }
};

Is it possible to write this more succinctly, whilst keeping all of the use cases?

Interesting question,

I've been looking at this for a while, I dont think there is a more succinct way. I've been playing with reconstructing className from the add or remove function calls but it became a less succinct and terrible hack.

The core reason for that is default JS does not have enough convenience methods for arrays. If you were open to use something like LoDash, you could do this:

var addClass = function addClass(element /*, classes... */){
  element.className = _.uniq( element.className.split(' ').concat( _.flatten( _.rest( arguments ) ) ) ).join(' ');
};

var removeClass = function removeClass(element /*, classes... */){
  element.className = _.difference( element.className.split(' '),  _.flatten( _.rest( arguments ) )  ).join(' ');
};

Other than that I really like your code; proper naming, indenting, size of functions, etc.

Update on illegal invocation, you are passing element as this to a function that is attached to classList, if you pass classList, then there will be no exception.

var classMethod = function(method) {
    return function(element /*, classes... */) {
        var classList = element.classList;
        element.classList[method].apply(classList,flatten(Array.prototype.slice.call(arguments, 1)));
    };
};

And then that works.. My mind is still boggling at that, but there you go.


DFS/BFS implementation of Cormen's pseudo code This is DFS/BFS C++ code for Cormen pseudo code. Please comment on this.

#include "iostream"
#include "vector"
#include "list"

enum color{
    WHITE,
    GREY,
    BLACK
};

struct edge{
    int destination_vertex;
    edge(int ver){
        destination_vertex = ver;
    }
};

struct vertex{
    int id;
    color visited;
    std::list<edge>  list;
    vertex(int _id){
        id = _id;
    }
};

class graph
{
private:
    std::vector<vertex> vertexes;
    void dfs_visits(vertex& source);
    int next;
public:
    graph(void){
        next = 0;
    }
    ~graph(void){}
    void dfs();
    void bfs();
};

void graph::dfs(void)
{
    for(std::vector<vertex>::iterator iter =vertexes.begin();iter < vertexes.end();iter++ ){
        iter->visited = WHITE;
    }

    for(std::vector<vertex>::iterator iter =vertexes.begin();iter < vertexes.end();iter++ ){
        if(iter->visited == WHITE){
            dfs_visits(*iter);
        }
    }
}

void graph::dfs_visits(vertex& source){
    source.visited = GREY;
    for(std::list<edge>::iterator iter = source.list.begin();iter != source.list.end();iter++){
        if(vertexes[iter->destination_vertex].visited == WHITE){
            dfs_visits(vertexes[iter->destination_vertex]);
        }
    }
    source.visited = BLACK;
    std::cout<< source.id <<std::endl;
}

void graph::bfs(){

    for(std::vector<vertex>::iterator iter=vertexes.begin();iter != vertexes.end();iter++){
        iter->visited = WHITE;
    }

    std::queue<vertex*> bsf_q;

    bsf_q.push(&vertexes[0]);

    while(!bsf_q.empty()){
        vertex * v = bsf_q.front();
        bsf_q.pop();

        for(std::list<edge>::iterator iter = v->list.begin() ;iter != v->list.end();iter++ ){
            if(vertexes[iter->destination_vertex].visited == WHITE){
                vertexes[iter->destination_vertex].visited = GREY;
                bsf_q.push(& vertexes[iter->destination_vertex]);
            }
        }

        v->visited = BLACK;
        std::cout << v->id <<std::endl;
    }
}

Preliminaries

It's conventional to include system headers with brackets instead of quotes and prefer std::vector over std::list unless proven wrong by a benchmark (note that the use of the word "list" in CLRS is not used in the same sense as std::list).

#include <vector>
#include <limits>

Data structures

Separate your algorithms and data structures and use a common naming convention where types have capitalized initial letters

enum {
    INFINITY = std::numeric_limits<int>::max()
};

enum Color {
    WHITE,
    GREY,
    BLACK
};

struct Vertex {
    int id;

    // BFS properties
    Color color;
    int discovery;
    Vertex* parent;

    // DFS properties
    int finish;
};

// adjacenty list representation (Figure 22.1 (b) of CLRS 3rd ed.)
struct Graph {
    std::vector<Vertex> vertices;
    std::vector< std::vector<Vertex*> > adjacent;
};

Several things can be said about my choice of data structures. For the purpose of illustrating BFS and DFS I wouldn't make them full-fledged classes with data hiding. You also need to set up some scaffolding code to initialize a Graph object, and make sure that the Vertex* inside adjacent are actually only pointing to Vertex objects inside vertices of the same Graph object (so that in fact adjacent only holds non-owning pointers). Real-world code would encapsulate the enforcement of such invariants inside the constructor and modifying member functions. For brevity this is not being show.

I also would remove the Edge class because it is implicitly represented as an Vertex* in the adjacency list graph representation. Finally, because the id in Vertex is an int, you get away with using a std::vector as a container. For std::string vertex labels, one would need to use a std::map<std::string, std::vector<Vertex*>> for the adjacent data inside Graph.

Note that the properties of BFS and DFS are put inside Vertex. This is to accomodate the style of notation of CLRS. They write on p592 that you can put that information in separate data structures as well (e.g. a std::map<Vertex*, SearchProperties>). Boost.Graph (the closest thing in C++ to a standardized graph implementation) uses such property maps to achieve the same effect more elegantly.

Breadth-first search

Document your algorithms pre- and post-conditions, as well as the runtime complexity as a function of the input size (look it up in CLRS!). I would give the function breadth_first_search the same signature as in CLRS, except that they are not very careful about whether they pass by value, reference or pointer.

#include <queue>

// pre-condition: graph of vertices with unitialized BFS properties
void breadth_first_search(Graph& g, Vertex* s)
{
    for (auto& v: g.vertices)
        if (v.id == s->id) continue;
        v.color = WHITE;
        v.discovery = INFINITY;
        v.parent = nullptr;
    }
    s->color = GRAY;
    s->discovery = 0;
    s->parent = nullptr;
    std::queue<Vertex*> q;
    q.push(s);
    while (!q.empty()) {
        auto u = q.front();
        q.pop();
        for (auto v: G.adjacent[u->id])) {
            if (v->color == WHITE) {
                v->color = GRAY;
                v->discovery = u->discovery + 1;
                v->parent = u;
                q.push(v);
            }
        }
        u->color = BLACK;
    }
}
// post-condition: graph with initialized vertex color and discovery times

Notice that I would strongly recommend to use the C++11 features auto type deduction and ranged for-loop. These enormously improve the readability of your code: the code above is in almost exactly the same notation as the listing of BFS(G,s) on p595 of CLRS (except that I use color, discovery and parent instead of the terser c, d and pi.

Except for debugging purposes, I would eliminate the std::cout calls from the BFS algorithm. Because the algorithm also keeps track of the parent information, you can completely retrace the algorithms steps after it finishes, should you want to, and print whatever information necessary.

Depth-first search

For the depth-first search, I again would follow the CLRS conventions as much as possible:

void depth_first_search(Graph& g)
{
    for (auto& u: g.vertices) {
        u.color = WHITE;
        u.parent = nullptr;
    }

    for (auto& u: g.vertices) {
        if (u.color == WHITE)
            depth_first_search_visit(g, &u, 1)
    }
}

void depth_first_search_visit(Graph& g, Vertex* u, int time)
{
    u->color = GRAY;
    u->discovery = time;
    for (auto v: g.adjacent[u->id]) {
        if (v->color == WHITE) {
            v->parent = u;
            depth_first_search_visit(G, u, time + 1)
        }
    }
    u->color = BLACK;
    u->finish = time + 1;
}

Note that I would pass the time variable by value along the recursive calls to depth_first_search_visit, which makes it slightly clearer to reason about the algorithm (and also avoids a few explicit increments here and there). Again note that the code above is in almost exactly the same notation as the listing of DFS(G) on p604 of CLRS.

Extra: note that BFS is written as a single function with an init phase + infinite loop over a std::queue, and DFS as an init function + a recursive fuction. It is a nice exercise to rewrite DFS as a single function with an init phase + infinite loop over a std::stack (instead of the implicit stack used by the recursion).

Further reading

If you want to know how the pros are doing it: familiarize yourself with Boost.Graph.


Finding the best matching block/patch in Python I wish to locate the closest matching NxN block within a WxW window centred at location (x,y) of a larger 2D array. The code below works fine but is very slow for my needs as I need to run this operation many times.

Is there a better way to do this?

Here N = 3, W = 15, x =15, y = 15 and (bestx, besty) is the centre of the best matching block

import numpy as np

## Generate some test data
CurPatch = np.random.randint(20, size=(3, 3))
Data = np.random.randint(20,size=(30,30))

# Current Location
x,y = 15,15
# Initialise Best Match
bestcost = 999.0
bestx = 0;besty=0

for Wy in xrange(-7,8):
    for Wx in xrange(-7,8):
            Ywj,Ywi = y+Wy,x+Wx

            cost = 0.0
            for py in xrange(3):
                for px in xrange(3):
                    cost += abs(Data[Ywj+py-1,Ywi+px-1] - CurPatch[py,px])
                    if cost >= bestcost:
                       break

            if cost < bestcost:
                bestcost = cost
                besty,bestx = Ywj,Ywi

print besty,bestx

The following outlines a couple variations of a numpy solution. For the problem sizes in your post, they all clock faster than 100us, so you should get your 30,744 calls in about 3 seconds.

import numpy as np

N = 3

W = 15
x, y = 15, 15

data = np.random.randint(20, size=(30, 30))
current_patch = np.random.randint(20, size=(N, N))

# isolate the patch
x_start = x - W//2
y_start = y - W//2

patch = data[x_start:x_start+W, y_start:y_start+W]

# take a windowed view of the array
from numpy.lib.stride_tricks import as_strided
shape = tuple(np.subtract(patch.shape, N-1)) + (N, N)
windowed_patch = as_strided(patch, shape=shape, strides=patch.strides*2)

# this works, but creates a large intermediate array
cost = np.abs(windowed_patch - current_patch).sum(axis=(-1, -2))

# this is much more memory efficient, but uses squared differences,
# and the fact that (a - b)^2 = a^2 + b^2 - 2*a*b
patch2 = patch*patch
ssqd =  as_strided(patch2, shape=shape,
                   strides=patch2.strides*2).sum(axis=(-1, -2),
                                                 dtype=np.double)
ssqd += np.sum(current_patch * current_patch, dtype=np.double)
ssqd -= 2 * np.einsum('ijkl,kl->ij', windowed_patch, current_patch,
                      dtype=np.double)

# for comparison with the other method
cost2 = windowed_patch - current_patch
cost2 = np.sum(cost2*cost2, axis=(-1, -2))

# with any method, to find the location of the max in cost:
best_X, best_y = np.unravel_index(np.argmax(cost), cost.shape)


Student class with std::string I am writing a class that has a few std::string as a member. I can't find clear guidelines on how use them in a safe way. Are std::string any different then int when we are talking about class members? Do I initialize the strings in the correct way? (How) can I improve the constructor?

#include <string>
#include <iostream>

class Student {
public:
    std::string name;
    Student(std::string name) {
        this->name = name;
    }
};

int main() {
    std::string name1 = "john";
    Student student1(name1);
    name1 = "jenny";
    std::cout << student1.name << std::endl;
    {
        std::string name2 = "ashley";
        student1.name = name2;
        name2 = "remy";
    }
    std::cout << student1.name << std::endl;
    return 0;
}

Which gave the following output as intended:

  john
  ashley

Consider replacing this:

Student(std::string name) {
    this->name = name;
}

with one of these two:

Student(std::string n)
: name{ std::move(n) } // assign here, not in function body
{
}

Student(const std::string& n)
: name(n) // assign here, not in function body
{
}

They are more efficient and more idiomatic.

The first takes the argument by value (constructs a string copy when it receives an argument) , then moves the constructed value into Student::name.

The second is marginally faster than the first (but this speed difference should not matter in most use cases, so which you choose is a matter of style), but it doesn't express as well as the first, the idea that Student gets it's own copy of the string (passing by value means "the function gets it's own copy of the value", or "the function takes ownership of the argument").


Java enum-based factory to calculate entry parameters I want to create a factory to return all Source instances that match given entry parameters- that is, I want to iterate through some Collection of objects and for each entry check boolean method.
I thought about enum, as  I'm guaranteed to iterate through them all.
I came up with something like this:

simple interface

public interface Source {

    public boolean isPresent(SourceCalculationData data);

    public Source2 getType();
}

my take on enum factory:

public enum Source2 {

    IB("IB", SourceIb.class),
    CCWEW("BOT", SourceCcwew.class),
    OBD("CUF", SourceCuf.class),
    PB("PB", SourcePb.class),
    COK("COK", SourceCok.class),
    CCZEW("CCZ", SourceCczew.class),
    CRM("CRM", SourceCrm.class),
    APP("APP", SourceApp.class);

    private static final Logger SENSITIVE_LOGGER = SensitiveLoggerFactory.getSensitiveLogger(Source2.class);

    private String configChannel;

    private Class<? extends Source> clazz;

    private Source2(String configChannel, Class<? extends Source> clazz) {
        this.configChannel = configChannel;
        this.clazz = clazz;
    }

    public Class<? extends Source> getClazz() {
        return clazz;
    }

    public String getConfigChannel() {
        return configChannel;
    }

    public Source getSource() {
        return getAction(clazz);
    }

    private Source getAction(Class<? extends Source> source2Class) {

        try {
            return source2Class.newInstance();

        } catch (InstantiationException | IllegalAccessException e) {
            SENSITIVE_LOGGER.warn("<< Exception occured during init of {}, returning unkown source.", source2Class);
            return new SourceUnkown();
        }

    }

and use case:

        final SourceCalculationData data = new SourceCalculationData();

        final EnumSet<Source2> presentSources = EnumSet.noneOf(Source2.class);
        for (final Source2 source2ToCheck : Source2.values()) {
            if (source2ToCheck.getSource().isPresent(data)) presentSources.add(source2ToCheck);
        }

I'm pretty sure this is not the best code in the world. Any suggestions what will make it better?

Your implementation is good, but I would suggest following points to improve:

It is really hard to test/mock source2ToCheck.getSource().
Source2 contains a lot logic/functionality (it is source type and
factory) it has to be simplified.
getSource() creates new course
object it is confusing
getSource(...) calls getAction(...) that create a source object. It is wired.
Be aware source2Class.newInstance() is deprecated since java 9
source2Class.getDeclaredConstructor().newInstance()

Suggestions:

I would propose to create dedicated interface/class for source factory
Rename Source2 to SourceType.
Throw custom exception CustomCantCreateSourceException instead of return new SourceUnkown(). Because it is real exceptional situation.
Remove public methods modifier from Source interface
Minor enhancements you can find in following code

Source Factory interface:

public interface SourceFactory {
    Source create(SourceType type);
}

Factory implementation:

public class SourceFactoryImpl implements SourceFactory {
    @Override
    public Source create(SourceType type) {
            try {
                return type.getClazz().getDeclaredConstructor().newInstance();
            } catch (InstantiationException | IllegalAccessException
                    | NoSuchMethodException | InvocationTargetException e) {
                SENSITIVE_LOGGER.warn("<< Exception occured during init of {}, returning unkown source.", source2Class);
                throw new CustomCantCreateSourceException();
            }
    }
}

Renamed Source2:

public enum SourceType {

    IB("IB", SourceIb.class),
    CCWEW("BOT", SourceCcwew.class),
    OBD("CUF", SourceCuf.class),
    PB("PB", SourcePb.class),
    COK("COK", SourceCok.class),
    CCZEW("CCZ", SourceCczew.class),
    CRM("CRM", SourceCrm.class),
    APP("APP", SourceApp.class);

    private static final Logger SENSITIVE_LOGGER = SensitiveLoggerFactory.getSensitiveLogger(SourceType.class);

    private String configChannel;

    private Class<? extends Source> clazz;

    private SourceType(String configChannel, Class<? extends Source> clazz) {
        this.configChannel = configChannel;
        this.clazz = clazz;
    }

    public Class<? extends Source> getClazz() {
        return clazz;
    }

    public String getConfigChannel() {
        return configChannel;
    }
}

Usage:

public class SourceService {
    private final SourceFactory factory;

    public SourceService(SourceFactory factory) {
        this.factory = factory;
    }

    public EnumSet<SourceType> fillSources() {
        final SourceCalculationData data = new SourceCalculationData();

        final EnumSet<SourceType> presentSources = EnumSet.noneOf(SourceType.class);
        for (final SourceType source2ToCheck : SourceType.values()) {
            if (factory.create(source2ToCheck).isPresent(data)) {
                presentSources.add(source2ToCheck);
            }
        }
        return presentSources;
    }
}


Thread safe logging class in C# to use from DLL I tried to sample up a logging class from this question and answer(I want to use this class from C# DLL).
Made slight modification mainly to file name, and also how data is written.
I am interested if this can be considered thread safe logging class?
I am particularly concerned by the comment under the answer. See below.
I didn't particularly understand the comment.

  You should also ditch the TextWriter.SyncTextWriter if you are going
  this way. Every method call on SyncTextWriter takes a lock on the
  instance. This is not necessary if you are wrapping the calls to the
  TextWriter in your own Logger classs methods. If you are passing a raw
  SyncTextWriter` around you can deadlock like this: lock(textWriter){
  textWriter.WriteLine("test"); // oops deadlock

Here is the class:

public static class Logger
{
    private static readonly object _syncObject = new object();

    static readonly TextWriter tw;

    static Logger()
    {
         // Note1:
         // My intent was that the log file name is same as the date on which the log
         // entry was added by the user - but apparently this approach as below is not
         // correct since it is in static constructor and the same date will be set for the file names even if log entries were added at different dates
         // Note2: The current answer doesn't address this, I would appreciate if someone can tell me where to move this code too to achieve what I want
         const string FMT = "yyyy-MM-dd";
         DateTime now1 = DateTime.Now;
         string strDate = now1.ToString(FMT);

        tw = TextWriter.Synchronized(File.AppendText(SPath() + "\\Log " + strDate + ".txt"));
    }

    public static string SPath()
    {
        return ConfigManager.GetAppSetting("logPath");
    }

    public static void Write(string logMessage)
    {
        try
        {
            Log(logMessage, tw);
        }
        catch (IOException e)
        {
            tw.Close();
        }
    }

    public static void Log(string logMessage, TextWriter w)    {
      // only one thread can own this lock, so other threads
      // entering this method will wait here until lock is
      // available.
      lock(_syncObject) {
       w.WriteLine("{0} {1} {2}", DateTime.Now.ToLongTimeString(),
          DateTime.Now.ToLongDateString(), logMessage);

       // Where to call flush??
       w.Flush();
     }
   }
}

PS. You can see classes like NLog have some requirements when they must be called from DLL(like config file must be in exe file which is calling DLL etc.). I'd like to avoid such things, as well as configurations which these classes require.

I don't think Log should be public, it is confusing. Me, as a client of your logging API, shouldn't need to create an instance of TextWriter to be able to log something. If I want to do some IO, I'll do it all by myself and won't pass by the logging API.

The naming is a little flawed. tw should be (I think), logTextWriter or something like that. Good variable name is worth more than the space you save by having bad ones. SPath should be GetLoggingPath or something like that.

If I understand your code correctly, if I leave the application running for 30 days, my log file will always be dated of the 1st day I opened the application, because of the static ctor, it may cause confusion.

In the Log method, there is a (very very very) small window for a bug. Instead of doing :

w.WriteLine("{0} {1} {2}", DateTime.Now.ToLongTimeString(), DateTime.Now.ToLongDateString(), logMessage);

you should do :

DateTime now = DateTime.Now;
w.WriteLine("{0} {1} {2}", now.ToLongTimeString(), now.ToLongDateString(), logMessage);

(If you don't want to consider the millisecond that could make DateTime.Now change, just consider I think it'd also be more readable.)

I'm no threading expert, but I know for a fact that your code is thread-safe, though there might (or not) be concurrency issues I'm not aware of if you have lot of calls on the Log method.

Finally, logging classes are usually interfaced. If you ever want to unit test your classes that uses the logging API, you don't want to be writing in a real file. Your class is static, meaning you can't interface it. This is a good opportunity to use the Singleton pattern :

public interface ILogger
{
    void Write(string logMessage);
}

public class Logger : ILogger
{
    private static readonly object _syncObject = new object();
    private static readonly Lazy<ILogger> instance = new Lazy<ILogger>(() => new Logger(),LazyThreadSafetyMode.ExecutionAndPublication);

    private readonly TextWriter textWriter;

    public static ILogger Instance => instance.Value;

    private Logger()
    {
        const string FMT = "yyyy-MM-dd";
        DateTime now1 = DateTime.Now;
        string strDate = now1.ToString(FMT);

        textWriter = TextWriter.Synchronized(File.AppendText(SPath() + "\\Log " + strDate + ".txt"));
    }

    public void Write(string logMessage)
    {
        try
        {
            Log(logMessage, textWriter);
        }
        catch (IOException e)
        {
            textWriter.Close();
        }
    }

    private void Log(string logMessage, TextWriter w)    {
        lock(_syncObject) {
            DateTime now = DateTime.Now;
            w.WriteLine("{0} {1} {2}", now.ToLongTimeString(), now.ToLongDateString(), logMessage);

            w.Flush();
        }
    }

    private static string GetPath()
    {
        return ConfigManager.GetAppSetting("logPath");
    }
}

Note the usage of Lazy<ILogger> which is a thread safe lazy loader generic utility to make sure the constructor isn't called twice by two different threads.

Using this interface, you can then use the dependancy injection to use your ILogger interface everywhere without caring about the implementation! :)


Json whitespace formatter I couldn't quickly find Java code with googling to make serialized Json easier to read (this is a debugging library, obviously implementation code wouldn't use this as it's just more characters). This is not intended to be viewed in a web browser, this is intended for console output mostly.  I'm fairly new to Json (that's probably why I don't know where to look for this), is there a use case that my code is missing?

public class JsonWhitespace {
    public static String toWhiteSpace(String noWhite) {
        StringBuilder sb = new StringBuilder();

        int tabCount = 0;
        boolean inArray = false;

        for(char c : noWhite.toCharArray()) {
            if (c == '{') {
                sb.append(c);
                sb.append(System.lineSeparator());
                tabCount++;
                printTabs(sb, tabCount);
            } else if(c == '}') {
                sb.append(System.lineSeparator());
                tabCount--;
                printTabs(sb, tabCount);
                sb.append(c);
            } else if(c == ',') {
                sb.append(c);
                if (!inArray) {
                    sb.append(System.lineSeparator());
                    printTabs(sb, tabCount);
                }
            } else if (c == '[') {
                sb.append(c);
                inArray = true;
            } else if (c == ']') {
                sb.append(c);
                inArray = false;
            } else {
                sb.append(c);
            }
        }

        return sb.toString();
    }

    private static void printTabs(StringBuilder sb, int tabCount) {
        for(int i = 0; i < tabCount; i++) {
            sb.append('\t');
        }
    }
}

I think you need to ignore the contents of quoted values.

Add the flag:

    boolean inQuotes=false;

then, at the top of your if statement:

        if (c == '"') {
            inQuotes=!inQuotes;
        }
        if (!inQuotes) {
            if (c == '{') {
                sb.append(c);
                sb.append(System.lineSeparator());
                tabCount++;
                printTabs(sb, tabCount);
            } else if(c == '}') {
                ...  // Your code
        } else {
            sb.append(c);
        }