The Geppy String: Regex vs Iteration We'll say that a lowercase 'g' in a string is "happy" if there is another 'g' immediately to its left or right. Return true if all the g's in the given string are happy.

gHappy("xxggxx") → true
gHappy("xxgxx") → false
gHappy("xxggyygxx") → false

I have written two functions to solve this problem, but I have some concerns:

Of the one with Regex, I don't like all the special cases.
Of the one with iteration, I don't like the weird helper method.

Both pass a fair number of tests and work correctly. Which one is better?

regex_geppy.rb

def geppy(string)
  return true if ! string.include?('g')
  return false if [0, 1].include?(string.length)
  return false if string.length == 2 && string != 'gg'
  return (not string.match("[^g]g[^g]"))
end

array_geppy.rb

def non_wrap_get(array, index)
  index < 0 ? nil : array[index]
end

def geppy(string)
  string
    .chars
    .each_with_index
    .all? { |curr, i|
      curr == 'g' ?
        non_wrap_get(string, i - 1) == 'g'  || non_wrap_get(string, i + 1) == 'g'
        : true
    }
end

geppy_tests

geppy("xxggxx") #=> true
geppy("xxgxx") #=> false
geppy("xxggyygxx") #=> false
geppy("g") #=> false
geppy("ag") #=> false
geppy("gg") #=> true
geppy("") #=> true
geppy("ab") #=> true
geppy("aaaa") #=> true
geppy('g'*1000) #=> true

Regular expressions are the right tool to use for this kind of test.  However, if you are going to use a regular expression to find lonely g characters, then you should use negative lookbehind and lookahead assertions instead of the negated character class [^g].

def geppy(str)
  !/(?<!g)g(?!g)/.match(str)
end


Cropping faces from images in a directory I am using the below code for cropping faces from images in a directory

import cv2
import glob
import os

def facechop(image,imgname):
    facedata = "haarcascade_frontalface_default.xml"
    cascade = cv2.CascadeClassifier(facedata)

    img = cv2.imread(image)

    minisize = (img.shape[1],img.shape[0])
    miniframe = cv2.resize(img, minisize)

    faces = cascade.detectMultiScale(miniframe)

    for f in faces:
        x, y, w, h = [ v for v in f ]
        cv2.rectangle(img, (x,y), (x+w,y+h), (255,255,255))

        sub_face = img[y:y+h, x:x+w]
        face_file_name = "faces/" + imgname + "_" + str(y) + ".jpg"
        cv2.imwrite(face_file_name, sub_face)

    # cv2.imshow(image, img)
    # cv2.waitKey(0)
    cv2.destroyAllWindows()

    return

# if __name__  '__main__':
path = "images"
images = glob.glob(path + "/*")
for imgpath in images[0:]:
    imagename = os.path.basename(imgpath)
    imgname = os.path.splitext(imagename)[0]
    facechop(imgpath, imgname)
    print(imgname,"exported")

# while(True):
#     key = cv2.waitKey(20)
#     if key in [27, ord('Q'), ord('q')]:
#         break

The code is too slow and sometimes even hangs the system. Please help improve.

So there are a couple of improvements that can be made to your code,

The first one would be, you are creating a new CascadeClassifier object for every picture this is a costly operation.

The next is the fact that you are resizing the image, to the size of the image? I am unsure why you are doing that, depending on the size of the faces in your images you could resize them to be smaller and the search for faces in the picture.

Harr cascade detection is costly you can customize the parameters depending on the sizes of faces in your image.

Your code draws a box around every face and then isolates it, why is that? Lots of this code looks to be vestiges of old code. Removing old code can help make it more clear what is going on.

You number the face images based on the y value of the box; it would be much cleaner to simply report the index of the face (e.g. face 0, 1, 2, 3, etc...)

Based on this, here is some straightforward revised code that is much cleaner, but detectMultiScale is still ~99% of the running time

import os

import cv2

image_path = "images"

def save_faces(cascade, imgname):
    img = cv2.imread(os.path.join(image_path, imgname))
    for i, face in enumerate(cascade.detectMultiScale(img)):
        x, y, w, h = face
        sub_face = img[y:y + h, x:x + w]
        cv2.imwrite(os.path.join("faces", "{}_{}.jpg".format(imgname, i)), sub_face)

if __name__ == '__main__':
    face_cascade = "haarcascade_frontalface_default.xml"
    cascade = cv2.CascadeClassifier(face_cascade)
    # Iterate through files
    for f in [f for f in os.listdir(image_path) if os.path.isfile(os.path.join(image_path, f))]:
        save_faces(cascade, f)


Extracting a number from provided URLs inside a text file I need to extract a ID (int number) from a Url.

Example: http://www.example.com/foo/bar/12345

For this I wrote 4 methods where the first one ReadIDsFromFile() is called by my constructor and the return value is set to a properties of this class. The methods are called in the order I posted them below.

private List<string> ReadIDsFromFile(string path)
    {
        // path is the full qualified path to a txt file. C:\text.txt
        List<string> TweetIDsList = new List<string>();
        string temp = string.Empty;

        using (StreamReader sr = new StreamReader(path))
        {
            while ((temp = sr.ReadLine()) != null)
            {
                if (ValidateUrl(temp))
                {
                    TweetIDsList.Add(ExtractID(temp));
                }
                else
                {
                    Logger.Log("Invalid URL: {0}", temp);
                }
            }
        }

        return TweetIDsList;
    }

private bool ValidateUrl(string url)
{
    Uri uriResult;
    bool result;
    return result = Uri.TryCreate(url, UriKind.Absolute, out uriResult) && (uriResult.Scheme == Uri.UriSchemeHttp || uriResult.Scheme == Uri.UriSchemeHttps);
}

private string ExtractID(string url)
{
    string id = string.Empty;
    char[] urlArray = url.ToCharArray();
    int result;
    for (int i = urlArray.Length - 1; i >= 0; i--)
    {
        if (Int32.TryParse(urlArray[i].ToString(), out result))
        {
            id += result;
        }
        else
        {
            break; // break loop. If tryparse fails it means we have reached a character which is not a number, probably a forward slash.
        }
    }

    return ReverseNumber(id);
}

private string ReverseNumber(string id)
{
    char[] tempArray = id.ToCharArray();
    string result = string.Empty;
    for (int i = tempArray.Length -1; i >= 0; i--)
    {
        result += tempArray[i];
    }
    return result;
}

My code is working without problems so far but I feel like that it is overly unnecessarily complicated. I am especially concerned about the reversed order of the ID and my attempt to reverse it. Is there a better way?

You can use a regular expression to find the ID. The pattern [0-9]+$ will match one or more occurrences of 0-9 at the end of the string. You can use it like this:

private static readonly Regex UrlId = new Regex("[0-9]+$");

private static string ExtractID(string url)
{
    var match = UrlId.Match(url);
    return match.Success
        ? match.Captures[0].Value
        : string.Empty;
}

Instead of using a StreamReader, consider using File.ReadLines

foreach (var line in File.ReadLines(path))
{
    if (ValidateUrl(line))
    {
        TweetIDsList.Add(ExtractID(line));
    }
    else
    {
        Logger.Log("Invalid URL: {0}", line);
    }
}

You can remove the variable result from ValidateUrl:

private static bool ValidateUrl(string url)
{
    Uri uriResult;
    return Uri.TryCreate(url, UriKind.Absolute, out uriResult) &&
        (uriResult.Scheme == Uri.UriSchemeHttp || uriResult.Scheme == Uri.UriSchemeHttps);
}


python: less ugly solution to the intersection of 3 lines in 3 dimensional space I've tagged this as homework because it was originally a school project, it was accepted but I'm not satisfied with the code.

I've used cramers rule to find the intersection of 3 lines in three dimensional space given the x, y and z coefficients but the code is hideous.

def trinomial():

       print "Trinomials require 3 equations with three variables each."
       firstX = input ("Please enter the coeficcient of X in the first equation: ")
       firstY = input ("Please enter the coefficient of Y in the first equation: ")
       firstZ = input ("Please enter the coefficient of Z in the first equation: ")
       firstA = input ("and what's after the = sign?")
       secondX = input ("Please enter the coefficient of X in the second equation: ")
       secondY = input ("Please enter the coefficient of Y in the second equation: ")
       secondZ = input ("Please enter the coefficient of Z in the second equation: ")
       secondA = input ("And what's after the = sign?")
       thirdX = input ("Please enter the coefficient of X in the third equation: ")
       thirdY = input ("Please enter the coefficient of Y in the third equation: ")
       thirdZ = input ("Please enter the coefficient of Z in the third equation: ")
       thirdA = input ("And what's after the = sign?")
       D= firstX*secondY*thirdZ+firstY*secondZ*thirdX+firstZ*secondX*thirdY-thirdX*secondY*firstZ-  thirdY*secondZ*firstX-thirdZ*secondX*firstY
       DX = firstA*secondY*thirdZ+firstY*secondZ*thirdA+firstZ*secondA*thirdY-thirdA*secondY*firstZ-thirdY*secondZ*firstA-thirdZ*secondA*firstY
       DY = firstX*secondA*thirdZ+firstA*secondZ*thirdX+firstZ*secondX*thirdA-thirdX*secondA*firstZ-thirdA*secondZ*firstX-thirdZ*secondX*firstA
       DZ = firstX*secondY*thirdA+firstY*secondA*thirdX+firstA*secondX*thirdY-thirdX*secondY*firstA-thirdY*secondA*firstX-thirdA*secondX*firstY
       #the next two if statements evaluate the D's, or determinants, to see if it's an inconsistant or identical systerm
       #if it's not, then it does the math to solve the equation
       if D == 0:
           print "Determinant is 0 evaluate for identical or inconsistant systems."
           if DX ==0 and DY == 0 and DZ == 0:
               print "System is dependant, there are infinite solutions"
               print
           else:
               print "System is inconsistant, there are no solutions"
               print
       if D != 0:
           AX = DX/D
           AY = DY/D
           AZ= DZ/D
       print "the solution to the set of equations is ("+str(AX)+","+str(AY)+","+str(AZ)+")."
       print

I'm sorry for the huge wall of text, the code works, and the solutions are correct, I'd just like to know if there's a neater way of doing this.

Instead of having variables firstX, secondX, thirdX, replace them with a list X
Don't put input and calculations in the same function, separate them
You do:

if D == 0:
     some stuff
 if D != 0:
     other stuff

use else instead

Your three lines calculating D, DX, DY, DZ are very similar. Make it a function.
The terms being subtractd parallel the ones being added. Extract the pair into a function.

Here is my result:

DEPENDENT = 0
INCONSISTENT = 1
GOOD = 2

def determinant(X, Y, Z):
    def term(x, y, z):
        return X[x]*Y[y]*Z[z] - X[z]*Y[y]*Z[z]
    return term(0, 1, 2) + term(2, 0, 1) + term(1,2,0)

def trinomial(X, Y, Z, A):

    D  = determinant(X, Y, Z)
    DX = determinant(A, Y, Z)
    DY = determinant(X, A, Z)
    DZ = determinant(X, Y, A)
