Decimal To Binary using only the basics Convert a decimal number to binary using only loops, strings, if/else statements.

This is my first C++ project and it took me a while to do it. Let me know what you think.

#include <iostream>
#include <string>

using namespace std;

string ConvertInt(int value);
string ConvertDec(double value);
void printBinary1(string str1);
void printBinary2(string str2);

int main()
{
    double number, decValue;
    int remainder, intValue;
    string binary1, binary2;

    cout << "Enter a deciaml value to be converted to binary. Then press ENTER: ";
    cin >> number;

    intValue = (int)number;
    decValue = number - intValue;

    binary1 = ConvertInt(intValue);

    binary2 = ConvertDec(decValue);

    printBinary1(binary1);

    cout << ".";

    printBinary2(binary2);

    int pause;
    cin >> pause;

    return 0;
}

string ConvertInt(int value)
{
    int remainder;
    string binary;

    while(value != 0)
    {
        remainder = value % 2;
        binary += remainder;
        value /= 2;
    }

    return binary;

}

string ConvertDec(double value)
{
    string binary;
    int integer;
    double decimal;

    for(int i = 1; i <= 5; i++)
    {
        integer = (int)value;
        binary += integer;
        decimal = value - integer;
        value = decimal * 2;
    }

    return binary;
}

void printBinary1(string str1)
{
    string binary1 = str1;

    for(int i = (binary1.length() - 1); i <= (binary1.length() - 1); i--)
    {
        cout <<  (int)binary1[i];
    }
}

void printBinary2(string str2)
{
    string binary2 = str2;

    for(int index = 1; index < binary2.length(); index++)
    {
        cout << (int)binary2[index];
    }
}

It is considered bad practice (by most people) to declare more than one variable on a line.

double number, decValue;
int remainder, intValue;
string binary1, binary2;

Though it is not a problem here there are some corner cases where the declaration gets confusing.

Second you declare all your variables at the top of the function.
This is a very C like style and in C++ it is usually better to declare the variable as close to the point of usage as possible (no need to invoke a constructor if you are never going to use the variable, also it makes reading the code easier as you can see the type at the fist point of usage).

I know all the books say to use:

using namespace std;

But in the long run its not a good idea. Get used to prefix things with their namespace std::string, or importing the specific bits you want using std::string (even then scope as tightly as possible (you would not believe how many time string has been reinvented)).

Variables should be descriptive (and more importantly unique and easy to find in the source).

for(int i = 1; i <= 5; i++)

Think about 10 years of maintenance on this code. The code now covers 5 long pages. Try and find and update all usages of i. This becomes a pane as i will appear in nearly every other identifier in all the comments in lots of places etc. So try and use variables that can be easily searched for.

In your ConvertXXX() functions. You are saving integer values into the string. This is fine. But you are then extracting those char values and converting to int for printing. Why not convert the values directly into the characters you want to print.

string ConvertDec(double value)
{
    // STUFF
         binary += '0' + integer; // each digit will be the character 0 or 1
    // MORE STUFF
}

When printing:

cout <<  (int)binary1[i];

// Can change this too:

cout <<  binary1[i];

More importantly you can replace the loop:

for(int index = 1; index < binary2.length(); index++)
{
    cout << (int)binary2[index];
}

// Can be changed too:

cout << binary2

Looking at your loops:

for(int i = (binary1.length() - 1); i <= (binary1.length() - 1); i--)

Does this even work?
It seems that i is getting smaller and smaller then goes negative.

It is easier to loop up and subtract in the range:

for(int loop = 0; loop < bianry1.length(); ++lop)
{
    std::cout << bianry1[binary1.length() - 1 - loop];
}

But the std containers (including string) provide iterators (and more importantly reverse iterators)

for(std::string::const_reverse_iterator loop = bianry1.rbegin(); loop != binary1.rend(); ++loop)
{
    std::cout << *loop;
}

Second loop:

for(int index = 1; index < binary2.length(); index++)

Problem here is that you are starting from 1. arrays/containers nearly everything is indexed from 0 and you can access the elements 0 -> (size() - 1). So here you are starting at the wrong point and going past the end of the string.

But as mentioned above you don't even need this as you can replace the loop.


TableCellEditor for multiple columns I am wanting to use a cell editor and a DocumentFilter to prevent the user from entering certain characters into the table cells.

The columns are as follows:

Product ID
Product Description
Inventory Count
Cost
Minimum Quantity

For Inventory Count, Cost and Minimum Quantity I am going to use a JFormattedTextField.

How can this be improved?

public class ProductTableCellEditor extends AbstractCellEditor implements TableCellEditor
{

    private TableCellEditor editor;

    @Override
    public Object getCellEditorValue()
    {
    if (editor != null) {
        return editor.getCellEditorValue();
    }
    return null;
    }

    @Override
    public Component getTableCellEditorComponent(JTable table, Object value, boolean isSelected, int row, int column)
    {
    JTextField textField = new JTextField();
    textField.setFont(ApplicationStyles.TABLE_FONT);

    switch (column)
    {
    case 0:
        ((AbstractDocument) textField.getDocument()).setDocumentFilter(new ProductTableDocumentFilter());
        editor = new DefaultCellEditor(textField);
        break;
    case 1:
        ((AbstractDocument) textField.getDocument()).setDocumentFilter(new ProductTableDocumentFilter());
        editor = new DefaultCellEditor(textField);
        break;
    case 2:
        throw new UnsupportedOperationException("Not yet supported....");
    case 3:
        throw new UnsupportedOperationException("Not yet supported....");
    case 4:
        throw new UnsupportedOperationException("Not yet supported....");
    default:
        throw new UnsupportedOperationException("Not yet supported....");
    }
    return editor.getTableCellEditorComponent(table, value, isSelected, row, column);
    }
    }

(AbstractDocument) textField.getDocument()

This is bad practice.

I recommend that you do one of these options:

The simple way would be to create a method toAbstractDocument(Document document)
The better way would be to create a field at TextField class such as ? extends Document document.


PIN class with SOLID principles I wrote this Pin class as a part of a project to learn SOLID. What do you think about it and what can be made better?

Interface

   public interface IAuthorization
    {
        bool Verify(object obj);
        void Change(object obj);
    }

Class

public class Pin : IAuthorization
    {
        private int _pin;

        public Pin(object pin)
        {
            _pin = ValidPin(pin);
        }

        public void Change (object pin)
        {
            _pin = ValidPin(pin);
        }

        public bool Verify (object pin)
        {
            return (_pin == ValidPin(pin));
        }

        private int ValidPin(object pin)
        {
            if (pin == null)
            {
                throw new ArgumentNullException("pin", "Pin must be exactly 4 digits long integer");
            }

            if (pin.ToString().Length != 4)
            {
                throw new ArgumentException("Pin must be exactly 4 digits long integer", "pin");
            }

            return Convert.ToInt32(pin);
        }
    }

The D says "depend on abstractions, not concrete types" - that doesn't mean to box your value types into object!

In fact, this:

private int _pin;

Contradicts this:

if (pin.ToString().Length != 4)
{
    throw new ArgumentException("Pin must be exactly 4 digits long integer", "pin");
}

If your pin is meant to work like a string, then it should be stored as a string. If my pin is 0072, I have a 4-digit pin, but you're storing it as 72 and throwing an ArgumentException at me.

Exposing object in your public API is horrible practice - nobody knows what you're expecting to receive! Worse, there's no XML documentation on your public members, so the only way to find out, is to dig into your code.

Come to think about it, with respect to the I of SOLID, I think your interface should look like this:

public interface IPinAuthorization
{
    bool Validate(SecureString pin);
}

And a PIN shouldn't be that easy to modify. Consider making the type immutable, and asking for the current PIN before accepting a new one:

public interface IModifiablePin
{
    IPinAuthorization Modify(SecureString current, SecureString @new);
}

You could have a more secure PinAuthorization implementation like this:

public class PinAuthorization : IPinAuthorization, IModifiablePin, IDisposable
{
    private readonly SecureString _pin;

    public PinAuthorization(SecureString pin)
    {
        ValidateInternal(pin); // see comments below
        _pin = pin;
    }

    private void ValidateInternal(SecureString pin)
    {
        if (pin.Length != 4)
        {
            throw new ArgumentException("PIN must be 4 digits.", pin);
        }

        // further validation logic would break the security of the string.
        // this is an indication that it's not this type's responsibility
        // to know how to validate itself.
    }

    public bool Validate(SecureString pin)
    {
        return pin == _pin;
    }

    public IPinAuthorization Change(SecureString current, SecureString @new)
    {
        if (_pin != current)
        {
            throw new ArgumentException("Invalid PIN.", current);
        }

        return new PinAuthorization(@new);
    }

    public void Dispose()
    {
        _pin.Dispose();
    }
}

Now, if you want to implement some FingerprintAuthorization, you can make it implement IPinAuthorization and validate against a SecureString-encoded finger print (however you want to do that), and the type will not be implementing IModifiablePin, since you don't change your finger prints. Or do you? Well, at least you have the flexibility of doing either. At a glance though, it seems the class has so little responsibility, that it possibly wouldn't even need to be reimplemented or adapted in any way.


Longest sequence of same subsequent number in a square matrix Given an N-by-N grid with each cell either occupied by an 'X', an 'O', or empty, write a program to find the longest sequence of consecutive 'X's either horizontal, vertically, or diagonally. To test your program, you can create a random grid where each cell contains an 'X' or 'O' with probability 1/3.

I would like a review of my answer. I'm not sure it's efficient enough, or even efficient at all. I chose to use numbers for now since they easier to work with. I hope it's all clear.

class Main {
    public static void main(String[] args){
        int n = 4;
        int[][] grid = new int[n][n];
        int max =  Integer.MIN_VALUE;

        for(int i = 0; i < n; i++){
            for(int j = 0; j < n; j++){

                // three nums are produced. there is 1/3 chance for each.
                int r = (int)(Math.random()*3);

                // both 'X' and 'O' have 1/3 chance to be produced.
                if(r == 1) grid[i][j] = 1;
                if(r == 2) grid[i][j] = 2;

                System.out.print(grid[i][j]+" ");
            }
            System.out.println();
        }

        // horizontal, vertical, backward diag, forward diag
        int hori  = 0, vert = 0, bdia = 0, fdia = 0;

        // assuming 1 represents 'X' and 2 represents 'O'
        int num   = (int)(1 + Math.random()*2);

        for(int i = 0; i < n; i++){
            hori = 0; vert = 0;
            // horizontal instances
            for(int j = 0; j < n; j++){
                if(grid[i][j] != num)
                    hori = 0;
                else hori ++;
                max = hori > max? hori: max;
            }

            // vertical  instances
            for(int j = 0; j < n; j++){
                if(grid[j][i] != num)
                    vert = 0;
                else vert ++;
                max = vert > max? vert: max;
            }

            // forward diag instances (fdia)
            for(int j = 0; j < n && i == 0; j++){
                if(grid[j][j] != num)
                    fdia = 0;
                else fdia ++;
                max = fdia > max? fdia: max;
            }

            // backward diag instances (bdia)
            for(int j = 0; j < n && i == 0; j++){
                if(grid[j][n-1-j] != num)
                    bdia = 0;
                else bdia ++;
                max = bdia > max? bdia: max;
            }
        }

        System.out.println("max : "+max);
        System.out.println("num : "+num);
    }
}

