Adding and removing PHP parameters on click I'm trying to optimize this repetitive piece of code. I think I need to set subsection href as a variable and then figure out a way to merge the functions?

$(document).ready(function () {
    $('#alphabetical').click(function () {
        $('.subsection').attr('href', function () {
            return this.href + '&sort=alphabetical';
        });
    });
    $('#chronological').click(function () {
        $('.subsection').attr('href', function () {
            return this.href.replace('&sort=alphabetical', '');
        });
    });
    $('#builtfilter').click(function () {
        $('.subsection').attr('href', function () {
            return this.href + '&status=built';
        });
    });
    $('#allfilter').click(function () {
        $('.subsection').attr('href', function () {
            return this.href.replace('&status=built', '');
        });
    });
});

First off, you have a couple of bugs:

You can keep clicking the #alphabetical or #builtfilter buttons, and each time you do, it'll add the parameter. Even if it's already there. Yet the "reverse" buttons only replace/remove 1 instance. So if you have:

 /some/path?foo=bar

and click the alphabetical button again a couple of times, you get

 /some/path?foo=bar&sort=alphabetical&sort=alphabetical&sort=alphabetical

But you'll have to click #chronological an equal number of times, to "reset" everything, since only one &sort=alphabetical chunk is removed each time.

Secondly, I don't know the URL you're manipulating, but if there are no parameters to begin with, you end up with an invalid URL like:

/some/path&sort=alphabetical

Note that there's no ? to separate the query string from the path. Instead the & has ended up there - where it shouldn't be.

Also, if you URL has a fragment like #something, you'll be adding paramters after that, which won't work right. Like:

/some/path?foo=bar#fragment&sort=alphabetical

So first off, I'd recommend using jQuery's param method to create a well-formatted query string from an object:

var params = { sort: "", status: "built" }; // an example
var queryString = jQuery.param(params); //=> "sort=&status=built"