#the next two if statements evaluate the D's, or determinants, to see if it's an inconsistant or identical systerm
#if it's not, then it does the math to solve the equation
    if D == 0:
       if DX == 0 and DY == 0 and DZ == 0:
           return DEPENDENT, None
       else:
           return INCONSISTENT, None
    else:
       return GOOD, [DX/D, DY/D, DZ/D]

def main():
    print "Trinomials require 3 equations with three variables each."
    X.append( input ("Please enter the coeficcient of X in the first equation: ") )
    Y.append( input ("Please enter the coefficient of Y in the first equation: ") )
    Z.append( input ("Please enter the coefficient of Z in the first equation: ") )
    A.append( = input ("and what's after the = sign?") )

    X.append( input ("Please enter the coeficcient of X in the second equation: ") )
    Y.append( input ("Please enter the coefficient of Y in the second equation: ") )
    Z.append( input ("Please enter the coefficient of Z in the second equation: ") )
    A.append( = input ("and what's after the = sign?") )

    X.append( input ("Please enter the coeficcient of X in the third equation: ") )
    Y.append( input ("Please enter the coefficient of Y in the third equation: ") )
    Z.append( input ("Please enter the coefficient of Z in the third equation: ") )
    A.append( = input ("and what's after the = sign?") )

    result, answer = trinomial(X, Y, Z, A)
    if result == DEPENDENT:
        print "System is dependant, there are infinite solutions"
        print
    elif result == INCONSISTENT:
        print "System is inconsistant, there are no solutions"
        print
    else:
        print "the solution to the set of equations is ("+ ",".join(map(str, answer)) + ")."
        print

But if you do any amount of python programming with this stuff you'll want to learn how to use numpy, which make your code:

def trinomial(X, Y, Z, A):
    system = numpy.array([X, Y, Z]).T
    unknowns = numpy.array([A])

    return numpy.linalg.solve(system, unknowns)


Dice roller in Ruby (mainly for D&D) I have been working with Ruby for about a week or two now, and I wanted to create a dice-rolling application that intakes any string (e.g. 1d4 + 1 or 3d20 + 4) and outputs the correct dice rolling.

def roll(amount = 0, sides = 0)
    #this is the function that gets a random roll with the amount of sides and the amount of dice inputted.
    srand
    total = 0
    (amount.to_i).times do
        total += rand(1..sides.to_i)
    end
    return total
end

while true
    raw_input = gets.chomp.to_s
    if raw_input == "exit"
        abort("May your rolls be ever natural.")
    end
    proc_input = raw_input.tr("^0-9", " ")
    #this keeps all the numbers from the string input.
    output = proc_input.split()
    if (output[2]) == nil
        mod = 0
    else
        mod = output[2]
    end
    fin = roll(output[0], output[1]) + mod.to_i
    puts "#{fin.to_s}"
end

Nice code overall, pretty readable, though there is space for improvements.

1) You don't have to return from ruby methods.

2) Good use of Enumerable methods will make code cleaner.

3) Use loop instead of while true

4) Also consider keyword arguments, as calling roll(2, 4) looks kind of cryptic.

Will all that in mind, we can turn your code into something like:

def roll(amount = 0, sides = 0)
  # rand(1..0) will return nil btw
  amount.to_i.times.sum { |t| rand(1..sides.to_i) }
end

loop do
  input = gets.chomp.to_s

  abort("May your rolls be ever natural.") if input == "exit"

  amount, sides, mod = input.tr("^0-9", " ").split
  fin = roll(amount, sides) + mod.to_i # nil converts to 0

  puts fin
end


Prime factorization in Python Beginner programmer here. What can I improve in my code? Are there things I'm doing wrong or are there easier ways of doing what I did?

def reduce(n):
    n = abs(n)
    ctr = 0
    factors_list = []
    for i in range(2,n+1):
        if ctr >= 1:
            break
        if n % i == 0:
            factors_list.append(i)
            factors_list.append(int(n/i))
            ctr += 1
    return factors_list

def isPrime(n):
    return 1 in reduce(n)

def primeFactorization(n):
    if isPrime(n):
        return reduce(n)
    factors = reduce(n)
    primeFactors = []
    while True:
        for e in factors:
            if isPrime(e):
                primeFactors.append(e)
                factors.remove(e)
            else:
                factors.extend(reduce(e))
                factors.remove(e)
        if len(factors) == 0:
            break
    return sorted(primeFactors)

Documentation and tests

Before improving your code, is it important to write tests for it to make you you don't break anything.

As you do it, you might want that you need to be a bit clearer about the behavior of your function. Let's see what your functions return with a simple piece of code:

def print_results():
    print("i, primeFactorization(i), reduce(i), isPrime(i)")
    for i in range(15):
        print(i, primeFactorization(i), reduce(i), isPrime(i))

Most results seem ok but why do we sometimes have "1" in the return value for primeFactorisation. In theory, we should have only prime numbers in it.

We could:

write doc to specify what the functions does (return prime factorisation)
write tests for the actual expected behavior
fix the code

Here are various snippets I've written to test the code. In a more serious project, you could use a testing framework.

def test_is_prime():
    primes = [2, 3, 5, 7, 11, 13]
    not_primes = [0, 1, 4, 6, 8, 9, 10, 12, 14, 15, 100, 100000]
    for p in primes:
        assert isPrime(p), p
    for np in not_primes:
        assert not isPrime(np), np

def test_prime_factorization():
    prime_factorisations = {
        2: [2],
        3: [3],
        4: [2, 2],
        5: [5],
        6: [2, 3],
        7: [7],
        8: [2, 2, 2],
        9: [3, 3],
        10: [2, 5],
        11: [11],
        12: [2, 2, 3],
        13: [13],
        14: [2, 7],
    }
    for n, factors in prime_factorisations.items():
        ret = primeFactorization(n)
        assert ret == factors, str(n) + ": " + str(ret) + "!=" + str(factors)

def test_prime_factorization_randomised():
    import random
    n = random.randint(2, 10000)
    ret = primeFactorization(n)
    m = 1
    assert sorted(ret) == ret, "return is not sorted for n:" + str(n)
    for p in ret:
        assert isPrime(p), "factor " + str(p) + " is not prime for n:" + str(n)
        m *= p
    assert m == n, "product of factors does not lead to original value:" + str(n) + ", " + str(m)

if __name__ == '__main__':
    print_results()
    test_is_prime()
    test_prime_factorization()
    for i in range(300):
        test_prime_factorization_randomised()

Now, it feels fuch safer to improve the code.

Also, you could use this to perform benchmarks: compute the operations many times and/or on huge numbers to measure the time it takes.

Style

Python has a style guide called PEP 8. I highly recommend reading it (many times) and trying to apply it as much as possible.

Among other things, the functions names should be in snake_case.

Algorithm

Splitting a problem into smaller problems and writing functions for these is usually a good idea.

Unfortunately, I am not fully convinced that the reduce functions really helps you here.

Let's see how things can be improved anyway.

Small simplifications/optimisations

We could use the divmod builtin to get both the quotient and the remainder of the division.

The ctr variable seems useless. It is used to break after we've incremented it. We could just break directly.

At this stage, we have:

def reduce(n):
    n = abs(n)
    factors_list = []
    for i in range(2,n+1):
        p, q = divmod(n, i)
        if q == 0:
            factors_list.append(i)
            factors_list.append(p)
            break
    return factors_list

Now it is clear that we either add i and p to the list only once or we add nothing as all. We could make this clearer:

def reduce(n):
    n = abs(n)
    for i in range(2,n+1):
        p, q = divmod(n, i)
        if q == 0:
            return [i, p]
    return []

Now, it is clear that the function:

returns [] in the cases 0 and 1
return [n, 1] when n is prime
return [d, n/d] where d is the smallest (prime) divisisors otherwise.

Also, reduce is called more than needed: everytime we do

if isPrime(n):
    ret = reduce(n)

on a prime number, we actually perform reduce twice which is very expensice, in particular for primes as we iterate up to n.

Thus, we could get an optimisation boost by writing:

def primeFactorization(n):
    """Return the prime factorisation of n in sorted order."""
    factors = reduce(n)
    if 1 in factors:
        return [n]
    primeFactors = []
    while True:
        for e in factors:
            new_factors = reduce(e)
            if 1 in new_factors:
                primeFactors.append(e)
                factors.remove(e)
            else:
                factors.extend(new_factors)
                factors.remove(e)
        if len(factors) == 0:
            break
    ret = sorted(primeFactors)
    return ret

Another key hindsight is prime factorisation is that the smallest divisors of n is at most sqrt(n) which limits the range you have to look in.

In your case, we could use this in reduce, change slightly how reduce behaves and write:

def reduce(n):
    """Return [a, b] where a is the smallest divisor of n and n = a * b."""
    n = abs(n)
    for i in range(2, int(math.sqrt(n)) + 1):
        p, q = divmod(n, i)
        if q == 0:
            return [i, p]
    return [n, 1]

def isPrime(n):
    """Return True if n is a prime number, False otherwise."""
    return n > 1 and reduce(n) == [n, 1]

def primeFactorization(n):
    """Return the prime factorisation of n in sorted order."""
    factors = reduce(n)
    if factors == [n, 1]:  # prime
        return [n]
    primeFactors = []
    while True:
        for e in factors:
            new_factors = reduce(e)
            if new_factors == [e, 1]:  # prime
                primeFactors.append(e)
            else:
                factors.extend(new_factors)
            factors.remove(e)
        if len(factors) == 0:
            break
    ret = sorted(primeFactors)
    return ret

which is much faster.

Then, you could get rid of:

    if len(factors) == 0:
        break

by looping with while factors.

Then using, list.pop(), you could get rid of remove (which takes a linear time):

def primeFactorization(n):
    """Return the prime factorisation of n in sorted order."""
    factors = reduce(n)
    if factors == [n, 1]:  # prime
        return [n]
    primeFactors = []
    while factors:
        e = factors.pop()
        new_factors = reduce(e)
        if new_factors == [e, 1]:  # prime
            primeFactors.append(e)
        else:
            factors.extend(new_factors)
    ret = sorted(primeFactors)
    return ret

Then it appears that the initial check is not really required as the logic is already performed inside the loop:

def primeFactorization(n):
    """Return the prime factorisation of n in sorted order."""
    primeFactors = []
    factors = [n]
    while factors:
        e = factors.pop()
        new_factors = reduce(e)
        if new_factors == [e, 1]:  # prime
            primeFactors.append(e)
        else:
            factors.extend(new_factors)
    ret = sorted(primeFactors)
    return ret

We can actually get rid of the sorting by popping the first element of the list, thus generating the factors in order:

def primeFactorization(n):
    """Return the prime factorisation of n in sorted order."""
    primeFactors = []
    factors = [n]
    while factors:
        e = factors.pop(0)
        new_factors = reduce(e)
        if new_factors == [e, 1]:  # prime
            primeFactors.append(e)
        else:
            factors.extend(new_factors)
    return primeFactors

Now, instead of a reduce function, we could write a somehow equivalent but easier to use get_smallest_div function. Taking this chance to rename all functions, the whole code becomes:

import math

def get_smallest_div(n):
    """Return the smallest divisor of n."""
    n = abs(n)
    for i in range(2, int(math.sqrt(n)) + 1):
        p, q = divmod(n, i)
        if q == 0:
            return i
    return n