Let me start out by saying that your solution certainly is efficient enough. You only loop over the grid once to do the 4 checks. And don't check the same sequence twice. (like going left to right AND right to left).

Since this is codereview.stackexchange let's instead start by reviewing the code :)

First of all, there is a mistake in your solution. Diagonally is not the same as on the main diagonals. The simplest example that you won't find in your solution but what I believe does count as a normal diagonal sequence is the following:

  O X O
  O O X
  O O O

This looks like a sequence of lenght 2 to me.

I dissagree with you when you say:

  I chose to use numbers for now since they easier to work with.

We can just as well work with char:

char[][] grid = new char[n][n];
...
if(r == 1) grid[i][j] = 'X';
if(r == 2) grid[i][j] = 'O';
...
if(grid[i][j] == 'X')

Doing this also makes it explicit that you're only checking for the X without having to arbitrarily choose if the 'X' is represented by a 1 or a 2 in your grid.

You have put all the code inside the main method. Although it kinda works for this small problem, it's generally a better idea to split your code up into more manageable blocks.

A few easy ones to separate are a grid generator, and a method to print a grid. By splitting this up into 2 functions we can also provide a fixed grid and still print that grid as well.

Printing is easy. We take the same loops as you already have with the same System.out... calls. But without the random setting of the cell.

public static void printGrid(char[][] grid){
    for(int row = 0; row < grid.length; row++){
        for(int col = 0; col < grid[0].length; col++){
            System.out.print(grid[row][col]+" ");
        }
        System.out.println();
    }
}

Notice how I named the loop variables row and col to make it clear what we're looping over.

Randomly generating a grid isn't much harder. We use the same loops, but now we randomly choose one of 3 characters. I'm using _ to represent "emtpy".

public static char[][] generateRandomGrid(int size){
    char[][] grid = new char[size][size];
    for(int row = 0; row < grid.length; row++){
        for(int col = 0; col < grid[0].length; col++){
            // three nums are produced. there is 1/3 chance for each.
            int r = (int)(Math.random()*3);

            // both 'X' and 'O' have 1/3 chance to be produced.
            if(r == 0) {
                grid[row][col] = '_';
            }
            if(r == 1) {
                grid[row][col] = 'X';
            }
            if(r == 2) {
                grid[row][col] = 'O';
            }
        }
    }
    return grid;
}

Now for the actual checking of the sequences. This part of your code confused me. You have the outer loop with index i. Then depending on what you check the meaning of this i changes to the row/the column/something you ignore entirely.

Instead let's provide 4 specific methods, that each calculate the maximum sequence in a specific direction for a given grid.

The horizontal one is easy. It's basically what you already did:

public static int findMaxHorizontalSequence(char[][] grid){
    int max = 0;
    for(int row = 0; row < grid.length; row ++) {
        int currentSeq = 0;
        for(int col = 0; col < grid[0].length; col ++){
            if(grid[row][col] == 'X'){
                currentSeq ++;
                if(currentSeq > max){
                    max = currentSeq;
                }
            } else {
                currentSeq = 0;
            }
        }
    }
    return max;
}

The vertical check is almost the same, except the for loops are swapped.

public static int findMaxVerticalSequence(char[][] grid){
    int max = 0;
    for(int col = 0; col < grid[0].length; col++){
        int currentSeq = 0;
        for(int row = 0; row < grid.length; row ++){
            if(grid[row][col] == 'X'){
                currentSeq ++;
                if(currentSeq > max){
                    max = currentSeq;
                }
            } else {
                currentSeq = 0;
            }
        }
    }
    return max;
}

You might have noticed by now that I always use grid.length and grid[0].length. This is so that the functions will all work correctly even if the grid was a rectangular shape instead of a square.

For the diagonals we'll have to do a bit more work. The easiest is probably to split it up in 2 loops. One for each of the following lines of starting locations:

X X X ?        O O O O
O O O O        X O O O
O O O O        X O O O
O O O O        ? O O O

We can ignore the spots with a ?. Those diagonals are only 1 char long, so it will already be counted by the horizontal/vertical checks. No need to repeat this.

The code could look like this:

public static int findMaxDownRightSequence(char[][] grid) {
    int max = 0;
    for (int startRow = 0; startRow < grid.length - 1; startRow++) {
        int currentSeq = 0;
        for (int row = startRow, col = 0; row < grid.length && col < grid[0].length; row++, col++) {
            if (grid[row][col] == 'X') {
                currentSeq++;
                if (currentSeq > max) {
                    max = currentSeq;
                }
            } else {
                currentSeq = 0;
            }
        }
    }
    for (int startCol = 1; startCol < grid[0].length - 1; startCol++) {
        int currentSeq = 0;
        for (int row = 0, col = startCol; row < grid.length && col < grid[0].length; row++, col++) {
            if (grid[row][col] == 'X') {
                currentSeq++;
                if (currentSeq > max) {
                    max = currentSeq;
                }
            } else {
                currentSeq = 0;
            }
        }
    }
    return max;
}

But at this point I really start noticing that I'm copy-pasting a lot. This usually means that we should refactor some common code. The common part of the code that we're copy-pasting is the part inside the inner for loop:

if (grid[row][col] == 'X') {
    currentSeq++;
    if (currentSeq > max) {
        max = currentSeq;
    }
} else {
    currentSeq = 0;
}

We have to find how to make the for loop work for all cases.
This means: for( find initial values for row and col ; check if they are still within the bounds of the grid ; and find how they change each step. )

Let's write a method that gets all those parameters and figure out how to map the existing ones to this method. And let's also write a specific method to check if the row and col are within the bounds of the grid, because otherwise the for statement will become unreadable.

public static int maxSequence(char[][] grid, int startRow, int startCol,
                              int verticalDir, int horizontalDir){
    int currentSeq = 0;
    int max = 0;
    for(int row = startRow, col = startCol;
                checkBounds(grid, row, col);
                row += verticalDir, col += horizontalDir) {
        if (grid[row][col] == 'X') {
            currentSeq++;
            if (currentSeq > max) {
                max = currentSeq;
            }
        } else {
            currentSeq = 0;
        }
    }
    return max;
}

private static boolean checkBounds(char[][] grid, int row, int col) {
    return row > 0 && col > 0 && row < grid.length && col < grid[0].length;
}

Now we can rewrite our other methods. The outer for loops stay the same, but we replace the inner loops with a call to our new method:

public static int findMaxHorizontalSequence(char[][] grid) {
    int max = 0;
    for (int row = 0; row < grid.length; row++) {
        int rowMax = maxSequence(grid, row, 0, 0, 1);
        if(rowMax > max){
            max = rowMax;
        }
    }
    return max;
}

public static int findMaxVerticalSequence(char[][] grid) {
    int max = 0;
    for (int col = 0; col < grid[0].length; col++) {
        int colMax = maxSequence(grid, 0, col, 1, 0);
        if(colMax > max){
            max = colMax;
        }
    }
    return max;
}

public static int findMaxDownRightSequence(char[][] grid) {
    int max = 0;
    for (int startRow = 0; startRow < grid.length - 1; startRow++) {
        int diagMax = maxSequence(grid, startRow, 0, 1, 1);
        if(diagMax > max){
            max = diagMax;
        }
    }
    for (int startCol = 1; startCol < grid[0].length - 1; startCol++) {
        int currentSeq = maxSequence(grid, 0, startCol, 1, 1);
        if (currentSeq > max) {
            max = currentSeq;
        }
    }
    return max;
}

All that's left is to write a method for the other diagonal.

    public static int findMaxDownLeftSequence(char[][] grid) {
    int max = 0;
    for (int startRow = 0; startRow < grid.length-1; startRow++) {
        int diagMax = maxSequence(grid, startRow, grid.length, 1, -1);
        if(diagMax > max){
            max = diagMax;
        }
    }
    for (int startCol = 1; startCol < grid[0].length; startCol++) {
        int currentSeq = maxSequence(grid, ?, ?, ?, ?);
        if (currentSeq > max) {
            max = currentSeq;
        }
    }
    return max;
}

Since this site requires working code in the first place to review I didn't want to give you the full solution for the diagonals. So try to fill in the four ? in the last part to see if you fully understood the solution :)

To run the new code (and find out if you filled in the last part correctly) you can use the following main method:

public static void main(String[] args) throws Exception {
    char[][] grid = new char[][]{{'O', 'X', 'O'},
            {'X', 'O', 'O'},
            {'O', 'O', 'O'}};
    printGrid(grid);
    System.out.println(findMaxSequence(grid));
}

or you can replace the grid with

char[][] grid = generateRandomGrid(100);

To test it out on a random grid.

Fun fact: on my PC it takes between 3.5 and 4 seconds to find the solution of a randomly generated 10000 x 10000 grid. About the same time it takes to randomly generate the grid.

I doubt it can get much faster on a single thread solution. Since you're stuck with looping over the entire grid anyway.


Mapping key value pairs between two JavaScript objects I have two JavaScript objects:

const widgetsObj = {
    inputWidget: "MyInputWidget",
    checkboxWidget: "MyCheckboxWidget",
    geoWidget: "MyGeoWidget"
};

 this.components = {
            "MyInputWidget": MyInputWidget,
            "MyCheckboxWidget": MyCheckboxWidget,
            "MyGeoWidget": MyGeoWidget
        };

The end goal is to map all keys from widgetsObj to all values from this.components. The end object should look like this.

let endObj = {
    inputWidget: MyInputWidget,
    checkboxWidget: MyCheckboxWidget,
    geoWidget: MyGeoWidget
}

Right now, I am doing this like so

let returnObj = {};
        for(const key of Object.keys(widgets)) {
            if(this.components.hasOwnProperty(widgets[key])) {
                returnObj[key] = this.components[widgets[key]];
            }
        }
        return returnObj;

where widgets represents widgetsObj.

Is there a better way to do this rather than look for each value by mapping through the whole object repeatedly?

It might be a little cleaner to use .reduce instead of for...of... loop:

Object.keys(widgetsObj).reduce( (obj, key) => {
    obj[key] = this.components[widgetsObj[key]];
    return obj;
}, {});


Operating on a list of files using recursion I'm starting just now with CoffeeScript and I found out that I can't solve problems like looping and recursives with just one line.  I would like to improve the code that I just wrote using built-in CoffeeScript helpers.

'use strict'
file_system = require 'fs'
Types       = require './types'