Secondly, you can use data-* attributes on your links to specify their action and classes to allow you to find all the links/buttons in one go (here's a related answer of mine).

For instance (simple example):

<button type="button" class="param" data-key="sort" data-value="alphabetical">Sort alphabetically</button>
<button type="button" class="param" data-key="sort" data-value="">Sort chronologically</button>

$(function () { // equivalent to $(document).ready(...)
  var params = { sort: "", status: "" }; // default params

  $(".param").on("click", function () {
    var $this = $(this),
        key = $this.data("param"),
        value = $this.data("value"),
        queryString;

    params[key] = value;
    queryString = jQuery.param(params);

    $(".subsection").attr("href", function () {
      // add or replace the query string as needed
      return this.href.replace(/(\?.*)?$/, "?" + queryString);
    });
  });
});

Note that this doesn't preserve fragments or any parameters that were already in the href, but not in the params object - but I'll leave that as an exercise.


Users model migration I have a model Users that each instance have 3 features. It was alright until I wanted to tell if this instance can do the 1st feature, the 3rd or all.

I did a migration:

class AddStatusFeaturesToUsers < ActiveRecord::Migration
  def change
    add_column :users, :status_feature_a, :boolean
    add_column :users, :status_feature_b, :boolean
    add_column :users, :status_feature_c, :boolean
  end
end

Is there a better way for doing this?

If your user and features model can have a many-to-many relationship, then you can use the following example:

  A has_and_belongs_to_many association creates a direct many-to-many
  connection with another model, with no intervening model

class CreateUsersAndStatusFeatures < ActiveRecord::Migration
  def change
    create_table :status_features do |t|
      t.string :name
      t.timestamps null: false
    end
    # Do not create users table if you already have.
    create_table :users do |t|
      t.string :name
      t.timestamps null: false
    end
    create_table :users_status_features, id: false do |t|
      t.belongs_to :user, index: true
      t.belongs_to :status_feature, index: true
    end
  end
end

class User < ActiveRecord::Base
  has_and_belongs_to_many :status_features
end
class StatusFeature < ActiveRecord::Base
  has_and_belongs_to_many :users
end

If you want to know in detail, go to this link.


Program to check if a date is valid or not I am a novice programmer and am currently learning Python programming by myself through a book. I stumbled upon a question in a chapter and it took me 2 days to figure out a solution for it. Although, it may not be an efficient code for the problem, I would be grateful if you guys could review it. Just to let everyone know of my Knowledge level on programming, I just have basic knowledge of functions, sequences and decision structure.

  Write a program to check if a date is valid or not? The date should be
  in the mm/dd/yyyy format.

  Pseudocode:

  Define 3 functions to check the validity of month,day and year, if valid return True for each function.
  input a string date separated by a "/" and assign it to a date variable.
  split the date variable to 3 separate variables.
  check if the 3 variables are True or not.
  Print if the date is valid or not.

I didn't take into account leap years where February has 29 days and also assumed the year to start at 1 A.D. to current years.

def monthcheck(month):
        if month > 0 and month <= 12: ## If month is between 1 and 12, return True.
            return True
        else:
            return False

def daycheck(month,day):
    monthlist1 = [1,3,5,7,8,10,12] ## monthlist for months with 31 days.
    monthlist2 = [4,6,9,11] ## monthlist for months with 30 days.
    monthlist3 = 2 ## month with month with 28 days.

    for mon in monthlist1: ## iterate through monthlist1.
        if month == mon: ## Check if the parameter month equals to any month with 31 days.
            if day >=1 and day <= 31: ## If the parameter day is between 1 and 31, return True.
                return True
            else:
                return False

    for mon in monthlist2: ## iterate through the monthlist with 30 days.
        if month == mon: ## check if the parameter month equals to any month with 30 days.
            if day >= 1 and day <= 30: ## if the parameter day is between 1 and 30,return True.
                return True
            else:
                return False

    if month == monthlist3: ## check if parameter month equals month with 28 days.
        if day >=1 and day <= 28: ## if the parameter day is between 1 and 28,return True.
            return True
        else:
            return False

def yearcheck(year):
    if len(year) >= 1 and len(year) <= 4: ## Check if year has between 1 to 4 numbers and return True.
        return True
    else:
        return False

def main():
    date = str(input("Enter the date in mm/dd/yyyy format: ")) ## Input date in the given format.

    month,day,year = date.split("/") ## split the date into 3 separate variables.

    monthvalidity = monthcheck(int(month))

    dayvalidity = daycheck(int(month),int(day))

    yearvalidity = yearcheck(year)

    if monthvalidity == True and dayvalidity == True and yearvalidity == True: ## check if all 3 variables are valid or True
        print("The date {0} is valid.".format(date))

    else:
        print("The date {0} is invalid.".format(date))

main()

Good work! While there is certainly room for improvement, it is good that you stuck it out and discovered a solution on your own.

def monthcheck(month):
    if month > 0 and month <= 12: ## If month is between 1 and 12, return True.
        return True
    else:
        return False

Can be more succinctly written as

def monthcheck(month):
    return 0 < month <= 12

While writing comments for practically every line of code can help you learn, and is a good way to keep track of where you are when working through pseudocode, don't just write what the code already told you.
PEP 8 - the style guide for Python - recommends using snake_case for naming functions, so monthcheck would become month_check.
What happens if I input "test" when asked for a date? The program crashes. It's a good idea to validate that any input from the user is in the format you expect.
The program asks for input in the format mm/dd/yyyy but will accept m/d/y, which probably isn't desired.
Don't compare explicitly to True / False

if monthvalidity == True and dayvalidity == True and yearvalidity == True:

Can be more easily read if it is written as

if monthvalidity and dayvalidity and yearvalidity

daycheck has several locations in which it could be improved. You could use a dictionary (as heather does), but if you haven't learned about those yet, you can still improve the algorithm.

Python's in operator can also check if a value is in a list, so instead of looping over monthlist1 and monthlist2, you can just check if month in monthlist1.
Since monthlist1 doesn't really describe what is in the variable, and I am having difficulty coming up with a better name, I would just inline the list with if month in [1, 3, 5, 7, 8, 10, 12] and add a clarifying comment.

Don't be afraid to use natural sounding variable names. I believe monthcheck is better named is_month_valid.
What about negative numbers? 1/1/-111 is apparently a valid date. This might be desired, but probably deserves a comment.
Converting numbers to strings to determine the size is generally not a good idea, and can get you into trouble in more complex cases with floats / doubles. I would prefer the check 1 <= year < 10000.
There is no need to have str() around an input function, input returns a string already.

Python, as a "batteries included" language, has a feature that can do this for us! In production code, this is what you should use.

from datetime import datetime

def main():
    user_input = input('Enter the date in mm/dd/yyyy format: ')

    try:
        datetime.strptime(user_input, '%m/%d/%Y')
        print('The date {} is valid.'.format(user_input))
    except ValueError:
        print('The date {} is invalid'.format(user_input))

if __name__ == '__main__':
    main()

This was rather long, but don't worry! It is completely normal to get a lot of feedback when you are starting out. The more you code, the more natural it becomes.


Factory design pattern with Shapes I'm making the very common example of Factory design pattern which creates a factory of shapes and return an instance of the type Shape. I wonder which is the best way to replace that bunch of nested if-else statements so that my code is more readable and maintainable.

I think there should exist a suitable design pattern to solve this issue but since I'm a beginner using design patterns I don't know where to start.

This is the code snippet for the getShape() method:

public Shape getShape(String type, String scope) {

    if (type == null) {
        return null;
    }
    if (type.equalsIgnoreCase("circle")) {
        if (scope.equalsIgnoreCase("singleton")) {
            return SingletonCircle.getInstance();
        }
        return new PrototypeCircle();
    } else if (type.equalsIgnoreCase("rectangle")) {
        if (scope.equalsIgnoreCase("singleton")) {
            return SingletonRectangle.getInstance();
        }
        return new PrototypeRectangle();
    } else if (type.equalsIgnoreCase("square")) {
        if (scope.equalsIgnoreCase("singleton")) {
            return SingletonSquare.getInstance();
        }
        return new PrototypeSquare();
    }
    return null;
}

Parameter checks

First of all, I'd be very strict about the check of the parameters: I do not see any use in gracefully trying to work around unset or illegal parameters instead of throwing an exception.

Thus, I recommend you start out with:

    Objects.requireNonNull(type);
    Objects.requireNonNull(scope);

These throw NullPointerException if the respective parameter is null. This is the clearest way to tell your user that this was unexpected. (If you use some kind of validation framework like javax.validation, you might even annotate the parameters as @NotNull).

Furthermore, I recommend IllegalArgumentException for illegal types instead of returning null.

DRY

Then, you repeat yourself with the check scope.equalsIgnoreCase("singleton") - you could refactor that into a variable.

switch

String comparisons can be done via a switch statement. To be case-insensitive, you can switch over toLowerCase of the string and use lowercase constants.

delegate to specific methods

Last advice: move the concrete object creation to specific methods for objects, and simply call the sub-method from the main method.

Putting it all together

public Shape getShape(String type, String scope) {
    Objects.requireNonNull(type);
    Objects.requireNonNull(scope);
    boolean isSingleton = scope.equalsIgnoreCase("singleton");
    switch(type.toLowerCase()) {
    case "circle":    return getCircle(isSingleton);
    case "rectangle": return getRectangle(isSingleton);
    case "square":    return getSquare(isSingleton);
    default: throw new IllegalArgumentException("unknown type");
    }
}

private Shape getCircle(boolean inSingletonScope) {
    return inSingletonScope
        ? SingletonCircle.getInstance()
        : new PrototypeCircle();
}

...


Is my approach structure good? The index file:

<?php
require_once 'app/run.php';

$_action = (isset($_GET['cmd']) && ctype_alnum($_GET['cmd'])) ? $_GET['cmd'] : 'index';

switch ($_action) {

    case 'index':
    case 'default':
    case 'home':
        include $con_dir . '/index.php';
    break;
    default:
        include $con_dir . '/404.php';

}

The run.php file:

<?php
session_start();

require_once 'config.php';

$app_dir = realpath(dirname(__FILE__));
$lib_dir = $app_dir . '/lib';
$con_dir = $app_dir . '/controllers';

require_once($app_dir . '/lib.php'); //Include libraries

//Database Connection
try {
  # MySQL with PDO_MYSQL
  $db = new PDO("mysql:host=$hostname;dbname=$database", $username, $password);
}
catch(PDOException $e) {
    echo $e->getMessage();
}
unset($password);

//Smarty
$tpl = new Smarty();
$tpl->template_dir = $app_dir . '/Smarty/templates/';
$tpl->compile_dir  = $app_dir . '/Smarty/templates_c/';
$tpl->config_dir   = $app_dir . '/Smarty/configs/';
$tpl->cache_dir    = $app_dir . '/Smarty/cache/';

The lib.php file

<?php
require_once($lib_dir . '/Smarty/Smarty.class.php');

The config.php file

<?php
$hostname = 'localhost';
$username = 'root';
$password = '';
$database = 'engine';

I think I can improve  this by doing the switch() more dynamic, let's say it, would it be possible to have that the ?cmd=filename would be appropriate, how would I do it? And how do I do it secure?

Else way what do you think about my basic structure, how should improve it? What would you change etc.

You code looks fine. One minor issue: in the catch(PDOException $e) branch maybe you want to

log the error (error_log),
show a fancy static error page to the user ("It's not your fault, we have logged the error and we're working on it." or something like this.),
and/or call die().

About the dynamic switch: I would use a whitelist array which prohibit including other files which are not in the array.

$pages['index'] = 'index.php';
$pages['default'] = 'default.php';
$pages['home'] = 'home.php';
$page = '/404.php'
if (isset($pages[$action])) {
    $page = $pages[$action];
}

include $con_dir . '/' . $page;


Small Python class for Lindenmayer Systems L-systems are basically rules for recursively rewriting a string, which can be used to characterize e.g. some fractal and plant growth.

I wrote a small class to represent deterministic L-systems and used it for two examples. Any comments would be greatly appreciated, especially about the class design, the structure of the second example, and how to make things more pythonic. I'm new to Python and don't have any training in "grammars", this is just a hobby.

The class LSystem.py:

class LSystem:
    """ Lindenmayer System
    LSystem( alphabet, axiom )
      axiom: starting "string", a list of Symbols
      rules: dictionary with rules governing how each Symbol evolves,
             keys are Symbols and values are lists of Symbols
    """
    def __init__(self, axiom, rules):
        self.axiom = axiom
        self.rules = rules

    """ Evaluate system by recursively applying the rules on the axiom """
    def evaluate(self,depth):
        for symbol in self.axiom:
            self.evaluate_symbol( symbol, depth )

    """ Recursively apply the production rules to a symbol """
    def evaluate_symbol( self, symbol, depth ):
        if depth <= 0 or symbol not in self.rules:
            symbol.leaf_function()
        else:
            for produced_symbol in self.rules[symbol]:
                self.evaluate_symbol( produced_symbol, depth - 1 )

class Symbol:
    """ Symbol in an L-system alphabet
    Symbol( leaf_function )
      leaf_function: Function run when the symbol is evaluated at the final
                     recursion depth. Could e.g. output a symbol or draw smth.
    """
    def __init__(self, leaf_function ):
        self.leaf_function = leaf_function

Example: Algae growth (example 1 from the wikipedia article)

import LSystem

# define symbols. their "leaf function" is to print themselves.
A = LSystem.Symbol( lambda:print('A',end='') )
B = LSystem.Symbol( lambda:print('B',end='') )
# define system
algae_system = LSystem.LSystem(
    axiom = [A],
    rules = { A: [A,B], B: [A] }
)
# run system
algae_system.evaluate(4) # prints "ABAABABA"

Example: Draw a Koch snowflake (as in my previous question)

import LSystem
import pygame
from math import pi, sin, cos

# some constants
WINDOW_SIZE = [300,300]
LINE_WIDTH = 1
LINE_LENGTH = 1

# global variables for "turtle drawing"
# maybe I should pass around a turtle/cursor object instead?
turtle_angle = 0
turtle_x = 0
turtle_y = WINDOW_SIZE[1]*3/4

# define drawing functions used to draw the Koch snowflake
def draw_forward():
    global turtle_angle, turtle_x, turtle_y
    start = [turtle_x, turtle_y]
    turtle_x += LINE_LENGTH * cos(turtle_angle)
    turtle_y += LINE_LENGTH * sin(turtle_angle)
    end = [turtle_x, turtle_y ]
    pygame.draw.line(window, pygame.Color('black'), start, end, LINE_WIDTH )
def turn_left():
    global turtle_angle
    turtle_angle += pi/3
def turn_right():
    global turtle_angle
    turtle_angle -= pi/3

# symbols in the L-system
Line = LSystem.Symbol( draw_forward )
Left = LSystem.Symbol( turn_left )
Right = LSystem.Symbol( turn_right )

# L-system axiom and rules
koch_curve_system = LSystem.LSystem(
    axiom = [ Line, Right, Right, Line, Right, Right, Line ],
    rules = { Line: [ Line, Left, Line, Right, Right, Line, Left, Line ] }
)

# init pygame
pygame.init()
window = pygame.display.set_mode(WINDOW_SIZE)
window.fill(pygame.Color('white'))

# evaluate the L-system, which draws the Koch snowflake
# (recursion depth was chosen manually to fit window size and line length)
koch_curve_system.evaluate(5)

# display
pygame.display.flip()

# wait for the user to exit
while pygame.event.wait().type != pygame.QUIT:
    1

This is a neat implementation of Lindenmayer systems. I have some suggestions for simplifying and organizing the code.

The docstring for a method or a function comes after the def line (not before, as in the code here). So you need something like:

def evaluate(self, depth):
    """Evaluate system by recursively applying the rules on the axiom."""
    for symbol in self.axiom:
        self.evaluate_symbol(symbol, depth)

and then you can use the help function from the interactive interpreter:

>>> help(LSystem.evaluate)
Help on function evaluate in module LSystem:

evaluate(self, depth)
    Evaluate system by recursively applying the rules on the axiom.

The Symbol class is redundant — it only has one attribute, and doesn't have any methods other than the constructor. Instead of constructing Symbol objects, you could just use functions:

def A():
    print('A', end='')

def B():
    print('B', end='')

and instead of calling symbol.leaf_function(), you could just call symbol().

In the Koch example, you already have functions so you can just omit the construction of the Symbol objects and write:

koch_curve_system = LSystem(
    axiom = [draw_forward, turn_right, turn_right, draw_forward, turn_right,
             turn_right, draw_forward],
    rules = {
        draw_forward: [draw_forward, turn_left, draw_forward,
                       turn_right, turn_right, draw_forward, turn_left,
                       draw_forward],
    }
)

Alternatively, you could rename the functions and leave the definition of the system unchanged.
The code in evaluate is very similar to the code in evaluate_symbol. This suggests that it would result in simpler code if you described the Lindemayer system in a different way, giving an initial symbol instead of an initial list of symbols. (And possibly giving an extra rule mapping the initial symbol to a list.)

If you try this, then you'll find that the LSystem class is redundant too: the only thing you can do with it is to call its evaluate method, so you might as well just write it as a function:

def evaluate_lsystem(symbol, rules, depth):
    """Evaluate a Lindenmayer system.

    symbol: initial symbol.
    rules: rules for evolution of the system, in the form of a
        dictionary mapping a symbol to a list of symbols. Symbols
        should be represented as functions taking no arguments.
    depth: depth at which to call the symbols.

    """
    if depth <= 0 or symbol not in rules:
        symbol()
    else:
        for produced_symbol in rules[symbol]:
            evaluate_lsystem(produced_symbol, rules, depth - 1)

Then the algae example becomes:

evaluate_lsystem(A, {A: [A, B], B: [A]}, 4)

In the snowflake example, there is persistent shared state (the position and heading of the turtle). When you have persistent shared state it makes sense to define a class, something like this:

class Turtle:
    """A drawing context with a position and a heading."""
    angle = 0
    x = 0
    y = WINDOW_SIZE[1]*3/4

    def forward(self, distance):
        """Move forward by distance."""
        start = [self.x, self.y]
        self.x += distance * cos(self.angle)
        self.y += distance * sin(self.angle)
        end = [self.x, self.y ]
        pygame.draw.line(window, LINE_COLOR, start, end, LINE_WIDTH)

    def turn(self, angle):
        """Turn left by angle."""
        self.angle += angle

and then:

turtle = Turtle()
forward = lambda: turtle.forward(1)
left = lambda: turtle.turn(pi/3)
right = lambda: turtle.turn(-pi/3)
initial = lambda: None
rules = {
    initial: [forward, right, right, forward, right, right, forward],
    forward: [forward, left, forward, right, right, forward, left, forward],
}
evaluate_lsystem(initial, rules, 5)


Nested trial and error in if-else condition Using Rails 3.2, I have the following:

g = Geocoder.search(address)

# if no address found, geocode with latitude and longitude
if g.empty?
  g = Geocoder.search(latitude, longitude)
  if g.empty?
    log.info "Geocode failed for #{latitude}, #{longitude}"
  end
else
  ---save---
end

If the first geocoding with address fails, it will try latitude and longitude. If still fails, then it logs error, else it will save.

Is this a good way to write this code?

Some notes:

g: use meaningful names.
Don't mix code with parentheses and without.
Check the pattern Object#presence.
# if no address found, ...: declarative code makes this kind of comments unnecessary.

I'd write:

results = Geocoder.search(address).presence || Geocoder.search(latitude, longitude)
if results.present?
  # save
else
  log.info("Geocode failed for addr='#{search}' and coords=#{latitude}/#{longitude}")
end


Summing CSV files row by row using Node.js events I need to read multiple CSV files that have the same number of rows, and compute some results for each row. For simplification purposes, and in the scope of this code review, this computation would be "sum each column of each row".

I would like to be able to do so using the fast-csv parser that triggers events on each row reading. Mostly to avoid having to load the entire file into variables and then compute what I need.

I came up with the following solution, with a dummy example to sum the values from each file. It seems to be working, but because I'm working with events, I'm not sure if there might be a case when some data could go missing, or if there's any other issue. I would appreciate any help to improve performance, functionality, maintainability, readability, etc..

Because executing the code requires an external dependency and some files, here's a Repl.it.

Here's the code if you just want to read:

const csv = require("fast-csv");
const fs = require('fs');
const EventEmitter = require('events');

const files = ['csv/file1.csv', 'csv/file2.csv'];

// Create parsers for each file
const parsers = files.map(file => csv.parseStream(fs.createReadStream(file), { delimiter: ';', headers: false }));

// Variables to hold temporary data and definitive results
const data = parsers.map(() => []);
let result = [];

// Handler for a set of the same row from all the different files
var eventEmitter = new EventEmitter();
eventEmitter.on('allRows', (rows) => {
  var sum = (r, a) => r.map((b, i) => Number(a[i]) + Number(b));
  result.push(rows.reduce(sum));
});

// Handler to display the result once all files have been read
eventEmitter.on('end', (rows) => {
  console.log("result", result);
});

// Handler for each row reading
const onDataHandler = function(row, idx) {
  data[idx].push(row);
  // When we have a row parsed for each file, we emit a signal
  if (data.reduce((acc, el) => acc && el.length > 0, true)) {
    if (data.reduce((acc, el) => acc && el[0] === null, true)) {
      // 'end' if all results have been shifted
      eventEmitter.emit('end');
    } else {
      // 'allRows' otherwise, with the first element of each file
      eventEmitter.emit(
        'allRows',
        data.reduce((acc, el) => {
          acc.push(el.shift());
          return acc;
      }, [])
      );
    }
  }
}

// We use readable to use 'flowing' mode and make sure we don't miss the last rows
parsers.forEach((parser, idx) => parser.on('readable', () => {
  onDataHandler(parser.read(), idx);
}));

If you want to check whether every item in an array passes a test, you should use .every for that, not .reduce. Eg

data.reduce((acc, el) => acc && el.length > 0, true)

can be switched with

data.every(arr => arr.length)

and

data.reduce((acc, el) => acc && el[0] === null, true)

can be switched with

data.every(arr => arr[0] === null)

Also, when you want to transform one array into another, .map is the right method, not .reduce. Or, when you just want to copy an array exactly, use .slice() or spread it. Change

data.reduce((acc, el) => {
  acc.push(el.shift());
  return acc;
}, [])

to

[...data]

On a broader note, I would consider the logic to be a lot easier to follow if it was separated into two parts: one part to read the files and deal with the asynchronicity, and another part to (synchronously) combine into a single array. I think mixing the two and using eventEmitter makes things more difficult to understand than they need to be.

A somewhat minor issue is that you're requiring all rows of a particular index to be kept in memory until they've all been parsed. In the case that you're dealing with a large number of files, or the rows are large, it would be better to combine them as soon as possible, as they come in, so that the only persistent data structure is the output array of arrays (rather than an array of arrays for each separate .csv).

When any row is parsed, you can call a function that combines it with the results array. You can also map each parseStream to a Promise that resolves once its end event fires, and use Promise.all to log the final results array once all parsers are completed:

const result = [];
const combineRow = (row, rowIndex) => {
  const targetRow = result[rowIndex];
  if (!targetRow) {
    result[rowIndex] = row;
    return;
  }
  row.forEach((cell, cellIndex) => {
    targetRow[cellIndex] = (targetRow[cellIndex] || 0) + cell;
  });
}
const getRows = parser => new Promise((resolve, reject) => {
  let rowIndex = 0;
  parser.on('readable', () => {
    const row = parser.read();
    if (row) {
      combineRow(row.map(Number), rowIndex);
      rowIndex++;
    }
  });
  parser.on('end', resolve);
});

Promise.all(parsers.map(getRows))
  .then(() => {
    console.log(result);
  });

https://repl.it/repls/EnchantedElaborateFiber


Format string processing similar to date or printf() I wrote a function that is supposed to take a format string similar to what date accepts and replace all format specifiers with strings provided by a callback function.

Format specifiers are sequences of a % plus another character; %% for a literal %
Replace the format specifier with the string provided by the callback function
If the callback function returns NULL, no replacement should take place (leave the specifier in)
The callback function receives the character following % as argument
An optional pointer can be passed through to the callback function
If the last character of the format string is %, it should be left as-is
The result of the processing is to be placed in the provided buffer
If the provided buffer isn't big enough, simply stop processing
Always null-terminate the provided buffer, even if it isn't big enough

The function works fine as far as I can tell. I'm looking for any feedback I can get.
char*
format(const char* format, char *buf, size_t len, char* (*cb)(char c, void* ctx), void *ctx)
{
    const char *curr;  // current char from format
    const char *next;  // next char from format

    size_t i = 0;      // index into buf
    char *ins = NULL;  // string to insert

    // iterate `format`, abort once we exhaust the output buffer
    for (; *format && i < (len-1); ++format)
    {
        curr = format;
        next = format+1;

        if (*curr == '%' && *next)
        {
            if (*next == '%') // escaped %, copy it over and skip
            {
                buf[i++] = *format++;
                continue;
            }
            if ((ins = cb(*next, ctx))) // get string to insert
            {
                // copy string, again aborting once buffer full
                while (*ins && i < (len-1))
                {
                    buf[i++] = *ins++;
                }
                ++format;
                continue;
            }
        }

        // any other character, just copy over
        buf[i++] = *curr;
    }

    // null terminate
    buf[i] = '\0';
    return buf;
}

Examples for (tested) input and output, assuming the callback function always returns FOO:

(empty string): (empty string)
%: %
%%: %
%f: FOO
%%f: %f
%%%f: %FOO
%%%%f: %%f

If so requested, I can provide a link to the context/project where this is used.

Return the number of bytes written to buf
You will notice that functions like sprintf() and strftime() don't return a pointer, but rather an integer that says something about the number of bytes that (would) have been written to the output buffer. This is much more useful than just copying the pointer to buf, which doesn't give the caller any new information.
Where is the string returned by the callback function allocated?
The callback function returns a pointer to a string. But where is this allocated? Your format() function doesn't call free(), so either the string should be stored in some statically allocated array, or it is allocated on the heap. In the former case, unless you return a pointer to a string literal, your format() function can only be used from one thread at a time. If you return memory that is allocated on the heap, then you have to keep track of it so the caller can clean up all the allocated memory once format() returns.
Consider having the callback function write into buf directly
To solve the above issue, and to avoid an unncessary copy, you can pass a pointer into the buffer and the remaining size to the callback function, and have the callback function write directly to the buffer. For example:
char*
format(const char* format, char *buf, size_t len, size_t (*cb)(char c, void* ctx, char *buf, size_t len), void *ctx) {
    ...
        if (*curr == '%' && *next)
        {
            if (*next == '%') // escaped %, copy it over and skip
            {
                buf[i++] = *format++;
                continue;
            }
            i += cb(*next, ctx, buf + i, len - i - 1);
            ++format;
            continue;
        }
    ...
}

And then your callback function can look like:
size_t example_cb(char c, void *ctx, char *buf, size_t len) {
    if (c == 'f') {
        if (len > 3)
            len = 3;
        memcpy(buf, "FOO", len);
        return len;
    }

    return 0;
}

You can create a helper function to avoid repeating the above construction, and to safely write any string to the buffer:
size_t emplace_string(const char *str, char *buf, size_t max_len) {
    size_t len = strlen(str);
    if (len > max_len)
        len = max_len;
    memcpy(buf, str, len);
    return len;
}

size_t example_cb(char c, void *ctx, char *buf, size_t len) {
    switch (c) {
    case 'f':
        return emplace_string("FOO", buf, len);
    case 'B':
        return emplace_string("bar", buf, len);
    ...
    default:
        return 0;
    }
}


Create `suffixes` Function on List I wrote the following suffixes function. Given a list, it returns the list + all sub-lists.

  suffixes [1,2,3,4] == [[1,2,3,4], [2,3,4], [3,4], [4], []]

Please critique my implementation.

suffixes :: [a] -> [[a]]
suffixes []         = [] : []
suffixes xxs@(_:xs) = xxs : suffixes xs

Looks good to me. The only thing I would change is

suffixes [] = [] : []

to

suffixes [] = [[]]

as it's a bit more readable.

When re-inventing functions, it can be instructive to look up their definition using Hoogle. Following the links, we find this definition in Data.List:

tails                   :: [a] -> [[a]]
tails xs                =  xs : case xs of
                                  []      -> []
                                  _ : xs' -> tails xs'


Binary Option Pricing using Card Game I shuffle a deck of cards(with an equal number of red and black
  cards) and start dealing them face up.
  After any card you can say “stop”, at which point I pay you $1 for
  every red card dealt and you pay me $1 for every black card dealt.
  What is your optimal strategy, and how much would you pay to play
  this game ?

I'd be very happy if you could judge the quality of the code.

using namespace std;

int main(int argc, char * const argv[])
{

    BinaryOptionPricing bop;
    int total_number_of_cards;

    sscanf_s(argv[1], "%d", &total_number_of_cards);
    cout << "Total Number of Cards = " << total_number_of_cards << endl;

    bop.set(total_number_of_cards);
    cout << "Value of the game = " << bop.get() << endl;

    return 0;
}

class BinaryOptionPricing
{

private: int red;
private: int black;
private: std::map<std::pair<int, int>, double> Cache;
private: typedef int card;
private: typedef double redValue;
private: typedef double blackValue;
private: typedef double gameValue;

private:
    double value(card red, card black)
    {
        redValue rValue;
        blackValue bValue;
        gameValue maxValue;

        if (0 == red)
            return ((double)black);
        if (0 == black)
            return (0);

        auto searchPair = Cache.find(std::make_pair(red, black));

        if (searchPair != Cache.end())
            return (searchPair->second);

        rValue= ((double)red / (red + black)) * value(red - 1, black);
        bValue = ((double)black / (red + black)) * value(red, black - 1);

        maxValue = std::max((rValue + bValue), (double)(black - red));
        Cache.insert(std::make_pair(make_pair(red, black), maxValue));

        return maxValue;
    }

public:
    double get()
    {
        return value(red, black);
    }

public:
    void set(int totalNumberOfCards)
    {
        this->red = (totalNumberOfCards/2);
        this->black = (totalNumberOfCards / 2);
    }
};

Your code has many, many stylistic issues. Some of them seem to be idioms you've invented coming from a Java background; some of them may be due to not really understanding how OOP classes work.

using namespace std;

int main(int argc, char * const argv[])
{

    BinaryOptionPricing bop;

You're missing a bunch of #include lines.
Prefer char * const *argv over char * const argv[] because you don't have to understand array-to-pointer decay to read it.
const-qualifying *argv here is incorrect, or at least the compiler is within its rights to reject it. The correct prototype for main is int main(int argc, char **argv) with no consts.
using namespace std; is frowned upon in C++; prefer to write out std::vector and so on, since that's what you'll be doing once you enter the workforce (or wherever you end up coming in contact with other programmers).
BinaryOptionPricing is undefined at this point. You need to provide the definition of this class before you try to use it.

class BinaryOptionPricing
{

private: int red;
private: int black;
private: std::map<std::pair<int, int>, double> Cache;
private: typedef int card;
private: typedef double redValue;
private: typedef double blackValue;
private: typedef double gameValue;

C++ isn't Java; you don't have to write public static void main and so on. In C++, public: and private: (and protected:) act as a kind of "section header" introducing a whole bunch of public or private (or protected) members at once. You should write the above code as

class BinaryOptionPricing
{
  private:
    int red;
    int black;
    std::map<std::pair<int, int>, double> Cache;

    using card = int;
    using redValue = double;
    using blackValue = double;
    using gameValue = double;

Notice that private: is the default inside a class, so you don't even need the line private: there; I just put it so you'd see how you could do it. Also notice that I'm using the C++11 using syntax for type aliases, instead of the old C/C++03 typedef syntax — the latter is still perfectly legal and supported, but the former syntax is becoming the de facto standard because it supports things like dependent type names and alias templates whereas the typedef syntax is basically an evolutionary dead end in those respects.

I would advise against using the typedef name card to refer to an integer count of cards. You could say cards or cardCount or NumberOfCards instead... but really, there's nothing wrong with using int directly. It saves you some typing, and clearly indicates what's going on in the code (namely, an integer count).

private:
    double value(card red, card black)
    {
        // ...
    }

public:
    double get()
    {
        return value(red, black);
    }

Here is where I say you may be fundamentally misunderstanding how classes work. Your class BinaryOptionPricing already has data members ("member variables") named red and black. You wrote a member function get() that grabs the value of those members and passes them as function parameters to another member function value(). Inside value, whenever you refer to red, you're referring to the function parameter red, which shadows the data member red defined in the class itself. Fortunately for you, the function parameter has the same value as the data member that it's shadowing... well, at least on the initial recursive call... but this is a very awkward way to write code! If you don't want access to your class's data members at all, you should make value(int, int) a static member function:

static double value(int red, int black)
{
    // ...
}

double get() const
{
    return value(this->red, this->black);
}

void set(int totalNumberOfCards)
{
    this->red = (totalNumberOfCards/2);
    this->black = (totalNumberOfCards / 2);
}

This looks like you want a constructor.

explicit BinaryOptionPricing(int totalNumberOfCards) :
    red(totalNumberOfCards/2), black(totalNumberOfCards/2)
{}

Even better, you should consider just not using OOP at all for this problem; there's no reason the member variables red and black really need to exist here. You could just write

double value_of_game(int red, int black)
{
    if (red == 0 || black == 0) return black;
    double total = red + black;
    double result = (red / total) * value_of_game(red-1, black)
                  + (black / total) * value_of_game(red, black-1);
    return std::max<double>(result, black - red);
}

and it would work just fine, modulo the lack of memoization making it slow.

Notice that you don't need (and shouldn't put) parentheses around your return expressions; that you're overusing explicit cast notation (frowned upon in C++); and that in C++ it is idiomatic to initialize your local variables and then rarely-if-ever change their initial values, rather than defining them uninitialized at the top of the function and then giving their "real" values farther down in the code.

So let's look at your memoization code.

    auto searchPair = Cache.find(std::make_pair(red, black));
    if (searchPair != Cache.end()) {
        return searchPair->second;
    }

    // ...

    Cache.insert(std::make_pair(make_pair(red, black), maxValue));

Good use of C++11 auto to avoid typing out the name of the iterator type!
Instead of insert, you could use Pythonesque syntax:

    Cache[make_pair(red, black)] = maxValue;

or even

    Cache[{red, black}] = maxValue;

although the latter is a bit tricksy so don't ask me to explain it. :)

Putting that all together, we get something along the lines of

double value_of_game(int red, int black)
{
    static std::map<std::pair<int,int>, double> cache;
    if (red == 0 || black == 0) return black;
    auto it = cache.find({red, black});
    if (it != cache.end()) return it->second;
    double total = red + black;
    double result = (red / total) * value_of_game(red-1, black)
                + (black / total) * value_of_game(red, black-1);
    result = std::max<double>(result, black - red);
    cache[{red, black}] = result;
    return result;
}

And this is where we might want to reintroduce a "noun" (a class) into our program, because maybe we want the ability to call value_of_game(x,y) from two different threads simultaneously and we don't want race conditions on cache. So we could do

class BinaryOptionPricing {
    std::map<std::pair<int,int>, double> cache;

  public:
    double value_of_game(int red, int black)
    {
        if (red == 0 || black == 0) return black;
        auto it = cache.find({red, black});
        if (it != cache.end()) return it->second;
        double total = red + black;
        double result = (red / total) * value_of_game(red-1, black)
                    + (black / total) * value_of_game(red, black-1);
        result = std::max<double>(result, black - red);
        cache[{red, black}] = result;
        return result;
    }
};

Alternatively we could directly expose the memoization mechanism to the caller:

using value_of_game_cache_t = std::map<std::pair<int,int>, double>;

double value_of_game(int red, int black, value_of_game_cache_t& cache)
{
    if (red == 0 || black == 0) return black;
    auto it = cache.find({red, black});
    if (it != cache.end()) return it->second;
    double total = red + black;
    double result = (red / total) * value_of_game(red-1, black, cache)
                + (black / total) * value_of_game(red, black-1, cache);
    result = std::max<double>(result, black - red);
    cache[{red, black}] = result;
    return result;
}

Notice that the second alternative here is just a very thin refactoring of the first alternative: we've merely renamed BinaryOptionPricing to value_of_game_cache_t and passed it as an explicit function parameter instead of an implicit this parameter.

BinaryOptionPricing bop;
printf("EV is %g\n", bop.value_of_game(26, 26));  // first approach

value_of_game_cache_t bop;
printf("EV is %g\n", value_of_game(26, 26, bop));  // second approach

The advantage of the first (class-based) approach, in this context, is that I could write my call as a one-liner and it would be just as fast and just as thread-safe as the alternative: the best of both worlds.

printf("EV is %g\n", BinaryOptionPricing().value_of_game(26, 26));


Merge 3 cells in 1 with VBA The below code is working uses formula to concatenate the result from Range("K:M") and write the result in column H.

Is there a better way to write the below code that can concatenate the result to the exact location as per below table? Please note that data in the before table is discontinued but sometimes will be from row 4 to 1000 rows down or the below example.

Sub GenerateStyleFabricColourV4()
    Dim srcData As Range
    Dim rowNum As Long, lastRow As Long

    With Worksheets(2)
        lastRow = Application.Max(4, _
                    .Cells(.Rows.Count, "K").End(xlUp).Row, _
                    .Cells(.Rows.Count, "L").End(xlUp).Row, _
                    .Cells(.Rows.Count, "M").End(xlUp).Row)
        With .Cells(4, "H").Resize(lastRow - 4 + 1, 1)
            .FormulaR1C1 = "=rc[3]&rc[4]&rc[5]"
            .Value = .Value2
        End With
    End With
End Sub

Sub ClearAllData()

Rows("4:" & Rows.Count).ClearContents

End Sub

To answer another question I provided some helpful information about optimization in vba here, VBA syntax that compiles sub data into master sheet.
This maybe help you to skip the blank rows (You did not clarify your question but based on the data you provided I think that would be your problem) and find the actual last row used. It is in reference to this post. Here's the code:
Dim srcData As Range, rng As Range
Dim rowNum As Long, lastRow As Long
Dim wsh As Worksheet

Sub GSFC_V4()
 Set wsh = Worksheets(2)

 wsh.Activate
 Set srcData = Range("K:M")
 Set rng = Range("K4")

 With wsh
  If Application.WorksheetFunction.CountA(.Cells) <> 0 Then
     lastrow = srcData.Find(What:="*", _
                  After:=rng, _
                  Lookat:=xlPart, _
                  LookIn:=xlFormulas, _
                  SearchOrder:=xlByRows, _
                  SearchDirection:=xlPrevious, _
                  MatchCase:=False).Row
 Else
     lastrow = 1
 End If

     With .Cells(4, "H").Resize(lastRow - 4 + 1, 1)
         .FormulaR1C1 = "=rc[3]&rc[4]&rc[5]"
         .Value = .Value2
     End With
 End With

End Sub

This is what I am using to optimize my macros. I call OptimizeVBA True at the beginning of each macro and at the end call OptimizeVBA False to set everything back to default. It increases the performance remarkably.
Sub OptimizeVBA(isOn As Boolean)

  Application.Calculation = IIf(isOn, xlCalculationManual, xlCalculationAutomatic)
  Application.EnableEvents = Not (isOn)
  Application.ScreenUpdating = Not (isOn)
  Application.StatusBar = Not (isOn)

End Sub

You need to be careful of how you refer to ranges to avoid getting errors.


Does this code fall into a design pattern? CustomDialog is the parent abstract class for OkExclamationDialog and ExclamationDialog with abstract method displayDialog() being overriden.

Does this fall into a design pattern?  Can this design be improved upon?

JudgeButton controls the other classes.

public class JudgeButton {
    private int typeCode;

    private void initialise(){
        setTypeCode();
        if(typeCode == 0){
            c = new ExclamationDialog("Episode not started");
            c.displayDialog();
        }
        else if(typeCode == 1){
            c = new OkExclamationDialog("Episode Finished, you will now be redirected");
            c.displayDialog();
        }
    }

    private void setTypeCode(){
        wrapperJsonObject = Utils.getCurrentWrapperJsonObjectFromServer();
        jsonObject = wrapperJsonObject.getJsonObject();
        NextRatingsScreen n = new NextRatingsScreen(jsonObject);
        RatingsCountDownTime r = new RatingsCountDownTime();

        if(!n.isEpisodeStarted()){
            typeCode = 0;
        }
        else if(!n.isPerformanceStarted()){
            typeCode = 1;
        }
    }
}

public class OkExclamationDialog extends CustomDialog{
    public OkExclamationDialog(String text) {
        super(text);
    }

    public void displayDialog() {
        DialogType dialogType = new DialogType();
        DialogBuilder dialogBuilder = new DialogBuilder();
        AbstractDialogFactory abstractDialogFactory = dialogType.getDialogType(DialogEnum.EXCLAMATION_DIALOG);
        Dialog nextScreen = dialogBuilder.buildDialog(abstractDialogFactory, super.getText());
        int result = nextScreen.doModal();
        ScreenController.displayNextScreenFadeTransition(nextScreen);

        if(result==Dialog.OK)
        {
            ScreenController.displayNextScreenFadeTransition(new ContestantsScreen());
        }
    }
}

public class ExclamationDialog extends CustomDialog{
    public ExclamationDialog(String text) {
        super(text);
    }

    public void displayDialog() {
        DialogType dialogType = new DialogType();
        DialogBuilder dialogBuilder = new DialogBuilder();
        AbstractDialogFactory abstractDialogFactory = dialogType.getDialogType(DialogEnum.EXCLAMATION_DIALOG);
        Dialog nextScreen = dialogBuilder.buildDialog(abstractDialogFactory, super.getText());

        ScreenController.displayNextScreenFadeTransition(nextScreen);
    }
}

public abstract class CustomDialog {
    private String text;

    public CustomDialog(String text){
        this.text = text;
    }

    public String getText(){
        return this.text;
    }

    public abstract void displayDialog();
}

The initialise reminds me a factory but it's not exactly a factory. It's fine if you don't use the same if-elseif structure in your code elsewhere. If you do pull out it to a factory class.

Just some idea:

1, Consider using an enum instead of the integer typeCode (eliminate magic numbers).

2, Maybe you should throw an IllegalStateException in the end of the initialise method with a message "Invalid typecode".

3, The same is true for the setTypeCode() method. Or set explicitly typeCode to 0 in the last line of the method if 0 is a valid value. (This is the implicit default value of the private int typeCode field.)

4, I'd rename the setTypeCode() to calculateTypeCode() which would return a TypeCode enum. Now it's more or less temporal coupling.

5, You should create a new getNextScreen() method in the CustomDialog class with the first four line of the displayDialog() method. Then call the getNextScreen() from the displayDialog() methods. It would remove some code duplication.

// CustomDialog class code
public Dialog createNextScreen() {
    DialogType dialogType = new DialogType();
    DialogBuilder dialogBuilder = new DialogBuilder();
    AbstractDialogFactory abstractDialogFactory =
        dialogType.getDialogType(DialogEnum.EXCLAMATION_DIALOG);
    Dialog nextScreen = dialogBuilder.buildDialog(abstractDialogFactory, text);
    return nextScreen;
}

// OkExclamationDialog, ExclamationDialog class code
public void displayDialog() {
    Dialog nextScreen = getNextScreen();
    ...
    ScreenController.displayNextScreenFadeTransition(nextScreen);
}


PHP Regex Input Sanitization This helper class is supposed to validate the user input from a text based wifi login form.

The entire process is basically this:

I use the create_voucher function of this API passing voucher_duration to set the amount of time for which the voucher is going to be valid and $this->clean['name']." ".$this->clean['surname'] as a note so I can later identify which device belongs to which user.

The code returned by create_voucher is then sent to the passed e-mail-adress using php-mailer so the user can login and use the wifi.

I am particularly unhappy with my error handling and would like to know if you see any obvious ways to break the code or inject malicious code.

class sanitizer
{
    public $clean;
    private $post=null;
    private $reg_email ='/^\S+@\S+\.\S+$/';        //Just some basic checking
    private $reg_name = '/^[\'\p{L} -]+[\n]?$/im'; //Allowing some wierd names
    private $reg_number='/^[[:digit:]]*$/im';      //A single integer no fuzz

    public function __construct($post){
         $this->post=$post;
     }

    private function sanitize($key, $regex){
       if (preg_match($regex, $this->post[$key])) {
           $this->clean[$key]= $this->post[$key];
       } else {
           $this->clean[$key]=null;
       }
   }

    public function clean_up(){
        if (isset($this->post['smt_sent'])) {
            if ($this->post['smt_sent']==1) {
                $this->sanitize('name', $this->reg_name);
                $this->sanitize('surname', $this->reg_name);
                $this->sanitize('voucher_duration', $this->reg_number);
                if ($this->post['voucher_duration'] > 0 && ($this->post['voucher_duration']/60 > 48)) {
                    $this->clean['duration']=null;
                }
                $this->sanitize('email_own', $this->reg_email);
                $this->clean['smt_received']=1;

                $this->clean['error'] = false; //No errors yet

                foreach ($this->clean as $field) { //Loop trough each field
                    if (!isset($field)) {
                        $this->clean['error'] = true; //Yup there are errors
                    }
                }
                return $this->clean;
            }
        }
    }
}

I think you need to think about inverting control here or you will find limited re-use in this class.  Why would you pass a specifically-formatted POST request here to this class in order to execute validation vs. having calling code just reference methods on this class that execute the validation logic?

For example:

// somewhere outside class
$valid = sanitizer::validateEmail($_POST['email_own']);

By doing this you can make a class like this more re-usable, in that it is not built just to handle POST data validation for a single script.

There is a distinct difference between sanitization and validation. You seem to be using the concepts interchangeably. I would say this class is a validation class not a sanitization class.

From looking at the rules in your class, I actually question if there is really much value here, as really the only validation rule you have here that could not be better fulfilled by built-in PHP validation functionality is you name validation.

You could just as easily use filter_input() or filter_input_array() here.  For example:

// not shown - checking of validation results
$email = filter_input(INPUT_POST, 'email_own', FILTER_VALIDATE_EMAIL);
$voucherDuration = filter_input(INPUT_POST, 'voucher_duration', FILTER_VALIDATE_INT);

This might pretty much eliminate your need for this class altogether.

Some other code review thoughts:

You should strive to use meaningful and specific names for classes, methods/functions, variables, etc. sanitizer as a class name here is very generic, especially since this class has a VERY specific use case that it supports. sanitize() as a method name incorrectly describes what is happening in this method, which is in fact a validation method. clean_up() as a method name doesn't seem to make sense.  What are you "cleaning up"? Don't shorten variable names.  Saving typing a few extra characters is trivial in comparison to having easily understandable code. For example, reg_* properties could be called regex_* to make the meaning much clearer, not just where defined, but also where used in the rest of the code.
Speaking of the regexes. Should these be class constants?  If you don't intend to have these items be mutable (which you don't in this case), don't allow them to be mutable.
You have unnecessary code nesting/branching. In your clean_up() method you could easily combine the first two conditionals and you could invert the conditional to exit early when the condition isn't met, de-nesting the entire rest of the code in the method. In your sanitize() method you could remove the else block altogether by setting null as default value.  As a rule of thumb, the more branching/nesting you have in your code, the more bug-prone it will be, so you should actively look to code it away.
You need to be consistent in your method returns. Your clean_up() method doesn't return anything if the first two conditions aren't met.
You should be consistent in where you write to data structures.  You are breaking the logic up for where you write to $this->clean across two methods.  Perhaps the sanitize method should  not write to this array at all, but rather just perform the regex and return values.
You have some stylistic problems you may want to address:

Classes should be named with first letter in uppercase by convention.
You use inconsistent spacing around assignment operators and flow-control structures.
Don't put comments at the end of lines where they are easily missed. Comments should go on line(s) before the lines they are applicable to.  Also don't add unnecessary comments. Here your comments add no value in understanding the code.


Control DataTable to CSV toString() formatting Following a question I asked and the advice on SO I implemented the following solution. Are there any downsides to such a solution? Is there a better way to make it changeable when I need to control formatting of other types like double and float where I have more than one occurrence of each in the dataLine[]?

    public void Save(DataTable newMyDataTable)
    {
        String dateTimeFormat = "yyyy-MM-dd HH:mm:ss";
        try
        {
            lock (writtingLock)
            {
                Thread.CurrentThread.CurrentCulture = CultureInfo.InvariantCulture;
                dataTable = newMyDataTable;
                fileId = dataTable.TableName;
                PathMaker path = new PathMaker(fileId);
                path.Make();
                // writing the table to a file
                using (StreamWriter swr =
                    new StreamWriter(File.Open(path.filePath, FileMode.Create), Encoding.Default, 1000000))
                // change buffer size and Encoding to your needs
                {
                    if (addHeader)
                    {
                        foreach (var dc in dataTable.Columns)
                        {
                            swr.Write(dc.ToString() + ",");
                        }
                        swr.WriteLine();
                    }
                    foreach (DataRow dr in dataTable.Rows)
                    {
                        Object[] dataLine = dr.ItemArray;
                        // extract date time and format it
                        var dateTime = dataLine.Where(o => o.GetType() == typeof(DateTime)).Select(t => (DateTime)t).Select(x => x.ToString(dateTimeFormat)).ToArray();
                        // collect data other than date time
                        var otherData = dataLine.Where(o => o.GetType() != typeof(DateTime)).ToArray();
                        // build your string
                        string dateTimeToWrite = string.Join(",", dateTime);
                        string otherDataToWrite = string.Join(",", otherData.Select(x => x.ToString().TrimEnd(null)).ToArray());
                        string lineToWrite = dateTimeToWrite + "," + otherDataToWrite;
                        swr.WriteLine(lineToWrite);
                    }
                }
            }
        }

What's the point of dataTable = newMyDataTable;? Why not simply work with newMyDataTable directly? Note that newMyDataTable is a bad variable name anyway.

Ditto fileId: this is only used to pass to PathMaker, so why then not simply use newMyDataTable.TableName?

Ditto dataLine: why not work with dr.ItemArray directly?

writtingLock has a spelling error.

What is dc? Ditto dr? Use descriptive names; using abbreviations isn't making your code faster.

Why assign dateTime and otherData when you could easily make this one line? Even better: why not move this code -- including the string.Join -- to separate methods?

Example:

string.Join(",", dr.ItemArray
     .Where(o => o.GetType() == typeof(DateTime))
     .Select(x => ((DateTime)x).ToString(dateTimeFormat)));

Why assign lineToWrite when you could simply write swr.WriteLine(dateTimeToWrite + "," + otherDataToWrite);?

filePath is a property, so it should be PascalCase.


Provide true clock-modulus operation for arbitrary sign values I wrote this template to do as stated in the title; did I miss anything?

template<typename T> T clockmod(T v, T m) //todo: test T is numeric
{
    typedef long double D;
    T t=(m-m); //zero default
    D dv=std::fabs((D)v);
    D dm=std::fabs((D)m);
    if ((dv>0.0)&&(dm>0.0))
    {
        int id=int(dv/dm);
        D d=(dm*id);
        D dr=(dv-d);

        if (v>0)
        {
            if (m>0) t=(T)dr;
            else if (m<0) t=(T)(dr-dm);
        }
        else if (v<0)
        {
            if (m>0) t=(T)(dm-dr);
            else if (m<0) t=(T)(-dr);
        }
    }
    return t;
}

Where:
v / m may be any numeric type;
m (the modulus base) always range from 0 to m, m<0 or m>0;
the sign of v indicates the 'direction of counting' round the clock, positive increments from 0, negative decrements from m

Style comments:

Generic typename T implies you do not know or care what kind of type it is or which properties it has. Why not name it Numeric or something in that direction?
You should really use better names for variables. At first glance, no one has any idea what which variable is intended to do.
Even a single line if deserves braces. When omitting these and later adding to the code, programmers often forget to add them and introduce bugs.
typedef is a relic of C compatibility. Consider the using keyword instead.
Do not just arbitrary rename a type especially into something like D. You have an IDE it can do completion for you, use it. (And yes, even vim and emacs do completion, no excuses unless you write your code in notepad.)
Comment on your intentions and assumptions of the pieces of your code.

Solution issues:

  the sign of v indicates the 'direction of counting' round the clock, positive increments from 0, negative decrements from m

You are probably looking for this, meaning you should declare the template function and then offer specific implementations for the different possible instances of T.
Is a cast to long double really required? If T is a numeric type, all required operations should be defined for T. There is no reason to fall back to std::fabs().
If you want to do it by hand, I would suggest something in the way of

bool clockwise = (v >= 0);
T absolute_v;
if(!clockwise){ // !clockwise implies v < 0
    absolute_v = -v; // we want the abs(v)
} else {
    absolute_v = v;
}


Using lock with value type (multithread context) I have to use value type to lock code sections in correspondence to objects identifiers (i.e. Guid, int, long, etc.).

So I've written the following generic class/method, which uses a Dictionary to handle object that is a reference in correspondence to the value type (so this objects can be used with the lock instruction).

I think the code and comments are self-explanatory enough.

public static class Funcall<TDataType, T>
{
    // Object used for lock instruction and handling a lock counter
    class LockObject
    {
        public int Counter = 0;
    }
    // Correspondance table between the (maybe) value type and lock objects
    // The TDatatype above allows to create an independant static lockTable for each given TDatatype
    static Dictionary<T, LockObject> lockTable = new Dictionary<T, LockObject>();

    static LockObject Lock( T valueToLockOn )
    {
        lock ( lockTable ) // Globally locks the table
        {
            // Create the lock object if not allready there and increment the counter
            if ( lockTable.TryGetValue( valueToLockOn, out var lockObject ) == false )
            {
                lockObject = new LockObject();
                lockTable.Add( valueToLockOn, lockObject );
            }
            lockObject.Counter++;

            return lockObject;
        }
    }

    static void Unlock( T valueToLockOn, LockObject lockObject )
    {
        lock ( lockTable ) // Globally locks the table
        {
            // Decrement the counter and free the object if down to zero
            lockObject.Counter--;
            if ( lockObject.Counter == 0 )
                lockTable.Remove( valueToLockOn );
        }
    }

    public static void Locked( T valueToLockOn, Action action )
    {
        var lockObject = default( LockObject );

        try
        {
            // Obtain a lock object
            lockObject = Lock( valueToLockOn );

            lock ( lockObject ) // Lock on the object (and so on the corresponding value)
            {
                // Call the action
                action?.Invoke();
            }
        }
        finally
        {
            // Release the lock
            Unlock( valueToLockOn, lockObject );
        }
    }

    // Same as above except this is returning a value (Func instead of Action)
    public static TResult Locked<TResult>( T valueToLockOn, Func<TResult> action )
    {
        var lockObject = default( LockObject );

        try
        {
            lockObject = Lock( valueToLockOn );

            lock ( lockObject )
            {
                return action == null ? default( TResult ) : action();
            }
        }
        finally
        {
            Unlock( valueToLockOn, lockObject );
        }
    }
}

Which can be used like this:

Guid anObjectId = ObtainTheId();
return Funcall<AnObjectClass, Guid>.Locked( anObjectId, () =>
{
    // do something
    return something;
} );

Are there better ways to do that? Any advice is welcome.

Edit/note: as asked in the comments, the use case is the following

This "lock" system is to be used by a module for which the entry points are dedicated to modify stored objects of a particular type (this object type will be modified by this module only). Externally the module is used by several concurrent workers.

More specifically, I use MongoDB 3.4 for objects storage and it does not provides transactions which are new in version 4.0 (I mean here session.Start, session.Commit).
Among that, I don't really need a full/complete transactional system, but simply ensure each step of the workers demands are each consistent at the time each demand occurs.
I'm aware that this simple "lock" system can be considered as weak, but it is simple and meet my need in the context I am working on.

Thread safety

I've not tested this, but it looks solid enough, since the only non-trivial logic is held in lock statements. Encapsulation is accordingly good.

Depending on the load, and if performance is critical, some reader/writer locking might help to ease contention (e.g. if the lock object already exists, you don't need to write-lock the dictionary to retrieve it, and if the count is non-zero when you release the lock, you don't need to write-lock to remove it), but it will dramatically increase the complexity.

Naming API

Lock isn't a great name: it doesn't lock anything, it just gives you a handle to a LockObject, which you happen to have to release. Unlock similarly. Personally I avoid split prepositions

Locked isn't a great method name either: Locked suggests a state, not an operation. I'd rather call these RunLocked, or RunExclusive or something.

I don't like this:

return action == null ? default( TResult ) : action();

It means that if action == null, then the system goes to all that effort just to return a meaningful-but-made-up value. What use case could this have? This may end up obscuring a bug later on, when something which shouldn't be null ends up being null, and this code covers up the fact by returning default(TResult). In the very least this behaviour should be clearly documented, but I'd much rather it just throw an ArgumentException with a helpful error message.

The same goes for action?.Invoke(); in the other overload.

As always, I'll suggest inline-documentation (///) on the type and public members: it is little (well spent) effort; makes it so much easier to exploit the API without having to leave the IDE; and improves maintainability, as anyone working on the code can see inline what it is meant to be doing and avoid breaking that contract.

Why is this static? I'd much sooner make this non-static, and provide a static instance if that is a meaningful concept. That way, if you need to lock on the same type for different purposes then you can. Making this static needlessly restricts the applicable use-cases.

Dodgy try...finally

The try...finally constructs in the Locked methods are a bit dodgy... Unlock will throw if lockObject is null, which means you should be entering the try knowing that it is not null. The quick-and-easy solution is to move the call to Lock out of the try. If Lock can crash (it's not immediately clear how under normal circumstances it would, thought), then you need to consider that specifically.

Other misc

allready is misspelt in one of the comments.
The lockTable can (and arguably should) be readonly.
I'd consider replacing the Counter++ and Counter-- with dedicated Increment()/Decrement() methods in LockObject (making counter private), since any other usage would (presently) make no sense.
Many people object to the if without {} construct: you only have one usage, so I'd consider putting the braces in.
Personally I like to make the accessibility of everything explicit (i.e. mark private members and classes private); it just saves the reader having to remember anything, and avoids any confusion when coming from languages with different defaults.


Assign value of nested array as array key I have a multi dimensional array like this:

[0] =>
    ['batch_id'] => '1'
    ['some_stuff'] => 'values'
    [0] =>
        ['unit_id'] => '12'
        ['some_stuff'] => 'values'
    [1] =>
        ['unit_id'] => '41'
        ['some_stuff'] => 'values'

This comes from an external API, so I have no control over the original formatting of the array.

What would be the preferred (least overhead) way to take all the 'unit_id' and 'batch_id' fields, remove them from the group of values about the item, and assign that value to the item's key?

What I've tried (and works, but I want to find something cleaner):

public function fixArray($batches) {
    $batchfix = array();
    foreach($batches as $batch) {
        $unitfix = $array;
        $batch_temp = $batch;
        array_shift(array_shift($batch_temp)); // Get rid of id details
        foreach($batch_temp as $unit)
            $unittemp = $unit;
            array_shift($unittemp);
            $unitfix[$unit['unit_id']] = $unittemp;
        $batchfix[$batch['batch_id']] = $batch;
    }
    return $batchfix;
}

Any suggestions?

I suppose the result you want to have is this:

    Array
    (
        [1] => Array
            (
                [stuff] => values
                [units] => Array
                    (
                        [12] => Array
                            (
                                [stuff] => values
                            )
                        [41] => Array
                            (
                                [stuff] => values
                            )
                    )
            )
    )

In this case you can use the code:

    public function fixArray($originalBatches) {
        $batches = array();
        foreach($originalBatches as $batch) {
            $units = array();
            $batch_id = $batch['batch_id'];
            unset($batch['batch_id']);
            foreach ($batch as $key => $value) {
                if (is_numeric($key) && !empty($value['unit_id'])) {
                    $units[$value['unit_id']] = array_slice($value, 1);
                    unset($batch[$key]);
                }
            }
            $batches[$batch_id] = array_merge($batch, array('units' => $units));
        }
        return $batches;
    }

Still the use of the 2 foreach is not a very good practice. You can change the code accordingly to use array_walk. This would be much faster.


Defining many HTTP request methods in a class using partialmethods I am using an object that expects a keyword argument for every call, but I never intend to change it. In this case a timeout of 10. What is the best way to add a partialmethods for each attributes.

In my example I want to set a timeout for my session requests http methods.

I could monkeypatch Session.request with a partialmethod. This works fairly well because all of the session methods go through request. Session().post is actually going through Session.request('post', <inputs>).

from requests import Session

Session.request = partialmethod(Session.request, timeout=10)

However, what about use cases where there isn't a single method I can fix or I simply don't want to do something potentially dangerous like monkeypatching.

I have a session helper object that configures the auth and the adapter (omitted here) for the client, but I want different default timeouts for different methods. I can't even monkeypatch the method in that case.

I could do it after standard definition of the class in a loop.

from functools import partialmethod
from requests import Session

class SeshTester:
    def __init__(self):
        self.s = Session()

    def request(self, method, *args, **kwargs):
        return self.s.request(method, *args, **kwargs)

for r_method in ['delete', 'get', 'head', 'options']:
    setattr(SeshTester, r_method, partialmethod(SeshTester.request, r_method, timeout=7))

for r_method in ['connect', 'put', 'push']:
    setattr(SeshTester, r_method, partialmethod(SeshTester.request, r_method, timeout=13))

# this call is equivalent to self.s.request('get', 'http://google.com', timeout=7
print(SeshTester().get('http://google.com'))

I don't think this is very legible, but am unaware how to do this inside of standard class definition. SeshTester object obviously isn't reference-able before it is made, but I don't want to repeat myself by doing post = partialmethod(request, 'post', timeout=13) for every possible http method.

I would like to perform this loop in the class, but don't know how to reference the object before it's fully defined.

It is certainly possible to do what was outlined that lies outside the class definition within the class definition itself through the usage of locals, and with that the intent of what is to be done can be expressed more clearly with less verbose code and lookup using setattr.

class SeshTester:
    def __init__(self):
        self.s = Session()

    def request(self, method, *args, **kwargs):
        return self.s.request(method, *args, **kwargs)

    for r_method in ['delete', 'get', 'head', 'options']:
        locals()[r_method] = partialmethod(request, r_method, timeout=7)

    for r_method in ['connect', 'put', 'push']:
        locals()[r_method] = partialmethod(request, r_method, timeout=13)

Seeing this working as intended from the shell:

>>> SeshTester().get
functools.partial(<bound method SeshTester.request of <__main__.SeshTester object at 0x7fde00176ef0>>, 'get', timeout=7)
>>> SeshTester().put
functools.partial(<bound method SeshTester.request of <__main__.SeshTester object at 0x7fddf8da0668>>, 'put', timeout=13)

One more thing: the documentation linked above does not actually describe what is implemented, at least for CPython; other implementations may have locals() behave differently within the class scope, however there is now an effort to properly standardize this underspecified behavior to the CPython implementation in PEP-558, such that the above code example will work as intended according to the standard (if the PEP gets ratified into Python 3.7).

However, as mentioned in the question, if the API provides a standardized helper method that provides a specific Session instance, certainly it can provide a subclassed Session instance with the specific features/methods that require overriding be applied where needed.  Consider the following:

class SessionExtras(Session):

    def request(self, method, url, *a, **kw):
        default_timeout = 7
        timeouts = {
            'CONNECT': 13,
            'PUT': 13,
            'PUSH': 13,
        }
        if kw.get('timeout') is None:
            kw['timeout'] = timeouts.get(method, default_timeout)
        return super().request(method, url, *a, **kw)

Perhaps this way the intent of what is to be done is expressed in a much more clear way, and allow the API users to make use of a real Session instance rather than some surrogate.  For this example, the default_timeout and timeouts is defined within the function definition, however they could conceivably be moved to the class or even module level, depending on the intended usage and how it is to be customized for the specific API that is being written.


Newton's square root This is the code I wrote to compute the newton's square root of a number.

I aimed for extreme clarity over efficiency and I have put an abundant docstring at the start of the newton_sqrt function where I describe the main formula and explain all the parameters.

This function is not meant to be fast (standard library is 4x times faster and more accurate), I wrote just to learn some good math coding techniques in Python.

This was written in Python 3 but is fully compatible with Python 2.
I created constants for the error messages. Is that a good practice? Or should I just write the errors down verbatim when need occurs?
I chose n / 2 as my initial guess. Do you know a better initial guess?
Are all the option parameters that I am using necessary or should I delete some of them?

from __future__ import division
import time

def newton_sqrt(n,x0=None,tolerance=10 ** (-3),max_iter=15,epsilon=10 ** (-5)):
    """
   This function computes the sqare root of a number
   using Newton's method.
   After chosing a starting approximate value for x runs:
   x = x - f(x) / derivative_of_f(x)
   After a given number of iterations or if the result is good enough
   the programme will stop and return x.

   Parameters:
       Compulsory:
           @ n: the number of which the square root must be taken.
       Optional:
           @ x0: the initial approximation of n.
           @ tolerance: the maximum error permitted.
           @ max_iter: the maximum number of iterations.
           @ epsilon: a number so small that dividing by it would break
           @        the programme, giving vastly inapproximate results.
   """
    DIVISION_BY_TINY_DERIVATIVE_ERROR = """The derivative of f is {} and therefore under the {} limit chosen.
   Dividing by it would give a result with a vast error."""
    NOT_ENOUGH_ACCURACY_IN_ITER_ERROR = """Solution of {} accuracy for {} was not found in {} iterations"""

    if x0 is None:
        x0 = n / 2

    f = lambda x: x ** 2 - n
    fprime = lambda x: 2 * x  # The derivative of f(x)

    for i in range(max_iter):
        y = f(x0)
        yprime = fprime(x0)
        if(abs(yprime) < epsilon):
            raise Exception(DIVISION_BY_TINY_DERIVATIVE_ERROR.format(
            yprime, epsilon))
        x1 = x0 - y / yprime
        if(abs(x1 - x0) / abs(x1) < tolerance):
            return x1
        x0 = x1

    raise Exception(NOT_ENOUGH_ACCURACY_IN_ITER_ERROR.format(
    tolerance,n,max_iter))

def test_and_timing(max_):
    start = time.time()
    n_sqrt = [newton_sqrt(i) for i in range(1, max_)]
    print("Newton's sqrt took {} seconds to execute for i 1:{}".format(
        time.time() - start, max_))

    start = time.time()
    b_sqrt = [i ** 0.5 for i in range(1, max_)]
    print("Built-in sqrt took {} seconds to execute for i 1:{}".format(
        time.time() - start, max_))

    print("""Sum of all newton sqrts is: {}
   Sum of all built-in sqrts is: {}""".format(sum(n_sqrt), sum(b_sqrt)))

if __name__ == "__main__":
    test_and_timing(100000)

Looks good overall. Some suggestions:

The newton_sqrt is written in a quite generic manner. Unfortunately it may only calculate square roots - because lambdas are defined inside the function. I would rewrite it along the lines of

def newton_raphson(x, x0, func, derivative, governing_options):
    your code verbatim, except the lambda definitions

def newton_sqrt(x, governing_options):
    return newton_raphson(x, x/2, lambda y: y*y - x, lambda y: 2*y, governing_options)

In any case, you most likely want to test for n > 0 and raise InvalidArgiment accordingly.
I am not sure about the epsilon. If an abs(prime) < epsilon condition triggers, the iteration would not converge anyway. I don't see an extra value here, but maybe it is just me.


Processing.js particle system After about half an hour of fooling around with Processing.js/JavaScript, I got this fairly decent particle system set up:

// main particle constructor class
var Particle = function(position) {
    this.velocity = new PVector(random(-0.1, 0.1), random(-0.1, 0.1));
    this.acceleration = new PVector(random(-0.01, 0.01), random(-0.01, 0.01));
    this.lifeTime = 300;
    this.position = position.get();
};

// update the particle
Particle.prototype.update = function() {
    this.velocity.add(this.acceleration);
    this.position.add(this.velocity);
    this.lifeTime--;
};

// render the particle
Particle.prototype.render = function() {
    strokeWeight(2);
    stroke(161, 153, 153, this.lifeTime);
    fill(140, 135, 140, this.lifeTime);
    ellipse(this.position.x, this.position.y, 15, 15);
};

// check the bounds of a particle
Particle.prototype.checkBounds = function() {
    if(this.position.x >= 385) {
        this.acceleration = new PVector(random(-0.02, -0.01), random(-0.01, 0.01));
    }
    if(this.position.x <= 15) {
        this.acceleration = new PVector(random(0.01, 0.02), random(-0.01, 0.01));
    }
    if(this.position.y >= 385) {
        this.acceleration = new PVector(random(-0.01, 0.01), random(-0.02, -0.01));
    }
    if(this.position.y <= 15) {
        this.acceleration = new PVector(random(-0.01, 0.01), random(0.01, 0.02));
    }
};

// check if the particle is dead
Particle.prototype.particleDead = function() {
    if(this.lifeTime > 0) {
        return false;
    } else {
        return true;
    }
};

// array containing particles
var particles = [];

// main draw loop
var draw = function() {

    // draw the background and push a new particle
    background(11, 100, 209);
    particles.push(new Particle(new PVector(mouseX, mouseY)));

    // iterate through particles and run their methods
    for(var p = particles.length-1; p >= 0; p--) {
        var particle = particles[p];
        particle.checkBounds();
        particle.update();
        particle.render();
        if(particle.particleDead()) {
            particles.splice(p, 1);
        }
    }
};

Now, I think that this could definitely be improved, as the Particle.prototype.checkBounds method seems to not work all the time. Anyways, what can I improve here?

Interesting question,

there are few things that could be improved:

Read up on the revealing module pattern, it would be excellent to

Reduce your usage of this
Make your class look like 1 piece of code

Something like this:

// main particle constructor class
function Particle(position) {

  var velocity = new PVector(random(-0.1, 0.1), random(-0.1, 0.1)),
      acceleration = new PVector(random(-0.01, 0.01), random(-0.01, 0.01)),
      lifeTime = 300,
      position = position.get();

  function update() {
    velocity.add(acceleration);
    position.add(velocity);
    lifeTime--;
  }

  function render() {
    strokeWeight(2);
    stroke(161, 153, 153, lifeTime);
    fill(140, 135, 140, lifeTime);
    ellipse(position.x,position.y, 15, 15);
  };

  return {
    update : update,
    render: render;
  }
};

I obviously did not rewrite the whole thing, but you should catch my drift.
You keep using random(-0.1, 0.1), I would create a helper function for this
fill(140, 135, 140, lifeTime); <- I would put a comment as to what color this should be
Do not use an if statement to determine true or false like you do :

// check if the particle is dead
Particle.prototype.particleDead = function() {
    if(this.lifeTime > 0) {
        return false;
    } else {
        return true;
    }
};

do this instead:

// check if the particle is dead
Particle.prototype.particleDead = function() {
    return this.lifeTime > 0
};

If you have time, read up on falsy values, you could also simply do return !this.lifetime
I am still playing with the code, I managed to fix the right hand side bounce with this:

if(this.position.x >= 385) {
  this.acceleration.x = random(-0.02, -0.01);
  this.velocity.x = random(-0.1, -0.01);
}

In essence you have to solve both the acceleration and the velocity.

I have to wonder, how do you access random? I can only access random as a function under the instance of Processing.

It is too bad that the snippets will now allow for localStorage access, so I can only give you a link to jsbin with my refactored version and the source code here in case jsbin ever dies:

var canvas = document.getElementById('partikals'),
    pjs = new Processing(canvas),
    width = 400,
    height = 400,
    padding = 15;

function Particle2(position) {

    var velocity = new PVector(pjs.random(-0.1, 0.1), pjs.random(-0.1, 0.1)),
        acceleration = new PVector(pjs.random(-0.01, 0.01), pjs.random(-0.01, 0.01)),
        lifeTime = 300;

    position = position.get();

    function update() {
        velocity.add(acceleration);
        position.add(velocity);
        lifeTime--;
    }

    function render() {
        pjs.strokeWeight(2);
        //Each particle is grey
        pjs.stroke(161, 153, 153, lifeTime);
        pjs.fill(140, 135, 140, lifeTime);
        pjs.ellipse(position.x, position.y, 15, 15);
    }

    function isDead() {
        return !lifeTime;
    }

    //I wonder if just *= -1 would work
    function bounce() {
        if (position.x > width - padding) {
            acceleration.x = -Math.abs(acceleration.x);
            velocity.x = -Math.abs(velocity.x);
        }
        if (position.x < padding) {
            acceleration.x = Math.abs(acceleration.x);
            velocity.x = Math.abs(velocity.x);
        }
        if (position.y > height - padding) {
            acceleration.y = -Math.abs(acceleration.y);
            velocity.y = -Math.abs(velocity.y);
        }
        if (position.y < padding) {
            acceleration.y = Math.abs(acceleration.y);
            velocity.y = Math.abs(velocity.y);
        }
    }

    return {
        update: update,
        render: render,
        isDead: isDead,
        bounce: bounce
    };
}

// array containing particles
var particles = [];

pjs.setup = function() {
    pjs.size(width, height);
    pjs.frameRate(24);
};

pjs.draw = function() {

    //Clear by drawing the background and push a new particle
    pjs.background(11, 100, 209);
    particles.push(new Particle2(new PVector(pjs.mouseX, pjs.mouseY)));

    // iterate through particles and run their methods
    for (var p = particles.length - 1; p >= 0; p--) {
        var particle = particles[p];
        particle.bounce();
        particle.update();
        particle.render();
        if (particle.isDead()) {
            particles.splice(p, 1);
        }
    }
};

pjs.setup();


It is a hashmap, using linear probing on an array I have been studying hashtable for two days and was musterring up the courage to write a code. Finally I have written code in which I have taken many shortcuts:

The first being I have used compression function as key % hash_size;
I have not used any hashing function.
I have implemented it using array. A better option could be a vector.
For linear probing I have choose to store in the next empty index.
My code is not commented but is pretty straightforward.

#include <string>
#include <iostream>
#include <algorithm>
using namespace std;
class data{
  private:
    int key;
    int value;
    bool deleteme;
  friend class hashtable;
};
class hashtable{
  public:
    hashtable(int m);
    hashtable(const hashtable& that);
    hashtable & operator= (hashtable src);
    ~hashtable();
    void add(int key, int value);
    bool exists(int key);
    int get(int key);
    void remove(int key);
    void display();
  private:
    int size;
    data* array;
};
hashtable::hashtable(int m):size(m){
    array = new data[m]();
    // data array[] = {NULL};
}
hashtable::hashtable(const hashtable& that){
  size = that.size;
  array = new data[that.size];
  for(int i = 0; i < that.size ; ++i){
    array[i] = that.array[i];
  }
}
hashtable & hashtable::operator=(hashtable src){
    std::swap(array, src.array);
    std::swap(size, src.size);
    return *this;
}
hashtable::~hashtable(){
    delete [] array;
}
void hashtable::add(int k, int val){
  int index = k % size;
  while(array[index].key != 0 || array[index].deleteme == 1){
    if(array[index].key == k)
      break;
    index += 1;
  }
  array[index].key = k;
  array[index].value = val;
}
bool hashtable::exists(int key){
  int index = key % size;
  while((array[index].key != key && index < size ) || (array[index].deleteme == 1 && index < size)){
    index += 1;
  }
  return index < size;
}
int hashtable::get(int key){
  int index = key % size;
  while((array[index].key != key && index < size) || (array[index].deleteme == 1 && index < size)){
    index += 1;
  }
  if(index < size)
    return array[index].value;
}
void hashtable::remove(int key){
  int index = key % size;
  while((array[index].key != key && index < size)|| (array[index].deleteme == 1 && index < size)){
    index += 1;
  }
  array[index].key = 0;
  array[index].value = 0;
  array[index].deleteme = 1;
}
void hashtable::display(){
    for (int i = 0; i < size; ++i)
    {
        cout << array[i].key << " " << array[i].value  << " " << array[i].deleteme << endl;
    }
}

int main(){
  hashtable h1(11);
  h1.add(3,134);
  h1.add(14,139);
  h1.add(742,963);
  h1.add(11,456);
  h1.add(22,635);
  h1.add(33,852);
  h1.add(44,968);
  h1.add(9,589);
  h1.add(9,7890);
  h1.remove(14);
  h1.remove(33);
  h1.add(55,786);
  h1.add(10,56);
  cout << h1.get(44) << endl;
  // cout << h1.(43) << endl;
  h1.display();
}

Use unsigned int for the keys/hashes.
Make functions like get, display or exists const, so you make clear that they are not modifying the map and you can pass the map as const& to a function, which may uses this functions but does not use add or remove or other functions modifying the map.

void display() const;

You can add operator[] which returns a reference to the value, here are the signatures:

int& operator[](int key); // this can be used for assignment (``map[3] = 5``) and to get a value (``std::cout << map[3] << '\n'``). This needs to insert an element, if there is no element with the key and initialize its value to a default value like ``int()``/``0``
int operator[](int key) const; // this is used when the map is accessed when it is const. No need for reference, but for other value-types like std::string, use std::string const& to avoid copying

You can make the class generic, to allow other value-types.
Make whitespace after each function or class, to seperate.


Generate HTML file from Handlebars template and data object I have a class called HtmlFile which has a method called generate. This method is responsible for generating an html file from handlebars template and data object provided in the HtmlFile class constructor.

I am confused on what code design should I use for this use case and what variable names to choose.

class HtmlFile {
    public file: any = null;
    private template: any = null;
    private data: any = null;
    constructor(templatePath: string, params: any) {
        this.template = path.resolve(templatePath);
        this.data = params;
    }

    public async generate() {
        try {
            const error = this._validate();
            if (error) {
                throw error;
            }
            const fileContent = fs.readFileSync(this.template, "utf8");
            const html = handlebars.compile(fileContent)(this.data);
            return html;
        } catch (e) {
            throw e;
        }
    }

    private _validate() {
        let errStr: any = null;
        if (typeof this.template !== "string") {
            errStr = `expected template path to be a string, instead got
            ${typeof this.template}`;
        }
        if (typeof this.data !== "object") {
            errStr = `expected data to be an object, instead got ${typeof
            this.data}`;
        }
        return errStr;
    }
}

And I am using this class in the following manner:

let htmlFile = new HtmlFile("./src/templates/sample.hbs", invoiceOptions);
const html: any = await htmlFile.generate();
return html;

I also want suggestions on what variable names I should use in place of htmlFile and html.

First things first - syntax:

Why is generate an async function? You should change the way the files are read. Please read the answers to this question: https://stackoverflow.com/questions/46867517/how-to-read-file-with-async-await-properly

Logic:

The _validate function checks integrity of your object. All the data it needs is available in the constructor and this is where this function should be called. Never defer throwing an exception if you cannot avoid it.

Structure:

As for what design to use - your class is doing multiple things at the same time. Reading a template file from disk is one thing - and it requires handling an asynchronous operation. Then there is template compilation - which in case of HBS is not asynchronous and there is rendering - not asynchronous again. I'd recommend to restructure the solution a bit:

Use one class to read a file and return you a compiled template object. In case you needed to reuse your template objects - do not read from disk on every template call.
The returned class (let's call this one HandlebarsTemplate) should take a string as its constructor input and attempt compiling it as HBS. If the template was not correct - you get an exception thrown on object creation.
Note that HandlebarsTemplate could be reused for templates coming from sources other than files e.g. database or just a string literal in the code.
The object HandlebarsTemplate should expose a method called generate(data). This method should attempt rendering data the with the compiled HBS template.
This way you get reusable template objects

This would be up to you if you want to read the template each time or store a compiled instance in memory. Both approaches have their pros and cons.