Levenshtein Distance with Haskell Vectors and Memoization Is the following an effective way to implement the Levenshtein Distance with Haskell vectors?

  import qualified Data.Vector as V

  levenshtein s1 s2 = levenshteinV (V.fromList s1) (V.fromList s2)

  levenshteinV p1 p2 = lev V.! l1 V.! l2
    where
      lev    = V.map levi      (V.enumFromN 0 (l1 + 1))
      levi i = V.map (levij i) (V.enumFromN 0 (l2 + 1))
      levij i j
        | i == 0    = j
        | j == 0    = i
        | otherwise =
          ((lev V.! (i - 1) V.!  j)      + 1) `min`
          ((lev V.!  i      V.! (j - 1)) + 1) `min`
          ((lev V.! (i - 1) V.! (j - 1)) + ind (i - 1) (j - 1))

      ind i j = if p1 V.! i == p2 V.! j then 0 else 1
      l1      = V.length p1
      l2      = V.length p2

In particular, should I be using V.map to construct the vectors or is there a better approach? Perhaps V.generate? Or does it not make a difference because of lazy evaluation?

Readability improves a lot with import Data.Vector ((!)) and replacing V.! with !.

generate looks better; performance-wise I do not expect a difference (with optimization).

lev    = V.generate (l1 + 1) levi

If you like point-free notation, you can short-cut levi, with intermediate manual steps:

  lev    = V.generate (l1 + 1) levi
  levi i = V.generate (l2 + 1) (levij i)

2

  lev    = V.generate (l1 + 1) (\i ->
           V.generate (l2 + 1) (levij i))

3

  lev    = V.generate (l1 + 1) (\i -> V.generate (l2 + 1) (levij i))

4

  lev    = V.generate (l1 + 1) (V.generate (l2 + 1) . levij)

Your code looks good. You might try it with Array next, to two-dimensional Index. The google results for haskell levenshtein Array are only marginally better than your Vector approach.


Create DataFrame from dictionary I request data from a table in a database and each line comes as a dictionary like this one :
{
    "timestamp" : 1234657890,
    "prices" : {
                   "AAA" : 111,
                   "BBB" : 222,
                    ...
                   "ZZZ" : 999
               }
}

From all those lines i wanted to create a dataframe like this:
Timestamp    AAA    BBB    ...   ZZZ
1234657890   111    222    ...   999
1234567891   110    223    ...   997
   ...
1324657899   123    208    ...  1024

So i did :
rawData = database_request()
listPrices = []
for row in rawData
    tmp = {'timestamp': row['timestamp']}
    tmp.update({name : price for name,price in row['prices'].items()})
    listPrices.append(tmp)
df = pd.DataFrame(listePrices)

So i was wondering if there were a more pythonic way to do this ?

Your rawData (which should be ideally named raw_data, python suggests a style guide to name variables and functions in lower_snake_case) is already in a list structure. You can manipulate this in place, without having to process the whole dataset manually.
for row in raw_data:
    row.update(row.pop("prices"))


Printing the n'th prime numbers This is an online problem.  The challenge is to print the kth prime number, for each k given in the input (1 ≤ k ≤ 15000).  The first line of input indicates the number of inputs on the subsequent N lines.

Example input:

4
1
2
3
4

Example output:

2
3
5
7

(the 1st 2nd 3rd and 4th prime numbers).

#include <stdio.h>
#include <stdlib.h>
#include <math.h>
#define MAX 100000000

void prime(int n)
{
    int i,j,temp,count=0;

    for(i=2;i<=MAX;i++)
    {
        temp=0;
        for(j=2;j<=sqrt(i);j++)
        {
            if(i%j==0)
            {
                temp=1;
                break;
            }
        }
        if(temp==0)
        {
            count++;
        }
        if(count==n)
        {
            printf("%d\n",i);
            break;
        }
    }
}

int main()
{
   int n,i;

   scanf("%d",&n);
   int arr[n];

   for(i=0;i<n;i++)
   {
       scanf("%d",&arr[i]);
   }
   for(i=0;i<n;i++)
   {
       prime(arr[i]);
   }

   return 0;
}

This code is working fine on codeblocks but is exceeding time limits on an online judge. I've tried manipulating this code in two more ways

using pointer and malloc in the main function
running the loop from j=1 to j=i/2 instead of upto sqrt(i)

(latter is more efficient due to fewer iterations of loop). Any suggestions?

You are trying to iterate over all numbers and check if each is prime which you do in O(n sqrt n) time. This might be feasible if you switch to a more efficient primality check, such as the Miller-Rabin primality test, as it is deterministic within certain limits depending on the probes.

A simpler way to do this is to generate primes until you are done. This can easily be done by the Sieve of Eratosthenes in roughly O(n log log n) time.

You can run the sieve once, get all the primes up to 175000 (15000th prime is 163841 according to a bit of googling) and then answer all the queries in constant time by dumping all the primes into an ArrayList (in order of course).

If those methods aren't fast enough, look into even faster sieving algorithms such as Sieve of Sundaram and Sieve of Atkin.


Get resources inside or outside constructor? I need your opinion on writing a constructor that's clean code and unit test friendly.

Option 1 :

var res1 = bigObject.GetResource1();
var res2 = bigObject.GetResource2();
var res3 = bigObject.GetResource3();
var myObject = new Foo(bigObject, res1, res2, res3);
//and the foo ctor just initializes its internal fields with given data

Option 2 :

var myObject = new Foo(bigObject);
[...]
public Foo(Bar bigObject)
{
    _bigObject = bigObject;
    _res1 = _bigObject.GetResource1();
    _res2 = _bigObject.GetResource2();
    _res3 = _bigObject.GetResource3();
}

Option 2 is cleaner (?) in the code (in option 1 I have the feeling I have to spoon-feed the Foo object because it is not smart enough to get what it needs by itself)), but requires helper methods in the tests to mock calls to Get* and pass what we want.

Of course, I can also write both ctors, and use one in the tests and the other in the "real" code, but it'd looks a bit weird and bloated to someone discovering the class, and the simple constructor wouldn't be tested.

What would you advise ?

EDIT :

Resources are not especially linked to the big object (i.e. there is no "wrong" combination). Imagine for example that the big object is a College, and resources are a list of teachers, a list of courses and a list of rooms, and we want to build, say, a timetable.

Are these resources always linked to the bigObject instance? Do they have something in common with certain data or functionality in bigObject? Are they somehow related mutually? It's hard to say exactly what is the better approach without knowing all this, but there are several sanity checks you can do:

Do GetResource1 to GetResource3 really all depend on the bigObject instance? Can you extract this functionality into a different class? Extract class refactoring might be an option.
Does bigObject know too much? Big kinda sounds like God, as in "god object".
If these three resources always come together (i.e. they all belong to the same interface), you might want to extract only them into a "parameter object". This means that you might end up with something in the middle, like:

public Foo(Bar bigObject, IResources resources)
{
   ...
}

Which means your bigObject will implement IResources, but you will also be able to implement them in a different way, and it will reduce the number of params.
If these resources are all of the same type, and there is a large number of them, then even a simple delegate might be enough. You might even want to make the call weakly typed for greater extensibility:

public Foo(Bar bigObject, Func<string, IRes> getResourceByName)
{
     var res1 = getResourceByName("1");
     var res2 = getResourceByName("2");
     ...
}

These suggestions popped out as I was writing, but ultimately, it's hard to give any exact guidance unless you state the actual problem you are trying to solve. Design should then follow from there.


Density-based clustering of image keypoints I have implemented the DBSCAN algorithm for clustering image keypoints.  I have been following the pseudocode on the wiki page pretty strictly, and it's working, but I get the feeling its a very naive implementation and could be improved in terms of performance.  I was hoping you could offer me some feedback on how to improve it.

/* DBSCAN - density-based spatial clustering of applications with noise */

vector<vector<KeyPoint>> DBSCAN_keypoints(vector<KeyPoint> *keypoints, float eps, int minPts)
{
vector<vector<KeyPoint>> clusters;
vector<bool> clustered;
vector<int> noise;
vector<bool> visited;
vector<int> neighborPts;
vector<int> neighborPts_;
int c;

int noKeys = keypoints->size();

//init clustered and visited
for(int k = 0; k < noKeys; k++)
{
    clustered.push_back(false);
    visited.push_back(false);
}

//C =0;
c = 0;
clusters.push_back(vector<KeyPoint>()); //will stay empty?

//for each unvisted point P in dataset keypoints
for(int i = 0; i < noKeys; i++)
{
    if(!visited[i])
    {
        //Mark P as visited
        visited[i] = true;
        neighborPts = regionQuery(keypoints,&keypoints->at(i),eps);
        if(neighborPts.size() < minPts)
            //Mark P as Noise
            noise.push_back(i);
        else
        {
            clusters.push_back(vector<KeyPoint>());
            c++;
            //expand cluster
            // add P to cluster c
            clusters[c].push_back(keypoints->at(i));
            //for each point P' in neighborPts
            for(int j = 0; j < neighborPts.size(); j++)
            {
                //if P' is not visited
                if(!visited[neighborPts[j]])
                {
                    //Mark P' as visited
                    visited[neighborPts[j]] = true;
                    neighborPts_ = regionQuery(keypoints,&keypoints->at(neighborPts[j]),eps);
                    if(neighborPts_.size() >= minPts)
                    {
                        neighborPts.insert(neighborPts.end(),neighborPts_.begin(),neighborPts_.end());
                    }
                }
                // if P' is not yet a member of any cluster
                // add P' to cluster c
                if(!clustered[neighborPts[j]])
                    clusters[c].push_back(keypoints->at(neighborPts[j]));
            }
        }

    }
}
return clusters;
}

vector<int> regionQuery(vector<KeyPoint> *keypoints, KeyPoint *keypoint, float eps)
{
float dist;
vector<int> retKeys;
for(int i = 0; i< keypoints->size(); i++)
{
    dist = sqrt(pow((keypoint->pt.x - keypoints->at(i).pt.x),2)+pow((keypoint->pt.y - keypoints->at(i).pt.y),2));
    if(dist <= eps && dist != 0.0f)
    {
        retKeys.push_back(i);
    }
}
return retKeys;
}

Just for correctness and not for performance reasons:
You nowhere mark any key as being clustered. As a result, keys may be clustered multiple times. In case all keys shall be clustered maximum one time, one may do the following modifications to the code above:

Add a following first line

clustered[i] = true;

behind

// add P to cluster c
clusters[c].push_back(keypoints->at(i));

and a following second line

clustered[neighborPts[j]] = true;

after

clusters[current_cluster].push_back(keypoints->at(neighborPts[j]));

Concerning the second line, make sure that it is inserted within the if-statement.