Extract =
  each_file: (index = 0) ->
    length  = @files.length
    file    = @files[index]
    if index < length
      Types.read file, @next.bind @
      @each_file index + 1

  next: (file) ->
    @result.push file
    if @result.length == @files.length
      return @cb.clean.call @cb, @result
    @each_file()
    return

  init: (files, cb) ->
    @files              = files
    @cb                     = cb
    @result             = []
    @each_file()
    return

module.exports = Extract

The script speaks for itself; I'm doing a recursive function to send files to Types.read, and I store the result in the result array.

Fake for-loops and comprehensions
At the beginning of your post, you say that you can't always achieve everything with looping in one line in CoffeeScript. When you say that, I assume you are taking about the each_file:

  each_file: (index = 0) ->
    length  = @files.length
    file    = @files[index]
    if index < length
      Types.read file, @next.bind @
      @each_file index + 1

This is kinda ugly right now because you seem to be using a method to fake a for-loop. Luckily, with CoffeeScript's comprehensions, we can turn this into a simple 1-line expression.
First, we need to be iterating through all of the @files. That can be written simply like this:
Types.read(file, @next.bind this) for file in @files

This is a comprehension that will go through all of the @files and substitute the file in the Types.read call with the current file it's looping over.
Now that we have this, you can remove that

file = @files[index]

line, along with that single index parameter. Why? Because, now that we have this loop, we no longer have any of that method-recursion-fake-for-loop-idness.
Along with those, since this is now a comprehensions that loops through all the values in an array, we don't need to do any checking to make sure the index is less than the length.

if index < length

Can be removed. Guess what your method looks like now?
  each_file: () ->
    Types.read(file, @next.bind @) for file in @files

The above method will do exactly what it was doing before: it will go through all of the files in @files and pass it into Types.read along with @next.bind @.
What's different now?

It's much shorter and much simpler than what you were doing before. As I already stated, you seemed to be reinventing the for loop with that recursion you were doing.

It's more idiomatic. CoffeeScript has those comprehensions so you can simplify long tasks into a few small and readable lines.

Misc.

  init: (files, cb) ->
    @files              = files
    @cb                     = cb

This can be shortened to this:
  init: (@files, @cb) ->

The CoffeeScript compiler treats parameters with a @ before them as a name of a property to set to the parameter. For example, @files will become:
this.files = files

in the method body.


A filled_multi_array Template Function for Boost.MultiArray in C++ This is a follow-up question for A ones Function for Boost.MultiArray in C++. Thanks to G. Sliepen's answer, the mentioned method which can set various initial value is better in generic purpose usage. However, the previous function specified the input type which must be boost::detail::multi_array::extent_gen<NumDims>. The type issue happened in some cases, an example is as below. The object filled_multi_array1 is created first with filled_multi_array(boost::extents[3][4][2], 1.0). This works well. I want to create a filled_multi_array2 object which size is as same as filled_multi_array1 and the initial value is 2.0. This doesn't work in existed version of filled_multi_array function.
auto filled_multi_array1 = filled_multi_array(boost::extents[3][4][2], 1.0);
auto filled_multi_array2 = filled_multi_array<double, filled_multi_array1.dimensionality>(filled_multi_array1.shape(), 2.0);

for (decltype(filled_multi_array2)::index i = 0; i != 3; ++i)
    for (decltype(filled_multi_array2)::index j = 0; j != 4; ++j)
        for (decltype(filled_multi_array2)::index k = 0; k != 2; ++k)
            std::cout << filled_multi_array2[i][j][k] << std::endl;

Then, I found that the type returned from .shape() function is this one const boost::detail::multi_array::multi_array_base::size_type*. It seems that the another function overloading is needed to handle the above filled_multi_array2 case. The experimental code of the filled_multi_array function with the input type const boost::detail::multi_array::multi_array_base::size_type* is as follows.
template<class T, std::size_t NumDims>
auto filled_multi_array(const boost::detail::multi_array::multi_array_base::size_type* size, const T& value)
{
    boost::multi_array<T, NumDims> output(reinterpret_cast<boost::array<size_t, NumDims> const&>(*size));
    std::fill_n(output.data(), output.num_elements(), value);
    return output;
}

With this overloaded function, filled_multi_array2 object can be generated successfully and the output is like:
2
2
2
2
2
2
2
2
2
2
2
2
2
2
2
2
2
2
2
2
2
2
2
2

The full implementation of filled_multi_array template function:
template<class T, std::size_t NumDims>
auto filled_multi_array(boost::detail::multi_array::extent_gen<NumDims> size, const T& value)
{
    boost::multi_array<T, NumDims> output(size);
    std::fill_n(output.data(), output.num_elements(), value);
    return output;
}

template<class T, std::size_t NumDims>
auto filled_multi_array(const boost::detail::multi_array::multi_array_base::size_type* size, const T& value)
{
    boost::multi_array<T, NumDims> output(reinterpret_cast<boost::array<size_t, NumDims> const&>(*size));
    std::fill_n(output.data(), output.num_elements(), value);
    return output;
}

All suggestions are welcome.
The summary information:

Which question it is a follow-up to?
A ones Function for Boost.MultiArray in C++

What changes has been made in the code since last question?
The another overloaded filled_multi_array function has been added here.

Why a new review is being asked for?
I am trying to handle both cases, including the one with the specified shape directly and the other one with .shape() function (the shape is referenced from another Boost.MultiArray object). If there is any possible improvement, please let me know.

It looks bad, but that's not your fault. boost::multi_array just has a terrible interface. In any case, you want to avoid passing the shape twice, because this can give problems:
auto large_array = filled_multi_array(boost::extents[1][2][3][4][5], 1.0);
auto small_array = filled_multi_array(boost::extents[1], 1.0);
auto another_array = filled_multi_array<double, large_array.dimensionality>(small_array.shape(), 2.0);

The above will compile without errors, but might crash at runtime, since small_array.shape() is a pointer to an array that doesn't have as many elements as expected. I don't know why the interface is so terrible, if they would have .shape() return a (reference to a) boost::array<size_t, NumDims> it would all be checkable at compile time and without needing reinterpret_casts.
So to prevent errors, you want to avoid having to pass two separate pieces of information. One possibility is just passing the array whose shape you want to copy as an argument:
template<class T, std::size_t NumDims>
auto filled_multi_array(const boost::multi_array<T, NumDims> &input, const T& value) {
    boost::multi_array<T, NumDims> output(reinterpret_cast<boost::array<size_t, NumDims> const&>(input.shape()));
    std::fill_n(output.data(), output.num_elements(), value);
    return output;
}

But that interface will create confusion: does the following line modify the input?
filled_multi_array(filled_multi_array1, 2.0);

Another possibility is to keep just the first filled_multi_array() variant, but create a helper function to create a extent_gen from a multi_array:
template<std::size_t NumDims>
auto get_extents(const boost::multi_array &input) {
    boost::detail::multi_array::extent_gen<NumDims> output;

    // Set the shape of output according to input.shape(),
    // left as an exercise to the reader.
    ...

    return output;
}

And then you can use it like so:
auto filled_multi_array1 = filled_multi_array(boost::extents[3][4][2], 1.0);
auto filled_multi_array2 = filled_multi_array(get_extents(filled_multi_array1), 2.0);


Simple Dal for Accessing Procedures How could I improve this code?

public class DALUser
    {
        SqlConnection conn;
        SqlCommand cmd;

        public void ExecuteSp(string Spname, Dictionary<dynamic, dynamic> Params)
        {
            try
            {
                initialize();
                cmd = new SqlCommand();
                cmd.Connection = conn;
                cmd.CommandText = Spname;
                cmd.CommandType = CommandType.StoredProcedure;
                cmd = GetParam(Params, cmd);
                cmd.ExecuteNonQuery();
            }
            finally
            {
                DeIntialize();
            }
        }

        public  DataTable ExecuteAdapter(string Spname, Dictionary<dynamic, dynamic> Params)
        {
            try
            {
                initialize();
                cmd = new SqlCommand();
                cmd.Connection = conn;
                DataTable dt = new DataTable();
                cmd.CommandText = Spname;
                cmd.CommandType = CommandType.StoredProcedure;
                cmd = GetParam(Params, cmd);
                SqlDataAdapter sda = new SqlDataAdapter(cmd);
                sda.Fill(dt);
                return dt;
            }

            finally
            {
                DeIntialize();
            }
        }

        SqlCommand GetParam(Dictionary<dynamic, dynamic> Params, SqlCommand cmd)
        {
            foreach (var objparam in Params)
            {
                cmd.Parameters.AddWithValue(Const.sParamPrefix + objparam.Key, objparam.Value);
            }
            return cmd;
        }

        #region SqlConnection Side

        void initialize()
        {
            if (CheckConnection())
            {
                conn = new SqlConnection(Const.ConnectionString);
                conn.Open();
            }
        }

        bool CheckConnection()
        {
            if (conn == null)
                return true;
            return false;
        }

        void DeIntialize()
        {
            if (!CheckConnection())
            {
                conn.Close();
                conn.Dispose();
            }
        }

        #endregion

    }

Your initialize and DeInitialize do the job of connecting and disconnecting your SQL Connection,  but not the SQL Command that you create in each method.

I am sure that closing and disposing the connection will take care of the Command but I like to make sure.

so what I would do is to use a using statement

initialize();
using (SqlCommand cmd = new SqlCommand() {
    .Connection = conn,
    .CommandText = Spname,
    .CommandType = CommandType.StoredProcedure
})
{
    cmd = GetParam(Params, cmd);
    cmd.ExecuteNonQuery();
}

your try/catch works, but I would rather have my connection opened and closed inside each method and not have to call 2 other methods and a try/catch to make it work. I would much rather just do this

using (SqlConnection conn = new SqlConnection(Const.ConnectionString))
using (SqlCommand cmd = new SqlCommand() {
    .Connection = conn,
    .CommandText = Spname,
    .CommandType = CommandType.StoredProcedure
})
{
    cmd = GetParam(Params, cmd);
    cmd.ExecuteNonQuery();
}

and that gets rid of the try catch and all the ugly close/dispose calls.

It also gets rid of the need for a check connection method because when you use a using statement there is no open connection outside of the using block.

And OCD tells me to change this as well

public  DataTable ExecuteAdapter(string Spname, Dictionary<dynamic, dynamic> Params)
{
    try
    {
        initialize();
        cmd = new SqlCommand();
        cmd.Connection = conn;
        DataTable dt = new DataTable();
        cmd.CommandText = Spname;
        cmd.CommandType = CommandType.StoredProcedure;
        cmd = GetParam(Params, cmd);
        SqlDataAdapter sda = new SqlDataAdapter(cmd);
        sda.Fill(dt);
        return dt;
    }

    finally
    {
        DeIntialize();
    }
}

and make it look like this instead

public  DataTable ExecuteAdapter(string Spname, Dictionary<dynamic, dynamic> Params)
{
    using (SqlCommand cmd = New SqlCommand(){
        .Connection = conn,
        .CommandText = Spname,
        .CommandType = CommandType.StoredProcedure
    })
    {
        cmd = GetParam(Params,cmd);
        using (DataTable dt = new DataTable())
        using (SqlDataAdapter sda = new SqlDataAdapter(cmd))
        {
            sda.Fill(dt);
            return dt;
        }
    }
}