def is_prime(n):
    """Return True if n is a prime number, False otherwise."""
    return n > 1 and get_smallest_div(n) == n

def get_prime_factors(n):
    """Return the prime factorisation of n in sorted order."""
    prime_factors = []
    factors = [n]
    while factors:
        n = factors.pop(0)
        div = get_smallest_div(n)
        if div == n:  # prime
            prime_factors.append(n)
        else:
            factors.extend([div, n//div])
    return prime_factors

def print_results():
    print("i, get_prime_factors(i), get_smallest_div(i), is_prime(i)")
    for i in range(15):
        print(i, get_prime_factors(i), get_smallest_div(i), is_prime(i))

def test_is_prime():
    primes = [2, 3, 5, 7, 11, 13]
    not_primes = [0, 1, 4, 6, 8, 9, 10, 12, 14, 15, 100, 100000]
    for p in primes:
        assert is_prime(p), p
    for np in not_primes:
        assert not is_prime(np), np

def test_prime_factorization():
    prime_factorisations = {
        2: [2],
        3: [3],
        4: [2, 2],
        5: [5],
        6: [2, 3],
        7: [7],
        8: [2, 2, 2],
        9: [3, 3],
        10: [2, 5],
        11: [11],
        12: [2, 2, 3],
        13: [13],
        14: [2, 7],
    }
    for n, factors in prime_factorisations.items():
        ret = get_prime_factors(n)
        assert ret == factors, str(n) + ": " + str(ret) + "!=" + str(factors)

def test_prime_factorization_randomised():
    import random
    n = random.randint(2, 10000)
    ret = get_prime_factors(n)
    m = 1
    assert sorted(ret) == ret, "return is not sorted for n:" + str(n)
    for p in ret:
        assert is_prime(p), "factor " + str(p) + " is not prime for n:" + str(n)
        m *= p
    assert m == n, "product of factors does not lead to original value:" + str(n) + ", " + str(m)

if __name__ == '__main__':
    start = time.perf_counter()
    import time
    print_results()
    test_is_prime()
    test_prime_factorization()
    for i in range(300):
        test_prime_factorization_randomised()
    print(get_prime_factors(9000000))
    print(get_prime_factors(9000000 * 701 * 701))
    print(time.perf_counter() - start)

My solution for this

If I was to write this from scratch, here is how I'd do it:

def is_prime(n):
    """Return True if n is a prime number, False otherwise."""
    if n < 2:
        return False
    return all(n % i for i in range(2, int(math.sqrt(n)) + 1))

def get_prime_factors(n):
    """Return the prime factorisation of n in sorted order."""
    prime_factors = []
    d = 2
    while d * d <= n:
        while n % d == 0:
            n //= d
            prime_factors.append(d)
        d += 1
    if n > 1:  # to avoid 1 as a factor
        assert d <= n
        prime_factors.append(n)
    return prime_factors


Simplify LINQ to Create Step Values Assume the following definition:

public class DataPoint {
    public DateTime Date { get; private set; }
    public double Value { get; private set; }
    public DataPoint(DateTime date, double value) { Date = date; Value = value; }
}

If var points = ... //a date ordered list of DataPoints

I want to convert points into a list or array that represents the steps between each subsequent value. E.g. [3, 5, 6, 10.5] would go to [3, 2, 1, 4.5] with dates.

Here's what I have:

var steps = (new[] {points.First()})
              .Concat(points
                        .Skip(1)
                        .Zip(points,
                            (curr, prev) => new DataPoint(curr.Date, curr.Value - prev.Value)))

I think my ruby background might have gotten the best of me as that doesn't seem very readable. Any thoughts on how to make things more approachable?

In my opinion, the Concat part is what's throwing it off. There is a lesser utilized Select overload that provides access to the index, which could be used like so (ignoring the DataPoint part to make a minimal example):

double[] points = new double[] { 3, 5, 6, 10.5 };

points.Select ((p, i) => p - (i == 0 ? 0 : points[i - 1]));


Optimizing word counter I took the Python class at Google code today, and this is what I made for the problem about building a word counter.

Please take a look and suggest any improvements that can be done.  Please do point out the bad practices if I have used any.

import sys

def make_dict(filename):
    """returns a word/count dictionary for the given input file"""
    myFile=open(filename,'rU')
    text=myFile.read()
    words=text.lower().split()
    wordcount_dict={}   #map each word to its count
    for word in words:
        wordcount_dict[word]=0
    for word in words:
        wordcount_dict[word]=wordcount_dict[word]+1
    myFile.close()
    return  wordcount_dict

def print_words(filename):
    """prints each word in the file followed by its count"""
    wordcount_dict=make_dict(filename)
    for word in wordcount_dict.keys():
        print word, "  " , wordcount_dict[word]

def print_top(filename):
    """prints the words with the top 20 counts"""
    wordcount_dict=make_dict(filename)
    keys = wordcount_dict.keys()
    values = sorted(wordcount_dict.values())
    for x in xrange (1,21): #for the top 20 values
        for word in keys :
            if wordcount_dict[word]==values[-x]:
                print word, "       ",wordcount_dict[word]

def main():
  if len(sys.argv) != 3:
    print 'usage: ./wordcount.py {--count | --topcount} file'
    sys.exit(1)

  option = sys.argv[1]
  filename = sys.argv[2]
  if option == '--count':
    print_words(filename)
  elif option == '--topcount':
    print_top(filename)
  else:
    print 'unknown option: ' + option
    sys.exit(1)

if __name__ == '__main__':
  main()

If you have to prepare a word counter, then the more adequate container is collections.defaultdict

Then your make_dict function could be written much more simply:

def make_dict(filename):
    """returns a word/count dictionary for the given input file"""
    wordcount_dict = defaultdict(int)
    with open(filename, 'rU') as myFile:
        for line in myFile:
            words = line.strip().lower().split()
            for word in words:
                wordcount_dict[word] += 1
    return wordcount_dict

Note that you don't need to care about initialization of dictionary entries for new keys for word counting, as defaultdict takes care of it.

Another different approach is to use OOP. That is, to create a word counter object with state initialization, methods and all the stuff. The code gets simplified, encapsulated and ready to be extended.

Below, there is a working OOP proposal. There are some improvements that can be implemented also in your functional version if you don't like OOP:

1) I simplified your methods. Now there is only one method print_words(self, number=None). If you want the best 20 then just indicate the number of words.

2) I included some optimizations to clean words that are splitted with punctuation characters (otherwise house, house. and house' would be counted as different), using constants from the string module.

non_chars = string.punctuation + string.whitespace
words = [item.strip(non_chars).lower() for item in line.split()]

3) I used operator.itemgetter for the sorting key (instead of lambdas. More readable, imho)

4) I used formatting for the print for a better look. Used classical %.

import operator
import string
from collections import defaultdict

class WordCounter(defaultdict):
    def __init__(self, filename):
        defaultdict.__init__(self, int)
        self.file = filename
        self._fill_it()

    def _fill_it(self):
        "fill dictionary"
        non_chars = string.punctuation + string.whitespace
        with open(self.file, 'rU') as myFile:
            for line in myFile:
                words = [item.strip(non_chars).lower() for item in line.split()]
                for word in words:
                    self[word] += 1

    def print_words(self, number=None):
        """prints the words with the top <number> counts"""
        wc_pairs = self.items()
        wc_pairs.sort(key=operator.itemgetter(1), reverse=True)
        number = number or len(wc_pairs)
        for word, count in wc_pairs[:number]:
            print "%-20s%5s" % (word, count)

my_wc = WordCounter('testme.txt')

print my_wc['aword']    # print 'aword' counts
my_wc.print_words()     # print all (sorted by counts)
my_wc.print_words(3)    # print top 3

And a final note: leaving a blank space before and after an operator and after commas in lists, increases readability of the text and is considered good practice.


Am I sufficiently cleaning incoming $_GET parameters, especially in the array? I'm trying to clean some incoming $_GET parameters.  I've not done this before, so I'd love feedback.

I'm especially concerned with the array.  While all the other parameters control simple logic, the array will be saved into the database and potentially output to users.

Please also feel free to point out any redundancies.

<?php
// int: only want a positive value
// array: wont know what this could be but it could be anything urlencoded
// string: three possible outcomes: foo, bar, fudge

( isset( $_GET["val1"] ) ? (bool) $val1 = true                              : (unset) $val1 );
( isset( $_GET["val2"] ) ? (int) $val2 = sanitize_absint( $_GET["val2"] )   : (unset) $val2 );
( isset( $_GET["val3"] ) ? (array) $val3 = sanitize_array($_GET["val3"])        : (unset) $val3 );
( isset( $_GET["val4"] ) ? (string) $val4 = sanitize_string($_GET["val4"])  : (unset) $val4 );

// further refine val4 string
val4_strip($val_4);

// sanitize
function gfahp_sanitize_absint( $int ) {
    $int = filter_var( $int, FILTER_SANITIZE_NUMBER_INT );
    $int = abs( intval( $int ) ); // positive number
    return $int;
}
function gfahp_sanitize_string( $string ) {
    // is this enough to prevent anything icky?
    $string = filter_var( $string, FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW );
    return $string;
}
function gfahp_sanitize_array( $array ) {
     $array = array_walk_recursive( $array, "gfahp_sanitize_string" );
     return $array;
}