Optimising Funny Marbles Problem statement is at http://www.codechef.com/DEC13/problems/MARBLEGF

  Lira is given array A, which contains elements between 1000 and 2000.
  Three types of queries can be performed on this array: add a given value to a single element on it, subtract a given value from a single element on it and find the sum of the values between indexes i and j, i.e. A[i]+...+A[j]. Check input and example section for details.

  Input

  The first line of the input contains two integers: N and Q, denoting respectively, the number of students that there are present to receive the marbles as a gift and the number of actions Lira's mom will perform.

  These actions can be of three different types:

  S i j - if the mom wants to find the sum of the number of marbles from students i to j.
  G i num_marbles - if the mom decides to give num_marbles to student i
  T i num_marbles - if the mom decides to take away num_marbles from student i

  Output

  The output should contain as many lines as the number of queries S and it should contain the answer for each query on a separate line

  Constraints

  2 ≤ N ≤ 1000000
  3 ≤ Q ≤ 50000
  The array is 0-indexed.
  1000 ≤ A[i] ≤ 2000
  A student can never have a negative value of marbles. (i.e. there's no data which can cause a student to have a negative value of marbles)
  0 ≤ i, j ≤ N-1, and i ≤ j for the sum query
  At any given time, it is assured that the maximum number of marbles each student can have (num_marbles) never exceeds the size of the int data type.

  Example

  Input:

5 3
1000 1002 1003 1004 1005
S 0 2
G 0 3
S 0 2

  Output:

3005
3008

My code gives time limit exceeded.

My code:

import java.util.*;

class funny_marbles
{
    public static void main(String[] ar)
    {
        Scanner sc = new Scanner(System.in);
        int n = sc.nextInt();
        int q = sc.nextInt();
        int[] a = new int[n];
        for(int i = 0; i < n; i++)
            a[i] = sc.nextInt();
        for(int i = 0; i < q; i++)
        {
            String s = sc.next();
            int t1 = sc.nextInt();
            int t2 = sc.nextInt();
            if(s.equals("S"))
            {
                long sum = 0;
                for(int j = t1; j <= t2; j++)
                    sum+=a[j];
                System.out.println(sum);
            }
            else if(s.equals("G"))
                a[t1]+=t2;
            else if(s.equals("T"))
                a[t1]-=t2;
        }
    }
}

Updated to use cumulative sum, but still exceeding time limit:

import java.util.*;

class funny_marbles
{
    public static void main(String[] ar)
    {
        Scanner sc = new Scanner(System.in);
        int n = sc.nextInt();
        int q = sc.nextInt();
        int[] a = new int[n];
        for(int i = 0; i < n; i++)
        {
            if(i == 0)
                a[i] = sc.nextInt();
             else
                a[i] = a[i-1] + sc.nextInt();
        }
        for(int i = 0; i < q; i++)
        {
            String s = sc.next();
            int t1 = sc.nextInt();
            int t2 = sc.nextInt();
            if(s.equals("S"))
            {
                int x;
                if(t1 == 0)
                    x = 0;
                else
                    x = a[t1-1];
                System.out.println(a[t2]-x);
            }
            else if(s.equals("G"))
            {
                update(a,t2);
            }
            else if(s.equals("T"))
            {
                update(a,-t2);
            }
        }
    }

    public static void update(int[] a, int t)
    {
        for(int i = 0; i < a.length; i++)
            a[i] += t;
    }
}

Given the constraints, the maximum sum, N * max(A), would be 2000000000, which fits within a Java int.  You don't need a long.

Your code is a straightforward implementation of the challenge; there's nothing to micro-optimize.  Your only hope is to use a different approach.

Assuming that the test is heavy on "S" queries, you would be better off keeping an array of cumulative sums.  Then you could answer an "S" query in constant time.

What to do with "G" and "T" operations, though?  The elegant approach would be to update the entire cumulative sums array for each "G" or "T".  However, since N is likely to be larger than Q, you might get better performance keeping the "G" and "T" operations as a list of exceptions instead.


Getting a value from an Excel sheet column The code below gets value from a column in an Excel sheet.  The values I get are B1, B2, B3, B4...., B100, ...Bn. All I need to do is strip out the char 'B' and convert the numeric string to integer and count how many times a particular integer has repeated.

The snippet below shows the string stripping the char 'B'. However I feel it can be done in better way but I don't know how.

if (range.Text.ToString() == "BayID")
{
    range = null;
    range = readSheet.get_Range(cell, Missing.Value);
    while (range.Text.ToString() != "")
    {
        range = null;
        string A1 = String.Empty, Val = String.Empty;
        char[] value = null;
        range = readSheet.get_Range(cell, Missing.Value);
        A1 = range.Text.ToString();
        value = A1.ToCharArray();
        j++;
        cell = "D" + j;
        for (int i = 1; i < value.Length; i++)
        {
            Val += value[i];
        }
   }
}

Don't be afraid of having more variables. I suspect that the value in range has a different contextual meaning when it is used in the first if clause vs the inside of the while loop. If this is the case, make two different variables with names that describe what the contextual meaning is for the specific variable. The type system will help you know the variable is a Range. A name like price or id (it all depends on what you are using it for) will make the code easier to read.

Building on what @Malachi said: You don't need to define your variables at the beginning and assign them a default value.

string A1 = String.Empty, Val = String.Empty;
char[] value = null;
range = readSheet.get_Range(cell, Missing.Value);
A1 = range.Text.ToString();
value = A1.ToCharArray();

//is the same as

range = readSheet.get_Range(cell, Missing.Value);
string A1 = range.Text.ToString();
char[] value = A1.ToCharArray();


Is there a way to refactor the c# code having multiple conditions? Is there a better way to refactor my below code where I have many conditions in if and else statement
The reason for refactoring is to make code cleaner and readable
    public static PaymentOptions GetPaymentOptions_Auto(TestConfigurationCDO testConfiguration, int siteId)
{
    var paymentOptions = new PaymentOptions();

    var paymentOptionList = SitePaymentRepository.GetSitePaymentInfoBySiteId(
        testConfiguration,
        siteId);

    var lowestPriority = paymentOptionList.Min(x => x.Priority);
    var paymentAuto = paymentOptionList.Where(x => x.Priority == lowestPriority).FirstOrDefault();

    if (paymentAuto.PaymentType == PaymentMethod.Klarna)
    {
        paymentOptions = new KlarnaOptions();
    }
    else if (paymentAuto.PaymentType == PaymentMethod.PayPalDirect ||
        paymentAuto.PaymentType == PaymentMethod.Braintree ||
    paymentAuto.PaymentType == PaymentMethod.BankTransfer || paymentAuto.PaymentType == PaymentMethod.AdyenDropIn)
    {
        paymentOptions = new ClientCheckoutOptions()
        {
            paymentMethod = paymentAuto.PaymentType
        };
    }
    else if (paymentAuto.PaymentType == PaymentMethod.PayPalExpress)
    {
        paymentOptions = new PaypalOptions();
    }
    else
    {
        paymentOptions = new PaypalOptions();
    }
    return paymentOptions;
}

You've got lots of tests for paymentAuto.PaymentType there, and many of the cases are the same. A switch statement will neaten that up, as it only names paymentAuto.PaymentType once, and it encourages you to put all of the cases nicely aligned together in code:
public static PaymentOptions GetPaymentOptions_Auto(TestConfigurationCDO testConfiguration, int siteId)
{
    var paymentOptionList = SitePaymentRepository.GetSitePaymentInfoBySiteId(
        testConfiguration,
        siteId);

    var lowestPriority = paymentOptionList.Min(x => x.Priority);
    var paymentAuto = paymentOptionList.First(x => x.Priority == lowestPriority);

    PaymentOptions paymentOptions;

    switch (paymentAuto.PaymentType)
    {
        case PaymentMethod.PayPalDirect:
        case PaymentMethod.Braintree:
        case PaymentMethod.BankTransfer:
        case PaymentMethod.AdyenDropIn:
            paymentOptions = new ClientCheckoutOptions()
            {
                paymentMethod = paymentAuto.PaymentType
            };
            break;

        case PaymentMethod.Klarna:
            paymentOptions = new KlarnaOptions();
            break;

        case PaymentMethod.PayPalExpress:
        default:
            paymentOptions = new PaypalOptions();
            break;
    }
}

The initial value you were assigning to paymentOptions wasn't used, so you can leave that variable uninitialised. The compiler will moan at you if you forget to initialise it somewhere in the switch statement.
That .FirstOrDefault() shouldn't fail to find a matching item. To make sure of this, I changed it to .First(), which throws an exception if it can't find a match. .Where(...).First() is the same as .First(...) (but a bit more expensive), so I changed that as well.
Annoyingly there's no .MinBy method in Linq (there are plenty of examples of implementations around, though). If you have this, you can write var paymentAuto = paymentOptionList.MinBy(x => x.Priority).


Generating the Collatz sequence in F# Below is my code for building the collatz sequence so a given number.  I'm trying to make the function tail recursive and I believe that this impelemtation is tail recursive but I'm not yet so comfortable with F# that I can convince myself 100%.

let collatzSeq n =
    let collatz m =
        match m%2 with
        | 0 -> m/2
        | _ -> 3*m + 1
    let rec loop (l,acc) =
        match l with
        | 1 -> acc
        | _ -> let l' = (collatz l) in loop (l',l'::acc)
    loop (n,[n]) |> List.rev

[<EntryPoint>]
let main argv =
    let col12 = collatzSeq 12
    printfn "%A %d" col12 col12.Length
    0 // return an integer exit code

My concern is does the let .. in loop construct stop this being tail recursive and if it does how do I improve the code to make it tail recursive?

By building two implementaions of a much simpler function you can see that the function in the question is in a form that is tail recursive.

let rec recList n =
    match n with
    | 1 -> [1]
    | _ -> n :: recList (n - 1)

let accList n =
    let rec loop (l,acc) =
        match l with
        |1 -> acc
        |_ -> let l' = l - 1 in loop(l',l::acc)
    loop (n,[]) |> List.rev

[<EntryPoint>]
let main argv =

    // Fails with a stack overflow!
    // printfn "%A" (recList 100000)
    printfn "%A" (accList 10000000)

The recList implementaion again demonstrates that a simple recursive function will cause the stack to explode, on my machine this happened between 10000 and 100000.

The accList version which mirrors the function in the question works with values much larger than the one that caused the recList function to fail.


Grouping sub-arrays with nonidentical items I have a 2D array like

[ [ 'B', 'O', 'C' ],
  [ 'J', 'B', 'F' ],
  [ 'F', 'B', 'D' ],
  [ 'F', 'D', 'W' ],
  [ 'N', 'F', 'X' ],
  [ 'X', 'J', 'F' ],
  [ 'T', 'Y', 'H' ],
  [ 'R', 'Q', 'P' ] ],

And i need to group the sub-arrays such that in the resulting 3D array each group should contain only the sub-arrays with items non identical with each other's items. Besides there shouldn't be any duplicate groups such as listing the same sub-arrays in a different order.

For the given array the result is;

[ [ [ 'B', 'O', 'C' ],
    [ 'F', 'D', 'W' ],
    [ 'T', 'Y', 'H' ],
    [ 'R', 'Q', 'P' ] ],
  [ [ 'J', 'B', 'F' ],
    [ 'T', 'Y', 'H' ],
    [ 'R', 'Q', 'P' ] ],
  [ [ 'F', 'B', 'D' ],
    [ 'T', 'Y', 'H' ],
    [ 'R', 'Q', 'P' ] ],
  [ [ 'N', 'F', 'X' ],
    [ 'T', 'Y', 'H' ],
    [ 'R', 'Q', 'P' ],
    [ 'B', 'O', 'C' ] ],
  [ [ 'X', 'J', 'F' ],
    [ 'T', 'Y', 'H' ],
    [ 'R', 'Q', 'P' ],
    [ 'B', 'O', 'C' ] ] ]