Naming

initialize();

should be capitalized for standards sake

Initialize();

Method names are PascalCase
variables are camelCase

I know this probably isn't the real SPROC name but Spname should be spName even though  Hungarian name scheme is no longer a recommended name scheme.


usart driver wrapper Could you review my usart driver wrapper please?

It contains the C language Xilinx usart driver, but I write C++ programs and need to C++ usart driver. How it was designed correctly and what I have to chage or add in the one on your view?

I will be thankful for your the strictest appraisal.

Thank you.

//**********************************************************************
// name:           usart.hpp
//
// test:           non.
//
// description:    a usart driver wrapper.
//
// author:         Artyom Shimko
//
// contact:        soikadigital@gmail.com
//
// created:        28.12.19
//
// release:        28.12.19
//
// changes:        non.
//
#ifndef USART_HPP
#define USART_HPP

#include "platform.h"                      // Platform depending settings.
#include "xuartps_hw.h"                    // The Xilinx header file for XUartPs device.

namespace UsartSapce {

    enum usartNumber { usart0 = XPAR_PS7_UART_0_BASEADDR,
                       usart1 = XPAR_PS7_UART_1_BASEADDR};
}

template <typename T>
class Usart : public Platform {

private:
   u32 usartNumber;

public:
   Usart(u32 usartNumber);                 // The set usart number method.

   status transmit(T *pData, u32 dataValue);
   status receive (T *pData, u32 dataValue);

   void reset(void);
};

//**********************************************************************
// The class constructor definition, it sets the usart number.
//**********************************************************************
// id:             A required usart number.
//
template <typename T>
Usart<T>::Usart (u32 usartNumber ) {

   this->usartNumber = usartNumber;
}

//**********************************************************************
// The usart reset method.
//**********************************************************************
// parameters:     non.
//
// return:         non.
//
template <typename T>
void Usart<T>::reset(void) {

   XUartPs_ResetHw(usartNumber);
 }

//**********************************************************************
// The data transmit usart method.
//**********************************************************************
// pData:          The pointer to transmited data.
//
// dataValue:      Value of transmited data.
//
// return:         ok aka 0 or error aka -1. The error occurs then pData
//                 is equal to nullptr or/and dataValue is equal to null.
//
template <typename T>
status Usart<T>::transmit(T *pData, u32 dataValue) {

   u8 *pByteData = nullptr;

   if(parameterCheck_(pData,dataValue) != StatusSpace::ok) {

       return StatusSpace::error;
   }

   pByteData = reinterpret_cast<u8*>(pData);

   for(u32 byteCounter = 0; byteCounter < dataValue; ++byteCounter) {

       XUartPs_SendByte(usartNumber, *(pByteData + byteCounter));
   }

    return StatusSpace::ok;
}

//**********************************************************************
// The data receive usart method.
//**********************************************************************
// pData:          The pointer to an array for received data.
//
// dataValue:      Value of received data.
//
// return:         ok aka 0 or error aka -1. The error occurs then pData
//                 is equal to nullptr or/and dataValue is equal to null.
//
template <typename T>
status Usart<T>::receive(T *pData, u32 dataValue) {

   u8 *pByteData = nullptr;

   if(parameterCheck_(pData,dataValue) != StatusSpace::ok) {

       return StatusSpace::error;
   }

   pByteData = reinterpret_cast<u8*>(pData);

   for(u32 byteCounter = 0; byteCounter < dataValue; ++byteCounter) {

       *(pByteData + byteCounter) = XUartPs_RecvByte(usartNumber);
   }

   return StatusSpace::ok;
}

#endif

Consider using enum class

Instead of having a regular enum inside its own namespace, use an enum class:

enum class UsartSpace: u32 {
    usart0 = XPAR_PS7_UART_0_BASEADDR,
    usart1 = XPAR_PS7_UART_1_BASEADDR,
};

Then, to ensure someone cannot instantiate a Usart with the wrong base address, make the constructor take this enum as a parameter:

template <typename T>
Usart<T>::Usart(UsartSpace usartSpace): usartNumber(static_cast<u32>(usartSpace)) {
}

Also, "Space" is a bit of a generic word that doesn't say much, but you can't call the enum Usart since you already have class Usart. Consider moving the enum to inside class Usart, and since it refers to a port, just call it Port. And perhaps keep the distinction between port number and address clear:

template <typename T>
class Usart: public Platform {
private:
    u32 baseAddress;
    static const u32 addresses[] = {
        XPAR_PS7_UART_0_BASEADDR,
        XPAR_PS7_UART_1_BASEADDR,
    };

public:
    enum class Port {
        usart0,
        usart1,
    };

    Usart(Port port);
    ...
};

...

template <typename T>
Usart<T>::Usart(Port port):
    baseAddress(addresses[static_cast<size_t>(port)])
{
}

In code that uses this class, you would then write:

Usart<sometype> usart(Usart::Port::usart0);

Use better names

What you call a usartNumber looks like an base address to me, so call it baseAddress instead. In any case, don't repeat the name of the class in the member variable names.

Also, dataValue is not the value of some piece of data, it's the size of the data you want to read or write. So call it size, and if possible use size_t as its type:

status transmit(T *pData, size_t size);
status receive (T *pData, size_t size);

Is it status or StatusSpace?

I feel this is another case of an enum in its own namespace being used as a way to declare constants of another type. I would instead define:

enum class Status {
    ok,
    error,
    ...
};

Use array notation where appropriate

Instead of writing *(pByteData + byteCounter), just write the more ideomatic pByteData[byteCounter].

Use const where appropriate

You wouldn't expect the function transmit() to modify the data that you want to send. So make this explicit:

template <typename T>
class Usart: public Platform {
    ...
    status transmit(const T *pdata, size_t size);
    ...
}

Consider templating transmit() and receive() instead of the whole class

The only things that depend on the template parameter T are the transmit() and receive() functions. By making the class templated, you basically lock the type of data you can send and receive when you instantiate the class. Consider instead templating just those two functions, so you can send and receive different data types on the same UART without having to reinstantiate the class:

class Usart: public Platform {
    ...
    template <typename T>
    status transmit(const T *pData, size_t size);

    template <typename T>
    status receive(T *pData, size_t size);
    ...
};

Use Doxygen to document your code

You are documenting your code, which is good practice, but consider doing it in Doxygen's format, so you can have Doxygen create cross-referenced documentation in HTML, PDF and other forms. Doxygen can also check that you documented all the functions and all the parameters to the functions.


Is it an anti-pattern to let ViewModel fill itself from a domain object? Let's say you have a Employees table mapped to a Employee class. Now you want to code a Edit page for general information. You need a ViewModel with all the properties you need to build the page, so you create EditEmployeeViewModel. Then you make your action EditEmployee and your view EditEmployee.

So far so good, but as you add more and more properties, all the code to fill your ViewModel ends up in your controller and as your edit page grows, your controller becomes more and more messy. Specially when your ViewModel starts depending on multiple models and/or lists. You also get the same problem when you get the data back; you have to update all the properties of entities involved in the model.

So my question is: Is it an anti-pattern or a bad practice to let the ViewModel map itself from the domain layer object(s) passed in the constructor or a method? If so, what would be a cleaner approach?

public class EmployeesController : Controller
{
    public ActionResult EditEmployee(int id)
    {
        var employee = repo.GetEmployee(id);
        return View(new EditEmployeeViewModel
        {
            Id = employee.Id,
            FirstName = employee.FirstName,
            LastName = employee.LastName
        });
    }
}

public class Employee
{
    public int Id { get; set; }
    public int FirstName { get; set; }
    public int LastName { get; set; }
    public int EmployeeNumber { get; set; }
}

public class EditEmployeeViewModel
{
    public int Id { get; set; }

    [StringLength(20, MinimumLength = 2)]
    public string FirstName { get; set; }

    [StringLength(20, MinimumLength = 2)]
    public string LastName { get; set; }
}

Example with the mapping in the ViewModel

public class EditEmployeeViewModel
{
    public int Id { get; set; }

    [StringLength(20, MinimumLength = 2)]
    public string FirstName { get; set; }

    [StringLength(20, MinimumLength = 2)]
    public string LastName { get; set; }

    public EditEmployeeViewModel(Employee employee)
    {
        Id = employee.Id;
        FirstName = employee.FirstName;
        LastName = employee.LastName;
    }

    public Employee UpdateModel(Employee employee)
    {
        employee.Id = Id;
        employee.FirstName = FirstName;
        employee.LastName = LastName;
        return employee;
    }
}

It's not an anti pattern. At some point, one layer must know about the layer underneath. You also don't want to rewrite view-model mapping code in multiple controllers or actions, nor do you want to introduce another layer to do the mapping for you. Instead, I opt for constructor injection.

The part of your code I do think is an anti pattern, or incorrect architecture, would be your view model having access to the data access layer.

Your last example using constructor injection will provide the least coupling and most code reuse:

public class EditEmployeeViewModel
{
    public EditEmployeeViewModel(Employee employee)
    {
        Id = employee.Id;
        // ...
    }
}

I would not include the reverse mapping in your view model, where you update an Employee domain model from the EditEmployeeViewModel object. Instead, I would opt for a command or service object to do this work, which would have access to the data access layer:

public class CreateEmployeeCommand
{
    public CreateEmployeeCommand(IEmployeeRepository repository, EditEmployeeViewModel model)
    {
        this.repository = repository;
        this.model = model;
    }

    private EditEmployeeViewModel model;
    private IEmployeeRepository repository;

    public void Execute()
    {
        if (repository.FindBySSN(model.SocialSecurityNumber) != null)
        {
            // cannot add duplicate employee
        }

        Employee employee = new Employee()
        {
            FirstName = model.FirstName,
            LastName = model.LastName,
            SocialSecurityNumber = model.SocialSecurityNumber,
            // ...
        };

        repository.Create(employee);
    }
}

Your controller becomes simplified. The key here is your data access layer should be created and destroyed at the controller level, not the command level. The controller should be controlling the transaction:

public class EmployeesController : Controller
{
    public ActionResult Create(EditEmployeeViewModel model)
    {
        if (!ModelState.IsValid)
            return View(model);

        using (IEmployeeRepository repo = new EmployeeRepository())
        {
            var command = new CreateEmployeeCommand(repo, model);

            command.Execute();

            repo.Commit();
        }

        return RedirectToAction("Index");
    }
}


Form a firstname.lastname@domain.se address, eliminating diacritics from a name I'm an amateur programmer who just started learning programming, my school gave me an assignment to create code that reads a surname and lastname from an input dialog box and then to edit those name in the format of "surname.lastname@nackademin.se" all of wich should be in lowercase letters and only letters of a-z since in sweden we sometimes use "å,ä,ö" in our names.
I have managed to do so, as you can see below but i was wondering if there was a simpler way to do this (perhaps by using less code).