function val4_strip($val_4){
// strip down val4
    if (!empty($val4){
        switch ($val4) {
            case 'foo':
                $val4 = 'foo';
            break;
            case 'bar':
                $val4 = 'bar';
            break;
            case 'fudge':
                $val4 = 'fudge';
            break;
            default:
            $val4 = (unset) $val4
            break;
        }
    }
    return $val4;
}

My 2 cents:

$int = abs( intval( $int ) ); // positive number

Silently fixing input values is evil, perhaps required, but still evil.

function gfahp_sanitize_array( $array ) {
     $array = array_walk_recursive( $array, "gfahp_sanitize_string" );
     return $array;
}

You assign and then return, I would just return immediately

function gfahp_sanitize_array( $array )
{
     return array_walk_recursive( $array, "gfahp_sanitize_string" );
}

Also, what is up with gfahp ? Hungarian notation of functions with acronyms is a slappable offence.

As for

function val4_strip($val_4){
// strip down val4
    if (!empty($val4){
        switch ($val4) {
            case 'foo':
                $val4 = 'foo';
            break;
            case 'bar':
                $val4 = 'bar';
            break;
            case 'fudge':
                $val4 = 'fudge';
            break;
            default:
            $val4 = (unset) $val4
            break;
        }
    }

}

You should probably go for a generic function that can look up a value in a set and assign it if found, otherwise return blank. ( Also that function does not return anything? ) (Also you are unsetting var4 in places ). Something like

$val4_set = array( "foo" , "bar" , "fudge" );

( isset( $_GET["val4"] ) ? (string) $val4 = sanitize_string_from_set($_GET["val4"], $val4_set)  : (unset) $val4 );

function sanitize_string_from_set( $string , $set )
{
  return array_key_exists( $string, $set )?$string:"";
}


Voting app in JavaScript using Ramda I'm just learning functional programming in JavaScript using Ramda.
I took this tutorial and implemented the server core.js component using Ramda.

The idea is that we'll have a collection of things to vote from: Movies, songs, programming languages, Horse JS quotes, anything. The app will put them against each other in pairs, so that on each round people can vote for their favorite of the pair. When there's just one thing left, that's the winner.

I attempted to use the point-free style as much as possible as I found this really forced me to avoid writing imperative code. I also avoided making any changes to the data structure defined by the tutorial though I felt that doing so could have potentially simplified some things quite a bit.  In real projects we can't always change an API.
Here is the data structure used:

(source: teropa.info)
Here's my code:
const R = require('ramda');

// overCalculatedValue :: Lens s a -> ( s -> (a -> a)) -> ( s -> s)
const overCalculatedValue = R.curry( (lens, func) => {
  return R.converge( R.over(lens), [func, R.identity]);
});
// setCalculatedValue :: Lens s a -> ( s -> a) -> ( s -> s)
const setCalculatedValue = R.curry( (lens, func) => {
  return R.converge( R.set(lens), [func, R.identity]);
});

// createEntryTallyLens :: string -> Lens s a
const createEntryTallyLens = R.useWith( R.lensPath, [R.flip(R.append)(['vote', 'tally'])] );
// isVotingOnLastCandidates :: State -> boolean
const isVotingOnLastCandidates = R.compose( R.equals(0), R.length, R.prop('entries'));
// pickWinner :: [[string],number] -> [ string, number] -> [[string],number]
const pickWinner = R.curry( ( result, candidate ) => {
  return result[1] === candidate[1] ? [ R.append(candidate[0], result[0] ), result[1]] :
            result[1] < candidate[1] ? [ [candidate[0]], candidate[1] ] : result;
});
const NoWinner = [[],-Infinity];

// getWinnerList :: State -> [string]
const getWinnerList = R.compose( R.nth(0), R.reduce(pickWinner, NoWinner), R.toPairs(), R.path(['vote', 'tally']));
// isWinnerDetermined :: State -> boolean
const isWinnerDetermined = R.compose( R.equals(1), R.length, getWinnerList);
// getOverallWinner :: State -> string
const getOverallWinner = R.compose(R.nth(0), getWinnerList);
// getNextCandidates :: State -> [string]
const getNextCandidates = R.compose(R.take(2), R.prop('entries'));
// addWinnerToList :: State -> ([string]->[string])
const addWinnerToList = R.compose(R.flip(R.concat()), getWinnerList);

// createNewVote :: State -> State
const createNewVote = R.compose( R.over(R.lensProp('entries'), R.drop(2)), setCalculatedValue(R.lensPath(['vote', 'pair']), getNextCandidates), R.omit('vote'));
// addWinnerBackToEntries :: State -> State
const addWinnerBackToEntries = overCalculatedValue( R.lensProp('entries'), addWinnerToList);

// doesWinnerExist :: State -> boolean
const doesWinnerExist = R.allPass([isVotingOnLastCandidates, isWinnerDetermined]);
// declareWinner :: State -> State
const declareWinner = R.compose( R.omit(['entries', 'vote']), setCalculatedValue( R.lensProp('winner'), getOverallWinner));
// setupNextElection :: State -> State
const setupNextElection = R.compose( createNewVote, addWinnerBackToEntries);

// setEntries :: State -> [string] -> State
exports.setEntries = (state, entries) => {
  return R.set(R.lensProp('entries'), entries, state);
};

// vote :: State -> string -> State
exports.vote = ( state, entry ) => {
  return R.over( createEntryTallyLens(entry), R.compose(R.inc, R.defaultTo(0)), state);
};

// startNextVote :: State -> State
exports.startNextVote = R.ifElse(doesWinnerExist, declareWinner, setupNextElection);

My questions:

Is converge with set or over the correct way to update a value in an object that must be calculated from other values in the object?
Any other areas where this could be simplified or improved?

You can use Destructuring assignment in importing functions from ramdajs.

const {compose, reduce, ....} = require('ramda'); instead of using R.compose.


Add functionality to object then remove those functions on export Outline

I have a form that needs validation. The problem is that the software requires custom validation that depends on multiple parameters.

I decided to pass the object into a function which will dynamically give the object the functions it needs to be validated.
Then once the validation is completed, I remove those functions from the object.

I'm adding the functions so I can chain them together, which I believe will provide me with great composition.

Usage

This is how I add the functions to the object:

let q = this.validator.validate(this.question)

I can then chain the added functions like this:

q.validateString().cleanUp();

That way my functions are composable and reusable.

Implementation

Now for the actual code to be reviewed:

export class Validator {
    validate(question) {
        let q = Object.assign(
            {},
            question,
            validationOptions
        );

        return q;
    }
}

let validationOptions = {
    validateString,
    cleanUp
};

function validateString() {
    return this;
}

function cleanUp() {
    Object.keys(validationOptions).map((i) => delete this[i]);
    return this;
}

I'm forced to use the class because I need Aurelia Dependency Injection. I compose my functions outside of that class Validator because they only need to work with the this binding.

This code works perfectly, and it executes as expected; however, I do have a few concerns:

Is there a downside to the way I add these functions?
The way I remove these functions works well, but are there better options? I can't use Symbol because I return the q object to another class.

Mild issue with assigning functions

It's possible that you could overwrite properties of a question if they have the same name as one of your validation functions. For example:

let question = {
  foo: "bar"
};

let validationOptions = {
    foo,
    cleanUp
};

When you execute Object.assign the later object's properties override earlier ones in the arguments array.

Alternative to deleting the functions

I think a better way to "delete" the added functions would be to just keep track of the original question object, and then return it from cleanUp:

export class Validator {
    validate(question) {
        let q = Object.assign(
            {},
            question,
            validationOptions
        );

        q.cleanUp = () => question;

        return q;
    }
}

let validationOptions = {
    validateString
};

function validateString() {
    // validate
    return this;
}

That way the initial object is unchanged, and you don't have to concern yourself with deleting the validation functions from the question when your done.

Wrapper Class

I think you also ought to consider making the validator a wrapper class for your question. That way your validation functions stay out of your data, you can still chain your methods, and you don't have to assign the validation functions for each object:

export class Validator {
    constructor(question) {
      this.data = question;
    }
    validateString() {
      return this;
    }
    cleanUp() {
      return this.data;
    }
}

If you want the validation functions to be free from an object, classes have prototypes that you can assign the methods to:

function validateString() {
  return this;
}

export class Validator {
    constructor(question) {
      data = question;
    }
}
Validator.prototype.validateString = validateString;

If you have a lot of functions you could do:

let validateOptions = {
  validateString,
  validateDate,
  validateAge
};

Object.keys(validateOptions).forEach((property) => {
  Validator.prototype[property] = validateOptions[property];
});


Converting a double to a std::string without scientific notation Problem

I need to convert a double to a std::string which contains no scientific notation and no unnecessary fillers.

Using std::fixed with iostream yielded ugly results - extra fillers (0's).  The closest working code I could get was to using sprintf("%g",...) and then dealing with exponentials, should they appear (beyond 6 places).

I am looking for improvements in robustness or clarity/simplicity in the implementation.

Details/Elaboration (addition)

I realize that the conversion between double and std::string and back can be expected to be lossy.  So I am not looking for an implementation that works for all cases.  For example, I don't expect to be able to convert "0.2000" to double and then get the same back (getting "0.2" back would be fine).  Similarly with "+0.2000": getting "0.2" back is ok.

I have attempted solutions involving sprintf("%g",...), but the result is automatic conversion to scientific notation when we go beyond six significant digits.  There appears to be no way to configure that setting.  Alternatively, sprintf("%f",...) requires that we manually set or calculate the exact precision that we want - which I don't know how to do if that information is lost inside a double.  Note that what I have found here seems to equally apply to most simple solutions involving  std::iostream and std::fixed.

#include <iostream>
#include <cstring>
#include <string>
#include <cassert>

static std::string
to_string( double d )
{
  char s[64];
  sprintf( s, "%g", d );

  char* e = std::strchr( s, 'e' );
  if ( e==nullptr )
    return s;
  char* p = std::strchr( s, '.' );

  // eg 2.4e-07
  int exp   = atoi( e+1 );   // -7
  int extra = 0;

  // snip period eg 24
  if ( p!=nullptr ) {
    extra = e-p-1;
    memmove( p, p + 1, strlen(s) - (p-s));
  }

  int coeff = atoi( s );
  // std::cerr << "DEBUG  = " << s << " coeff = " << coeff << " exp = " << exp << "\n";
  if ( exp<0 ) {
    sprintf( s, "0.%0*d", -exp+extra, coeff);
    return s;
  }

  // eg 7.3e+09
  sprintf( s, "%d%0*d", coeff, exp-extra, 0 );
  return s;
}

static void
test_string_conversion( std::string const& s )
{
    assert( to_string( atof( s.c_str() ))==s );
}

int main()
{
   test_string_conversion( "1.5203" );
   test_string_conversion( "-1.025" );
   test_string_conversion( "3.1415" );
   test_string_conversion( "-38271" );
   test_string_conversion( "0.8382" );
   test_string_conversion( "-0.8382" );
   test_string_conversion( "7" );
   test_string_conversion( "-3" );
   test_string_conversion( "0.0000024" );
   test_string_conversion( "7300000000" );
   return 0;
}

As mentioned in the comments of the other answer, your decimal floating point numbers are stored as binary and rarely have finite representations.

If you simply want code that passes your test I would mention that it is much easier to remove padding from std::fixed than it is to expand the scientific notation.

#include <iostream>
#include <string>
#include <sstream>
#include <limits>
#include <iomanip>
#include <cstdlib>
#include <cassert>
#include <cstddef>

static std::string to_string(double d)
{
    std::ostringstream oss;
    oss.precision(std::numeric_limits<double>::digits10);
    oss << std::fixed << d;
    std::string str = oss.str();

    // Remove padding
    // This must be done in two steps because of numbers like 700.00
    std::size_t pos1 = str.find_last_not_of("0");
    if(pos1 != std::string::npos)
        str.erase(pos1+1);

    std::size_t pos2 = str.find_last_not_of(".");
    if(pos2 != std::string::npos)
        str.erase(pos2+1);

    return str;
}

static void test_string_conversion(const std::string& str)
{
    assert( to_string( atof(str.c_str()) ) == str );
}

int main(void)
{
    test_string_conversion( "1.5203" );
    test_string_conversion( "-1.025" );
    test_string_conversion( "3.1415" );
    test_string_conversion( "-38271" );
    test_string_conversion( "0.8382" );
    test_string_conversion( "-0.8382" );
    test_string_conversion( "7" );
    test_string_conversion( "-3" );
    test_string_conversion( "0.0000024" );
    test_string_conversion( "7300000000" );
}


Clustering markers on Google map on zoom I have a method that is getting called when someone zooms camera on the map (Google map).
I recalculate the positions of markers of map and the code is clunky and not effective. I iterate over arrays three times per method call and that is really slow. When I have 600 markers the performance is really lacking.

What could I improve to make it faster?

I use following local variables:

float lastZoomLevel_; // last zoom value on last zoom
BOOL forceRecalculateMap; // when new data arrives I use this to force repositioning
NSArray *pointsArrayV_; // Array of BankPoint instances, object that holds location and description of marker

- (void)mapView:(GMSMapView *)mapView didChangeCameraPosition:(GMSCameraPosition *)position
{
    float currentZoomLevel = mapView.camera.zoom;
    float diff = fabs(currentZoomLevel - lastZoomLevel_);

    if (diff > 0.3 || self.forceRecalculateMap){

        self.forceRecalculateMap = NO;

        lastZoomLevel_ = currentZoomLevel;

        markersGroupArray_ = [[NSMutableArray alloc] init];

        for (BankPoint *bpoint in pointsArrayV_) {

            CLLocationCoordinate2D coord = bpoint.coordinates.coordinate;

            CGPoint currentPoint = [mapView.projection pointForCoordinate:coord];

            if ([markersGroupArray_ count] == 0){

                NSMutableArray *array = [[NSMutableArray alloc] initWithObjects:bpoint, nil];

                [markersGroupArray_ addObject:array];

            }

            else {

                BOOL flag_groupadded = NO;

                int counter= 0;
                for (NSMutableArray *array in markersGroupArray_){

                    for (BankPoint *bbpoint in array){

                        CLLocationCoordinate2D mcoord = bbpoint.coordinates.coordinate;
                        CGPoint mpt = [mapView.projection pointForCoordinate:mcoord];

                        if ([self distance:mpt point:currentPoint] < 20){
                            flag_groupadded = YES;
                            break;
                        }
                    }
                    if (flag_groupadded){

                        break;
                    }
                    counter++;

                }

                if (flag_groupadded){

                    if ([markersGroupArray_ count] > counter){
                        NSMutableArray *groupArray = [markersGroupArray_ objectAtIndex:counter];
                        [groupArray insertObject:bpoint atIndex:0];
                        [markersGroupArray_ replaceObjectAtIndex:counter withObject:groupArray];
                    }
                }
                else if (!flag_groupadded){

                    NSMutableArray * array = [[NSMutableArray alloc]initWithObjects:bpoint, nil];
                    [markersGroupArray_ addObject:array];
                }
            }
        }

        [mapView clear];

        for (NSArray *array in markersGroupArray_){

            BankPoint *item = [array objectAtIndex:0];

            CLLocationCoordinate2D coord = item.coordinates.coordinate;

            GMSMarker *marker = [[GMSMarker alloc] init];
            marker.position = coord;
            marker.map = mapView;
            marker.userData = item;
        }
    }
}

- (float)distance :(CGPoint)pointA point:(CGPoint) pointB
{
    return sqrt( (pow((pointA.x - pointB.x),2) + pow((pointA.y-pointB.y),2)));
}

Naming and Comments

The postfix underscore is quite strange to me, and I don't understand it at all.  It matches no convention I've ever seen or heard of, and it's just not obvious to me what's gained by sticking out there.  What's it for?

In your 93 lines of code, the only comments to be found are on the first 3 lines:
float lastZoomLevel_; // last zoom value on last zoom
BOOL forceRecalculateMap; // when new data arrives I use this to force repositioning
NSArray *pointsArrayV_; // Array of BankPoint instances, object that holds location and description of marker

And these comments are almost completely unnecessary.  And if they are to be considered necessary, then we easily make them unnecessary by improving the naming of the variables.
CGFloat _lastZoomLevel;

Needs no comment.
BOOL forceRecalculateMap;

Probably needs no comment.  Worst case scenario, we might leave a comment on any line that sets this variable.  Though the implications of this being set to YES should be pretty clear based on the variable name.
NSArray *pointsArrayV_;

This "needs" a comment, and therefore, it probably simply needs a better variable name.  Why not just call it _bankPoints?  This should make it sufficiently clear what sorts of objects the array is intended to contain.  And we needn't comment here about what those objects do.  If someone wants to know what a BankPoint instance does, they can go to BankPoint.h or BankPoint.m, or any documentation you've provided about the BankPoint class.

The bigger crime though is that there are no comments at all in this method:
- (void)mapView:(GMSMapView *)mapView didChangeCameraPosition:(GMSCameraPosition *)position

That wouldn't be so terrible if the method were simpler.  But it's a complicated method, and it's hard to understand what it's trying to do--which makes it hard to optimize.
You say you iterate over arrays three times per method call.  That's not actually true.  One over your fast iteration loops is nested.  You run the nested loop once per iteration of the outer loop.
A better plain-English description of exactly what this method is trying to do along with some embedded comments will already make this method significantly better even if it still performs exactly the same.  Single lines of code don't generally need comments.  I can read a single line of code and know what that line does.
What we need comments for are the bigger picture things.  What does that specific line do in context of the current scope?  In context of the method as a whole?  In context of the class as a whole?  Use comments to give your code more clear context.  This way, when we get lost in the line-by-line details, there's always a helpful comment to re-orient us to the context.
Adding these comments will help you remember the big picture and that can help you come up with a better algorithm for doing whatever it is you're doing.


Sum the remainders of ai%aj given an array "a" Given an array of numbers, I am creating a table where, the cell at the intersection of row i and column j contains a number ai % aj.
Numbers in the array can occur multiple times.

Example:
For numbers = [2, 5, 3, 3, 5], the output should be 32.
Here is the table that gets created:

  | 0 1 2 3 4
--------------
0 | 0 2 2 2 2
1 | 1 0 2 2 0
2 | 1 3 0 0 3
3 | 1 3 0 0 3
4 | 1 0 2 2 0

The sum of its elements is 32.

Guaranteed constraints:
1 ≤ numbers.length ≤ 10^5,
1 ≤ numbers[i] ≤ numbers.length.

function sumOfRemainders($numbers) {
    $resSum = 0;
    $len = count($numbers);

    if($len == 1)
        return 0;

    for($i=0; $i<$len; $i++)
        for($j=0; $j<$len; $j++)
            $resSum += $numbers[$i]%$numbers[$j];
    return $resSum;
}

The code works and is of course a brute force approach, but will take huge amount of time for large inputs, or will timeout in case of any testing environment.

How can I possibly make my code faster?
Any suggestions are most appreciated.

A drop in the bucket – but it will increase the performance a little bit at least.

Use foreach instead of for

foreach is faster than a for-loop. So using foreach-loops will decrease the computing time for large inputs. It will also make your function a bit easier, as you don't have to create the $len-variable:

function sumOfRemainders($numbers) {
    $sum = 0;

    if (1 === count($numbers)) {
        return 0;
    }

    foreach ($numbers as $a)
        foreach ($numbers as $b)
            $sum += $a % $b;

    return $sum;
}

Now, for a single run on a small input like your example [2, 5, 3, 3, 5] the difference is neglectable.

However, here a a few benchmarks, when running with large input arrays:

 1000 values  |  original:  0.16s  |  new:  0.08s
 7000 values  |  original:  7.05s  |  new:  4.46s
10000 values  |  original: 14.70s  |  new:  8.84s
20000 values  |  original: 58.67s  |  new: 35.91s

The tests where run with MAMP's PHP 7.2.1 on the CLI on a i7 2.5GHz with 16 GB RAM.

Here are both versions running 5000 values on 3v4l.org:

Original
New version

Unfortunately you can't run more then 5000 values before running into their time limit.

Here's the original script I used for the benchmark.

Basically this is a variant of the Cartesian product. Here's a short read about Efficient Cartesian Product algorithm on Stackoverflow.


Generating random number inside a paralleled loop (openmp) What is the most fast and safe way of generating random numbers inside a pararelled loop? I did as below but I'm not sure if it is correct or not:

using namespace std;
std::minstd_rand gen(std::random_device{}());
std::uniform_real_distribution<double> unirnd(0, 1);
int main(){
double eta=something;
double x[N];
//initializing x[]
for (i=0;i<N;i++)
{
double z= unirnd(gen) * eta;
x[i] = some function of x[i] and z
}
}

To compile the code, I use g++ -fopenmp -std=gnu++11 -Wall -Wextra -g code.cpp -o a.out

Generating good random numbers is a hard problem, and generating parallel random numbers is harder. (Consider that you don't want to replicate the same sequence in different threads). You should use a library which provides a parallel random number generator (such as Intel MKL [now free for anyone]). It has an implementation of the generator described in Parallel Random Numbers: As Easy as 1, 2, 3 which may be appropriate.


PHP - using static variables to store "client" for later use I had this S3Helper class before such that, every time I called a function to any of its methods (which I haven't shown here, but which do things like upload files to s3 or get temporary s3 links), it called the getClient() method and it would make a new client every time.

Now I don't know about the performance hit on that, if calling new S3Client([]) makes http requests every time it's called, but I supposed it did so decided I'd rather just make the client once and then use the same client for future calls.

So I cached the result in a static variable, and now my getClient function checks if the cache exists first, and if it does it returns it, otherwise it makes a new client and sets the cache.

Is this a recommended PHP way of doing things? Can it be improved?

<?php // Code within app\Helpers\Helper.php

namespace App\Helpers;
use Aws\S3\S3Client;

class S3Helper {
    protected static $client = null;

    private static function getAuth() {
        return [
                'endpoint' => env('S3_ENDPOINT'),
                'key' => env('S3_KEY'),
                'secret' => env('S3_SECRET'),
                'bucket' => env('S3_BUCKET')
            ];
    }

    private static function getClient() {
        if (self::$client) return self::$client;

        $auth = self::getAuth();
        self::$client = new S3Client([
            'region' => 'eu-west-2',
            'version' => '2006-03-01',
            'endpoint' => $auth['endpoint'],
            'credentials' => [
                'key' => $auth['key'],
                'secret' => $auth['secret']
            ],
            'use_path_style_endpoint' => true
        ]);
        return self::$client;
    }
}

Before going through advises just wanted to remind you can alternatively use Aws\Sdk class as a client factory.
Sample usage:

// The same options that can be provided to a specific client constructor can also be supplied to the Aws\Sdk class.
// Use the us-west-2 region and latest version of each client.
$sharedConfig = [
    'region' => 'us-west-2',
    'version' => 'latest'
];

// Create an SDK class used to share configuration across clients.
$sdk = new Aws\Sdk($sharedConfig);

// Create an Amazon S3 client using the shared configuration data.
$client = $sdk->createS3();

Observe AWS SDK for PHP guide, just in case you hadn't yet.They give a notation there:

  Note

  We highly recommended that you use the Sdk class to create clients if
  you're using multiple client instances in your application. The Sdk
  class automatically uses the same HTTP client for each SDK client,
  allowing SDK clients for different services to perform nonblocking
  HTTP requests. If the SDK clients don't use the same HTTP client, then
  HTTP requests sent by the SDK client might block promise orchestration
  between services.

But, proceeding with your approach, you have a variation of Singleton pattern (restricts the instantiation of a class to a single object, which can be useful when only one object is required across the system).In your case S3Helper acting as a factory producing/returning the same S3Client instance.

To prevent instantiation/cloning of S3Helper from the outer code you can close __construct, __clone (and even __wakeup) methods:

class S3Helper {
    ...
    private function __construct() { }
    private function __clone() { }

To eliminate multiple return statements in your static getClient() method use the following condition (set client if it's not set yet):

private static function getClient() {
    if (self::$client === null) {   # use strict comparison here
        $auth = self::getAuth();
        self::$client = new S3Client([
            'region' => 'eu-west-2',
            'version' => '2006-03-01',
            'endpoint' => $auth['endpoint'],
            'credentials' => [
                'key' => $auth['key'],
                'secret' => $auth['secret']
            ],
            'use_path_style_endpoint' => true
        ]);
    }
    return self::$client;
}


Uploading a video in S3 Using Future I've posted a similar program previously, but I have not made any major modifications in the code, so I am posting it again by deleting the previous question.

I am afraid of the thread keyword and I am not too familiar with threads and blocking, so can you please review this code for me?

Is the use of future enough or not?

This code comes from here.

Note: The AWS credentials are fake.

package controllers

import java.awt.image.BufferedImage
import java.util.ArrayList

import scala.collection.JavaConversions.mapAsJavaMap
import scala.concurrent.Future

import org.apache.http.NameValuePair
import org.apache.http.message.BasicNameValuePair

import com.amazonaws.auth.BasicAWSCredentials
import com.amazonaws.services.s3.AmazonS3Client
import com.twilio.sdk.TwilioRestClient
import com.twilio.sdk.TwilioRestException

import net.liftweb.json.DefaultFormats
import net.liftweb.json.Serialization.write
import play.api.Play.current
import play.api.i18n.Lang
import play.api.i18n.Messages
import play.api.libs.concurrent.Execution.Implicits._
import play.api.mvc.Action
import play.api.mvc.Controller

object Application extends Controller {

  val bucketImages = "images35"
  val bucketVideos = "videos35"
  val bucketAudios = "audios35"

  val AWS_ACCESS_KEY = "qqqqqqqqqqqqqqqqqqqqqqqqqqq"
  val AWS_SECRET_KEY = "xxxxxxxxxxxxxxxxxxxxxxxxxxx"

  val yourAWSCredentials = new BasicAWSCredentials(AWS_ACCESS_KEY, AWS_SECRET_KEY)
  val amazonS3Client = new AmazonS3Client(yourAWSCredentials)

  val ACCOUNT_SID = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
  val AUTH_TOKEN = "tytytytytyyyyyyyyyyyyyyyyyyyyyyyyyy"

  implicit val formats = DefaultFormats

 def uploadVideo = Action.async(parse.multipartFormData) { implicit request =>
session.get("userI").map { userId =>
  try {
    var videoName: String = null
    val upload = for {
      done <- Future {
    request.body.files.map { mov =>
      videoName =System.currentTimeMillis() + ".mpeg"
      amazonS3Client.putObject(bucketVideos, videoName, mov.ref.file)
      }
      }
    } yield done
    upload.map { res =>
      val map = Map("result" -> "success", "videoName" -> videoName)
      Ok(write(map))
    }
  } catch {
    case e: Exception =>
      Future{
      Ok(write(Map("result" -> "error")))
      }
  }
}.getOrElse {
  Future{
  Ok(write(Map("result" -> "nosession")))
   }
  }
 }
}

application.conf

play {
  akka {
    event-handlers = ["akka.event.slf4j.Slf4jEventHandler"]
    loglevel = WARNING
    actor {
      default-dispatcher = {
        fork-join-executor {
          parallelism-min = 300
          parallelism-max = 300
        }
      }
    }
  }
}
contexts {
  simple-db-lookups {
    fork-join-executor {
      parallelism-factor = 10.0
    }
  }
  expensive-db-lookups {
    fork-join-executor {
      parallelism-max = 4
    }
  }
  db-write-operations {
    fork-join-executor {
      parallelism-factor = 2.0
    }
  }
  expensive-cpu-operations {
    fork-join-executor {
      parallelism-max = 2
    }
  }
}

Contexts Object

object Contexts {
  implicit val simpleDbLookups: ExecutionContext = Akka.system.dispatchers.lookup("contexts.simple-db-lookups")
  implicit val expensiveDbLookups: ExecutionContext = Akka.system.dispatchers.lookup("contexts.expensive-db-lookups")
  implicit val dbWriteOperations: ExecutionContext = Akka.system.dispatchers.lookup("contexts.db-write-operations")
  implicit val expensiveCpuOperations: ExecutionContext = Akka.system.dispatchers.lookup("contexts.expensive-cpu-operations")
}

First, remove import play.api.libs.concurrent.Execution.Implicits._. You don't want the default Play execution context. Instead, you should use one of the ExecutionContexts you've defined and aren't using. Perhaps define one for S3 operations alone, and you don't need the others if you aren't going to use them. Future.apply requires the implicit ExecutionContext, so you can either import the correct one, or explicitly pass it.

Assuming you have a context defined like this in your configuration:

contexts {
    s3-ops {
        fork-join-executor {
            parallelism-factor = 10.0
        }
    }
}

Then in your controller you could add:

implicit val ec: ExecutionContext = Akka.system.dispatchers.lookup("contexts.s3-ops")

Now in the body of the controller function, I'll work my way inward. With the handling of the session, why bother returning data to the user if they don't have a session? It would seem more appropriate to return Forbidden, which can shorten that line to Future.successful(Forbidden). Note that Future.successful creates a Future that has already been completed.

The use of try/catch is not very Scala-like. There are other nicer ways to handle errors in Scala like Try. Future is actually very similar to Try, though, and it should provide all the error handling you need in this case. If an exception occurs within a Future, then it becomes a failed Future. No exceptions will propagate upward. Most of the code inside your try block can go inside the Future apply instead.

var videoName: String = null is something that should be avoided altogether. First, it's best to avoid mutable vars, and second you should never find yourself assigning null to anything.

I'm not sure the body of your current Future where you're mapping request.body.files is doing what you think it's doing. request.body.files is a Seq[FilePart], which means if there are multiple FileParts, you're going to upload them all separately, overwrite videoName and only return the name of the last uploaded FilePart. Or worse, if there are no FileParts, that part will silently fail. If you're only going to have a single FilePart, it'd be better to name it in the upload body and access via request.body.file("filename") which will be an Option[FilePart]. From there you can map the Option[FilePart] to the actual upload.

Since you're not actually doing anything with the PutObjectResult from the S3 response, it'd be better to return the name of the file in the Future, so that you have access to it when you map the Future, and would no longer have a need for a var. Following the suggestions above, it would now be a Future[Option[String]].

This would leave you with three return states for the Future:

A successful Future containing Some[String] (upload success on both ends).
A successful Future containing None (no file was uploaded to the server).
A failed Future (the upload to S3 failed somehow).

You'd then have something that looks roughly like this:

Future {
    request.body.file("filename").map{ file =>
        val videoName = ...
        amazonS3Client.putObject(...)
        videoName
    }
}.map{ upload =>
    upload.map{videoName => Ok(...)}.getOrElse{BadRequest(...)}
}.recover{ case _ =>  // recover the failed `Future` to a default value, in this case a `Result`
    InternalServerError
}

Much cleaner. Note that recover accepts a PartialFunction[Throwable, ?], similarly to a catch block, so you can be more fine grained in your error handling if you need to.

A couple more notes:

There's no need to use Lift's json API for your responses. Play's implementation works just fine:

import play.api.libs.json.Json
Ok(Json.obj("result" -> "success", "videoName" -> videoName))

Your imports take up a lot of space. Many can be condensed into one liners:

 import play.api.i18n.Lang
 import play.api.i18n.Messages

Can be:

 import play.api.i18n.{Lang, Messages}


Calculating direction based on index of array I am comparing the index of the current component displayed in the viewport against an array of all the components on the screen to determine to direction the user in scrolling.

Does this function have too many concerns? and can the if statements be reduced to something cleaner?

function getDirection(component) {
    let direction;
    if (!this.lastComponentInViewport) {
      direction = "none";
    } else {
      let oldIndex = this.components.indexOf(this.lastComponentInViewport);
      let newIndex = this.components.indexOf(component);

      if (newIndex === oldIndex) direction = "none";

      if (newIndex > oldIndex) direction = "next";

      if (newIndex < oldIndex) direction = "previous";
    }
    return direction;
  }),

"none" can be treated as a default returned value, thus eliminating multiple occurrences of direction = "none" assignment. In that way it'd be enough to just consider non-none cases:

function getDirection(component) {
    let oldIndex, newIndex,
        lastComp = this.lastComponentInViewport;

    if (lastComp) {
        oldIndex = this.components.indexOf(lastComp);
        newIndex = this.components.indexOf(component);
        if (newIndex > oldIndex) return "next";
        if (newIndex < oldIndex) return "previous";
    }
    return "none";
}


Calculating company-wise total expenses I have a variable List. Its declaration is like :

List<HashMap<String, Object>>

Sample data in it:

  [{total-expense=228, company=HiHard, year=2009}, {total-expense=2936, company=POTENT, year=2009}, {total-expense=1362412.65, company=HiHard, year=2010}, {total-expense=9007.96, company=POTENT, year=2010}, {total-expense=240427.43, company=HiHard, year=2011}, {total-expense=1956.11, company=POTENT, year=2011}]

I have written a program to fetch the below result:

  {"HiHard":[[228,1362412.65,240427.43]],"POTENT":[[2936,9007.96,1956.11]]}

i.e. to fetch company-wise total expense incurred for financial years 2009, 2010, 2011.

Current program logic:

1) getDistinctCompanies from the list

2) Iterate through the distinct companies[from step 1] and get the expense incurred by passing in company and list.

Complete program:

public class JSONConstruction {
    public static void main(String[] args) {
        List<HashMap<String, Object>> list = new ArrayList<HashMap<String, Object>>();

        ... ASSUME list has the SAMPLE data as described before.

        List<String> companyList = getDistinctCompanies(list);

        Iterator<String> iterCompany = companyList.iterator();
        String company;
        JSONObject obj = new JSONObject();
        while(iterCompany.hasNext()){
            company = iterCompany.next();
            JSONArray jsonList = new JSONArray();
            jsonList.add(getExpenseList(list, company));
            obj.put(company, jsonList);
        }

        System.out.print(obj);
    }

    private static List<String> getDistinctCompanies(List<HashMap<String, Object>> list) {
        Iterator<HashMap<String, Object>> iterList = list.iterator();
        String company;
        List<String> companyList = new ArrayList<String>();
        while (iterList.hasNext()) {
            HashMap<String, Object> map = iterList.next();
            company = (String) map.get("company");
            if(companyList==null || (!companyList.contains(company))) {
                companyList.add(company);
            }
        }
        return companyList;
    }

    private static List<Object> getExpenseList(List<HashMap<String, Object>> list, String company) {
        Iterator<HashMap<String, Object>> iterList = list.iterator();
        List<Object> expenseList = new ArrayList<Object>();
        String companyMatch;
        while (iterList.hasNext()) {
            HashMap<String, Object> map = iterList.next();
            companyMatch = (String) map.get("company");
            if(companyMatch.equals(company)) {
                Object obj = map.get("total-expense");
                expenseList.add(obj);
            }
        }
        return expenseList;
    }

Though the data is fetched correctly, I am eager to know a better way of coding this program.

private static List<String> getDistinctCompanies(List<HashMap<String, Object>> list) {
    Iterator<HashMap<String, Object>> iterList = list.iterator();
    String company;
    List<String> companyList = new ArrayList<String>();
    while (iterList.hasNext()) {
        HashMap<String, Object> map = iterList.next();
        company = (String) map.get("company");
        if(companyList==null || (!companyList.contains(company))) {
            companyList.add(company);
        }
    }
    return companyList;
}

There is no need to check if companyList==null because that won't ever happen because 4 lines prior you create the list.
The scope of map is defined to be inside the while loop, but company is defined outside whereas it is only used in the while loop as well.
You should name your variables proper. Shortening names doesn't add any value. Instead of iterList a simple iterator or listIterator would be much better.
You should leave your variables and operators some space to breathe. This helps to make your code more readable. companyList==null vs companyList == null.


SSH autokiller for connections from other systems I was messing around with ssh and decided to write a python program that scans for any connections that aren't from the system, then terminates them. It works well for many connections from my iMac to my MacBook. A more in-depth explanation of the program is written at the top of the code. I'd appreciate feedback on any nitpicks you can find, since I threw this together in about thirty minutes. Any and all feedback is appreciated and considered.

scanner.py

""" Import Statements """
import os
import subprocess
import time

"""

Objective: Scan all connections to this computer, every two seconds, and if any are not
system, as in not `console` or `s000`, find the PID associated with the connection,
then terminate the connection with `kill -9 ...`

"""

def format_pid(text):
    """
    Formats the output to only include PIDS that are not from the system
    """
    ids = text.split("\n")
    all_ids = []
    del ids[0] #removes uptime stuff
    for line in ids:
        pid = line[:13]
        if pid[12:] != '0':
            all_ids.append(pid[:5])
    return all_ids

def get_pids():
    """ Returns all users logged into the computer """
    return format_pid(subprocess.Popen(['ps'], stdout=subprocess.PIPE).communicate()[0])

def kill():
    """ Terminates all processes that are not system """
    processes = get_pids()
    for process in processes:
        if process: # Makes sure process isn't null/empty
            os.system('kill -9 ' + process)
            print("KILLED CONNECTION PROCESS: " + process)

    time.sleep(2)
    kill()

if __name__ == '__main__':
    kill()

Replace magic numbers like 13 there line[:13] with constansts.
IMHO:

rename ids to lines
all_ids to ids
processes to pids


Parse through an XDocument efficiently and match values to matching object Minimum viable test program:

class Program
{
    private static List<Param> paramList;

    private class Param
    {
        public string xmlName;
        public double scalingFactor;
        public double Value { get; set; }

        public Param( string xmlName, double scalingFactor)
        {
            this.xmlName = xmlName;
            this.scalingFactor = scalingFactor;
        }
    }

    private static void UpdateParams(XDocument xDoc)
    {
        IEnumerable<Param> filteredParamList;
        foreach (XElement xElement in xDoc.Root.Elements())
        {
            filteredParamList = paramList.Where(param => param.xmlName == xElement.Name.LocalName);
            if (filteredParamList != null && filteredParamList.Any())
            {
                filteredParamList.Single().Value = double.Parse(xElement.Value) * filteredParamList.Single().scalingFactor;
            }
        }
    }

    static void Main(string[] args)
    {
        paramList = new List<Param>
        {
            new Param("paramA",1),
            new Param("paramB",.5),
            new Param("paramC",.01)
        };

        /* For testing, this XDocument is hard coded here. */
        UpdateParams(XDocument.Parse("<root><paramA>121</paramA><paramB>100</paramB><paramC>197</paramC></root>"));

        foreach(Param param in paramList)
        {
            Console.WriteLine("Name: {0} Value: {1}", param.xmlName, param.Value);
        }

        Console.Read();
    }
}

Let's focus on UpdateParams. So I have an XDoc that has a root and values. Looks like this:

<root>
 <paramA>121</paramA>
 <paramB>100</paramB>
 <paramC>105</paramC>
  ...
</root>

I have a List of my param objects with an xmlName field which matches my XML key's. The xmlName of each Param object in my list will be unique; I'm manually creating every Param class in the main class's constructor (or in this test program, the main function). For context, the param objects will eventually have UI/WPF bindings from Value to textboxes or other UI elements as needed. For each key in the XML, i want to find the matching param object from my list, and fill in the Value field. If there's something weird in my XDocument that doesn't match my object list, I want to ignore it (and not throw an exception).

Is there a simpler/better way to write the parsing? Seems a little verbose, and i don't like that I have that extra IEnumerable filteredParamList just to work around null checking.

I'm going to revew only this part as I think the rest of it is just for demonstration purposes.

private static void UpdateParams(XDocument xDoc)
{
  IEnumerable<Param> filteredParamList;
  foreach (XElement xElement in xDoc.Root.Elements())
  {
      filteredParamList = paramList.Where(param => param.xmlName == xElement.Name.LocalName);
      if (filteredParamList != null && filteredParamList.Any())
      {
          filteredParamList.Single().Value = double.Parse(xElement.Value) * filteredParamList.Single().scalingFactor;
      }
  }
}

Try to avoid writing methods like this one, where one of the parameters is some static field. This makes it very difficult to test because of the external dependency. If possible you should pass the parameter as anoter argument. I had a hard time understanding your code.
Try to also avoid modifying methods parameters like you do here with the paramList. It's usually much better to return a new result. Such methods are called pure methods and are much easier to test and to maintain. Modifying global objects can make debugging very painful.
Use the smallest possible type for parameters (or the most general/abstract). This means, your method doesn't need the XDocument, it can do its job just fine with an IEnumerable<XElement>.
Choose more meaningful method names. UpdateParams is too general and doesn't say anything about what is going to be updated.
You neither need the foreach, nor Where, nor Any, nor Single, nor the null check. All this can be replaced by a much nicer join.

This is how it could look like after refactoring:

private static IEnumerable<Param> ScaleParameterValues
(
    IEnumerable<XElement> elements,
    IEnumerable<Param> parameters
)
{
    return
        from xe in elements
        join p in parameters on xe.Name.LocalName equals p.xmlName
        select new Param(p.xmlName, p.scalingFactor)
        {
            Value = p.scalingFactor * double.Parse(xe.Value)
        };
}


Fruit score leaderboard I made a basic website to practice HTML and CSS that will display 3 fruits with their given scores in an Olympic podium sort of way.

Have I used any bad practices and what I should do instead?

What can I do to fully optimize and shorten my code?

How readable is my code? How can I make it more readable?

    <link href='https://fonts.googleapis.com/css?family=Lato' rel='stylesheet' type='text/css'>

Highest Fruit Score: 83

65

83

34

HTML

Your HTML should have a DOCTYPE declaration.  Nowadays, the HTML 5 doctype (<!DOCTYPE html>) is considered standard practice.

You should always declare a charset, and the charset should be UTF-8.  The <title> element is required.

In an  tag, the alt attribute is considered mandatory, and in this case there are very obviously appropriate descriptions to assign to each image.

You are using a
 to do the layout, for content which is not semantically tabular.  This is considered vulgar in HTML: the document should represent the meaning, and the layout should be performed using CSS.  (The user agent might not be a typical graphical browser; it might be a screen reader for the sight-impaired.)  The semantically appropriate element to use should be an
 ordered list.  The fruits should be listed in first, second, and third place in the document.

CSS

For ease of maintenance, avoid repeating declarations.  For example, you can set the font-family just once on an ancestor element.  Also, you can write the transform: scale(0.75) declaration just once, using a .silver, .bronze selector.

How can we change the display order using CSS?  Using the CSS order property would be really nice, but it relies on CSS flex boxes, which is a newer feature.

body {
    background-color: #AAFF00;
    font-family: 'Lato', sans-serif;
    text-align: center;
}

div.fruits {
    display: inline-block;
    width: auto;
}
div.fruits p {
    margin-top: 0;
    color: #000000;
    font-size: 50px;
}
div.fruits p span.score {
    font-weight: bold;
}

ol, li { /* CSS reset: make lists not work like lists */
    list-style: none;
    margin: 0;
    padding: 0;
}

ol {
    display: flex;
}
li {
    display: inline-block;
}
li.silver {
    order: -1;
}
.silver, .bronze {
    transform: scale(0.75);
}

li img {
    display: block;
}
li span.score {
    position: relative;
    top: -25px;
    border: 1px solid black;
    padding: 12.5px;
    font-size: 25px;
}
li.gold .score {
    background-color: gold;
}
li.silver .score {
    background-color: silver;
}
li.bronze .score {
    background-color: #CD7F32;
}
<!DOCTYPE html>

    <meta charset="UTF-8">
    <title>Top three fruits</title>
    <link rel='stylesheet' type="text/css" href='https://fonts.googleapis.com/css?family=Lato'>

Highest Fruit Score: 83

 83

 65

 34

For compatibility, we can resort to using floats to put the silver element on the left.  A very unfortunate consequence of that hack is that the width of the list needs to be hard-coded to help position the floats properly.

body {
    background-color: #AAFF00;
    font-family: 'Lato', sans-serif;
    text-align: center;
}

div.fruits {
    margin-left: auto;
    margin-right: auto;
    width: 750px; /* Unfortunately hard-coded width */
}
div.fruits p {
    margin-top: 0;
    text-align: center;
    color: #000000;
    font-size: 50px;
}
div.fruits p span.score {
    font-weight: bold;
}

ol, li { /* CSS reset: make lists not work like lists */
    list-style: none;
    margin: 0;
    padding: 0;
}

li {
    display: inline-block;
    width: 250px;
}
.silver, .bronze {
    transform: scale(0.75);
}

/* Change the display order to silver-gold-bronze */
li.silver {
    float: left;
}
li.bronze {
    float: right;
}
ol:after {
    clear: both;
}

li span.score {
    position: relative;
    top: -25px;
    border: 1px solid black;
    padding: 12.5px;
    font-size: 25px;
}
li.gold .score {
    background-color: gold;
}
li.silver .score {
    background-color: silver;
}
li.bronze .score {
    background-color: #CD7F32;
}
<!DOCTYPE html>

    <meta charset="UTF-8">
    <title>Top three fruits</title>
    <link rel='stylesheet' type="text/css" href='https://fonts.googleapis.com/css?family=Lato'>

Highest Fruit Score: 83

 83

 65

 34


On demand JS loader review At my work, I need to create a script that third-party webmasters could include in their pages without need to include something else. But this script had dependencies on jQuery and some amount of their plug-ins.

On the Internet, I have found libraries that have same functionality except for an important one: they don't check are needed library already exist on a page.

Yes, I could execute the needed libraries in local scope of my script, but I've decided to reduce the number of HTTP connections and traffic with this function:

var require = function (lib, obj, libs_obj) {
    // if obj is function than `require` called directly by user and we
    // must transform it to object. It's for reduce number of used
    // variables. When we call `require` recursively, we use this object
    // instead of function
    var lib_is_list = typeof(lib) === 'object';
    if (typeof obj === 'function') obj = { callback: obj, count: lib_is_list ? lib.length : 1 }
    if (lib_is_list) { // this is list of libs
        for (var i in lib) require(lib[i], obj, libs_obj);
        return;
    }
    var lib = libs_obj[lib];
    if (lib.callbacks === undefined) lib.callbacks = [];
    if (lib.check()) { if (obj.callback) obj.callback(); return; }
    lib.callbacks.push(obj);
    if (lib.pending) { return; }
    lib.pending = true;

    function ready() {
        function script_downloaded() {
            lib.pending = false;
            var obj;
            while (obj = lib.callbacks.pop()) {
                obj.count--; if (obj.count == 0) obj.callback();
            }
        }

        download_script(lib.link, script_downloaded);
    }

    function download_script(src, callback) {
        var script = document.createElement('script');
        script.type = 'text/javascript';
        script.async = 'async';
        script.src = src;

        // Based on jQuery jsonp trick
        if (callback) {
            script.onload = script.onreadystatechange = function() {
                if (!script.readyState || /loaded|complete/.test(script.readyState)) {
                    script.onload = script.onreadystatechange = null;
                    callback();
                }
            };
        }
        document.getElementsByTagName('head')[0].appendChild(script);
    }

    var deps_count = lib.deps ? lib.deps.length : 0;
    if (deps_count < 1) { ready(); return; }
    var new_obj = { callback: ready, count: deps_count };

    require(lib.deps, new_obj, libs_obj);
};

This function work in IE6+ (and, of course, in other browsers) and written in pure JS.
To call this function, use syntax like this:

require(['list', 'of', 'libraries'], function () { alert 'All libs loaded'; }, libs_obj),

where libs_obj is object like this:

{
    list: {
        check: function() { return list_exist(); },
        // function to check whether the required functionality
        link: 'js/list_js_file.js',
        // link to script
        deps: ['libraries', 'of']
        // list of dependencies of current library
        // If not, library doesn't have dependencies
    },
    of: {
        check: function() { return of_exist_and_version_is('1.2.3'); },
        link: 'js/another_file.js',
    },
    libraries: {
        check: funcion() { return libraries_exist() || analog_exist(); },
        link: 'http://www.example.com/js/libraries.js',
        deps: ['of']
    }
}

Callback function are optional - if we don't need it, we can just type false or undefined.  Of course, this function must be called after all third-party scripts. Bottom of page is better place to script with this function.  Please tell me where I went wrong or give me useful advice.

My 2 cents:

please use lowercase camelcase ( lib_is_list -> libIsList )
The first 5 lines seem clumsy

Checking for an array with typeof 'object' is odd
Changing the type of a variable/parameter is odd and not recommended, you do this twice
Dont drop newlines ( for (var i in lib) require(lib[i], obj, libs_obj); )

You can use lib.callbacks = lib.callbacks || [] instead of if (lib.callbacks === undefined) lib.callbacks = [];
Why would you assign first callbacks, and then check() whether you should return
Your handling of obj , count and callbacks is roundabout, the lack of newlines dont help,
The function name ready() is unfortunate, ready is most commly used for the callback after http requests
You should consider merging/reworking ready/script_downloaded/download_script
The src parameter is unfortunate, since it does not contain the source, maybe url ?
I like defensive programming, but checking for callback seems much, since your code guarantees it

Overal, I have to say this seems over-engineered, I think you are trying to be too smart.


Comparing corresponding elements of two equal-length arrays I have a helper function I wrote, but it feels like something _.lodash already does or could do if I knew the library better.

function changed(before:any[], after:any[]):boolean[]
{
    if (before.length !== after.length)
    {
        throw new Error(`Length of arrays are not equivalent: ${before.length}, ${after.length}`);
    }
    else
    {
        let changed:boolean[] = [];
        for(let i = 0; i < before.length; i++)
        {
            changed[i] = (after[i] !== before[i]);
        }
        return changed;
    }
}

An example of how this might be used and what would return:

console.log(changed([0,1,0,1], [0,1,1,0]))
// => [false, false, true, true]

Does anyone know if this can be elegantly achieved with lodash?

I don't know TypeScript or lodash, but I hope the following code is at least almost correct. You should be able to use zip and map to solve the problem in a functional manner.

function changed(a, b) {
    return _.zip(a, b).map(x => { return x[0] !== x[1] });
}

Basically, the problem of what has changed can be rephrased as which elements are equal.


Segmenting brain images (Dicom format) using region-growing algorithm I have a list of images in a folder where num denotes the index of image and I used num to run the iterations in a "for loop". The code is supposed to segment brain part from the brain CT images. Here is the link for region growing algorithm.

How do I increase the performance of my region-growing algorithm?

% CORD_XY(1),CORD_XY(2) is the seed point
% LEN Denotes the number of images in a folder
% num is used to iterate in "for loop"
% "img" here is single image from the folder in DICOM format

for num= 1:LEN
    img = dicomread(files(num,1).name);
    [row, col] = size(img);
    th = 20;
    diff = 10;

    J = regiongrowing(img, CORD_XY(1), CORD_XY(2), th);
    value1 = sum(sum(J > 0));

    if num > 10)
        th = 55;
    elseif num <= 10
        while diff < 0.2 * value1 && diff ~= 0 && th < 60
            value1 = sum(sum(J > 0));
            th = th + 10;
            J = regiongrowing(double(img), CORD_XY(1), CORD_XY(2), th);
            value2 = sum(sum(J > 0));
            diff = value2 - value1;
        end
    end
end

J = regiongrowing(img, CORD_XY(1), CORD_XY(2), th-10);

I don't have the Image Processing Toolbox, so I can't test your code, but I'll go through what I can.

Never do diff = 10; diff is a useful builtin function in Matlab, so using it as a variable name will cause the function to be useless. The same goes with max, sum, size and so on.

value1 = sum(sum(J > 0)); is a bit faster than value1 = sum(J(:)>0);, so with regards to performance you did the right thing. In my opinion however, sum(J(:)>0) is a bit cleaner, and it can also be scaled to more dimensions.

Instead of elseif N <= 10 you can simply do else, as they mean the same thing in this context.

If it takes 4-5 iterations pretty consistently, then maybe you can increase the initial value of th to e.g. th = 40. It will result in fewer iterations for most images, so a faster algorithm. You might benefit from guessing a good value, run the algorithm, if you don't hit convergence, jump to a lower value. This will perform better for most of the images, but will be slower for some. In total, it's likely faster.


Circular Singly Linked List using Java Please review my implementation of a Circular Singly Linked List.

public class NodeS {

    public int num;
    public NodeS next;

    public NodeS(int n){
        this.num = n;
    }
}

public class SinglyCircularList {

    private static NodeS head = null;
    private static NodeS tail = null;
    private static int size = 0;

    public static int getSize() {
        return size;
    }

    public static void insert(int n) {
        NodeS node = new NodeS(n);
        node.next = tail;
        if (tail == null) {
            tail = node;
        } else {
            head.next = node;
        }
        head = node;
        size++;
    }

    public static int delete() {
        if (!isEmpty()) {
            NodeS deq = tail;
            tail = deq.next;
            size--;
            if (size == 0) {
                tail = null;
            }
            head.next = tail;
            return deq.num;
        } else {
            System.out.println("List Empty !!");
        }
        return -1;
    }

    public static boolean isEmpty() {
        return size == 0;
    }

    public void printList() {
        NodeS temp = tail;
        for (int i = 0; i <= size; i++) {
            if (temp != null) {
                System.out.print(temp.num);
                temp = temp.next;
            }
        }
        System.out.println();
    }

    /**
     * @param args
     */
    public static void main(String[] args) {
        SinglyCircularList scl = new SinglyCircularList();
        scl.insert(1);
        scl.insert(2);
        scl.insert(3);
        scl.insert(4);
        scl.insert(5);
        scl.insert(6);
        scl.insert(7);
        scl.printList();
        System.out.println("Del->" + scl.delete());
        scl.insert(8);
        scl.printList();
        System.out.println("Del->" + scl.delete());
        System.out.println("Del->" + scl.delete());
        System.out.println("Del->" + scl.delete());
        System.out.println("Del->" + scl.delete());
        System.out.println("Del->" + scl.delete());
        System.out.println("Del->" + scl.delete());
        System.out.println("Del->" + scl.delete());
        System.out.println("Del->" + scl.delete());
        scl.printList();
        scl.insert(9);
        scl.insert(10);
        scl.printList();

    }

}

I have only a couple of points:

1

You implementation will be able to store only one list throughout your Java program:

SinglyCircularList list1 = new SinglyCircularList();
SinglyCircularList list2 = new SinglyCircularList();

list1.insert(1); // Here list1 = [1], and list2 = [1];
list2.insert(2); // Now  list1 = [1, 2], and list2 = [1, 2]

So, basically, you should remove the keyword static from everywhere except the main(String[] args).

2

It would be nicer if your delete method would throw an exception on deleting from an empty list.

3

Instead of printList() you could override the public String toString().

4

private static NodeS head = null;
private static NodeS tail = null;
private static int size = 0;

Whenever declaring class or object fields, references are initialized by default with null, and numeric fields to zero. You can write simply:

private NodeS head;
private NodeS tail;
private int size;

Summa summarum

I had something like that in mind:

import java.util.NoSuchElementException;

public class SinglyCircularList<E> {

    private static final class Node<E> {

        private final E datum;
        private Node<E> next;

        Node(final E datum) {
            this.datum = datum;
        }

        E getDatum() {
            return datum;
        }

        Node<E> getNext() {
            return next;
        }

        void setNext(final Node<E> next) {
            this.next = next;
        }
    }

    private Node<E> head;
    private Node<E> tail;
    private int size;

    public int size() {
        return size;
    }

    public void insert(final E datum) {
        final Node<E> newnode = new Node<>(datum);

        if (head == null) {
            head = newnode;
            tail = newnode;
            size = 1;
            return;
        }

        tail.setNext(newnode);
        tail = newnode;
        size++;
    }

    public E delete() {
        if (isEmpty()) {
            throw new NoSuchElementException("Deleting from an empty list.");
        }

        final E ret = head.getDatum();

        if (size == 1) {
            head = null;
            tail = null;
        } else {
            head = head.getNext();
            tail.setNext(head);
        }

        size--;
        return ret;
    }

    public boolean isEmpty() {
        return size == 0;
    }

    @Override
    public String toString() {
        final StringBuilder sb = new StringBuilder("[");

        if (size > 0) {
            sb.append(head.getDatum());
        } else {
            return "[]";
        }

        Node<E> currentNode = head.getNext();

        for (int i = 1; i < size; ++i, currentNode = currentNode.getNext()) {
            sb.append(", ").append(currentNode.getDatum());
        }

        return sb.append("]").toString();
    }

    public static void main(String[] args) {
        SinglyCircularList<Integer> scl = new SinglyCircularList<>();

        System.out.println("Creating the list:");

        for (int i = 1; i <= 8; ++i) {
            System.out.println(scl);
            scl.insert(i);
        }

        System.out.println(scl);

        System.out.println("Removing from the list:");

        while (!scl.isEmpty()) {
            scl.delete();
            System.out.println(scl);
        }
    }
}

Hope that helps.