My code works fine but something back in my mind says i am doing some overkill and it can be done better. Any ideas how this job might be done any better?

var arr = [ [ 'B', 'O', 'C' ],
            [ 'J', 'B', 'F' ],
            [ 'F', 'B', 'D' ],
            [ 'F', 'D', 'W' ],
            [ 'N', 'F', 'X' ],
            [ 'X', 'J', 'F' ],
            [ 'T', 'Y', 'H' ],
            [ 'R', 'Q', 'P' ] ],
 result = arr.reduce((s,t,i,a) => t.used ? s
                                         : (s.push(a.map((_,j) => a[(i+j)%a.length])
                                                    .reduce((p,c,k) => k-1 ? p.every(t => t.every(n => c.every(v => n !== v))) ? (c.used = true, p.push(c),p) : p
                                                                           : [p].every(t => t.every(n => c.every(v => n !== v))) ? (c.used = true, [p,c]) : [p])),s),[]);
 console.log(JSON.stringify(result,null,2))

The code is slightly complicated but in the heart of it lies;

.reduce((p,c,k) => k-1 ? p.every(t => t.every(n => c.every(v => n !== v))) ? (c.used = true, p.push(c),p) : p
                                                                           : [p].every(t => t.every(n => c.every(v => n !== v))) ? (c.used = true, [p,c]) : [p]))

which will take

[ [ 'B', 'O', 'C' ],
  [ 'J', 'B', 'F' ],
  [ 'F', 'B', 'D' ],
  [ 'F', 'D', 'W' ],
  [ 'N', 'F', 'X' ],
  [ 'X', 'J', 'F' ],
  [ 'T', 'Y', 'H' ],
  [ 'R', 'Q', 'P' ] ],

and turn it into

[ [ 'B', 'O', 'C' ],
  [ 'F', 'D', 'W' ],
  [ 'T', 'Y', 'H' ],
  [ 'R', 'Q', 'P' ] ]

and will mark all sub arrays as used. Then we will rotate the given array  with a.map((_,j) => a[(i+j)%a.length]), up until an unused sub array is at index 0 position and repeat the same job.

Cascaded ternaries, generic variable names, obfuscated logic produces write-only and mind-boggling code that requires deciphering for anyone, including you in the future.

Let's try to write a self-explanatory code:

function groupUnique(input) {
    let results = [];
    for (let inputRow of input) {
        let collectedCells = new Set(inputRow);
        let draft = [inputRow].concat(
            input.filter(row => {
                if (row.some(cell => collectedCells.has(cell)))
                    return false;
                for (let cell of row)
                    collectedCells.add(cell);
                return true;
            })
        );
        let alreadyAdded = results.some(res =>
            res.length == draft.length && res.every(row => draft.includes(row))
        );
        if (!alreadyAdded)
            results.push(draft);
    }
    return results;
}


Shortcut script for elusive grep command This is an example of my filesystem:

/code/

internal/

dev/
main/

public/

dev/
main/
release/

tools/

/code/internal/dev/, /code/public/dev/ and /code/tools/ contain subdirectories for multiple projects. I work almost exclusively in the dev branches of /code/internal/ and /code/public/, and often I want to search for a text string in those directories along with /code/tools/ (which has no branches). In these instances I run a command like this:

grep -I -r FooBar /code/internal/dev/ /code/public/dev/ /code/tools/

Additionally, sometimes I am only interested in certain file types. Then the command becomes:

grep -I -r FooBar /code/internal/dev/ /code/public/dev/ /code/tools/ | grep .c:\|.h:

I usually forget this command between usages and end up having to relearn it. To alleviate that problem, I created a script - which I would appreciate feedback on :)

search() {
    local t
    local OPTIND
    local pattern
    local files
    local types

    if [ $1 = --help ]; then
        echo "Usage: search [OPTION] ... PATTERN [FILE] ..."
        echo "Search for PATTERN in each FILE."
        echo "Example: search -t c -t h 'hello world' /code/internal/dev/ /code/public/dev/"
        echo
        echo "Output control:"
        echo "  -t    limit results to files of type"
        return
    fi

    while getopts ":t:" opt; do
        case $opt in
            t) t=(${t[@]} $OPTARG);; # create an array
        esac
    done
    shift $((OPTIND-1))

    pattern=$1
    files=${@:2}

    if [ -n "$t" ]; then
        # cast the array to a string
        types=${t[@]}

        # convert the string to a pattern usable by grep
        # example: "c h" becomes ".c:\|.h:"
        types=.${types// /':\|.'}:

        grep -I -r $pattern $files | grep $types
    else
        grep -I -r $pattern $files
    fi
}

With this and a couple more shortcut scripts, I can (relatively) quickly find anything I'm looking for:

search-all-code() {
    search $@ /code/internal/dev/ /code/public/dev/ /code/tools/
}

Aside: I realize some versions of grep support --include and --exlude options, but the version of grep I'm stuck with doesn't.

I'd lean towards find.  I'll provide some code review type comments at the bottom, but first a rewrite:

search() {
    local extensions
    local pattern
    local find_cmd
    local OPTIND opt

    local usage=$( cat - << END
Usage: $FUNCNAME [OPTION] ... PATTERN [FILE] ...
Search for PATTERN in each FILE.
Example: $FUNCNAME -t c -t h 'hello world' /code/internal/dev/ /code/public/dev/

Output control:
  -t    limit results to files of type
END
)
    if [[ $1 == --help ]]; then
        echo "$usage"
        return
    fi

    extensions=()
    while getopts ":ht:" opt; do
        case $opt in
            h) echo "$usage"; return;;
            t) extensions+=("$OPTARG");;
            ?) echo "invalid option: -$OPTARG";;
        esac
    done
    shift $((OPTIND-1))

    if (( $# == 0 )); then
        echo "no search term provided"
        return
    fi

    pattern=$1
    shift

    if (( $# == 0 )); then
        echo "no directories provided"
        return
    fi
    # your directories to search are now the positional parameters

    find_cmd=(find "$@" '(')
    or=""
    for type in "${extensions[@]}"; do
        find_cmd+=($or -name "*.$type")
        or="-o"
    done
    find_cmd+=(')' -exec grep -I "$pattern" '{}' +)

    "${find_cmd[@]}"
}

Notes

[ $1 = --help ] will give you syntax error if $1 is blank -- you get [  = --help ] and the = operator requires 2 operands. bash's [[ ... ]] is smarter about not dropping operands just because they're empty
You can build up arrays bit-by-bit with arr+=("$element")
you want to use more double quotes for your variables. If the pattern is "hello world", then grep $pattern file1 will look for the pattern "hello" in files "world" and "file1" -- grep "$pattern" file1 will work as you expect.
I use (( ... )) arithmetic expression for numeric comparisons.
I build up the find command piece by piece and execute it with "${arr[@]}" -- that specific syntax, indexing the array with [@] and surrounding it with double quotes, is the way you'll want to expand arrays most of the time. That specific syntax will expand the array into elements, but it will keep elements containing whitespace as a single argument.

This includes using "$@" over $@ -- the former gives you the actual arguments given by the user, the latter gives you all the words in the arguments. A demonstration:

set -- "foo bar" "hello world"    # set the positional parameters
printf "%s\n" "$@" | wc -l        # print the parameters one per line
                                  # and count the lines -- 2
printf "%s\n" $@ | wc -l          # here you get 4

if t is an array, $t gives you the first element only.


Store coordinates in 2d grid I`m writing a function for small pygame program. The ideas is that I have a 500 pixels square screen and I want to place 5 objects per row with the size of width and height in random order and I need a function to generate and store coordinates.

from random import randint

def gen_coords(width=50, height=50, side_of_scr=500):
    """Generates random coordinates for objects in rows
    that are positioned on 5x5 grid."""

    coords = []
    for row in range(5):
        # creating row container
        coords.append([])
        for pos in range(5):
            # range of values in randint reflects that i want the objects
            # to be placed with offset of the sides of the screen
            x = randint(0.5 * width, side_of_scr - 0.5 * width)

            # the distance between the rows is equal to the height of an object
            y = 0.5 * height + row * 2 * height
            coords[row].append((x, y))
    return coords

So this

for coord in gen_coords():
    print(coord)

returns

[(177, 25.0), (367, 25.0), (214, 25.0), (284, 25.0), (125, 25.0)]
[(425, 125.0), (446, 125.0), (126, 125.0), (409, 125.0), (187, 125.0)]
[(210, 225.0), (188, 225.0), (215, 225.0), (132, 225.0), (431, 225.0)]
[(303, 325.0), (471, 325.0), (252, 325.0), (26, 325.0), (111, 325.0)]
[(468, 425.0), (127, 425.0), (452, 425.0), (30, 425.0), (345, 425.0)]

I've done this using list of lists and it's working so far, but im looking for the better way to do it. Thanks for help.

I am not entirely sure what you are unhappy about, so I am going to assume you find it too cumbersome, so here is a "one line" way to do it:

import random
gen_coords = (lambda rows=range(5), cols=range(5): lambda width=50, height=50, side_of_scr=500: [[
    (random.randint(width/2, side_of_scr - width/2), height/2 + row * 2 * height)
    for col in cols] for row in rows])()

That is still ugly, so let us make an elegant solution, with better performance:

import random
def gen_coords(width=50, height=50, side_of_scr=500, rows=5, cols=5):
    lower = width/2
    upper = side_of_scr - width/2
    height_step = 4 * height
    gen_x = lambda : random.randint(lower, upper)
    def gen_row(row):
        y = (row + 0.25) * height_step
        return [(gen_x,y) for col in range(cols))]
    return [gen_row(row) for row in range(rows)]


Synchronization of new data with previous data I am working with synchronization for the first time. I wrote some code that I have to implement in my current work. I wanted to know if I am doing it right or am I making a mistake.

Whenever a new list of data is downloaded, NewDataArrived() is called. This method then calls the ListData() that returns the list of previously saved data. There is another class running in parallel that uses the ListData(). What I want to do is whenever the List is being updated, the Thread class should just wait until the list is updated.

public class ListData() {

/*
*
*

*/
public List < name_holder > getList() {
    synchronized(LOCK_LIST) {
        if (newDataArrived)
            LOCK_LIST.notifyAll();

        return full_list;
    }
}

void NewDataArrived() {

    newDataArrived = true;
    parallelDownload p = new parallelDownload();
    p.start();
    getList().addAll(my_list);
    newDataArrived = false;

}

class parallelDownload extends Thread {
    private Object o = new Object();

    @Override
    public void run() {
        synchronized(o) {
            for (name_holder n: getList()) {
                while (newDataArrived) {
                    try {
                        o.wait();
                    } catch (InterruptedException e) {
                        e.printStackTrace();
                    }
                }
            }
        }
    }
}
}

Coding Conventions

First of all: You should follow common naming conventions for Java.  It makes it harder to read your code if you don't -- or to put it differently: It is easier to help if the code does follow common conventions.  Maybe have a look at Google's Java Style or at least:

write methods in lowerCamelCase style and
classes in UpperCamelCase style.

Furthermore it would be good to provide a compilable minimal example.

Threadsafe List

From what I understood you want to have a list which is capable of being accessed and modified from multiple threads.  If so, you may have a look at this stackoverflow discussion.

Java Concurrency

Some general things:

You don't need to instantiate an object for your synchronized blocks.  If you do not specify an object, the current object is used.  That should be sufficient for your use cases.
Don't start a thread directly.  Rather use an ExecutorService instead.
Don't subclass Thread, but rather use a Runnable (also see here).
Very good book on the whole topic: Java Concurrency in Practice.


Executing URL using Apache HttpClient I have XML below which is stored as a String in xmlData variable and I need to pass this String to my URL in client_data variable and then execute the URL:

<?xml version="1.0"?>
<ClientData
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
 xsi:schemaLocation="http://www.google.com model.xsd"
 xmlns="http://www.google.com">
    <client id="100">
        <clock>
                <for>
                    <etc>val(tery) = 1</etc>
                    <while><![CDATA[val(tery)  < 20]]></while>
                </for>
        </clock>
    </model>
</ClientData>

Below is the URL I am hitting on the browser by URL encoding client_data value manually to get the response back and it works fine through browser -

http://localhost:8080/test_tmp?max_time=30&users=1000&client_data=<?xml version="1.0"?>
<ClientData
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
 xsi:schemaLocation="http://www.google.com model.xsd"
 xmlns="http://www.google.com">
    <client id="100">
        <clock>
                <for>
                    <etc>val(tery) = 1</etc>
                    <while><![CDATA[val(tery)  < 20]]></while>
                </for>
        </clock>
    </model>
</ClientData>

Now I have started using Apache HttpClient to execute the URL and I have got below code which is working fine -

Using Apache HttpClient

This code works fine and I am able to get the response back. I am getting JSON response back as a string which is expected.

public static void main(String[] args) {
    for(int i= 1; i<=100; i++) {
        // this will return xml as a string
        String xmlData = getXMLData(i);
        String url = generateURL(xmlData);

        // do I need to create this everytime for each call?
        HttpClient httpclient = HttpClientBuilder.create().build();
        HttpGet request = new HttpGet(url);
        StringBuilder response = new StringBuilder();

        try {
            HttpResponse httpResponse = httpclient.execute(request);
            StatusLine statusLine = httpResponse.getStatusLine();
            int statusCode = statusLine.getStatusCode();
            String phrase = statusLine.toString();

            if (statusCode == HttpStatus.OK.value()) {
                BufferedReader rd = new BufferedReader(new InputStreamReader(httpResponse.getEntity().getContent()));

                String line = "";
                while ((line = rd.readLine()) != null) {
                    response.append(line);
                }
            }
        } catch (ClientProtocolException e) {
            System.out.println(e.getMessage());
        } catch (IOException e) {
            System.out.println(e.getMessage());
        }

        // prints response
        System.out.println(response.toString());

        // and then I am serializing json "response" here to my POJO if it not null
    }
}

private static String generateURL(final String clientData) {
    StringBuilder url = new StringBuilder();
    url.append("http://localhost:8080/test_tmp?max_time=30&users=1000&client_data=");
    url.append(URLEncoder.encode(clientData, "UTF-8"));

    return url.toString();
}

I am opting for code review to see whether there is any better way to execute the URL efficiently using Apache HttpClient or if there is any way to simplify the above code.

Efficiency

In response to

// do I need to create this everytime for each call?
HttpClient httpclient = HttpClientBuilder.create().build();

Then no. The construction doesn't take any parameter, so it's independent from the loop and you can initialize only once before the loop. And although the javadoc for those methods is empty, by the looks of .create().build() I'd say that it's not a cheap instantiation.

Simplification

You can save some lines by using IOUtils to read the content from the response, as explained in this answer.

Readability

Your function is a bit too long. You can probably divide it into smaller functions. I would start extracting the try/catch block into a readResponse() method.

Miscellaneous

This

HttpStatus.OK.value()

is from Spring. There's nothing wrong with it, but since you're using an Apache library, why don't you use HttpStatus from the Apache library?


Flattening a Binary Tree I have this below Data Structure.

The output will be like

m - n - o - p - q - r - s -t

Please review the below code and let me know if any optimization can be done.

public class Application {

    public static Node root = null;
    public static Node temp = root;
    public static Node anotherTemp;

    public static void main(String[] args) {

        Node m = new Node("m");
        Node n = new Node("n");
        Node o = new Node("o");
        Node p = new Node("p");
        m.setDown(n);
        n.setDown(o);
        o.setDown(p);

        Node q = new Node("q");
        Node r = new Node("r");
        Node s = new Node("s");

        m.setRight(q);
        q.setDown(r);
        r.setDown(s);

        Node t = new Node("t");
        q.setRight(t);
        root = new Node(m.getValue());
        temp = root;

        anotherTemp = m;

        flattern(m);
        // Here is my output
        System.out.println(temp);
    }

    public static void flattern(Node m) {

        if (m.getRight() == null)
            return;

        while (m.getDown() != null) {
            if (!m.getValue().equals("m")) {
                root.setRight(new Node(m.getValue()));
                m = m.getDown();
                root = root.getRight();
            }
            else
                m = m.getDown();
        }
        if (m.getDown() == null) {
            root.setRight(new Node(m.getValue()));
            if (anotherTemp.getRight() != null) {
                anotherTemp = anotherTemp.getRight();
                root.getRight().setRight(new Node(anotherTemp.getValue()));
                root = root.getRight();
            }
        }

        if (m.getDown() == null)
            flattern(anotherTemp);
        System.out.println(temp);
        System.out.println(root);
    }

}

class Node {
    String value;
    Node right;
    Node down;

    Node(String val) {
        this.value = val;
    }

    public String getValue() {
        return value;
    }

    public void setValue(String value) {
        this.value = value;
    }

    public Node getRight() {
        return right;
    }

    public void setRight(Node right) {
        this.right = right;
    }

    public Node getDown() {
        return down;
    }

    public void setDown(Node down) {
        this.down = down;
    }

}

Possible bug

If you were to set a right node to the node r with

r.setRight(new Node("v"));

the result would not contain this node.

OOP paradigm

There are several fundamental issues with your solution. Mainly, it breaks completely the OOP paradigm; this is shown by:

The use of static global variables:

public static Node root = null;
public static Node temp = root;
public static Node anotherTemp;

This is not a good idea: this creates variables that can be seen and modified by anyone, when in reality, they only make sense when flattening the nodes.
The flattening algorithm depends on the value itself!

if (!m.getValue().equals("m")) {

The issue with this line is that you have just made your algorithm dependent on your data. It cannot flatten any node since "m" is hard-coded. If tomorrow your first node has a value of "a", it will be broken.

In Object-Oriented Programming, you should give behaviour to objects. Your object in this case is a Node, and you are trying to flatten it. Therefore, it makes sense to have a method flatten() inside the class Node: any node can be flattened and it is the responsibility of a Node to be able to flatten itself.

Let's reason through what it means for a node n to be flattened:

The new right of node n will be the result of flatting what was below it.
After that, the right-most node of n will have at its right set to the flattened previous right of n.

This shows a simple recursive solution:

class Node {

    // ...

    public Node flatten() {
        Node node = new Node(value);
        Node prevRight = right;
        if (down != null) {
            node.setRight(down.flatten());
        }
        if (prevRight != null) {
            Node rightMost = node;
            while (rightMost.getRight() != null) {
                rightMost = rightMost.getRight();
            }
            rightMost.setRight(prevRight.flatten());
        }
        return node;
    }

}

Be careful of typos also, flattern is mis-spelled: it should be flatten.


Function to produce unique random numbers I've been learning Haskell for a few weeks after coming from a C# background. I've written a function to return a number of unique Ints from a specified range:

module MrKWatkins.Random (
    uniqueRandomInts
) where

import System.Random
import qualified Data.IntSet as I

uniqueRandomInts :: RandomGen g => (Int, Int) -> Int -> g -> ([Int], g)
uniqueRandomInts range@(min, max) n rng
    | n < 0             = error "n must be positive"
    | n > max - min + 1 = error "n is too large for range"
    | otherwise         = recursion n I.empty ([], rng)
    where
        recursion n used (xs, rng)
            | n == 0          = (xs, rng)
            | I.member x used = recursion n used (xs, rng')
            | otherwise       = recursion (n-1) (I.insert x used) (x:xs, rng')
            where
                (x, rng') = randomR range rng

Then to produce 5 numbers in the (inclusive) range 1 -> 10 the call would be something like:

uniqueRandomInts (1, 10) 5 $ mkStdGen 42

Firstly I have a few specific questions:

I've prefixed my module name with MrKWatkins to distinguish it as my module (much as I would with a namespace in C#). Is this common practice, or would I be better with a single word for the module name that is more descriptive?
I've done some pre-condition checking on the parameters, throwing exceptions if they're not satisfied. Is this common practice in Haskell, and is this the best way to do it?
I've used a nested function for the recursion. Would I be better off separating this out into a top level function that I don't export from the module?
I've reused some variable names in the recursion function, hiding the ones from the parent scope. Should I use different names instead?

On top of those if there is any advice on style, better ways to implement the above or just general advice would be greatly appreciated.

Haskell is very different than other languages.

Here is how I would solve the problem:

import Control.Arrow
import System.Random

uniqueRandomInts :: (RandomGen g, Random a)
    => (a, a) -> Int -> g -> ([a], g)
uniqueRandomInts range n =
  (map fst &&& snd . last) . take n . iterate getNext . randomR range
  where getNext = randomR range . snd

What does this do?

First note that the g (generator) parameter isn't shown in the example, but it is there....  uniqueRandomInts range n returns a function of type

RandomGen g => g -> ([a], g)

so you will still need to apply this parameter in order to get the values.

I've also refrained from specifying the type of the return value at this stage....  This function is a tool that could be used for any type, why artificially bind it at this point?

The iterate getNext $ randomR range g part (where I've shown where the g will be applied) creates an infinite list of (a, g), where the a's are the random values, and the g's are the generators returned at each step.  (for reference, iterate f x is a function that returns [x, f x, (f.f) x, (f.f.f) x, ....]).

Passing around infinite lists is one of those mind blowing things to people who are coming to Haskell from other languages, but remember that the language is lazy, so no actual value will be calculated until requested (this is sort of how the human mind works....  If I ask you to imagine an infinite sequence of 1's, you can reason about it without actually sitting there trying to enumerate all the 1's in your brain before you go on).

We then apply take n to this, to take the first n values of the sequence (thus turning it into something finite before we actually need to calculate any of the values).

Finally, we have the mysterious (map fst &&& snd . last) part.  This isn't actually needed for the calculations themselves, but just to format the values in the way that you wanted them.  (func1 &&& func2) x applies the functions on the value x, and returns (func1 x, func2 x).  We are pulling out the random values in the first slot, and getting the last random generator for the second spot.

You can use this function as follows (because we kept things generic, you need to specify the type)

main = do
  g <- getStdGen
  print $ randomValues (1, 10::Int) 10 g

As to your particular questions:

As to the prefixing, Haskell has a vibrant community ecosystem where many people contribute code to the public, and some sort of prefixing is needed to keep things sane....  Of course this is only needed if you plan to contribute your code.  I personally don't prefix at all when I am writing something small for myself (why complicate things?), and can always add it if I need later.  If a personal project becomes larger, I can always add it.  This is pretty much how I programmed in Java also....
Pre-checking is very common and valuable in Haskell, however....  so is generality.  There is no reason that this particular function shouldn't be used by an numerical type over any range (would you want to rewrite the exact same function for a set of random floats over the range -1.0 to +1.0?).  I would personally place the limit check and type fixing higher up in the code....  And when I did, I would use the Data.Ix.inRange function.
I didn't use explicit recursion (it was used in a lower level function that I used), however I'll answer the general philosophy that I follow to determine if a subfunction should be top level or not....  I make a function top level if it describes a generic tool that others could use.  I usually use a "where" clause generally only for function documentation (ie- when the name of the variable describes what it is doing).  Sometimes you need to define a where clause variable for other reasons (ie- recursion or to avoid repetition), but strangely enough these are usually cases that you are better off writing around anyway (recursion and repetition are usually abstracted away in lower level calls like iterate and (&&&), which are cleaner to use anyway).  This isn't a hard fast rule though!  Not everything can be abstracted in some high level function yet.
I don't know :), however if you ever turn on -Wall in the compiler, it will complain every time you shadow another variable, so to avoid the nagging I usually try to change the name.  Sometimes choosing a different name becomes annoying, so I just turn off -Wall :)

EDIT-----

Oops, as you pointed out, I misread the problem....  You need unique values.

Luckily we can still use all the code above, with a slight addition.  Add this-

removeDuplicatesUsing::Ord b=>(a->b)->[a]->[a]
removeDuplicatesUsing f theList = [x|(Just x, _, _) <- iterate getNext (Nothing, theList, empty)]
  where
    getNext (_, x:xs, used)
      | f x `member` used = (Nothing, xs, used)
      | otherwise = (Just x, xs, insert (f x) used)

Then put one more filter in your randomValue pipeline-

(map fst &&& snd . last) . take n . removeDuplicatesUsing fst . iterate getNext . randomR range

Let me explain what is going on here

There are many algorithms to generate random unique values....  The unique part complicates things a bit, and depending on your circumstances, different algorithms work better.  I will discuss those cases later in this answer, but for now I chose the same algorithm that you used in your answer.
Because Haskell is lazy, adding another function filter in a bunch of composed functions barely takes a speed, memory or latancy performance hit.  Data will flow through the functions from right to left as it is generated, you will start to see results even before all the data has started to flow into the pipeline.  In this way, Haskell function composition is more like Unix pipe chaining than anything else.
In fact breaking things apart like this is my preferred way to do things.  By now I might have more code than if I wrote it all in one function, but each of these are simple, easy to debug tools that you could imagine reusing.
A removeDuplicates function (which I didn't write) would read a stream of values in, and filter out anything that it has previously seen.  This is almost what we need, but we want to base uniqueness on only the resulting random value (if we included the generator, it would always be unique!).  It is common in Haskell to write generalized -Using or -With functions that allow you to pass a function in to give the "key" that you want to use for comparisons (....and note that you can easily define removeDuplicates = removeDuplicatesUsing id).
I wrote the removeDuplicates in the same style as the uniqueRandomInts function (using a list of tuples generated by iterate)....  Writing out using recursion here might be easier to read, I guess it is just a matter of taste.  This feels more Haskell-y to me, and that was the point of this comparison.
I still left out the range check, but after reading your comment I reverse what I said above, I think it is a good idea to put it in!  (I looked too quickly and thought it was a value range, not a count range).

Finally, a note about algorithms (independent on the language chosen to write this).  If you are choosing n unique items from a range of N values....

If N and n are small --- It doesn't matter what algorithm you choose, they will all be fast.
If N is huge and n is small --- the given algorithm above works great.  It breaks down however....
as n approaches N in size --- Once a sizable percentage of N has been filled, you will start hitting many repeat hits (if 90% of the range has already been taken, only one in 10 tries will get something new).  You can of course flip the chosen and not chosen values (ie- generate the 10% you don't want, then return all the others), or you can move on to a solution of this type- https://stackoverflow.com/questions/196017/unique-random-numbers-in-o1.  While this is doable in Haskell, the part where you start swapping values in an array is trickier than in any mutable language (you basically either have to use IO around a mutable array, or start making copies of the initial array, which Haskell can do much more efficiently by reusing parts of the initial array in the copied version, but still, less efficient than just doing mutating swaps....  Someone else might have a better suggestion than this....).
If N is really large, holding all values in an array in memory might not be practical.  In this case, you might want to use some sort of "unsort" command line tool (a few of these exist in Unix), do a one time pass over all the values, and then take the first n elements of the list.
If N is huge, and you don't care that n come out precisely, you can always do something statistical (do one linear pass and accept each value with probability n/N).  This should be much faster than method (4), but of course you won't get exactly n values, and you can't just truncate early, or else you will be favoring earlier values over latter ones.


Fixed point combinator in C++1x This has been tested and compiled under Visual Studio 2010. Are there any serious problems with this implementation?

PS. This implementation is fully lambda expression based. If I can make fixpoint::fix a real member function it will be simpler than this.

//As decltype(variable)::member_name is invalid currently,
//the following template is a workaround.
//Usage: t2t<decltype(variable)>::t::member_name
template<typename T>
struct t2t
{
    typedef T t;
};

template<typename R, typename V>
struct fixpoint
{
    typedef std::function<R (V)> func_t;
    typedef std::function<func_t (func_t)> tfunc_t;
    typedef std::function<func_t (tfunc_t)> yfunc_t;

    class loopfunc_t {
    public:
        func_t operator()(loopfunc_t v)const {
            return func(v);
        }
        template<typename L>
        loopfunc_t(const L &l):func(l){}
        typedef V Parameter_t;
    private:
        std::function<func_t (loopfunc_t)> func;
    };
    static yfunc_t fix;
};
template<typename R, typename V>
typename fixpoint<R, V>::yfunc_t fixpoint<R, V>::fix =
[](fixpoint<R, V>::tfunc_t f) -> fixpoint<R, V>::func_t {
    fixpoint<R, V>::loopfunc_t l = [f](fixpoint<R, V>::loopfunc_t x) ->
        fixpoint<R, V>::func_t{
            //f cannot be captured since it is not a local variable
            //of this scope. We need a new reference to it.
            auto &ff = f;
            //We need struct t2t because template parameter
            //V is not accessable in this level.
            return [ff, x](t2t<decltype(x)>::t::Parameter_t v){
                return ff(x(x))(v);
            };
        };
        return l(l);
    };

int _tmain(int argc, _TCHAR* argv[])
{
    int v = 0;
    std::function<int (int)> fac =
    fixpoint<int, int>::fix([](std::function<int (int)> f)
        -> std::function<int (int)>{
        return [f](int i) -> int{
            if(i==0) return 1;
            else return i * f(i-1);
        };
    });

    int i = fac(10);
    std::cout << i; //3628800
    return 0;
}

I hav tried the same thing in G++ (version 4.5 from Ubuntu 11.04 original), and the result is quite impressing me. We now do have a lot more features to generise the solution.

we can now support any number of parameters without modifying the code.
the t2t class is no longer needed as G++ lambda supports accessing all identifiers in scope, including global things.

I also tried write a free template function to access the internal function. And the result looks good. I just wanted to say: it is elegant!

#include <iostream>
#include <functional>

template<typename R, typename... V>
struct fixpoint
{
    typedef std::function<R (V...)> func_t;
    typedef std::function<func_t (func_t)> tfunc_t;
    typedef std::function<func_t (tfunc_t)> yfunc_t;

    class loopfunc_t {
    public:
        func_t operator()(loopfunc_t v)const {
            return func(v);
        }
        template<typename L>
        loopfunc_t(const L &l):func(l){}
        loopfunc_t(){}
    private:
        std::function<func_t (loopfunc_t)> func;
    };
    static func_t Fix(tfunc_t f){
        return [](loopfunc_t x){ return x(x); }
        ([f](loopfunc_t x){ return [f, x](V... v){ return f(x(x))(v...); }; });
    }
};
template<typename T>
struct getfixpoint
{
    typedef typename getfixpoint<decltype(&T::operator())>::fp fp;
};
template<typename R, typename T, typename... V>
struct getfixpoint<std::function<R (V...)> (T::*)(std::function<R (V...)>)>
{
    typedef fixpoint<R, V...> fp;
};
template<typename R, typename T, typename... V>
struct getfixpoint<std::function<R (V...)> (T::*)(std::function<R (V...)>)const>
{
    typedef fixpoint<R, V...> fp;
};
template<typename R, typename... V>
struct getfixpoint<std::function<R (V...)> (*)(std::function<R (V...)>)>
{
    typedef fixpoint<R, V...> fp;
};

template<typename T>
auto getFix(T f) -> typename getfixpoint<T>::fp::func_t {
    return getfixpoint<T>::fp::Fix(f);
};

int main(int argc, char* argv[])
{
    auto fac = getFix([](std::function<int (int)> f) -> std::function<int (int)>{
        return [f](int n)->int { if(n==0) return 1; else return n * f(n-1); };
    });
    std::cout<<fac(10)<<std::endl; //3628800
    return 0;
}


Abuse/Misuse of C# BackgroundWorker? I have finished a program, and it does what I want it to do, but I feel I am "doing it wrong", even though it's seemingly efficient enough. I have prepared a small example of what I feel I am handling wrong with the backgroundworker class and would like to see if I could have handled this any more cleanly.

First, I have a form with 1 button, 1 statusstrip, 1 toolstripprogressbar, and 1 toolstriplabel. I update the statusstrio items while the backgroundworker is running, as well as show messageboxes across classes. See below.

Form1.cs

namespace StatusStripTest
{
    public partial class Form1 : Form
    {
        public Form1()
        {
            InitializeComponent();
        }

        private void prepareToRun()
        {
            if (!backgroundWorker1.IsBusy)
            {
                backgroundWorker1 = new BackgroundWorker();
                backgroundWorker1.DoWork += delegate
                {
                    Looping.loop(statusStrip1);
                };
                backgroundWorker1.RunWorkerAsync();
            }
        }

        private void button1_Click(object sender, EventArgs e)
        {
            prepareToRun();
        }

    }
}

Looping.cs

namespace StatusStripTest
{
    public class Looping
    {
        public static void loop(StatusStrip strip)
        {
            Form1 Form1 = new Form1();
            string alphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz";
            while (true)
            {
                MessageBox.Show("Time to loop");
                foreach (char c in alphabet)
                {
                    StripHandler.UpdateProgress(strip, alphabet.IndexOf(c), alphabet.Length);
                    StripHandler.UpdateStatus(strip, c.ToString());
                }
            }

        }
    }

}

StripHandler.cs

namespace StatusStripTest
{
    public class StripHandler
    {
        public static void UpdateStatus(StatusStrip ss, String Status)
        {
            ss.Invoke((MethodInvoker)delegate
            {
                if ((ss.Items[1] as ToolStripStatusLabel) != null)
                {
                    ss.Items[1].Text = Status;
                }
            });
        }

        public static void UpdateProgress(StatusStrip ss, long position, long len)
        {
            ss.Invoke((MethodInvoker)delegate
            {
                if ((ss.Items[0] as ToolStripProgressBar) != null)
                {
                    ToolStripProgressBar tspb = (ToolStripProgressBar)ss.Items[0];
                    tspb.Minimum = 0;
                    tspb.Maximum = (int)len;
                    tspb.Value = (int)position;
                }
            });
        }
    }
}

Am I passing the statusstrip incorrectly?

In general, passing controls to the BackgroundWorker should not be necessary at all. Think about it as it simply has some job to do and that's all. However, it can of course report about progress of its job. Those two ideas were taken into account when designing BackgroundWorker class and are reflected by DoWork and ProgressChanged events respectively. There is also a dedicated BackgroundWorker.ReportProgress method which raises ProgressChanged event.

This event should be handled on GUI level so it has access to the controls without passing them down to worker. Draft of solution:

private void prepareToRun()
{
    if (!backgroundWorker1.IsBusy)
    {
        backgroundWorker1 = new BackgroundWorker();
        backgroundWorker1.DoWork += backgroundWorker_DoWork;
        backgroundWorker1.ProgressChanged += backgroundWorker_ProgressChanged;
        backgroundWorker1.RunWorkerAsync();
    }
}

public static void backgroundWorker_DoWork(object sender, DoWorkEventArgs e)
{
    BackgroundWorker worker = sender as BackgroundWorker;

    // use worker.ReportProgress(percent, someData) somewhere here

}

private void backgroundWorker_ProgressChanged(object sender, ProgressChangedEventArgs e)
{
    // Update your statusStrip1 here according to data passed in e.ObjectState
}


Dropping rows from a PANDAS dataframe where some of the columns have value 0 I am dropping rows from a PANDAS dataframe when some of its columns have 0 value. I got the output by using the below code, but I hope we can do the same with less code — perhaps in a single line.

df:

    A   B   C
 0  1   2   5
 1  4   4   0
 2  6   8   4
 3  0   4   2

My code:

 drop_A=df.index[df["A"] == 0].tolist()
 drop_B=df.index[df["C"] == 0].tolist()
 c=drop_A+drop_B
 df=df.drop(df.index[c])

[out]

    A   B   C
 0  1   2   5
 2  6   8   4

I think you need create boolean DataFrame by compare all filtered columns values by scalar for not equality and then check all Trues per rows by all:

df = df[(df[['A','C']] != 0).all(axis=1)]
print (df)
   A  B  C
0  1  2  5
2  6  8  4

Details:

print (df[['A','C']] != 0)
       A      C
0   True   True
1   True  False
2   True   True
3  False   True

print ((df[['A','C']] != 0).all(axis=1))

0     True
1    False
2     True
3    False
dtype: bool

I think you need create boolean DataFrame by compare all values by scalar and then check any Trues per rows by any and last invert mask by ~:

df = df[~(df[['A','C']] == 0).any(axis=1)]

Details:

print (df[['A','C']])
   A  C
0  1  5
1  4  0
2  6  4
3  0  2

print (df[['A','C']] == 0)
       A      C
0  False  False
1  False   True
2  False  False
3   True  False

print ((df[['A','C']] == 0).any(axis=1))
0    False
1     True
2    False
3     True
dtype: bool

print (~(df[['A','C']] == 0).any(axis=1))
0     True
1    False
2     True
3    False
dtype: bool


Designing object oriented clean code structure with spring services I'm writing spring app for storing and manipulating recipes. I wrote ShoppingList service which generates, based on UUIDs of recipes, shopping list. This is my current model for which i wrote tests and works as intended.
Service uses another service called IngredientConverter which converts to response models. Now i wanted to move some dependency to ShoppingList as this function is jealous of functions, but approached a problem. How can a this class use IngredientConverter service without ugly passing it to ShoppingList function? Are there any tips you could give my code and structure planning?
@Service
@AllArgsConstructor
public class ShoppingListService {

    private final RecipeService recipeService;
    private final IngredientConverter ingredientConverter;

    public ShoppingList generateShoppingList(List<UUID> uuidsOfRecipes) {
        List<Recipe> recipes = recipeService.getAllByIDIn(uuidsOfRecipes);
        ShoppingList shoppingList = ShoppingList.empty();

        Map<Ingredient, Integer> ingredients = new HashMap<>();

        recipes.forEach(recipe ->
                recipe.getIngredients().forEach(
                        ingredientQuantity ->
                                ingredients.compute(
                                        ingredientQuantity.getIngredient(),
                                        (key, value) ->
                                                value == null ?
                                                        ingredientQuantity.getAmount() :
                                                        value + ingredientQuantity.getAmount())));

        ingredients.keySet().forEach(
                ingredient ->
                        shoppingList.getIngredients().add(
                                ingredientConverter.convertWithAmount(
                                        ingredient.getName(),
                                        ingredients.get(ingredient),
                                        ingredient.getUnit())
                        )

        );

        return shoppingList;
    }

}

@Data
@AllArgsConstructor
public class ShoppingList {

    private final List<IngredientQuantity> ingredients;

    public static ShoppingList of(List<IngredientQuantity> ingredients) {
        return new ShoppingList(ingredients);
    }

    public static ShoppingList empty(){
        return ShoppingList.of(new ArrayList<>());
    }

}

@Service
public class IngredientConverter {

 public IngredientQuantity convertWithAmount(String name, int amount, Unit unit) {
        return IngredientQuantity.builder()
                .amount(amount)
                .ingredient(convert(name, unit))
                .build();
    }
}

@Data
@Entity
@SuperBuilder
@NoArgsConstructor
@AllArgsConstructor
@EqualsAndHashCode(callSuper = true)
public class IngredientQuantity extends IdentifiableEntity {

    private int amount;
    @ManyToOne
    private Ingredient ingredient;
}

@Data
@Getter
@Entity
@SuperBuilder
@NoArgsConstructor
@AllArgsConstructor
@EqualsAndHashCode(callSuper = false)
public class Ingredient extends IdentifiableEntity {

    private String name;
    @Enumerated(EnumType.STRING)
    private Unit unit;
}

@Getter
@NoArgsConstructor
@SuperBuilder
@MappedSuperclass
public class IdentifiableEntity {

    @Id
    @GeneratedValue(strategy = GenerationType.AUTO)
    private UUID ID;
}
```

I would inject the converterservice into ShoppingList and implement an add() function, which adds an element to the ShoppingList after conversion.
void add(name, ingredient, unit) {
    IngredientQuantity toAdd = this.converterService.convert(name, ingredient, unit); // terrible naming, todo!
    this.ingredients.add(toAdd);
}

Now the shopping list is the owner of conversion which (arguably) is modelling real life. When I need to shop for milk, I convert the American recipes to litres instead of ounces or whatever.
This also simplifies your last lambda function, you just need to iterate over the keyset and call ShoppingList::add, which is Java8+ method ref syntax.
The other huge lambda should probably also be split into parts, I mean, I love functional java as much as anyone, but I believe this is cutting the line of functional utility over readability.
Pseudocode
foreach Recipe:
    foreach Ingredient : Recipe
        compute(ingredient)

These are my 2 cents.


100 Doors Problem in perl I'm solving this in perl:

https://github.com/kennyledet/Algorithm-Implementations/tree/master/100_Doors_Problem

  Puzzle:
  There are 100 doors in a long hallway. They are all closed. The first time you walk by each door, you open it. The second time around, you close every second door (since they are all opened). On the third pass you stop at every third door and open it if it’s closed, close it if it’s open. On the fourth pass, you take action on every fourth door. You repeat this pattern for 100 passes.
  At the end of 100 passes, what doors are opened and what doors are closed?

This is my solution:

#!/usr/bin/env perl

use strict;
use warnings;
use utf8;
use feature qw(say);

my @doors = (0) x 100;

sub check_hallway {
    say "door ". ++$_ , $doors[$_] ? ' is open' : ' is closed' foreach 0 .. $#doors;
}

sub walk_hallway {
    for my $step ( 1 .. $#doors + 1 ) {
        for ( my $i = $step; $i < $#doors + 1; $i += $step ) {
            $doors[ $i ] ^= 1;
        }
    }
}

walk_hallway();
check_hallway();

Is there any better way to write this or any advice to keep in mind?

You have a bug. Lists are 0-indexed in Perl.

Take a closer look at this:

my @doors = (0) x 100;

sub check_hallway {
    say "door ". ++$_ , $doors[$_] ? ' is open' : ' is closed' foreach 0 .. $#doors;
}

@doors has 100 elements, indexed from 0 to 99.
In check_hallway, you have a loop from 0 to 99, but due to the ++$_ earlier on the line,
$doors[$_] will be used with indexes 1..100.
$books[0] is not used, and $books[100] is used, out of bounds.

To make the problem more visible, simply add $books[$_] in the say statement:

say "door ". ++$_, $doors[$_], $doors[$_] ? ' is open' : ' is closed' foreach 0 .. $#doors;

When accessing $doors[0], the program will emit an error:

  Use of uninitialized value in say at prog.pl line 11.

So then why does it appear to be working?
Because in walk_hallway you are not using index 0..99,
but 1..99.

As it is, the last door is missed. Door 100 should be open, but the program shows it closed.

An easy fix is to use 101 doors and ignore index 0. Only the first part of the code needs to change to make that work:

my @doors = (0) x 101;

sub check_hallway {
    say "door ". $_, $doors[$_] ? ' is open' : ' is closed' foreach 1 .. $#doors;
}

But if take a look at the open doors in the output, a pattern becomes obvious:

door 1 is open
door 4 is open
door 9 is open
door 16 is open
door 25 is open
door 36 is open
door 49 is open
door 64 is open
door 81 is open
door 100 is open

The only doors open are the square numbers. This is because all other numbers have an even number of unique divisors. Only square numbers have an odd number of unique divisors.
So, to print the open and closed doors, we can use a much simpler and faster algorithm, for example:

my $doors = 100;

my %squares;
foreach (1..sqrt($doors)) {
    $squares{$_ * $_} = 1;
}

foreach my $door (1..$doors) {
    print 'door ', $door, ' is ', $squares{$door} ? 'open' : 'closed', "\n";
}


Simple responsive page Can someone tell me if my code is okay so far? I've read quite some resources online for responsive webdesign but it seems like everyone's opinion on best practices is different.

The page I'm working on is this. It's a very simple page with very little content. Right now it works quite well on smaller devices but I want to be sure my approach is correct. In particular, I'm not sure whether my use of the units is correct.

html, body { height: 100% }
body { margin: 0; padding: 0; font-family: 'Lato', sans-serif; background-color: #eee; overflow: hidden;}

/* Content */

.container {
  position: relative;
  height: 80%;
}

img[alt="Pikachu"] {
  width: 198px;
  height: 198px;
  border: 1px solid #000;
  position: absolute;
  left: 50%;
  top: 50%;
  transform: translate(-50%, -50%);
}

/* Footer */

footer {
  position: absolute;
  bottom: 0;
  left: 0;

  width: 100%;
  border-top: 1px solid #cbcbcb;
  height: 20%;
  max-height: 100px;
  min-height: 60px;
  background-color: #eee;
  text-align: center;
}

footer:after {
  content: '';
  border-top: 1px solid #fff;
  position: absolute;
  top: 0;
  left: 0;
  width: 100%;
}

footer > div {
  position: absolute;
  left: 50%;
  top: 50%;
  transform: translate(-50%, -50%);
  white-space: nowrap;
}

/* Footer Images */

a > img {
  width: 45px;
  vertical-align: middle;
  margin-right: 15px;
}

  img[alt="Twitch"] {
    margin-right: 0;
  }

/* Typography */

h1 {
  line-height: 48px;
  margin: 24px 0;

  color: #fff;
  text-shadow: hsl(15, 84%, 10%) 1px 1px, hsl(15, 84%, 10%) 2px 2px, hsl(15,             84%, 10%) 3px 3px, hsl(15, 84%, 10%) 4px 4px;
    text-transform: uppercase;
    text-align: center;
    white-space: nowrap;
  }

    h1::selection {
      color: hsl(15, 84%, 10%);
      background-color: #fff;
      text-shadow: none;
    }

  p {
    line-height: 24px;
    margin: 24px 0;
  }

body { overflow: hidden; }

You should really never hide overflow on content-based elements. People with small browsers may not see the full content, rendering the site useless to them. overflow: auto is a much better alternative because scrollbars will only be shown if there is overflow. You can tune this even more with overflow-X, overflow-Y.

Using purely px based sizing creates very small text when people have large displays. People who browse on a TV will likely find the page to have tiny text and large side margins.

footer {
  position: absolute;
  bottom: 0;
  left: 0;
}

Using absolute positioning on block elements can cause portions of the content to get obscured. To counter this you can add padding or margin to the bottom of the body element.

You have odd indentation throughout the entire code. It's better to set a set style for readability.

For instance, instead of this:

h1 {
  line-height: 48px;
  margin: 24px 0;

  color: #fff;
  text-shadow: hsl(15, 84%, 10%) 1px 1px, hsl(15, 84%, 10%) 2px 2px, hsl(15,             84%, 10%) 3px 3px, hsl(15, 84%, 10%) 4px 4px;
    text-transform: uppercase;
    text-align: center;
    white-space: nowrap;
  }

This:

h1 {
  line-height: 48px;
  margin: 24px 0;

  color: #fff;
  text-shadow: hsl(15, 84%, 10%) 1px 1px,
               hsl(15, 84%, 10%) 2px 2px,
               hsl(15, 84%, 10%) 3px 3px,
               hsl(15, 84%, 10%) 4px 4px;
  text-transform: uppercase;
  text-align: center;
  white-space: nowrap;
}

  I'm not sure whether my use of the units is correct.

This looks pretty good. Here is information regarding use of units:

px is the pixels relative to pixel density. The pixel density can affect the size of a pixel with

<meta name="viewport" content="width=device-width, initial-scale=1">

where initial scale is the amount that the pixel density affects the size of a pixel.

Meta tags go in the head element.

em is the measure of one font size on the element, therefore if an element has font-size: 16px one em is 16px. Em is relative to the parent element as well so font-size: 1.5em is 1.5 times the parent's font size.

rem is the measure of one :root element font size. If :root { font-size: 16px } then most elements will base their font size off of a 6px multiplier. Some people use calc() in their :root font size to create a scaling font. This can be done like:

:root { font-size: calc(.5em + .5vw) }

which means that the root font size will be .5 of the browsers default font size plus .5% of the viewport width.

em and rem are usable on more than just the font-size property.

% is a percentage of that parent's measurement for the tag. If the parent has width: 50px and the child has width 50% the child's width will calculate to 25px.

Other units you might need are vw, vh, vmin, vmax. These are scaling units, always relative to the viewport's size. These are helpful for scaling elements so that they always fit in the browser window. These values can be negative or greater than 100.

vw is 1% of the viewport's width. If the viewport is 500px wide then 1vw is 5px, 2vw is 10px and 7vw is 35px.
vh is 1% of the viewport's height. If the viewport is 243px then 1vh is 2.43px.
vmin is 1% of the smaller viewport side's length. If the viewport is 624px wide and 437px tall then 1vmin is 4.73px.
vmax is 1% of the larger viewport side's length. If the viewport is 1242px wide and 804px tall then 1vmax is 12.42px.


Determining if two words are anagrams I was given a this problem at a technical interview and failed miserably in the time given. Afterwards, I sat down and worked out what I think is a good solution. The task was actually to build an AngularJS app to do this, which I did, but the guts of my solution is in JavaScript. I wanted to ask if there is a better way to do this:

// convert strings to LC for case insensitivity
// split strings into arrays
// sort the arrays (spaces will sort first and be trimmed later)
// join the arrays back into strings
// we're only concerned about the printable characters in the anagram so,
// trim() to remove any spaces and then compare the resulting strings
var str1 = this.word1.toLowerCase().split('').sort().join('').trim();
var str2 = this.word2.toLowerCase().split('').sort().join('').trim();

if (str1 === str2) {
    this.isAnagram = true;
} else {
    this.isAnagram = false;
}

My original assertion (Outdated):

  Sorting a string (Or any array) is inefficient because even the
  fastest algorithm will sort no faster than O(n log n) in an average
  case. The most efficient way would use a hash map to count letters in
  each word. Something like:

Although reading from a hash map can be as quick as O(1), writing to a hash map is significantly slower. By using a 26-value array (0-25) to represent lowercase letters, the speed of operations can be sped up significantly:

function isAnagram(word1, word2) {
  if (typeof word1 !== 'string' || typeof word2 !== 'string') {
    throw new Error('isAnagram requires two strings to be passed.')
  }

  var normalizedWord1 = word1.replace(/[^A-Za-z]+/g, '').toLowerCase();
  var normalizedWord2 = word2.replace(/[^A-Za-z]+/g, '').toLowerCase();

  var counts = [];
  var word1Length = normalizedWord1.length;
  var word2Length = normalizedWord2.length

  if (word1Length !== word2Length) { return false; }

  for (var i = 0; i < word1Length; i++) {
    var index = normalizedWord1.charCodeAt(i)-97;
    counts[index] = (counts[index] || 0) + 1;
  }

  for (var i = 0; i < word2Length; i++) {
    var index = normalizedWord2.charCodeAt(i)-97;
    if (!counts[index]) { return false; }
    else { counts[index]--; }
  }

  return true;
}

EDIT: A speed comparison between using a hash and using a 26-value array:
http://jsperf.com/anagram-algorithms


StringBuilder vs string.concat I was reading this article about StringBuilder vs string.concat and couldn't figure out which one would be better in the following situation:

StringBuilder address = new StringBuilder();

address.Append(retailer.Street.Replace(",", "
").Replace("\n", "
"));
address.Append("
");

if (!string.IsNullOrWhiteSpace(retailer.City))
{
  address.Append(retailer.City);
  address.Append("
");
}

if (!string.IsNullOrWhiteSpace(retailer.County))
{
  address.Append(retailer.County);
  address.Append("
");
}

address.Append(retailer.Postcode);

return MvcHtmlString.Create(address.ToString());

According to the article, as it is not in a loop, I should be using string.Concat. But as there are if statements while building the string, I was wondering if that still applied.

Or would I be better off doing the following:

string address = string.Concat(retailer.Street.Replace(",", "
").Replace("\n", "
"), "
");

if (!string.IsNullOrWhiteSpace(retailer.City))
{
    address += string.Concat(retailer.City, "
");
}

if (!string.IsNullOrWhiteSpace(retailer.County))
{
    address += string.Concat(retailer.County, "
");
}

address += retailer.Postcode;

return MvcHtmlString.Create(address);

DISCLAIMER: You haven't properly defined "better" so I'll assume you're talking about performance

You're fixing the wrong problem.

Under the assumption that your retailer is an instance of some Retailer POCO, you should properly normalize your data and extract the Address data into a separate Table / POCO.

Then you can stop generating HTML in some Controller code and instead do something like:

retailer.Address.ToHTMLString();

This is significantly cleaner and allows central modification of "ToHTML" rules.

In addition to that I don't think you actually have to care about the performance difference between these two. Have you profiled your code and proved that this is the bottleneck? If it isn't.... It's good enough I'd say.
You seem to be falling prey to the problems described in Eric Lippert's Performance Rant


Simulate a pool using multithreading in Python I was tasked with solving this particular problem:

  A water pool which can hold a total of 800 liters of water has 3
  pipes. 2 of them pour water in, the third one pours water out. 1st
  pipe pours 100 liters in, 2nd pipe pours 80 liters in and the third
  one pours 110 liters out. The pool must keep its volume between 600 and
  800 liters. When the volume is greater than 700 liters, no water
  should be poured in, when the volume is less than 600, no water should
  be poured out.

This is the code I came up with:

import threading
import logging
import time
import concurrent.futures

class Pipe:
    waterVolume = 0
    def __init__(self, type):
        self.typeOfPipe = type
        self._lock = threading.Lock()

    def behaviour(self, type):
      while True:
        if type == 0:
            if Pipe.waterVolume < 700:
                with self._lock:
                    local_copy = Pipe.waterVolume
                    local_copy += 100
                    Pipe.waterVolume = local_copy
                    print('Pipe %d added 100 liters' % (self.typeOfPipe + 1))
                    print('volume is: %d' % Pipe.waterVolume)
        if type == 1:
            if Pipe.waterVolume < 700:
                with self._lock:
                    local_copy = Pipe.waterVolume
                    local_copy += 80
                    Pipe.waterVolume = local_copy
                    print('Pipe %d added 80 liters' % (self.typeOfPipe + 1))
                    print('volume is: %d' % Pipe.waterVolume)
        if type == 2:
            if Pipe.waterVolume > 600:
                with self._lock:
                    local_copy = Pipe.waterVolume
                    local_copy -= 110
                    Pipe.waterVolume = local_copy
                    print('Pipe %d removed 110 liters' % (self.typeOfPipe + 1))
                    print('volume is: %d' % Pipe.waterVolume)
        time.sleep(1.5)

if __name__ == '__main__':
    with concurrent.futures.ThreadPoolExecutor(max_workers=3) as executor:
        for i in range(3):
            p = Pipe(i)
            executor.submit(p.behaviour, i)

From what I've seen when satisfies the condition 600 < waterVolume < 800 for the first time, it stays there. That combined with the fact that there's no deadlocks or race conditions makes me think that this is a correct solution to the problem. Nevertheless, suggestions about improving the script are very welcome.

Naming

type is a built-in function in Python and therefore a name to avoid. Also, the widely followed Style Guide for Python Code aka PEP 8 recommends to use lower_case_with_underscores for variable names. CamelCase or more specifically TitleCase is usually "reserved" for class names.

Duplicate code

The behaviour function has an enormous amount of duplicated code. The code code for each of the if branches is basically the same, the only variable parts are the thresholds on which those branches are triggered and the amount of water that is either drained or added. Also the type parameter of behavior is redundant, since self already knows its type. Another thing that might need a closer look is the way you use the look. My threading is a bit rusty, but if I understood your code correctly, there is a separate lock for each instance of Pipe. As a consequence, there is no real synchronization between the threads and the happily check and modify Pipe.waterVolume as they go. But maybe I'm wrong here. Under the assumption that I'm right, let's look at a refactored version of your code:

class Pipe:
    water_volume = 0
    _lock = threading.Lock()

    def __init__(self, type_, flow, activation_condition):
        self.pipe_type = type_
        self._flow = flow
        self._activation_condition = activation_condition

    @classmethod
    def pipe1(cls):
        """A pipe of type 1 adding 100l when the volume is below 700"""
        return Pipe(1, 100, lambda volume: volume < 700)

    @classmethod
    def pipe2(cls):
        """A pipe of type 2 adding 80l when the volume is below 700"""
        return Pipe(2, 80, lambda volume: volume < 700)

    @classmethod
    def pipe3(cls):
        """A pipe of type 3 draining 110l when the volume is above 600"""
        return Pipe(3, -110, lambda volume: volume > 600)

    def behaviour(self):
        while True:
            with Pipe._lock:
                if self._activation_condition(Pipe.water_volume):
                    Pipe.water_volume += self._flow
                    action = "added" if self._flow > 0 else "drained"
                    print(f"Pipe {self.pipe_type+1} {action} {abs(self._flow)} liters")
                    print(f"Volume is: {Pipe.water_volume}")
            time.sleep(1.5)

A lot has changed here, so lets go through step-by-step.

I changed the naming to follow the PEP8 recommendation linked above.
The lock is now shared between all instances of the class (and their associated threads) the same way the water volume is.
The code duplication is gone.

To reach that goal, the Pipe class was made more general in that it now accepts a flow rate (positive or negative) and a condition under which this pipe is active. To have to same "preconfigured" pipes as before, I added three classmethods which act as a simple factory. Each factory method also got a short description what to expect from the returned pipe.
behaviour (in lack of a better idea for a name) is now quite a bit shorter since it's basically the same code for all instances which add live to add via their properties. The lock is now also acquired before checking on the current volume. From a coding standpoint that absolutely makes sense, it's however up for discussion if a real system could/would be setup with this level of synchronization. The console output is now also generically built from what is known about the pipe object currently in action. I used f-strings (available from Python 3.6+) to format the output contrary to the "old-style" % formatting. Maybe have a look at this blog post to learn more about the different kinds of string formatting available to Python programmers.

Make it stop

I don't know if you have ever heard of the poem "Der Zauberlehrling" (The Sorcerer's Apprentice) by the German writer Johann Wolfgang von Goethe. In the poem, a young sorcerer summons a broom to help him carry water in order to clean his masters house. When he is done with the task, he tries to stop the broom from carrying more and more water into the house but fails to do so. From there on the events quickly spiral out of control until his master returns and finally takes control over the broom again.

Sounds vaguely familiar? Well, that's because once one has started your program, not even Ctrl+C will stop it (at least under Windows). This answer on SO could be used as a starting point to make that work.

A few closing words on threading in Python, especially in the most widely used reference implementation CPython. CPython has something called the global interpreter lock or GIL for short. The GIL takes care that at any time there is only a single thread executing Python code. That has significant impact on what you can expect to see when changing your pure Python code to multi-threading. Positive gains are mainly found when you have blocking IO operations or something like calls to a web API where the underlying code actively releases the GIL in order to allow other threads to do their work. Apart from that, the Python interpreter will also stop threads from time to time in order to allow over threads to run. When this happens depends on which Python version you are using (Python 2 vs. Python 3).

Since this is a topic that can quite involved, let me give you another link to a blog post on realpython and to a question/answer on Stack Overflow to read more about that topic.


Drawing stars around the center of the screen I have an homework as follows:

  Write a program where you enter from the keyboard a number n. Then n
  stars are drawn onto the screen, circling around the center of your
  screen. You can assume that 1 < n < 20.

The solution I came up with is below.

import turtle

window=turtle.Screen()
window.screensize(1200,1200)

draw=turtle.Turtle()
draw.pensize(2)

draw.speed('fastest')

angle = [36,144,144,144,144]

GetNumber = int(input("Enter a number: "))

while GetNumber <= 1 or GetNumber >= 20:
    print("Enter a number between from 2 to 19")
    GetNumber = int(input("Enter a number: "))

for shape in range(GetNumber):
    for i in angle:
        draw.left(i)
        draw.forward(180)
    draw.penup()
    draw.forward(60)
    draw.pendown()
    draw.left(170)

Is there any way to improve this code? I think my code is OK, but I believe there are perhaps more concise or better solutions (working faster perhaps?)

Perhaps angle (currently 170) should be changed or perhaps an dynamic angle changer code can be implemented for the sake of improving the solutions. What do you think?

It would be user friendly and make testing easier if the program accepted the input number as a command line argument too, for example:

import sys

def main():
    if sys.argv[1:]:
        num = int(sys.argv[1])
        if not is_valid_number(num):
            warn_invalid_number()
            sys.exit(1)
    else:
        num = prompt_valid_number()

    draw_stars_in_a_spiral(num)

if __name__ == '__main__':
    main()

While doing this, to avoid code duplication, I split the original functionality to multiple functions, which is a good practice in general anyway.

For validating the number, the N < var < M operator of Python is very handy:

def is_valid_number(num):
    return 1 < num < 20

Then the warning and prompting functions:

def warn_invalid_number():
    print("Number must between 2 to 19 (inclusive)")

def prompt_number():
    return int(input("Enter a number: "))

When prompting the user until he gives a valid number, change to a while True loop to avoid the duplication when asking for a number:

def prompt_valid_number():
    while True:
        num = prompt_number()
        if is_valid_number(num):
            return num
        warn_invalid_number()

Finally, the main method to draw the stars:

def draw_stars_in_a_spiral(num):
    window = turtle.Screen()
    window.screensize(1200, 1200)

    draw = turtle.Turtle()
    draw.pensize(2)

    draw.speed('fastest')

    for _ in range(num):
        for angle in angles:
            draw.left(angle)
            draw.forward(180)
        draw.penup()
        draw.forward(60)
        draw.pendown()
        draw.left(170)

Some points to note:

I moved all the drawing operations here. If validation fails when invoking with command line parameters, there's no need to open a window and draw anything.
I replaced the unused shape variable in for shape in range(num) with _
I replaced the angle array with angles to be more intuitive, and the loop variable i with the more intuitive name angle


Find two numbers in a sorted array whose sum is closest to a given number Given a sorted array and a number x, find a pair in array whose sum is closest to x.

Here's my solution:

import Data.List (minimumBy)
import Data.Ord (comparing)

closestPairSum :: (Num a, Ord a) => [a] -> a -> (a, a)
closestPairSum xs n = snd $ minimumBy (comparing fst) pairs
  where
    pairs = sums xs (reverse xs)
    sums xxs@(x:xs) yys@(y:ys) = let sum = x + y in
      if n < sum
      then (sum - n, (x, y)) : sums xxs ys
      else (n - sum, (x, y)) : sums xs yys
    sums _ _ = []

I'm mostly self-taught, so I would love to know in which ways my code is not according to common practice. More specifically:

How can I more clearly align my let-in and if-then-else expressions?
I use an intermediate list of (a, (a, a)) tuples, which I'm not thrilled about. Is there a more elegant way to do this, without sacrificing performance?
I'm on the fence about sum xs (reverse xs) vs sum xs $ reverse xs. Is this just considered nitpicking, or do people have a strong opinion about this?
Would it make more sense to swap the parameters of this function? I have the feeling that a would usually precede [a] with pointfree style in mind, but I'm not sure about this one.
Anything else that I might not have thought of :)

The "intermediate array" takes no extra space due to lazy evaluation. minimumBy . comparing is minimumOn. Yes, swapping the parameters makes sense. Inline then and else once more. I wouldn't worry so much about constant factors - have you compiled it with -O2 and tested (eg with criterion)?

import Data.List (minimumOn)

closestPairSum :: (Num a, Ord a) => a -> [a] -> (a, a)
closestPairSum n xs = minimumOn (abs . (n-) . uncurry (+)) pairs where
  pairs = sums xs (reverse xs)
  sums xxs@(x:xs) yys@(y:ys) = (x, y) : if n < x + y
    then sums xxs ys
    else sums xs yys
  sums _ _ = []


Recursive Level Order traversal I have come up with this code to do level order traversal recursively for a binary search tree.

private void printLevelOrder_Recursive(Node root2) {
    Queue<Node> queue = new LinkedList<Node>();
    queue.add(root2);

    Stack<Node> stack = new Stack<Node>();
    stack.add(root2);
    if(stack.isEmpty() == false) {

        Node node = stack.pop();
        System.out.println(node.data);

        stack.addAll(printLevelOrder_Recursive_Children(node.left, node.right));
    }

    while(stack.isEmpty() == false) {
        Node node = stack.pop();
        System.out.println(node.data);
    }
}

private Stack<Node> printLevelOrder_Recursive_Children(Node left, Node right) {
    Stack<Node> stack = new Stack<Node>();
    if (left == null && right == null) {
        return stack;
    }

    Stack<Node> leftChildren = new Stack<Node>();
    Stack<Node> rightChildren = new Stack<Node>();
    if (left != null && (left.left != null || left.right != null)) {
        leftChildren = printLevelOrder_Recursive_Children(left.left, left.right);
    }
    if (right != null && (right.left != null || right.right != null)) {
        rightChildren = printLevelOrder_Recursive_Children(right.left, right.right);
    }

    if (rightChildren.isEmpty() == false) {
        stack.addAll(rightChildren);
    }
    if (leftChildren.isEmpty() == false) {
        stack.addAll(leftChildren);
    }
    if (right != null) {
        stack.add(right);
    }
    if (left != null) {
        stack.add(left);
    }
    return stack;
}

where Node is:

public class Node {
    public int data;
    public Node left;
    public Node right;
}

I looked at various implementations, but I find mine much easier. Could you please chime in with your review and thoughts?

A few things that catch my eye, just skimming over your code:

Unconventional method names:

Java method names are supposed to be named strictly in camelCase. You're using some bastardized camel-snake-pascal-case which is something I'd expect when mixing together PHP, Python and C++... ~shivers
Use of outdated API:

Stack is a class in Java retroactively added to the Collections API, but it should've been officially deprecated since Java 1.5. It's been officially replaced by Deque, which is an interface that supports all stack-operations, see also Stack's javadoc:

  A more complete and consistent set of LIFO stack operations is
  provided by the Deque interface and its implementations

Returning a Collection from a method which begins with print

A method name should not lie. For a method named print... I'd expect to give something to be printed and some output channel (e.g. BufferedOutputWriter) with no return value. Instead you should name it something like traverseLevelOrder.
Overcomplicated helper method:

For a recursive traversal it's definitely not necessary to have a method that takes two nodes. Instead of relying on the caller to maintain the Stack you have, you may want to give the Stack (or better the Deque) into the recursive traversal for a level-order traversal. Also it's a significant difference whether you only want to traverse the nodes or also keep the traversal results in memory...
General recursive approach:

It might be a better option to actually do the level-order traversal with an "iterative" approach, by maintaining the nodes you still have to traverse in a queue that builds up by level and just process them back again. Following code should give you an idea:

void traverse(Node node) {
    Queue<Node> traversalQueue = new LinkedList<>();
    traversalQueue.add(node);
    while (!traversalQueue.isEmpty()) {
        // add nodes to queue by level
        // and pop the current:
        traversalQueue.add(node.left); // null-checks omitted
        traversalQueue.add(node.right);
        // use node.data
    }
}