import javax.swing.*;
public class Namn {
    public static void main(String[] arg) {
        char c = '.';
        String s = JOptionPane.showInputDialog(null, "Ange namn och efternamn");
        s = s.replaceAll("\\s{2,}", " ").trim();
        s = s.replace(' ', c);
        s = s.toLowerCase();
        s = s.replace('å', 'a');
        s = s.replace('ä', 'a');
        s = s.replace('ö', 'o');
        JOptionPane.showMessageDialog(null, s + "@nackademin.se");
        System.exit(0);
    }
}

You can chain replaces and you don't need to replace "n. spaces" -> " ", you can just replace them with a "." if you trim first

String s = JOptionPane.showInputDialog(null, "Ange namn och efternamn");
s = s.trim()
      .replaceAll("\\s{2,}", ".")
      .toLowerCase()
      .replace('å', 'a')
      .replace('ä', 'a')
      .replace('ö', 'o');
JOptionPane.showMessageDialog(null, s + "@nackademin.se");


Preventing session hijacking I'm implementing a login script and I want it to be more secure:

function sec_session_start() {
        $session_name = 'sec_session_id';   // Set a custom session name
        $secure = SECURE;
        // This stops JavaScript being able to access the session id.
        $httponly = true;
        // Forces sessions to only use cookies.
        if (ini_set('session.use_only_cookies', 1) === FALSE) {
            //header("Location: ../error.php?err=Could not initiate a safe session (ini_set)");
            exit();
        }
        // Gets current cookies params.
        $cookieParams = session_get_cookie_params();
        session_set_cookie_params($cookieParams["lifetime"], $cookieParams["path"], $cookieParams["domain"], $secure, $httponly);
        // Sets the session name to the one set above.
        session_name($session_name);
        session_start();            // Start the PHP session
        session_regenerate_id();    // regenerated the session, delete the old one.
    }

    function  checkIp($mysqli) {
        if(isset($_SESSION['user_id'])){
            $user_id = $_SESSION['user_id'];
            $user_ip = $_SERVER['REMOTE_ADDR'];
            $user_browser = $_SERVER['HTTP_USER_AGENT'];

            if($stmt === $mysqli->prepare("SELECT UserIP, BrowserInfo FROM users WHERE UserID = ? LIMIT 1")){
                $stmt->bind_param('i', $user_id);
                $stmt->execute();
                $stmt->store_result();

                if($stmt->num_rows == 1){
                    $stmt->bind_result($db_ip,$db_browser);
                    $stmt->fetch();

                    if($user_ip == $db_ip && $user_browser == $db_browser){
                        return true;
                    }else{
                        return false;
                    }
                }else{
                    return false;
                }
            }else{
                return false;
            }
        }else{
            return false;
        }
    }

Are these functions secure enough to prevent session hijacking? If they are not, what can I do to make them secure enough?

Checking an "constant" IP address is an bad idea because of ISP with load balancing. IP address should change to less busy proxy.

In most of cases IP address belongs to the same block. It is called CIDR matching:

function cidr_match($ip, $range)
{
    list ($subnet, $bits) = explode('/', $range);
    $ip = ip2long($ip);
    $subnet = ip2long($subnet);
    $mask = -1 << (32 - $bits);
    $subnet &= $mask; # nb: in case the supplied subnet wasn't correctly aligned
    return ($ip & $mask) == $subnet;
}

usage:

if(!empty($_SESSION['ip'])){
  //192.168.001.xxx = 192.168.001.000/24
  if(!cidr_match($_SERVER['REMOTE_ADDR'],$_SESSION['ip'].'/24')){
     session_destroy();
  }
} else {
  $_SESSION['ip'] = $_SERVER['REMOTE_ADDR'];
}

http://en.wikipedia.org/wiki/Classless_Inter-Domain_Routing

Personally, i recommend to you to check IP address range and some client data (but data may be fake).

PS: do not store session data in MySQL, you should store them just in session.


Compute a value (and update a CSV record), based on prior value The F# code below embodies the two specific questions I have.  It is part of a working transformation utility, the entirety of which, including sample data, is in this gist:
http://bit.ly/2i3kUwF.

Generically, the task is: given a file of discrete records, compute a value for a subsequent record that depends on a value of the previous record. Since I want a transformed stream and not just a computed summary value, this seems a perfect use of Seq.scan.  Here is the relevant portion of main:

File.ReadLines(inFile)
    |> Seq.map appendEmptyColumnForBarValue  // add the col to hold the new barValue
    |> Seq.scan updateBarValue ("", target)  // compute the barValue, based on prev state and target
    |> Seq.skip 1                            // exclude initial blank state above from output
    |> Seq.iter outFile.WriteLine

But I have some questions concerning proper usage of Seq.scan.

Since, by definition, there is no initial state prior to reading the first record, and because I want to return an entire record to the stream, I initialize state to an empty string (ignore the "target" for a bit - that is question 2).  This creates the need to distinguish between this special case of the of the state being the empty string that first time through, vs all other invocations:

let updateBarValue (state:string, target:int32) (line:string) =
    // This is the "folder" function to maintain state for the scan function in main.
    // Since the first time through there will be no accumulated state, the inital state is set to "".
    // The match below handles the decision for first time through, or all other cases.
    //
    // From doc, usage for Seq.scan:
    // Seq.scan folder state source
    //
    // Because scan accepts a single parameter for state AND we need the target passed from the command line
    // invocation, we bundle state and target into a tuple.
    // However, target remains unchanged, while the true state is continually updated
    match state with
    | "" -> buildLineWithBarValue  (line, target) line  // first time through we pass line instead of accumulated state
    | _  -> buildLineWithBarValue (state, target) line

And this use of the empty string, forces the

    |> Seq.skip 1                            // exclude initial blank state above from output

in the main routine above to prevent a blank line from being output.

So, question #1: Am I using Seq.scan correctly? Is there a different/better way to accomplish the goal, that would not force the inclusion of the "Seq.skip 1", which seems less-than-tidy? This would be an easy-to-forget item in this pattern.

Secondly, let's deal with that "target" value.  As you can see from the comment above, I'm cheating a bit by adding it to the state for Seq.scan because it is necessary for the computation and I saw no other way to pass it to my Seq.scan's folder function, updateBarValue.

So, question #2: it seems to me that passing target in as part of the state, when it is clearly not, is a code smell of some sort.  How might this be achieved without my cheat?

The code works, but I'm not happy with it as is; looking for ideas for improvements (in the gist there are a couple of clear TODOs- not talking about those). And, of course, if there is something stupid/crazy that I have not asked about or flagged with a TODO, don't be shy.

First question - no, there is no way to get rid of this "initial" value completely. There is, however, a way to make it more intuitive that the values is "missing" - the Option type. Pass None as initial state, and return Some from then on. This will also allow you to elegantly get rid of that initial value by using Seq.choose.

Second question - easy: just make target first parameter of the folder function and pass it in via partial application:

let updateBarValue target state (line:string) =
   let effectiveState =
      match state with
      | None -> line
      | Some s -> s
   Some (buildLineWithBarValue (effectiveState, target) line)

File.ReadLines(inFile)
   |> Seq.map appendEmptyColumnForBarValue // add the col to hold the new barValue
   |> Seq.scan (updateBarValue target) None // compute the barValue, based on prev state and target
   |> Seq.choose id // get rid of the initial input
   |> Seq.iter outFile.WriteLine

As an aside, I noticed your tendency to tuple function arguments. That is generally considered bad form, unless arguments inherently "belong together" - e.g. point coordinates, - because curried arguments give more freedom. To this end, I would make buildLineWithBarValue's arguments curried as well.


Using Delegates and BeginInvoke with .NET 2.0 WebServices I'm attempting to implement a simple long-polling/comet/reverse AJAX solution, and came across the concept of delegates, specifically with the BeginInvoke and EndInvoke methods. I've built a Web Service that uses these concepts, but, having never used these, they make me a little nervous, and I have my doubts as to whether they are actually what I want.

The goal, of course, is to offload the actual processing from IIS processes onto other framework processes, for a reduction in server load when waiting on a long-poll request for data. I can't have gotten this right, but I want to know why and at least get some direction in how it might be better accomplished (not necessarily a solution, but maybe an article or some documentation).

The Web Service code:

Imports System.Web
Imports System.Web.Services
Imports System.Web.Services.Protocols
Imports System.Threading
Imports System.Runtime.InteropServices

<WebService(Namespace:="http://tempuri.org/")> _
<WebServiceBinding(ConformsTo:=WsiProfiles.BasicProfile1_1)> _
<Global.Microsoft.VisualBasic.CompilerServices.DesignerGenerated()> _
Public Class WebService
    Inherits System.Web.Services.WebService

    <WebMethod()> _
    Public Function HelloWorld() As String
        Dim threadId As Integer
        Dim thing As New Things
        Dim caller As New AsyncDoStuff(AddressOf thing.doStuff)
        Dim result As IAsyncResult = caller.BeginInvoke(3000, threadId, Nothing, Nothing)

        result.AsyncWaitHandle.WaitOne()

        Dim returnValue As String = caller.EndInvoke(threadId, result)

        result.AsyncWaitHandle.Close()

        Return returnValue
    End Function
End Class

Public Class Things
    Public Function doStuff(ByVal callDuration As Integer, <Out()> ByRef threadId As Integer) As String
        ' Imagine this method was accessing a database, looking for new information, and returning when it found some
        Thread.Sleep(callDuration)
        threadId = Thread.CurrentThread.ManagedThreadId()
        Return String.Format("My call time was {0}.", callDuration.ToString())
    End Function
End Class

Public Delegate Function AsyncDoStuff(ByVal callDuration As Integer, <Out()> ByRef threadID As Integer) As String

This Stack Overflow answer to the question "Is there a VB.NET equivalent of C# out parameters" is pretty clear:

  No, there is no equivalent construct that allows a non-initialised variable to be passed to a method without a warning [...]

However specifying the <Out()> attribute, despite being ignored by VB (ByRef is all you need, really), does allow a C# client to use it as an out parameter - so good call then!

Declaring the delegate at the bottom of the module, after it's being used, makes it uselessly confusing and prompts for unnecessary scrolling; I would have put it at the top... actually, I would have made the delegate a public member of the WebService class:

Public Class WebService
    Inherits System.Web.Services.WebService

    Public Delegate Function AsyncDoStuff(ByVal callDuration As Integer, <Out()> ByRef threadID As Integer) As String

    '...

I very, very seldom declare my own delegate types, if at all. I don't like ref ByRef and out <Out()> parameters either - I think they should be avoided.

In this case I would have gone with a Func<int,AsyncStuffResult> (please excuse the C# notation), where the int parameter is the callDuration, and the AsyncStuffResult is a class that encapsulates everything you want to return from the function call - whatever that is.

This way if/when you need to return more things, your signature doesn't need to change, you only add new members to your result class - note that the EventArgs class fulfills the same goal.

Public Class AsyncStuffResult
    '- expose ThreadId as a get-only property
    '- expose your String result as a get-only property, too
    '- expose a constructor to assign the members
End Class


JavaScript queue implementation using linked list Are there better ways of modularizing the code? Does it contain things JavaScript experts would avoid? Is it idiomatic? Is there a better way of signaling an empty queue than returning null?

function makeQueue () {
  // types
  let Node = {
    next : null,
    data : null
  };

  // data
  let head = null;
  let size = 0;

  // methods
  function enqueue(x) {
    let newNode = {...Node, next : head, data : x};
    head = newNode;
    size++;
  }

  function dequeue() {
    if (size === 0) {
      return null;
    }

    let x = head.data;
    head = head.next;
    size--;

    return x;
  }

  function getSize() {
    return size;
  };

  // public
  return {
    enqueue : enqueue,
    dequeue : dequeue,
    getSize : getSize
  };
};

Some observations in addition to the other answer:

inconsistent space between function name and () - use eslint or similar to prevent typos
Node is used as a template for extra properties, not as a type, so I doubt its name should be capitalized as though it's a class. Also, it's effectively unused in your code.
const in JS is not for named constants only, but for any value that's assigned just once:

  The reason is simple, it's much easier to reason about code when it is dead obvious what can change and what cannot change. [...]

  Going back and changing a const to a let is dead simple. And going const by default makes you think twice before doing so. And this is in many cases a good thing.

  How many bugs have you seen that involved variables changing unexpectedly? I'd guess a lot. I know that the majority of bugs that I see involve unexpected state changes. You won't get rid of all of these bugs by liberally using const, but you will get rid of a lot of them!

+ if you use a JavaScript IDE, it'll warn about accidental reassignment of a const identifier.
A separate assignment statement with newNode doesn't seem warranted as the code would be just as obvious with a direct assignment to head.
getSize is too verbose. I suggest exposing it as size just like the read-only size getter in Map and Set.
Since you're using ES2015, also use object literal shorthand:

return {
  enqueue,
  dequeue,
  size: getSize
};


Developing a multistep form I implemented a multistep form with the method described by Ryan Bates in ep217 but I had some weird behavior when refreshing or moving between the steps Acts_as_good_style (old but still good) has a tip redirect when moving on that lead me to the change in the code that follows.

In a nutshell, it says that if you're in the create action you should not render new (as I was doing).

This solved the problem. But I had to manage the case of errors in the form, so I ended up with this code.

#profiles_controller.rb

def create
  # [...] save etc [...]

  # render
  if @profile.new_record?
    # render 'new' # OLD
    session[:profile_valid] = @profile.errors.blank?  # NEW
    redirect_to new_profile_path # NEW
  else
    # [...]
  end
end

def new
  @profile = Profile.new(session[:profile_params])
  # [...]

  # rebuild errors (see create)
  # check false because first time is nil and no error have to be displayed
  @profile.valid? if session[:profile_valid] == false
  session[:profile_valid] = true
end

Where in the new action I reload the errors otherwise lost depending on session[:profile_valid], that works fine.

BUT this way doesn't look very good to me and I would appreciate to have your opinion, OR how do you manage your controllers in multispets forms?

What was the strange behavior? Refreshing or going back and forth through the steps sometimes you jump to the wrong page, not clear what the logic is, probably depends on the validations in the model and the params hash.

I can't speak for that particular Railscast episode, but rails provides an idiomatic way to deal with validation errors. Taking your code as an example, I'd change it to look like this (using Rails 4.2):

app/controllers/profiles_controller.rb:

class ProfilesController < ActionController::Base
  def show
    @profile = Profile.find(...)
  end

  def new
    @profile = Profile.new(profile_params)
  end

  def create
    @profile = Profile.new(profile_params)

    if @profile.save
      redirect_to profile_path(@profile)

    else
      render :new
    end
  end

  def edit
    @profile = Profile.find(...)
  end

  def update
    @profile = Profile.find(...)

    if @profile.update_attributes(profile_params)
      redirect_to profile_path(@profile)

    else
      render :edit
    end
  end

  private

  def profile_params
    # call params.require(:profile).permit(...) to whitelist attributes as needed
    params.require(:profile)
  end
end

app/views/profiles/new.html.haml:

%h2 New Profile
= render partial: 'form'

app/views/profiles/edit.html.haml:

%h2 Edit Profile
= render partial: 'form'

app/views/profiles/_form.html.haml:

= form_for(@profile) do |f|
  - # Display any form errors, if any
  - if @profile.errors.any?
    .errors
      %h2 Errors
      %ul
        - @profile.errors.full_messages do |message|
          %li= message

  - # Assuming profiles have first_name and last_name as attributes
  %section
    = f.label :first_name
    = f.text_field :first_name

  %section
    = f.label :last_name
    = f.text_field :first_name

  %section
    = f.submit

This is a very common pattern in rails apps that you can also see being used in the rails guides. The form_for helper is smart enough to generate the correct path to submit the form to depending on weather or not the @profile instance variable contains a record that has or has not been persisted. Persisted records (new ones or those with errors) will be posted to create. Records being updated will be set to whatever the update action/path for that controller is.

Whenever updating or saving a record fails, the controller will render the appropriate view (new or edit) and use the record that failed validation when rendering the form. From here, your view can inspect the record and display any errors if they exist (@profile.errors).

This won't fix an issue of a user navigating backwards and forwards through your form, but it should reduce the need to do so and it'll let you avoid weirdness you may have encountered while persisting form data within the session variable (which gets serialized and de-serialized into whatever your session store is during each request).

PS: I find that thinking of multi-step forms with more than one or two steps as state machines helps greatly in trying to conceptualize the different states and transitions you need to handle when you need to determine at what point in a form a particular session should be in. To that effect, I would highly recommend checking out the wicked gem which does a lot of this grunt work for you.


Linked list implementation I wrote some code for linked lists and would like to know what things could/should be done differently.

I'm using an index to keep track of the most important information and have 2 sets of functions, one to use the list just to point to data and the other to store the data in the list.

Here's the code so far:

#include <stdlib.h>
#include <string.h>

//Basic structure
typedef struct nNode Node;
struct nNode {
    void *content;
    Node *next;
    Node *previous;
};

//An index will make everything simpler
typedef struct {
    Node *first;
    Node *last;
    size_t node_count;
} List;

//Initialize list index with default values
void init_list(List *root)
{
    root->first = NULL;
    root->last = NULL;
    root->node_count = 0;
}

//Link new element and return pointer to it if successful or NULL
Node *add_node(List *root, void *content)
{
    Node *temp = malloc(sizeof(Node));
    if(temp == NULL){
        return NULL;
    }

    //Fill node
    temp->content = content;
    temp->next = NULL;
    temp->previous = root->last; //NULL if first node

    //Handle first element insertion
    if(root->first == NULL){
        root->first = temp;
    }

    //Or update previous element
    else {
        root->last->next = temp;
    }

    //Update index
    root->last = temp;
    ++root->node_count;

    return temp;
}

//Unlink element, always successful
void delete_node(List *root, Node *node)
{
    //If it's the first, update index reference
    if(node->previous == NULL){
        root->first = node->next;
    }

    //Or it's safe to update previous node
    else {
        node->previous->next = node->next;
    }

    //If it's the last, update index reference
    if(node->next == NULL){
        root->last = node->previous;
    }

    //Not the last? Safe to update next node
    else {
        node->next->previous = node->previous;
    }

    //Update count and free memory
    free(node);
    --root->node_count;
}

//Iterate through all nodes and call a function on every iteration
void iterate(const List *root, void(*function)(Node*))
{
    Node *next; //store in case the node changes
    for(Node *ite = root->first; ite != NULL; ite = next){
        next = ite->next;
        (*function)(ite);
    }
}

//Iterate through all nodes backwards and call a function on every iteration
void iterate_backwards(const List *root, void(*function)(Node*))
{
    Node *previous; //store in case the node changes
    for(Node *ite = root->last; ite != NULL; ite = previous){
        previous = ite->previous;
        (*function)(ite);
    }
}

//Delete all elements in the list
void delete_list(List *root)
{
    iterate(root, (void(*)(Node*))free);
    init_list(root);
}

////////////////
////// The following functions also store the content in the list
///// instead of just pointing to it. They should be used together
//// in order to avoid memory leaks.
/////////////

//Add node and store a copy of content
Node *add_node_store(List *root, const void *content, size_t content_size)
{
    //Allocate space
    void *copy = malloc(content_size);
    if(copy == NULL){
        return NULL;
    }

    //Copy content
    memcpy(copy, content, content_size);

    //Check if node insertion succeeded
    void *response = add_node(root, copy);
    if(response == NULL){
        free(copy);
    }

    return response;
}

//Unlink node and free stored content
void delete_node_store(List *root, Node *node)
{
    free(node->content);
    delete_node(root, node);
}

//Free node and its content
static void free_node_store(Node *node)
{
    free(node->content);
    free(node);
}

//Delete all nodes on the list, free them and their contents
void delete_list_store(List *root)
{
    iterate(root, free_node_store);
    init_list(root);
}

You use root as parameter name when you pass the list around but I think a better name would just be list. root implies that you pass the first node of the list around rather than the container.
++root->node_count is correct but root->node_count += 1 reads nicer (imho).
I would consider changing the interface slightly:

Have a List* new_list() method which creates the list (and initializes the object).
delete_list should also delete the list object (probably needs signature change to delete_list(List **list))
Add a method clear_list which just clears the contents of the list.

Right now the programmer has to remember whether a node was added via add_node or add_node_store in order to call the proper delete function. However this could be solved by having a flag in nNode which indicates whether the node owns the content or not. Delete could then automatically do the right thing.


MySQL Query to check at least one field is greater than zero I want to retrieve the row if any of the 3 integer field in a row is greater than zero. All the field has default value of zero.

Which of the following query is better?

-- Query 1
SELECT * FROM my_table WHERE number_A > 0 OR number_B > 0 OR number_C > 0

-- Query 2
SELECT * FROM my_table WHERE (number_A + number_B + number_C) > 0

While checking the execution time both the query gives same numbers and the result data of the query is exactly the same.

In addition to the other differences between the two statements

WHERE (number_A + number_B + number_C) > 0

Could cause you problems if any of those three are negative. That may not be relevant in your case, but if one of these is negative the total might fail to exceed zero even if another of the values does exceed zero.


Deployment Automatization for docker builds, kubernetes deploys and secret injection I've written a small script that automates the tasks I previously did manually for deploying onto our kubernetes clusters and I am quite happy with the results I have, but I do think that what my approach was (building a command and executing it at the end) could probably be improved.

Especially the fn_magic part is something I dont know could be done better.

team="webdev"
project="sick-project"
registry="myfancyregistry"
tagPrefix="$registry/$team/$project"
namespace="webdev-dev"
kubectl="kubectl -n=$namespace"

declare -A NAME_TO_TAGS
declare COMMANDBUILDER
declare -A REPLACEMENTS

# Colors :)
RED='\033[0;31m'
GREEN='\033[0;32m'
BLUE='\033[0;34m'
CYAN='\033[0;36m'
YELLOW='\033[1;33m'
NC='\033[0m' # No Color

function setReplacements() {
  while IFS= read -r line
  do
    [ -z "$line" ] && continue
    key=${line%%=*}
    value=${line#*=}
    REPLACEMENTS[$key]=$value
  done < "${BASH_SOURCE%/*}/secrets"
}

function buildAndPush() {
  function fn_magic() {
    local dockerfile=$1
    local name=$2
    local context=$3
    local tag=$(tar cf - --exclude='node_modules' $context | sha1sum | grep -o "\w*")
    NAME_TO_TAGS["$name"]="$tag"
    local output=$(docker images $tagPrefix/$project-$name:$tag | awk '{print $2}' | sed 1,1d)
    printf "SHA1 for ${CYAN}$name${NC} is ${YELLOW}$tag${NC}.\n"
    if [[ "$output" ]]; then
      printf "Skipping building ${CYAN}$name${NC} because image already exists locally.\n"
      COMMANDBUILDER="$COMMANDBUILDER && printf \"Pushing ${CYAN}$name${NC}.\n\" && docker push $tagPrefix/$project-$name:$tag"
      return
    fi
    COMMANDBUILDER="$COMMANDBUILDER && printf \"Building ${CYAN}$name${NC}.\n\" && docker build -f dockerfiles/$dockerfile.dockerfile -t $tagPrefix/$project-$name:$tag $context"
    COMMANDBUILDER="$COMMANDBUILDER && printf \"Pushing ${CYAN}$name${NC}.\n\" && docker push $tagPrefix/$project-$name:$tag"
    return
  }
  local calls=(
    "fn_magic ReactNginx frontend ./frontend"
    "fn_magic DockerfileNode backend ./backend"
  )

  for call in "${calls[@]}"; do
    eval $call
  done
}

function deploy() {
  local deployName="$1"
  local tag="$2"

  local filter=""
  REPLACEMENTS[TAG]="$tag"
  for key in "${!REPLACEMENTS[@]}"; do
    local REPLACE=$(echo ${REPLACEMENTS[$key]} | sed -e 's/\\/\\\\/g; s/\//\\\//g; s/&/\\\&/g')
    filter="$filter | sed \"s/{{$key}}/$REPLACE/g\""
  done

  COMMANDBUILDER="$COMMANDBUILDER && cat "${BASH_SOURCE%/*}/$deployName.yaml" $filter | $kubectl apply -f -"
}

setReplacements
buildAndPush
for name in "${!NAME_TO_TAGS[@]}"; do
  deploy $name ${NAME_TO_TAGS[$name]}
done
COMMANDBUILDER="echo $COMMANDBUILDER"
eval "$COMMANDBUILDER"

Some suggestions in rough order of decreasing importance:

eval is evil, and I'm pretty sure you can completely avoid it in your script. Other than a massive gain in sanity, all that sed stuff to escape strings is not going to be necessary if you instead just use plain shell.
Naming is really important, and comments are a good way to detect things which should be renamed.
set -o errexit -o nounset -o pipefail would be very handy in this script. Just make sure you're aware of the caveats (that page is needlessly snarky but has some useful bits, and that site is fantastic).
A simple array is the best way to build a command. I would thoroughly recommend not building complex commands using a shell script. Instead, run each command as soon as you have all the pieces. Bundling up commands to run isn't going to make things faster; in most cases it'll make things significantly slower by making >50% of the code about building the command rather than just running it.
Nested functions aren't. There are no closures in Bash, so defining a function within another function just makes it harder to read.
In Bash the convention is that uppercase names are reserved for exported variables.
Single use variables can often be inlined without loss of clarity.
printf is meant to be used with format strings for readability. So

printf "SHA1 for ${cyan}$name${end_format} is ${yellow}$tag${end_format}.\n"

would normally be written as

printf "SHA1 for %s is %s.\n" "${cyan}${name}${end_format}" "${yellow}${tag}${end_format}"

local is considered a command for exit code purposes, which means that local foo=[variable or command substitution] only gets the exit code of local, not of the variable or command substitution. So if you start using nounset and the like you should declare variables local before assigning them separately:

deploy() {
    local deploy_name tag
    deploy_name="$1"
    tag="$2"
    …
}

shellcheck can give you more tips.


Comparison functions for 2D points I have the following class for 2D points:

class Point {
private:
    double _x, _y;

public:
    Point(double x, double y) : _x(x), _y(y);

    double x() const { return _x; }
    double y() const { return _y; }
};

I wanted to add a few comparison functions that compared objects of this class in different ways.

One of them is just a simple function:

/// Returns whether A has a smaller y-coordinate than B
/// (and smaller x-coordinate in case of equality).
bool yComp(const Point &A, const Point &B) {
    if (A.y() != B.y()) return A.y() < B.y();
    return A.x() < B.x();
}

The other is a struct with operator() defined on it:

struct angleComp {
    Point origin;
    angleComp(const Point &P);
    bool operator()(const Point &A, const Point &B);
};

// Constructs a new angle comparison with P as origin
angleComp::angleComp(const Point &P) : origin(P) {}

// Compares according to the angles that the lines joining each point
// to a given origin form with the x-axis.
bool angleComp::operator()(const Point &A, const Point &B) {
    GeometricVector u = A - origin, v = B - origin;
    return u.x()*u.x()*u.sqrNorm() < v.x()*v.x()*v.sqrNorm();
}

Do I put the comparisons inside the class definition? The first function could be made a static method, but the problem is I can't put angleComp inside the class (as a static member) since one of its fields is of type Point itself, which would be an incomplete type (inside the class declaration). And I would prefer these comparison functions to be grouped together (instead of one being a static member, and the other being a top-level thing).

So, an alternative would be to put both outside the class, but to retain some grouping, putting all of them in a namespace:

namespace PointComp {
    bool yComp(const Point &A, const Point &B);

    struct angleComp {
        Point origin;
        angleComp(const Point &P);
        bool operator()(const Point &A, const Point &B);
    };
}

The overall design seems a bit complicated to me. Setters and getters are often more of an hindrance than a real asset, and the benefits of having a simple struct should not be under-estimated. In particular, it reminds you that you don't have to encapsulate everything, and that defining functions at the point of use is often the most convenient way to proceed:

struct Point { float x, y; };

// ...

// here I need to compare points by y and then x
auto best = std::max_element(v.begin(), v.end(), [](auto point_a, auto point_b) {
    return std::tie(point_a.y, point_a.x) < std::tie(point_b.y, point_b.x);
    //      -> tie gives you the lexicographic comparison for free
});

// and there by angle with etc.
auto widest = std::max_element(v.begin(), v.end(), [origin](auto point_a, auto point_b) {
    // ...
});

In a nutshell, why having a namespace and custom classes where you can simply use a lambda?


Optimize Cython code with np.ndarray contained Can someone help me further optimize the following Cython code snippets? Specifically, a and b are np.ndarray with int value (range(256)) in them.  They are one dimension arrays with dynamic length.  resultHamming is a one-dimension array with float value in it (dynamic length).  bits is an int list (size 256).

The function is to compare two dynamic length bit vector, and return a similarity value as the distance of the two, where the length of each vector is a multiple of 2048-bit (256 bytes). I want to find the best match between these two bit vector by comparing each 2048-bit block, where each bit vector is represented as ndarray (read the bit sequence byte by byte, thus each position is range from 0 to 2^8 = 256). Rule for matching is to find global minimum distance between all block pairs and allow one block in A to be matched with more than one block in B if they have smaller distance. Always compare the smaller size vector against the larger one.

The following code assumes b vector is smaller.  We can limit resultHamming to be smaller than size of numArrayB and only record numArrayB smallest distance value, but need to track the current size when inserting new value into it. Even with current case (record all the pairwise distance), we actually know the final size of resultHamming at the beginning.

def compare(a, b):
    cdef double dis, hammingTotal = 0
    cdef int numArrayA = int(a.size/256)
    cdef int numArrayB = int(b.size/256)
    cdef int i, j, k, l, index
    bits = list(xrange(256))
    # Prepare a bit number table for fast query
    for l in xrange(256):
        # nnz() counts the number of 1s in value
        bits[l] = nnz(l)

    resultHamming = []
    for i in xrange(numArrayB):
        # Count the number of 1-bits in i-th block of B
        onesB = sum(bits[b[k+256*i]] for k in xrange(256))
        for j in xrange(numArrayA):
            # Count the number of 1-bits in j-th block of A
            onesA = sum(bits[a[k+256*j]] for k in xrange(256))
            # Calculate the hamming distance between i-th block of B and j-th block of A
            hammingCur = sum(bits[b[i*256+k] ^ a[j*256+k]] for k in xrange(256))
            dis = (hammingCur) / (onesA + onesB)
            # Insertion current dis to resultHamming with sorted order
            k = len(resultHamming) - 1
            if dis >= resultHamming[-1]:
                resultHamming.append(dis)
            else:
                resulthamming.append(resultHamming[k])
                while k > 0 and resultHamming[k-1] > dis:
                    resultHamming[k] = resultHamming[k-1]
                    k -= 1
                resultHamming[k] = dis

    # Extract k smallest distance from the distance array
    for k in xrange(numArrayB):
        hammingTotal += resultHamming[k]
    return round(hammingTotal/(numArrayB), 3)

As a general rule for optimizing with cython, try to avoid python function calls if possible, i.e. replace pythonic sum with numpy sum or explicit for loop, replace bit number table with numpy array, and try to declare types for all variables.

You can also precalculate most of the variables used in distance calculation and use a separate array to hold minimum distances for each block. I quickly put something together and got the code below.

Just wondering, what is the rationale for normalizing the distance with the number of set bits?

import cython
import numpy as np
cimport numpy as np

cdef bitCount(unsigned char value):
    cdef count = 0
    while(value):
        value &= value - 1
        count += 1
    return(count)

def compare(np.ndarray[np.uint8_t, ndim=1, mode='c'] a,
            np.ndarray[np.uint8_t, ndim=1, mode='c'] b):

    cdef int nblocksA = int(a.size/256)
    cdef int nblocksB = int(b.size/256)
    cdef int i, j, k

    cdef double partial_hamming_distance

    # Prepare a bit number table for fast query
    cdef np.ndarray[np.uint8_t, ndim=1] bits = np.zeros(256, dtype=np.uint8)
    for i in range(256):
        bits[i] = bitCount(i)

    cdef np.ndarray[np.float_t, ndim=1] min_distances = np.ones(nblocksB,
                                                                dtype=np.float)
    min_distances *= 1000000

    cdef np.ndarray[np.float_t, ndim=1] set_bitsA = np.zeros(nblocksA, dtype=np.float)
    for i in range(nblocksA):
        for k in range(256):
            set_bitsA[i] += bits[a[k + 256 * i]]

    cdef np.ndarray[np.float_t, ndim=1] set_bitsB = np.zeros(nblocksB, dtype=np.float)
    for i in range(nblocksB):
        for k in range(256):
            set_bitsB[i] += bits[b[k + 256 * i]]

    for i in range(nblocksB):
        for j in range(nblocksA):
            partial_hamming_distance = 0

            for k in range(256):
                partial_hamming_distance += (bits[b[i*256+k] ^ a[j*256+k]]
                                          / (set_bitsB[i] + set_bitsA[j]))

            if partial_hamming_distance < min_distances[i]:
                min_distances[i] = partial_hamming_distance

    return np.sum(min_distances)


Composing functionality of sealed class with interfaces public sealed class Client
{
    private Simulation _simulation;
    public Client(ITickProvider tickProvider, ICommandReceiver commandReceiver)
    {
        _simulation = new Simulation();
       tickProvider.Tick += HandleTick;
       commandReceiver.CommandReceived += HandleCommandReceived;
    }
    private void HandleTick ()
    {
        //Distribute tick to _simulation
    }
    private void HandleCommandReceived(Command command)
    {
        //Inject command to _simulation
    }
}

Client operates a multithreaded Simulation, distributing ticks and external input to the Simulation.
ITickProvider provides a Tick event that runs every e.g. fixed interval, button click, etc..
ICommandReceiver receives external input (Commands); i.e. User input, server input, serialized input, etc..
Because the interfaces' implementations are usually exclusive, I decided to make Client sealed and inject the functionality with the constructor. This also helps with controlling thread-sensitive access to Simulation.
What is the pattern of composing a sealed class with modular interfaces? Is this a well-known pattern and if so, have I implemented it correctly?

It depends on the actual class requirements along with the overall code factors.
For instance,

Do you think that Client class can be extended in the future? (not by inheritance surely since it's sealed).
How often this class is going to be called?
How simple you need to simplify the initiation of this class? (as a way to automate handling of its dependencies)
Are these interfaces going to be recalled or extended in any other way except this class ?
Do interfaces have many unneeded members? (say you only need some members and not all of them).

You must investigate that, and decide the best approach that would be feasible to your current project.
As kick-starter that would give you some ideas after looking at your class, and interfaces, without knowing your actual implementations, I could say the safest way is to create ClientHandler and pass it to the constructor instead. something like this :
public class ClientHandler : ITickProvider, ICommandReceiver
{
    private readonly Simulation _simulation;
    public ClientHandler(Simulation simulation)
    {
        if(simulation == null)
        {
            throw new ArgumentNullExecption(nameof(simulation));
        }
        _simulation = simulation;
    }

    // ITickProvider implementation
    // Do the handeling here
    public void Tick() { ... }

    // ICommandReceiver implementation
    // Do the handeling here
    public void CommandReceived() { ... }
}

public sealed class Client
{
    private readonly ClientHandler _handler;

    public Client(ClientHandler clientHandler)
    {
        if(clientHandler == null)
        {
            throw new ArgumentNullExecption(nameof(clientHandler));
        }

        _handler = clientHandler;
    }
}

This is better, because you'll have more room in your code to be able to control the handler and the client side-by-side, with a benefit of extending the handler as per requirement. Also, Implementing interfaces would ease your work, especially if your work includes Reflection. You can also keep the handler as abstract class and inherit it to have more customizable handlers.


Yet another Sieve of Eratosthenes in Java I wanted a reusable class for generating a list of primes till some arbitrary (but reasonable) limit. So, I wrote a class Primes that implements the Sieve of Eratosthenes and another method that makes use of this sieve. The implementation is done on the basis of clarity and swiftly obtaining results.

I admit that using a BitSet would have improved memory usage and using the Sieve of Atkin would have given me a (slightly) better runtime, but I avoided extra trouble and went for it.

Primes.java

/**
 * This is a utility class that contains methods related to prime numbers
 * that are used in many problems in Project Euler.
 *
 * @author Subhomoy Haldar
 * @version 1.0
 */
public class Primes {

    /**
     * Returns a boolean array that is essentially a sieve. In order to check
     * if a number is prime, check if the boolean at that index is true. For
     * example,
     *

     *  // ...
     *  boolean[] sieve = Prime.siftTill(someLimit);
     *  int n = (yourNumber);
     *  if (sieve[n])
     *      // ... do operations with the prime number
     *  // ...
     *

     *

     *
Time complexity:

     * O(n log log n)
     *
Space complexity:

     * O(n)
     *
     * @param n The number till which the sieve must be generated.
     * @return The sieve in the form of a boolean array.
     */
    public static boolean[] siftTill(int n) {
        // create an array of size (n + 1), to include the index n as well
        boolean[] sieve = new boolean[++n];
        // the outer loop will be limited to ~ sqrt(n) iterations
        int limit = (int) Math.sqrt(n);
        // initially set all to true except 0 and 1
        for (int i = 2; i < n; i++) sieve[i] = true;
        // start from 2 and remove all multiples of primes
        for (int i = 2; i < limit; i++) {
            if (sieve[i]) {
                for (int j = i * i; j < n; j += i) {
                    sieve[j] = false;
                }
            }
        }
        return sieve;
    }

    /**
     * Returns an array of primes upto the given inclusive limit.
     *

     *
Time complexity:

     * O(n)
     *
Space complexity:

     * O(n)
     *
     * @param limit The inclusive upper limit.
     * @return An array of primes upto the given inclusive limit.
     */
    public static long[] getPrimesTill(int limit) {
        boolean[] isPrime = siftTill(limit);
        // 1st pass: count the number of primes
        int count = 0, index = 0;
        for (int i = 2; i <= limit; i++) {
            if (isPrime[i])
                count++;
        }
        long[] primes = new long[count];
        // 2nd pass: store the primes
        for (int i = 2; i <= limit; i++) {
            if (isPrime[i])
                primes[index++] = i;
        }
        return primes;
    }
}

I welcome comments on all aspects of the code.

Documentation
The class shouldn't mention who the intended client is, just what it does.
Most of your comments inside methods are noise. Document why, not what.
Error Handling
You don't handle bad inputs (e.g. negative values). It doesn't make sense for siftTill() to return an ArrayIndexOutOfBoundsException in that case. Explicitly throw a relevant exception, like IllegalArgumentException.
Naming
You use 'till' as an abbreviation for 'until'. Don't! A "till" is a real word which means something different, and it may be confusing, especially since 'until' only has one 'l'.
Best Practices
Don't reassign method parameters.
Declare variables as close as possible to their first use.
Don't use increment/decrement operations inside array references. Junior programmers will not be able to tell the order of operations.
It doesn't make sense to have a getPrimesTill() take an int as the maximum value but return a long[].
You're better off keying loops off of the array's size explicitly, rather than the variable you used to set its length. That way, if either changes your array traversal won't break.
The Primes class should be final and have a private constructor to indicate it should not be instantiated. Alternately, make an instantiable class that can be optionally be reused and runs the sieve for numbers it hasn't yet computed. That lets you keep computed information, such as the number of primes, in a member variable rather than having to walk your sieve array to find it.
Putting it all together might look something like:
import java.util.Arrays;

/**
 * This is a utility class that contains methods related to prime numbers.
 *
 * @author Subhomoy Haldar
 * @version 1.0
 */
public final class Primes {

    /**
     * Returns a boolean array that is essentially a sieve. In order to check
     * if a number is prime, check if the boolean at that index is true. For
     * example,
     *
     *

     *
     *  // ...
     *  boolean[] sieve = Prime.siftTill(someLimit);
     *  int n = (yourNumber);
     *  if (sieve[n])
     *      // ... do operations with the prime number
     *  // ...
     *
     *

     *

     *
Time complexity:

     * O(n log log n)
     *
Space complexity:
 O(n)
     *
     * @param n The number until which the sieve must be generated.
     * @return The sieve in the form of a boolean array.
     */
    public static boolean[] siftUntil(final int n) {

        final boolean[] sieve = new boolean[n + 1];
        Arrays.fill(sieve, true);
        sieve[0] = false;
        sieve[1] = false;

        removePrimeMultiplesFrom(sieve);

        return sieve;
    }

    private static void removePrimeMultiplesFrom(final boolean[] sieve) {
        final int limit = (int) Math.sqrt(sieve.length);
        for (int i = 2; i < limit; i++) {
            if (!sieve[i]) {
                continue;
            }

            for (int j = i * i; j < sieve.length; j += i) {
                sieve[j] = false;
            }
        }
    }

    /**
     * Returns an array of primes up to the given inclusive limit.
     *

     *
Time complexity:

     * O(n)
     *
Space complexity:
 O(n)
     *
     * @param limit The inclusive upper limit.
     * @return An array of primes up to the given inclusive limit.
     */
    public static int[] getPrimesUntil(final int limit) {

        final boolean[] isPrime = siftUntil(limit);

        final int[] primes = new int[countPrimes(isPrime)];
        for (int i = 2, primeIndex = 0; i <= limit; i++) {
            if (isPrime[i]) {
                primes[primeIndex] = i;
                primeIndex++;
            }
        }
        return primes;
    }

    private static int countPrimes(final boolean[] isPrime) {
        int count = 0;
        for (int i = 2; i < isPrime.length; i++) {
            if (isPrime[i]) {
                count++;
            }
        }
        return count;
    }

}


Extracting the last component (basename) of a filesystem path fn basename<'a>(path: &'a str, sep: char) -> Cow<'a, str> {
    let pieces = path.split(sep);
    match pieces.last() {
        Some(p) => p.into(),
        None => path.into(),
    }
}

Usage:

println!("'{}'", basename("foo", '/'));    // outputs 'foo'
println!("'{}'", basename("bob/", '/'));   // outputs ''
println!("'{}'", basename("/usr/local/bin/rustc", '/')); // outputs 'rustc'

I think the split() into a match on last() is kind of elegant.
I know there is some work needed to handle both str and String, I am not sold on the use of Cow and needing to define a lifetime for the string.

I am not sold on Cow because later on I need to extract from it.

let prog = basename(&args[0], '/').into_owned();

It feels like I am working too hard.

Firstly, you should use rsplit and next rather than split and last, as it starts at the more appropriate end:

fn basename<'a>(path: &'a str, sep: char) -> Cow<'a, str> {
    let pieces = path.rsplit(sep);
    match pieces.next() {
        Some(p) => p.into(),
        None => path.into(),
    }
}

Secondly, you shouldn’t be using strings for this; you should be using paths, because that’s semantically what you’re dealing with.

The easiest way to get a path tends to be to take a &Path or a generic parameter implementing AsRef<Path> and calling .as_ref() on it; str, String, Path, PathBuf and more implement it.

You can get the base name from a &Path with file_name; this admittedly produces a Option<&OsStr>, so if you want to display the path you’d need to convert it back towards a string with e.g. .and_then(|s| s.to_str()).

Anyway, the point of this latter part is just that for something that is semantically a path, you should be handling it specially, as a rule; a path need not be Unicode. Think on it more.