mapping a tictactoe board in javascript I'm creating a Tic Tac Toe solver algorithm. The idea is to iterate through each row, column and both diagonals and put them into a map in JavaScript. With that map I can run other logic to check for a winner and where that winner's position is.

I'm not building an actual TTT game nor am I writing any AI for now. My question is if there's a better way of achieving the same results without having to use two for loops to get the columns and second diagonals.

let board = [
  ['x','o','x'],
  ['o','o','o'],
  ['o','o','x']
];

const mapper = board => {
  let map = {},
  d1 = [],
  d2 = [];

  for (let i = 0; i < board.length; i++) {
    let tmp = [];
    // get all rows
    map[`ROW${i}`] = board[i];

    // get second diagonals
    d2.push(board[i][board.length-1-i]);

    for (let j = 0; j < board.length; j++) {
      // get all columns
      tmp.push(board[j][i]);

      // get first diagonals
      if (i === j) {
        d1.push(board[i][j])
      }
    }

    map[`COL${i}`] = tmp;
  }

  map[`DIA1`] = d1;
  map[`DIA2`] = d2;

  return map;
}

const checkForWinner = board => {
  return mapper(board);
}

checkForWinner(board);

Not the board, store the moves

A TicTacToe board can be mapped to two 9bit numbers that represent each player's moves then the test for winning moves is just a simple bitwise test against the 8 possible combinations of move that are a win.

  My question is if there's a better way of achieving the same results without having to use two for loops to get the columns and second diagonals.

The following may be what you want

// top left is low bit, bottom right is high bit
// coord to bit pos is x + y * 3
// Bit positions on board
// 0,1,2
// 3,4,5
// 6,7,8

const board = [['x','o','x'], ['o','o','o'], ['o','o','x']];
const players = ["x","o"];
const winMoves = [448,56,7,292,146,73,273,84];

const getMoves = (board, player) => {
  var moves = 0;
  for(var i = 0; i < 9; i ++){
     moves += board[i / 3 | 0][i % 3] === player ? 1 << i : 0;
  }
  return moves;
}
const checkWin = (board, player) =>{
  const moves = getMoves(board, player);
  return winMoves.some(win => (win & moves) === win);
}
if(!players.some(player => {
    if(checkWin(board, player)) {
        console.log("Player " + player + " wins");
        return true;
    }
}){
  console.log("No winner");
}

Complete game state in 18bits

If you look into the problem further you can reverse the board/moves relationship so that the board is created from two sets of moves. This means you can store the complete game state with only 2 9bit numbers and you would not have to examine the board to find the win

const move = (player, x, y) => player |= 1 << (x + y * 3);
const checkWin = moves => winMoves.some(win => (win & moves) === win);
const winMoves = [448,56,7,292,146,73,273,84];
var movesX = 0, movesO = 0; // moves for each player game start

// or from your example the moves would be
// x o x
// o o o
// o o x
movesX = 0b100000101;
movesO = 0b011111010;

// with
console.log(checkWin(movesX)); // >> false
console.log(checkWin(movesO)); // >> True


Find largest prime factor Here is my attempt at solving Project Euler problem 3. It works for test numbers up to 9 digits long, but overflows when I input the real 12-digit behemoth.

I've looked at other methods of solution, but I wanted to challenge myself to code one that doesn't search with unnecessary numbers - only primes - and never any of their factors.

I would sincerely appreciate it if someone with more coding experience could show me where in my program I am taking up a lot of memory/computational efficiency/etc.

#The prime factors of 13195 are 5, 7, 13 and 29.
#What is the largest prime factor of the number 600851475143 ?
import numpy as np
import math

def lpf(n):
    orig=n #Store the original n as a reference.
    ceiling=int(math.floor(np.sqrt(n)))+1 #Don't check for prime factors larger than sqrt(n).
    thelist=[]
    for i in xrange(2,ceiling):
        thelist.append(i) #Make a list of integers from 2 up to floor(sqrt(n))+1.
    plist=[2]  #Initialize a running list of primes.
    pfpairs=[] #Initialize a list to store (prime factor, multiplicity) pairs.
    removed=[] #Initialize a list to store removed prime multiples.
    for p in plist:
        if p>=ceiling: #Again, don't check for prime factors larger than sqrt(n).
            break
        i=2
        pexp=0      #Set the multiplicity of the prime to zero initially.
        if n%p==0:  #If the prime divides n, set the new n to n/p and see if it divides again.
            #print n, p
            pexp=1
            n=n//p
            while True:
                if n%p==0: #As long as there is no remainder after division by p, keep dividing n by p,
                    n=n//p #making sure to redefine n and increment p's multiplicity with each successful divison.
                    pexp+=1
                else:
                    pfpairs.append((p,pexp)) #Once a divison fails, store the prime and its multiplicity.
                    break
        while (p*i)<=ceiling: #As long as the prime multiples dont exceed the ceiling,
            if (p*i) in removed: #if the current multiple has already been removed, move on.
                i+=1
            elif (p*i) not in thelist:
                i+=1
            else:
                removed.append(p*i) #Else, add that multiple to the removed pile, and then remove it from the list.
                thelist.remove(p*i)
                i+=1
        for number in thelist: #After all prime multiples (not including p*1) are deleted from the list,
            if number>p:       #the next number in the list larger than p is guaranteed to be prime, so set
                plist.append(number) #p to this new number and go through the logic again.
                break
    print '%d =' % orig, #Print the prime factorization of n to the user.
    for pair in pfpairs:
        if pair != pfpairs[-1]:
            print '(%d^%d) x' % (pair[0], pair[1]),
        if pair == pfpairs[-1]:
            print '(%d^%d)' % (pair[0], pair[1])
    return max(pfpairs, key = lambda pair: pair[0])[0] #Return the largest prime factor.

print lpf(78618449)

>>>
78618449 = (7^1) x (13^1) x (29^1) x (31^3)
31

The problem is:

  What is the largest prime factor of the number 600851475143?

Your solution has a function named lpf, which computes the complete prime factorization of the number and prints it and returns the largest prime factor. That is doing too many things! Your function should do one thing: find the largest prime factor.

Doing that, we can simplify your code a lot. Let's start with:

ceiling=int(math.floor(np.sqrt(n)))+1 #Don't check for prime factors larger than sqrt(n).
thelist=[]
for i in xrange(2,ceiling):
    thelist.append(i) #Make a list of integers from 2 up to floor(sqrt(n))+1.

You imported numpy just for sqrt, but math already has it. Is there a reason for that? int also truncates already, so you don't need floor on top of that. Furthermore, why loop through xrange when we can just use range? So a simpler one would be:

ceiling = int(math.sqrt(n)) + 1
thelist = range(2, ceiling)

Now, as saulspatz points out and I totally missed, sqrt(n) is a ceiling for determining if a number is prime. But it's not a ceiling for determining all the factors. For example, the largest prime factor of 10 is 5... but sqrt(10) ~ 3.16. A valid ceiling would be:

ceiling = n/2 + 1

We can refine the ceiling once we find the first prime factor as an optimization.

Next, what are you actually doing with thelist? It looks like you're sort of doing the Sieve of Eratosthenes. But not really. You're doing all sorts of removal from this list, and that's not entirely necessary. You could just use the sieve directly:

def lpf_me(n):
    largest = None
    ceiling = n/2 + 1
    thelist = [True] * ceiling
    for i in xrange(2, ceiling):
        if thelist[i]:                        # i is prime
            for j in xrange(2*i, ceiling, i): # turn off multiples of i
                thelist[j] = False
            if n % i == 0:
                largest = i                   # new largest prime factor
    return largest

Which when run:

>>> lpf(78618449)
31

Now, this is valid, but pretty slow (run with timeit, 10 times):

me                        148.2s

That's pretty bad. Can we build on this? Sure we can!

Let's go back to the drawing board with the sqrt(n). Any factor larger than that is either prime or divisible by something less than sqrt(n). But we've already put in the work to find all the primes less than sqrt(n)! So we can just reuse that work:

def lpf_me(n):
    largest = None
    ceiling = int(math.sqrt(n)) + 1
    thelist = [True] * ceiling
    others = []

    for i in xrange(2, ceiling):
        if thelist[i]:                        # i is prime
            for j in xrange(2*i, ceiling, i): # turn off multiples of i
                thelist[j] = False
            if n % i == 0:
                largest = i                   # new largest prime factor
                other = n/i
                if other > ceiling:           # hold onto this for later
                    others.append(other)

    for factor in others:
        for i in xrange(2, ceiling):
            if thelist[i] and factor % i == 0: # only have to check divisibility
                break                          # by primes.
        else:
            # this is prime and necessarily larger than largest
            return factor

    return largest

How fast is this? Pretty speedy. There aren't going to be very many candidates, and we can run through those pretty quickly:

up to n/2       148.22s
up to sqrt(n)
- holding on      0.02s
OP solution      21.83s ** note this is incorrect as stops at sqrt(n)
                        ** instead of n/2

I tried running on the original Euler problem value but yours never finished. But, to be fair, mine does less work. So let's actually do the prime factorization. We'll return a list of pairs like you did in the original:

def prime_factorization(n):

Now, again, we can use the Sieve approach. But instead of modifying n throughout, we can just write a helper function to do it for us:

def factor(num, p):
    exp = 0
    while num % p == 0:
        exp += 1
        num /= p
    return (p, exp) if exp > 0 else None

Simple. We don't have to touch n, keep an orig, or pollute our main function with extraneous logic. The flow of the function is the same - same sieve, except instead of keeping track of largest, we keep track of all_factors:

def factorize(n):
    def factor(num, p):
        exp = 0
        while num % p == 0:
            exp += 1
            num /= p
        return (p, exp) if exp > 0 else None

    ceiling = int(math.sqrt(n)) + 1
    thelist = [True] * ceiling
    all_factors = []
    others = []

    for i in xrange(2, ceiling):
        if thelist[i]:                        # i is prime
            for j in xrange(2*i, ceiling, i): # turn off multiples of i
                thelist[j] = False

            next_factor = factor(n, i)
            if next_factor:
                all_factors.append(next_factor)
                other = n/i
                if other > ceiling:           # hold onto this for later
                    others.append(other)

    for f in reversed(others):
        for i in xrange(2, ceiling):
            if thelist[i] and f % i == 0: # only have to check divisibility
                break                          # by primes.
        else:
            # this is prime
            all_factors.append(factor(n, f))

    return all_factors

Now it's a fair comparison, but mine still runs in 0.027s. It's all about algorithmic complexity.

Summary

Avoid erasing from lists. It's bad.
Break large functions up into smaller functions. It'll make your code easier to read and, in this case, even make the logic a lot simpler.
When you need to solve a problem, just solve the problem. Worry about extraneous similar problems when, and if, they come up.


Bash script for getting geolocation (latitude, longitude and country code) I have a bash script, where I need to get the user's geolocation (latitude, longitude and country code). If a GPS device is present, I use gpspipe. But a lot of laptops/computers don't have GPS devices (yet). I need the location as accurate as possible, and getting the geolocation by IP address is (by far) too unreliable.

I ended up with the following bash script with the following dependencies: Python with sys, urllib and json libraries, wget and a working internet connection.

LAT=""
LNG=""
CTRY=""
IFS=$'\n'
printf "Give your location (street name, city, country, etc): "
read LOCATION
while [ -z "$LAT" ]; do
    printf "Querying Google Geo API... "
    LOCATION=$(echo $LOCATION | python -c "exec(\"import urllib,sys;\\nprint(urllib.quote_plus(sys.stdin.readline()))\")")
    URL="https://maps.googleapis.com/maps/api/geocode/json?address=$LOCATION"
    GOOGLEAPIS=$(wget -qO- $URL)
    STATUS=$(echo $GOOGLEAPIS | python -c "exec(\"import json,sys;\\nobj=json.load(sys.stdin);\\nprint(obj['status'])\")")

    case "$STATUS" in
    *OK*)
        OPTIONS=($(echo $GOOGLEAPIS | python -c "exec(\"import json,sys;\\nobj=json.load(sys.stdin);\\nfor x in obj['results'][::]:print(x['formatted_address'])\")"))
        OPT=$OPTIONS
        NRFOUND=${#OPTIONS[@]}
        IDX="0"

        if [[ $NRFOUND -ge 2 ]]; then
            printf "More than one option found. Please select your location from the list:\n"
            select OPT in "${OPTIONS[@]}";
            do
                IDX=$(($REPLY - 1))
                break
            done
        fi
        LAT=$(echo $GOOGLEAPIS | python -c "exec(\"import json,sys;\\nobj=json.load(sys.stdin);\\nprint(obj['results'][$IDX]['geometry']['location']['lat'])\")")
        LNG=$(echo $GOOGLEAPIS | python -c "exec(\"import json,sys;\\nobj=json.load(sys.stdin);\\nprint(obj['results'][$IDX]['geometry']['location']['lng'])\")")
        CTRY=$(echo $GOOGLEAPIS | python -c "exec(\"import json,sys;\\nobj=json.load(sys.stdin);\\nfor x in obj['results'][$IDX]['address_components']:\\n\\tif x['types'][0] == 'country':print(x['short_name'])\")")
        break
        ;;
    *)
        printf "Google could not find your location based on your query. Google says: $STATUS\nPlease enter another location description.\n"
        read LOCATION
        ;;
    esac
done
unset $IFS
printf "Selected location: %s\nYour latitude/longitude location set to: (%s, %s).\nYour countycode is set to: %s\n" $OPT $LAT $LNG $CTRY

The code works (as required here on CR) and a sensible geolocation is found for a lot of queries. But, as I'm no seasoned shell writer, I'd like your review on this code. Do you see any pitfalls, corrections, improvements (on the coding), etc?

Bugs:

echo "$my_variable" | my_command can behave unexpectedly if the variable starts with one of the echo options. You can do my_command <<< "$my_variable" instead.
unset $IFS does not unset IFS. For that you need unset IFS.

Simplifications:

You shouldn't need to set IFS - read will save the full input to the variable anyway (see help [r]ead).
A lot of the code is already Python, and Python has much fewer caveats than Bash, so changing to it would very much benefit the maintainability of the code. Strictly speaking it's not a pure Bash script anyway, and you really don't want to be parsing JSON manually in Bash. If you really need a Bash script (which you shouldn't unless it's for a Bash programming exercise, and even then, somebody is wrong on the Internet if they asked you to use pure Bash to parse JSON) you could make it a wrapper script.
Bash's read takes a -p prompt option, so you can avoid the first printf.
Unchanging variables such as URL should be assigned outside of the loop.

Style issues:

Non-exported variables should be lowercase to distinguish them from exported variables.
It is customary to exit with a non-zero exit code when a command fails.


Two similar MongoDB-querying functions to find distinct and non-distinct documents I'd like to remove some of the repeated code in my mongo data access layer. The code is all the same except for these key areas:

Function signature parameters differ between collectionFind and collectionDistinct though they both have a callback cb.
The line below // HERE IS THE LINE THAT VARIES

I suspect I can use a closure to pass in cb to the (innerErr, result) callback but that only solves part of the problem so was curious about other ideas.

  collectionFind: function collectionFind(collectionName, query, fields, options, cb) {
    var url = getUrl();
    if (!url) {
      logger.error('No mongodb connection set');
    } else {
      getConnection(url, (err, db) => {
        if (err) {
          logger.error(`Cannot connect to mongo: ${err}`);
          clearConnection();
        } else {
          // HERE IS THE LINE THAT VARIES
          find(db, collectionName, query, fields, options, (innerErr, result) => {
            if (innerErr) {
              clearConnection();
              cb(innerErr);
            } else {
              cb(null, result);
            }
          });
        }
      });
    }
  },
  collectionDistinct: function collectionDistinct(collectionName, field, filter, cb) {
    let url = getUrl();
    if (!url) {
      logger.error('No mongodb connection set');
    } else {
      getConnection(url, (err, db) => {
        if (err) {
          logger.error(`Cannot connect to mongo: ${err}`);
        } else {
          // HERE IS THE LINE THAT VARIES
          distinctDocuments(db, collectionName, field, filter, (innerErr, result) => {
            if (innerErr) {
              clearConnection();
              cb(innerErr);
            } else {
              cb(null, result);
            }
          });
        }
      });
    }
  }

You could return early on errors. This way, you don't nest as much.

I'm not sure if getConnection, find and distinctDocuments are custom functions or library functions based on the example. But if you are able to update their implementation or swap it out for another library, use Promises. That way you avoid callback hell.

Also, I really hate functions that do too much. Some developers tend to just put multiple similar functionalities in one function and distinguish them via an argument. But when more functionality is added or when the functionalities diverge, the implementation becomes monolithic and hard to refactor. So I suggest that until a third or fourth function with the same flow comes in, I would keep it as is.

If the implementation used promises, it would be as easy as:

collectionFind: function collectionFind(collectionName, query, fields, options) {
  const url = getUrl();

  if (!url) return logger.error('No mongodb connection set');

  return getConnection(url)
    .then(db => find(db, collectionName, query, fields, options))
    .catch(err => {
      logger.error(err);
      clearConnection();
      throw err; // rethrow
    });
}
collectionDistinct: function collectionDistinct(collectionName, field, filter) {
  const url = getUrl();

  if(!url) return logger.error('No mongodb connection set');

  return getConnection(url)
    .then(db => distinctDocuments(db, collectionName, field, filter))
    .catch(err => {
      logger.error(err);
      clearConnection();
      throw err; // rethrow
    });
}

// Usage
ns.collectionFind(...).then(result => {

}, err => {

});

ns.collectionDistinct(...).then(result => {

}, err => {

});


Project Euler #22 - Name scores I have written code in C++ 11 and check output with Project Euler site, and it is correct. I am not showing output, just to keep it secret, at least from my end.

Please review my C++ 11 code.

#include <fstream>
#include <string>
#include <algorithm>
#include <iostream>
#include <vector>
#include <sstream>

using namespace std;

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

    ifstream file("E:\\names.txt");

    if (!file.is_open())
    {
        cout << "Unable to open file";
        return -1;
    }

    /* Get all file data with ',' delimiter*/
    vector<string> words;
    string token;

    while (getline(file, token, ','))
    {
        words.push_back(token);
    }

    //sort all the words
    sort(words.begin(), words.end());

    int counter = 0;
    unsigned int grand_total = 0;

    //get total of each word and multiply with it's position
    for (auto it = words.begin(); it != words.end(); it++)
    {
        counter++;
        //copy word from token excluding " "
        string word((*it).begin() + 1, (*it).end() - 1);

        int sub_total = 0;
        for (string::iterator it = word.begin(); it != word.end(); it++)
        {
            //ascii value of A is 65
            //A is 1, B is 2, so *it - 64
            sub_total += *it - 64;
        }

        //grand total will be final answer
        grand_total += sub_total*counter;
    }

    return 0;
}

The code is generally pretty clean, however, the major sticking point is that everything here is just shoved into main. Just breaking this up with a few functions that do one thing would be a good start:

std::vector<std::string> read_file(const std::string& path)
{
    std::ifstream file(path);

    if (!file.is_open())
    {
        std::cerr << "Unable to open file" << "\n";
        std::exit(-1);
    }

    std::vector<string> words;
    std::string token;

    while (std::getline(file, token, ','))
    {
        words.push_back(token);
    }

    return words;
}

I've also put back all the std:: namespace qualifier. Using namespace std is something that is ok for very short programs like this, but is something you shouldn't get into the habit of doing.

How about a function to sum the values in a word?

int sum_word(const std::string& word)
{
    constexpr static int ascii_to_integer = 64;
    int total = 0;
    for(auto c : word) {
        total += (c - ascii_to_integer);
    }
    return total;
}

In fact, if you want to be fancy, this can be even shorter:

int sum_word(const std::string& word)
{
    constexpr static int ascii_to_integer = 64;
    return std::accumulate(word.begin(), word.end(), 0, [](int i) { return i - ascii_to_integer; });
}

Finally, we can have a function to encompass the outer loop (processing each word in the vector):

int total_word_position(const std::vector<std::string>& v)
{
    int counter = 1;
    int total = 0;

    for(const std::string& s : v) {
        // Exclude " "
        std::string word(s.begin() + 1, s.end() - 1);
        total += (sum_word(word) * counter);
        ++counter;
    }
    return total;
}

Your main method would then become:

int main()
{
    std::vector<std::string> all_words = read_file("E:\\names.txt");
    std::sort(all_words.begin(), all_words.end());
    int grand_total = total_word_position(all_words);
}

This has a number of benefits:

Variable scope is minimized to a single smaller function, instead of a large main function.
It's generally easier to read and debug, as there is less to keep in your head.
It separates parts of the program that do very different things (reading a file vs sorting vs summing numbers).


Check if list contains a pair which adds up to a given sum Kindly review this scala code for given problem and suggest improvements.

Problem - Given an array of integers and a target sum, check if array contains a pair which adds up to sum.

Example -

i/p - (1,2,4,5,6) 7
o/p - true

i/p - (1,1,1,5,6) 2
o/p - true

i/p - (1,4,4,5,6) 13
o/p - false

Scala implementation

import scala.collection.immutable._

object FindMatchingSum {
  def containsMatchingSum(data: Array[Int], complementTable: Set[Int], sum: Int): Boolean = {
    if (data.isEmpty) false
    else if (complementTable contains data.head) true
    else containsMatchingSum(data.drop(1), complementTable ++ Set(sum-data.head), sum)
  }

  def main(args: Array[String]):Unit = {
    println(containsMatchingSum(Array(1,2,4,5,6), Set[Int](), 7))
    println(containsMatchingSum(Array(1,1,1,5,6), Set[Int](), 2))
    println(containsMatchingSum(Array(1,4,4,5,6), Set[Int](), 9))
    println(containsMatchingSum(Array(1,4,4,5,6), Set[Int](), 13))
  }
}

Sample run

scalac FindMatchingSum.scala
scala FindMatchingSum
true
true
true
false

If you're worried about efficiency you could copy the Array into a Vector or use an index instead of paring down the input.


Building a string from multiple inputs with some variations I am accepting a form from the user with a bunch of inputs and I am constructing a string from those inputs.  I am trimming each input and 2 of the inputs (GID & Clone) are not required fields so I'm doing null checks before building the string to avoid null object errors.

I wrote this method and it works, but I was wondering if there is a less repetitive way of doing this.

private string ConstructSeedReference(SeedViewModel model)
{
    string SeedRefId;

    if (!String.IsNullOrEmpty(model.Seed.GID) && !String.IsNullOrEmpty(model.Seed.Clone))
    {
        SeedRefId = String.Format("{0}{1}{2}{3}{4}{5}{6}{7}{8}",
        model.Customer.CustomerCode.TrimEnd(), model.Seed.CollectionCodeId.TrimEnd(), model.Seed.SpeciesId.TrimEnd(),
        model.Seed.Zone.TrimEnd(), model.Seed.Elevation.TrimEnd(), model.Seed.ColYear.TrimEnd(), model.Seed.OrchardId, model.Seed.GID.TrimEnd(), model.Seed.Clone.TrimEnd());
        return SeedRefId;
    }
    else if (!String.IsNullOrEmpty(model.Seed.GID) && String.IsNullOrEmpty(model.Seed.Clone))
    {
        SeedRefId = String.Format("{0}{1}{2}{3}{4}{5}{6}{7}",
        model.Customer.CustomerCode.TrimEnd(), model.Seed.CollectionCodeId.TrimEnd(), model.Seed.SpeciesId.TrimEnd(),
        model.Seed.Zone.TrimEnd(), model.Seed.Elevation.TrimEnd(), model.Seed.ColYear.TrimEnd(), model.Seed.OrchardId, model.Seed.GID.TrimEnd());
        return SeedRefId;
    }
    else if (String.IsNullOrEmpty(model.Seed.GID) && !String.IsNullOrEmpty(model.Seed.Clone))
    {
        SeedRefId = String.Format("{0}{1}{2}{3}{4}{5}{6}{7}",
        model.Customer.CustomerCode.TrimEnd(), model.Seed.CollectionCodeId.TrimEnd(), model.Seed.SpeciesId.TrimEnd(),
        model.Seed.Zone.TrimEnd(), model.Seed.Elevation.TrimEnd(), model.Seed.ColYear.TrimEnd(), model.Seed.OrchardId, model.Seed.Clone.TrimEnd());
        return SeedRefId;
    }
    else
    {
        SeedRefId = String.Format("{0}{1}{2}{3}{4}{5}{6}",
        model.Customer.CustomerCode.TrimEnd(), model.Seed.CollectionCodeId.TrimEnd(), model.Seed.SpeciesId.TrimEnd(),
        model.Seed.Zone.TrimEnd(), model.Seed.Elevation.TrimEnd(), model.Seed.ColYear.TrimEnd(), model.Seed.OrchardId);
        return SeedRefId;
    }
}

String.Format("{0}{1}{2}{3}{4}{5}{6}", ...);

Since you are just doing concatenation, string.Concat is more appropriate here.

if (!String.IsNullOrEmpty(model.Seed.GID) && !String.IsNullOrEmpty(model.Seed.Clone))
{
    SeedRefId = String.Format("{0}{1}{2}{3}{4}{5}{6}{7}{8}",
    model.Customer.CustomerCode.TrimEnd(), model.Seed.CollectionCodeId.TrimEnd(), model.Seed.SpeciesId.TrimEnd(),
    model.Seed.Zone.TrimEnd(), model.Seed.Elevation.TrimEnd(), model.Seed.ColYear.TrimEnd(), model.Seed.OrchardId, model.Seed.GID.TrimEnd(), model.Seed.Clone.TrimEnd());
    return SeedRefId;
}

Between your ifs and else, there is a lot of repeated code. And, we can observe a pattern there:

The first 7 elements are always present.
The last 2 elements are present if it is not null/empty
The elements are always in the same order.

The method can be reduced to this:

private string ConstructSeedReference(SeedViewModel model)
{
    var reference = string.Concat(
        model.Customer.CustomerCode.TrimEnd(),
        model.Seed.CollectionCodeId.TrimEnd(),
        model.Seed.SpeciesId.TrimEnd(),
        model.Seed.Zone.TrimEnd(),
        model.Seed.Elevation.TrimEnd(),
        model.Seed.ColYear.TrimEnd(),
        model.Seed.OrchardId);

    if (!string.IsNullOrEmpty(model.Seed.GID))
    {
        reference += model.Seed.GID.TrimEnd();
    }

    if (!string.IsNullOrEmpty(model.Seed.Clone))
    {
        reference += model.Seed.Clone.TrimEnd();
    }

    return reference;
}

EDIT: Just reread your question, if your concern is only about the NullReferenceException. You can simply use null-coalescing operator ?.:

private string ConstructSeedReference(SeedViewModel model)
{
    return string.Concat(
        model.Customer.CustomerCode.TrimEnd(),
        model.Seed.CollectionCodeId.TrimEnd(),
        model.Seed.SpeciesId.TrimEnd(),
        model.Seed.Zone.TrimEnd(),
        model.Seed.Elevation.TrimEnd(),
        model.Seed.ColYear.TrimEnd(),
        model.Seed.OrchardId,
        model.Seed.GID?.TrimEnd(),
        model.Seed.Clone?.TrimEnd());
}


Simple load balancer model in Scala For one of my cases in algorithms I am required to create a model of a load balancer model for multiple processors. Lately I became interested in Scala so I decided that it would be great to create this program in Scala. I'm not really familiar with the concept of functional programming so any feedback will be welcome. Please note that I am not asking about the correctness of my algorithm as this is my homework.

object HelloWorld {
  def randomElements(arg: Integer): Double = {
    val r = scala.util.Random
    return r.nextDouble()
  }
  // used for debugging
  def return1Elements(arg: Integer): Double ={
    return 1
  }

  def fillJobs(size: Integer, fillingFunction: (Integer) => Double): Array[Double] = {
    val array = new Array[Double](size);
    for (el <- 0 to size -1) {
      array(el) = fillingFunction(el);
    }
    return array
  }

  def fillProcessors(size: Integer): Array[Double] = {
    val array = new Array[Double](size);
    for (el <- 0 to size -1) {
      array(el) = 0
    }
    return array
  }
  def result(processors: Array[Double]): Double = {
    return processors.max
  }

  def findMaxSolver(processorsArray: Array[Double], jobsArray: Array[Double]): Double = {
    val sortedJobs = jobsArray.sortWith(_ > _)
    var sortedProcessors = processorsArray

    for(processorIndex <- 0 to processorsArray.size -1){
      sortedProcessors(processorIndex) += sortedJobs(processorIndex)
    }

    for(job <- sortedJobs.slice(sortedProcessors.size, sortedJobs.size)){
      sortedProcessors = sortedProcessors.sortWith(_<_)
      sortedProcessors(0) += job
    }
    return result(sortedProcessors)
  }

  def solve(processorsArray: Array[Double], jobsArray: Array[Double],
            solvingFunction: (Array[Double], Array[Double]) => Double): Double = {
    val processors = processorsArray.clone()
    val jobs = jobsArray.clone()

    return solvingFunction(processors, jobs)
  }

  def main(args: Array[String]) {
    val JOB_NUMBER = 1024
    val PROCESSOR_NUMBER = 8
    val jobs = fillJobs(JOB_NUMBER, randomElements)
    val processors = fillProcessors(PROCESSOR_NUMBER)

    println("Job times")
    println("============================")
    for (element <- jobs) {
      println(element)
    }
    println("============================")
    println("Results")
    println("Find max solver " + solve(processors, jobs, findMaxSolver))
  }
}

Below are some of the changes I would make to your code. In my opinion one of the nice aspects of Scala is that it gives you all sorts of ways to reduce the amount of work your mind has to do in order to decipher code. As an example, one of the first things I did was declare a type ArrD that is equivalent to Array[Double]. I then just substitued where necessary and the code (to my mind) became more readable. The choice of ArrD was arbitrary on my part, you could if you wanted use ArrayDouble instead. Along these lines I shortened all of you variable and value names.

As the program is set up right now you don't need to pass in a procs array, but I left it in anyway. As you mentioned this is a homework assignment so I'll leave a bit of mystery as to why this is. And really you don't need the take and the drop.

Next note that jobs.sorted is equivalent to jobs.sortWith(_ > _).

And finally (for now) checkout how I initialized jobs and procs. If you still would like to use your randomElements function you should look into the method called tabulate.

Cheers.

object O {

  type ArrD = Array[Double]

  def findMaxSolver(procs: ArrD, jobs: ArrD): Double = {
    val sJobs  = jobs.sorted
    var sProcs = sJobs.take(procs.size)

    for(job <- sJobs.drop(procs.size)){
      sProcs     = sProcs.sorted
      sProcs(0) += job
    }
    sProcs.max
  }

  def solve(procs: ArrD, jobs: ArrD, f: (ArrD, ArrD) => Double) = f(procs, jobs)

  def run = {
    val jobNum  = 1024
    val procNum = 8
    val jobs    = Array.fill(jobNum)(scala.util.Random.nextDouble())
    val procs   = Array.fill(procNum)(0.0)
    println(s"RESULTS:\nFind max solver ${solve(procs, jobs, findMaxSolver)}")
  }
}


A more optimized way of finding prime factors I've solved Project Euler #3, but solving the same in Hacker Rank which has time constraint of < 4 seconds doesn't work. I've tried using the square root solution for speeding up but it fails.

Example:

The prime factors of 10000001 are 11 and 909091, we have sqrt(10000001) = 3162, if we check all primes from 2 to 3162, we can only have 11, after loop terminated, the result will be 11, incorrect answer.

So, when I use num/2 the running time is significantly increased.

public class Solution {

static boolean checkPrime(long num){
boolean toret = true;
long sq = (long) Math.sqrt(num);
for(long i=2; i<=sq; i++){
    if(num%i ==0)
        toret = false;
}
return toret;
}

static long gimmeAns(long num){
ArrayList<Long> al  = new ArrayList<Long>();
int q = 0;
long[] list = new long[10];
long sq = (long)Math.sqrt(num);
long sqint;
boolean flag = true;
if(num < 0){
    for(long i =3; i<=num; i+=2){
    if(num%i == 0)
        al.add(i);
        }
}else{
    //for(long i =3; i<=sq; i+=2){
    //if(num%i == 0)
        //al.add(i);
        //}
    boolean isPrimeCheck = checkPrime(num);
    if(isPrimeCheck){
        al.add(num);
    }
    for(long i =3 ; i<= num/2 ; i+=2){
        if(num%i==0){
            al.add(i);

        }
    }
}
Iterator it = al.iterator();
while(it.hasNext()){
    long curr = (long) it.next();
     flag = true;
    long squrt = (long) Math.sqrt(curr);
        for(long i=2; i<=squrt; i++){
            if(curr%i == 0)
                flag = false;
        }
    if(flag == true){

        list[q++] = curr;
    }
}

return list[q-1];

}

public static void main(String[] args) {
    long numberCases;
    Scanner in = new Scanner(System.in);
    numberCases = in.nextLong();
    long result = 0;
    for(long i=0; i< numberCases; i++){
        long num = in.nextLong();
        result = gimmeAns(num);
        System.out.println(result);
    }

}
}

Format Your Code

It's extremely difficult to follow your logic with inconsistent indenting. 4-spaces after each brace please.

Stop When You Know The Answer

Here is your function to find out if a number is prime. Which you don't actually need for this problem (more on that in a bit), but I wanted to talk about it anyway:

static boolean checkPrime(long num){
    boolean toret = true;
    long sq = (long) Math.sqrt(num);
    for(long i=2; i<=sq; i++){
        if(num%i ==0)
            toret = false;
    }
    return toret;
}

So it is indeed true that you only have to go up to the square root of num. However, let's say we want to find out if 1 trillion is prime. We check 2, and 2 is indeed a divisor. And then we check 3, and 4, and ... and we still check another million numbers. That is hugely inefficient. If you found a divisor, it's already not prime, so just stop:

static boolean checkPrime(long num){
    long sq = (long) Math.sqrt(num);
    for(long i=2; i<=sq; i++){
        if(num%i ==0)
            return false; // we know it's not prime!
    }
    return true;
}

The Algorithm

We want the largest prime factor. We don't care about any other factors, so all the logic around keeping a list (and then going through it again? I don't know what that's about) is unnecessary extra work. Furthermore, you're checking if the number is prime and then going through all the rigmarole of checking all the divisors again. You definitely want to check every divisor at most one time. And even better, divide num as you go:

long largest = 1;
for (long i=2; i*i<=num; ++i) {
    while (num % i == 0) {
        largest = i;
        num /= i;
    }
}

return num > 1 ? num : largest;

So from the Euler example, we start with num == 13195 and i == 2. That doesn't divide, neither does 3 or 4. We get to 5, set largest to 5, and reduce num to 2639. That is no longer divisible by 5. Then we get to 7, largest is now 7 and num is reduced to 377. Then we get to 13, and num is reduced to 29.

At this point we're done, since 14*14 > 29. We know that the remainder must be prime (otherwise we would've found a smaller factor already), so we return 29.

Note that the square root of 13195 is 114, but we didn't even go as high as 14. Your original solution would've have gone up to 114, then gone up to 6597. I just saved you something like 6700 mod operations. And mod operations are not cheap!

Optimization Potential

Once we check for 2, we don't have to check any more even numbers. Once we check for 3, we don't have to check any more multiples of 3. So a simple optimization would be to loop over the factors 5, 7, 11, 13, 17, 19, ... by alternating adding 2 and 4. This is going to be a relatively minor improvement compared to the algorithm improvement I just suggested.


Pull data from database to generate XML files I have the following script in Perl that does the following:

Prompts user for time in epoch. If user enters '########', the query will look for all reports generated after that date/time. If user does not input time, it will just look for all reports in past day.
Connects to a database, runs a query to receive reports
I create a directory of the current date (i.e. Jun25)
I iterate through every row in the results and I save the contents of the xml file into an actual file and name the file [customer name]-[epoch time].xml

I am trying to clean up my code and have it run more efficiently.

#!/usr/bin/perl

#use strict;
use DBI;
use File::Slurp;
use Encode;
use utf8;
use POSIX qw(strftime);
use File::Slurp;
use XML::XPath;
use XML::XPath::XMLParser;
$| = 1;
print "Please enter epoch time of which to receive reports or leave blank to receive last day of reports: ";
my $epoch = <STDIN>;
chomp($epoch);
if ($epoch =~ /\D/) {$epoch = time() - 86400}
###Start Database
my $dsn = join "", (
    "dbi:ODBC:",
    "Driver={SQL Server};",
    "Server=SERVER;",
    "Database=DATABASE",
);
my $user = q/USERNAME/;
my $password = q/PASSWORD/;

my $db_options = {
    PrintError => 1,
    RaiseError => 1,
    #LongTruncOk => 1,
    LongReadLen => 100000000,
};

print "CONNECTING TO DATABASE...";
# Connect to the data source and get a handle for that connection.
my $dbh = DBI->connect($dsn, $user, $password, $db_options)
    or die "Can't connect to $dsn: $DBI::errstr";
print "CONNECTED\nFETCHING ROWS AND PARSING...\n";

my $sql = "SELECT [receivedTime], [reportXml] from UsageReport WHERE [receivedTime] > " . $epoch . " AND [reportXml] NOT LIKE '%exclude%'";

# Prepare the statement.
my $sth = $dbh->prepare($sql)
     or die "Can't prepare statement: $DBI::errstr";

$sth->execute();
###End Database

#Make Directory of Current Date
$datestring = strftime "%b%d", localtime;
mkdir $datestring;
chdir $datestring;
# Fetch and display the result set value.
my @matrix;
my $count = 0;
while (my @row = $sth->fetchrow_array ) {
    my ($time, $report) = @row; #time, report
    $count++;
    eval {
        my $xp = XML::XPath->new(xml => $report);
        my $customer = $xp->getNodeText('/report/instances/instanceroot/instanceheader/customer');
        if ($customer eq "") {next;}
        our $ext = $customer . "-" . $time . ".xml";
        write_file($ext, $report);
    };
    if ( $@ ) {printf "ERROR ";}
    else {printf "$count ";}
}

# Disconnect the database from the database handle.
$dbh->disconnect;
print "\nCOMPLETED REPORT PARSING\nLatest Report Date: ";

The program asks to input the time first,
before trying to connect to the database.
If the database connection fails,
the user might be frustrated for having entered something only to watch the program crash.
It would be better to move the input part right before it's really needed.

There are some unused elements:

use File::Slurp; appears twice, delete the second
@matrix is never used

The comment at the end of this line is pointless:

my ($time, $report) = @row; #time, report


Simple C++ Dynamicly allocated Matrix with only one memory allocation I was bothered with existing matrix allocation methods that take variable rows and columns, so I made a small class that only needs one memory allocation.

template <typename T>
class Matrix{
    T* data;
    size_t rows, columns;

public:
    Matrix(const size_t rows, const size_t columns) : rows(rows), columns(columns) {
        // One single allocation to store data
        data = new T[rows+columns + 1];
    }

    T* operator[](const size_t r) {
        return data+(r*columns);
    }
};

// Example

int main() {

    Matrix<int> mat(5,3);

    mat[2][1] = 1;

    //...

}

Since you have the new keyword, you should make a destructor and delete[].

~Matrix()
{
    delete[] data;
    data = nullptr;
}

Note that it is not necessary to set data = nullptr in the destructor, because it will be deleted as soon as the destructor is called.

You are missing a copy constructor which can cause problems when assigning one matrix to another

What happens if the number passed in [] when slicing from the matrix is out of bounds?

Have a glance at std::initialzer_list in C++, it will simplify assigning values to the matrix.

You will have to overload the arithmetic operators ( +, -, *, /) if you want to perform any kind of operations between two matrices

Lastly, I was a little confused to see size_t rather than std::size_t, if you have using std::size_t in your source code, you should have it here too to avoid confusion.


Calculator to add time intervals I have written a little program that calculates the time by given strings.

A time string in the context of this program consists of a number and a following letter that can be s, m, h, or d.

You can enter something like this:

java TimeCalculator 443432s 34234m 34323m 32344234s 8h 23d 12h

It than calculates the time and prints it out in an readable format:

450 days 22 hours 18 minutes 6 seconds

I want to ask you, how I can improve the readability and maybe also the performance. I dont tried to design the program object oriented because I think the program is to small for that. But if you have another opinion on that, please let me know!

public class TimeCalculator {
    private static final int MINUTE_IN_SECONDS = 60;
    private static final int HOUR_IN_SECONDS = 3600;
    private static final int DAY_IN_SECONDS = 86400;

    private static int seconds = 0;

    public static void main(String[] args) {
        if (args.length == 0 || args[0].contains("help")) {
            showHelp();
            return;
        }

        for (String arg : args) {
            timeStringToSeconds(arg);
        }

        printNewTimeString();
    }

    private static void timeStringToSeconds(String timeString) {
        if (timeString.contains("s")) {
            timeString = timeString.replace("s", "");
            seconds += Integer.parseInt(timeString);
        } else if (timeString.contains("m")) {
            timeString = timeString.replace("m", "");
            seconds += Integer.parseInt(timeString) * MINUTE_IN_SECONDS;
        } else if (timeString.contains("h")) {
            timeString = timeString.replace("h", "");
            seconds += Integer.parseInt(timeString) * HOUR_IN_SECONDS;
        } else if (timeString.contains("d")) {
            timeString = timeString.replace("d", "");
            seconds += Integer.parseInt(timeString) * DAY_IN_SECONDS;
        }
    }

    private static void printNewTimeString() {
        String newTimeString = new String();
        // calculate days
        int days = 0;
        while (seconds >= DAY_IN_SECONDS) {
            days++;
            seconds -= DAY_IN_SECONDS;
        }
        if (days > 0) {
            newTimeString += days + " days ";
        }

        // calculate hours
        int hours = 0;
        while(seconds >= HOUR_IN_SECONDS) {
            hours++;
            seconds -= HOUR_IN_SECONDS;
        }
        if (hours > 0) {
            newTimeString += hours + " hours ";
        }

        // calculate minutes
        int minutes = 0;
        while (seconds >= MINUTE_IN_SECONDS) {
            minutes++;
            seconds -= MINUTE_IN_SECONDS;
        }
        if (minutes > 0) {
            newTimeString += minutes + " minutes ";
        }

        // calculate seconds
        if (seconds > 0) {
            newTimeString += seconds + " seconds ";
        }

        System.out.println(newTimeString);
    }

    private static void showHelp() {
        System.out.println("This Program converts time strings to an ordered string that makes the time");
        System.out.println("information better understandable\n");
        System.out.println("You can enter something like this: ");
        System.out.println("java TimeCalculator 534s 400d 32453s 234h");
        System.out.println("The output will be this: ");
        System.out.println("410 days 3 hours 9 minutes 47 seconds");
        System.out.println("Available characters: d, h, m, s");
    }
}

Instead of subtracting the day_in_seconds repeatedly you could divide the total seconds by day_in_seconds and then set seconds equal to (seconds % days_in_seconds).

Something like:

if(seconds/days_in_seconds > 0) {
    days = seconds/days_in_seconds;
    seconds = seconds%days_in_seconds;
}

And so on for the other values..

Hope that helps (Posting from mobile)


Customized date and time display A simple reworking of the Bash date command to my own personal liking. Possibly a rather trivial task; I just wanted the date and time to be in the form of:

Fri Sep 2 2016
11:37 P.M.

I think this accomplishes that:

#!/bin/bash

today() {
    now="$(date | awk '{ OFS=" "; print $1,$2,$3,$6 }')"
    h="$(date "+%H")"
    m="$(date "+%M")"
    if [ $h -gt 12 ]; then
        echo -e "$now\n$(( $h - 12 )):$m P.M."
    elif [ $h = 12 ]; then
        echo -e "$now\n$h:$m P.M."
    elif [ $h = 00 ]; then
        h=12; echo -e "$now\n$h:$m A.M."
    elif [ $h -le 11 ]; then
        if [ $h -lt 10 ]; then
            h="$(echo $h | cut -c2)"
        fi
        echo -e "$now\n$h:$m A.M."
    fi
}

today

Is there anything I could do to improve this?

Parsing formatted output — as you have done with $(date | awk '{ OFS=" "; print $1,$2,$3,$6 }') — is a dangerous game: it's hard to say exactly what awk should expect as input.  For example, on GNU/Linux, I get:

$ LC_TIME=en_CA.UTF-8 date
Fri Sep  2 23:54:56 PDT 2016
$ LC_TIME=C date
Fri Sep  2 23:54:56 PDT 2016

However, on macOS, I get:

$ LC_TIME=en_CA.UTF-8 date
Fri  2 Sep 2016 23:54:56 PDT
$ LC_TIME=C date
Fri Sep  2 23:54:56 PDT 2016

Note the difference in the output in the en_CA.UTF-8 locale.  More "exotic" locales could produce even wilder output.  I don't recommend parsing the output of date, but if you do, be sure to explicitly state the locale using LC_TIME=C date.

What would be a better approach, then?  The date command's format specifiers are meant for just this kind of thing, and you can get quite close to your desired output.  On GNU/Linux:

$ date +'%a %b %e %Y%n%I:%M %p'
Fri Sep  2 2016
23:54 PM

On macOS:

$ date +'%a %b %e %Y%n%I:%M %p'
Fri Sep  2 2016
23:54 pm

Although the format specifiers are only loosely standardized, I would still consider this to be a more idiomatic and reliable solution than yours.  If you insist on getting exactly the format that you specified in your example, then a bit of post-processing might be needed.

$ date +'%a %b %e %Y%n%I:%M %p' | sed -e 's/  //' -e 's/[Aa][Mm]$/A.M./' -e 's/[Pp][Mm]$/P.M./'


Function to convert a title to a slug I'm creating a function that converts a title, to a URL slug. I'm not all to familiar with regular expressions, but I've done my best. Can you see any problems or improvements with the following function below?

The only thing allowed in the slug is letters, numbers and - charchaters.

function slugify($input) {
    // Convert multiple spaces to single spaces
    $slug = preg_replace("/[[:blank:]]+/",' ', $input);
    // Convert to lower case
    $slug = strtolower($slug);
    // Remove anything that's not a number, letter or space
    $slug = preg_replace('/[^a-z0-9\s]+/', '', $slug);
    // Trim, and replace spaces with hyphens
    $slug = preg_replace('/\s/', '-', trim($slug));

    return $slug;
}

You should prepare a set of sentences to slugify and verify by yourself if your function is ok.

Below are the step I use to slugify text:

Use iconv() if available:

$slug = iconv('utf-8', 'us-ascii//TRANSLIT', $text);
Lowercase the text, taking Unicode into account:

$slug = mb_strtolower($slug);
Remove unwanted characters like you do:

$slug = preg_replace('/\W+/', '-', $slug);

These are the steps used in Propel ORM or symfony framework for example.
The complete code can be:

function slugify($text, $separator = '-')
{
  // transliterate
  if (function_exists('iconv'))
  {
    $slug = iconv('utf-8', 'us-ascii//TRANSLIT', $text);
  }

  // lowercase
  if (function_exists('mb_strtolower'))
  {
    $slug = mb_strtolower($slug);
  }
  else
  {
    $slug = strtolower($slug);
  }

  // remove accents resulting from OSX's iconv
  $slug = str_replace(array('\'', '`', '^'), '', $slug);

  // replace non letter or digits with separator
  $slug = preg_replace('/\W+/', $separator, $slug);

  // trim
  $slug = trim($slug, $separator);

  return $slug;
}

I think $text is a better name than $input for the string to slugify.


Update user's last login date This function updates the user last login date on the user collection.  I guess there are too many brackets and much spaghetti. How can I shorten this code?

(function () {
    'use strict';

    var database = require('mongodb');
    var util = require('util');

    var constants = require('../constants');

    var dbhelper = {
        updateLastSeenDate: function (me) {
            database.connect(constants.variables.connection, function (err, db) {
                if (err) {
                    db.close();
                    throw err;
                }
                var bsonID = database.BSONPure.ObjectID(me);
                var collection = db.collection('users');
                collection.findOne({
                    _id: bsonID
                }, function (err, result) {
                    if (result) {
                        result.lastLoginDate = Date.now();
                        collection.update({
                            _id: bsonID
                        }, result, function (err, u) {
                            if (err) {
                                db.close();
                                throw err;
                            }
                            util.log('Update complete: updateLastSeenDate');
                            db.close();
                        }); //end of update
                    }
                });
            });
        }
    };

    module.exports = dbhelper;
}());

Here's some points. I commented directly in the code.

(function () {
    'use strict';

    //Why do you use `database` instead of `mongo` or `mongodb`.
    //It can become confusing later in the code when you are using `db`.
    //It will be hard to tell the diffence between database and db.
    var database = require('mongodb');
    var util = require('util');

    var constants = require('../constants');

    var dbhelper = {
        //`me` Will it be you all the time ?
        //You should think a more meaningful name such as `logedInUserId`
        updateLastSeenDate: function (me) {
            //Do you really want to connect and close the database everything you want to update a field ?
            //You should consider keeping the connection alive and share it across the whole process.
            database.connect(constants.variables.connection, function (err, db) {
                if (err) {
                    db.close();
                    throw err;
                }
                var bsonID = database.BSONPure.ObjectID(me);
                var collection = db.collection('users');

                //nesting calls to async function is affecting readability, testability and maintability.
                //You should consider using workflow library such as async
                collection.findOne({
                    _id: bsonID
                }, function (err, result) {
                    if (result) {
                        result.lastLoginDate = Date.now();

                        //Here's another nested call
                        collection.update({
                            _id: bsonID
                        }, result, function (err, u) {
                            //Using async you could handle errors in only one place.
                            if (err) {
                                db.close();
                                throw err;
                            }
                            util.log('Update complete: updateLastSeenDate');
                            db.close();
                        });
                    }
                });
            });
        }
    };

    module.exports = dbhelper;
}());

I would use async https://github.com/caolan/async#waterfall, change a few variable names and share the connection. Since node.js in single threaded you can open only one connection and share it to all your modules.


Calculating the difference between 2 arrays I made this function to calculate the difference between 2 arrays and return it in a third array. I'm using this function as a part of stocktaking functionality I apply in my accounting system.

function stockdiff ($array2,$array1){
    //inputs :
    //$array2 is the stocktaking result in an array that has the format "$product_id=>$quantity"
    //$array1 is the stock according to the data in the system in an array that has the format "$product_id=>$quantity"
    foreach($array2 as $key=> $value){
        if(isset($array1[$key])){$result[$key] = $array2[$key] - $array1[$key];}
        else{$result[$key] = $array2[$key];}
    }
    $array1diff = array_diff_key($array1,$array2);

    foreach($array1diff as $key=> $value){
        $result[$key] = -1 * $array1[$key];
    }
return $result;
}

Case example

When the user do the stocktaking, its result are going to be saved in $array2 in the format $product_id=>$quantity.

This is a stocktaking result example:

$array2[2] = 500;
$array2[3] = 7;
$array2[1] = 302;
$array2[105] = 7000;
$array2[7] = 304;
$array2[8] = 20;
$array2[9] = 20;
$array2[11] = 20;
$array2[73] = 32;
$array2[21] = 35;

I then compare it against the stock quantities according to the data in the system $array1 which might be something like this:

$array1[1] = 30;
$array1[2] = 60;
$array1[3] = 202;
$array1[4] = 200;
$array1[7] = 0;
$array1[8] = 0;
$array1[9] = 0;
$array1[11] = 52;
$array1[21] = 70;
$array1[99] = 21;

I made this function to do that job. I need it to tell me the changes that happened for every product between the data in the system and the actual data come from the stocktaking, for example, let's take product_id 2.

In the system it was $array1[2] = 60; 60 units from the product 2, and the stocktaking told us no, we found 500 units from the product 2 $array2[2] = 500;. So, the function must return [2]=>440.

The function also needs to consider about these points:

If there is a product in the stocktaking array ($array2) that has no matching product in the stock-in-system array($array1), then the function will assume it's whole quantity has been increased and return it's quantity from $array2.
If there is a product in the stock-in-system array ($array1) that has no matching product in the stocktaking array ($array2), then the function will assume it's whole quantity has been decreased and return -1 * quantity.

What do you think about the function?

Regarding the way to achieve this task I couldn't find any better strategy than yours.

On the other hand your code may be made cleaner and more readable by:

first merely following best-practices (don't put several statements in the same line,
using more significant names (such as $initial and $final instead of $array1 and $array2, $final_value and $initial_value rather than only $value)
avoiding to declare useless variables (like $array1diff, used only once)
using the more direct expressions when possible (e.g. $array2[key] is already available as $value, while -1 * $array1[$key] is merely -$array1[$key])
using the ternary operator where it can simplify the code (see also the note below)

It results in a reduced code:

function stock_diff($initial, $final) {
    foreach ($final as $key => $final_value) {
        $result[$key] = $final_value - @$initial[$key] ?: 0;
    }
    foreach(array_diff_key($initial, $final) as $key => $initial_value) {
        $result[$key] = -$initial_value;
    }
    return $result;
}

NOTE: regarding the use of @ in the ternary operator, I know that many people automatically banish it as globally evil.
I don't agree with this too general point of view: sure it must be avoided in many situations where it might lead to some issues, but it's pretty legal when used carefully and with discernment.

In the case above, the only "error" it may hide is the one we know it may happen (undefined index), so it keeps sure.
And so it helps making the code more readable and reduced:
(isset($initial[$key]) ? $initial[$key] : 0)
is replaced by
@$initial[$key] ?: 0

The only con is about performance: using @ works slightly slower than isset(). So avoid it when you know that a piece of code will be executed a significantly huge number of times.


Removing image records if no physical file exists I have a working script that selects image fields in all tables and empty their values if the physical file doesnt exist.

    $query1 = "SELECT table_name,column_name
           FROM information_schema.columns
           WHERE table_schema='schemaname' AND column_name like '%image%' or column_name='video'";

$result1 = mysql_query($query1) or die(mysql_error() . " -- " . $query1);

while($row1 = mysql_fetch_row($result1)){
    if (!strpos($row1[0],'backup') > 0){
            $sql = "Select COLUMN_NAME FROM information_schema.columns WHERE TABLE_NAME = '".$row1[0]."' AND EXTRA = 'auto_increment'";
            $resultcol = mysql_query($sql);
            $rowcol = mysql_fetch_row($resultcol);

        $query2 = "SELECT " . $row1[1] . ", " .$rowcol[0] . "
               FROM " . $row1[0] . "
               WHERE " . $row1[1] . " != '' AND " . $row1[1] . " IS NOT NULL
               ";
        echo $query2 . "
";
        $result2 = mysql_query($query2) or die(mysql_error() . " -- " . $query2);

        while ($rowdb =  mysql_fetch_row($result2)){
            if (!strpos($rowdb[0],'facebook') > 0 && !file_exists($img_root.'/'.$rowdb[0])){
                $sql = "UPDATE ".$row1[0]." SET ". $row1[1] . " = '' WHERE " . $rowcol[0]. "= ".$rowdb[1];
                echo $sql . "

";
                $delete_count++;
                //mysql_query("UPDATE ".$row1[0]." SET ". $row1[1] . " = '' WHERE id = ".$row1["id"]);
            }
        }
    }
}

The script is working fine, but it takes time.  I was wondering if there is a smarter (more optimized) way to get the same function.

The mysql_* functions are deprecated.  Use mysqli or PDO instead.

Each query that you issue has a rather large overhead: you have to communicate the query to the server; the server has to parse, plan, and execute the query; the results have to be returned to your application.  Therefore, you want to avoid running any query in a loop.  You're much better off executing fewer complex queries than many simple queries.

First, note that your $query1 has a bug: AND has higher precedence than OR, so you need parentheses.  Also, you eventually ignore any column named …backup, so you might as well filter those out immediately.

SELECT TABLE_NAME AS table_name, COLUMN_NAME AS image_col
    FROM INFORMATION_SCHEMA.COLUMNS
    WHERE
        TABLE_SCHEMA = 'schemaname'
        AND TABLE_NAME NOT LIKE '%backup%'
        AND (COLUMN_NAME LIKE '%image%' OR COLUMN_NAME = 'video');

Let's clarify the naming.

Your $row1[0] is what I call table_name
Your $row1[1] is what I call image_col

Notionally, you want to execute this query for each result returned by that $schema query.

UPDATE `$schema['table_name']`
    SET `$schema['image_col']` = ''
    WHERE
        `$schema['image_col']` != '' AND `$schema['image_col']` IS NOT NULL
        AND `$s['image_col']` NOT LIKE '%facebook%'
        AND NOT file_exists('$img_root/$schema['image_col']')

Of course, there are two problems with that idea:

There is no file_exists() function in MySQL (and furthermore, if such a function existed, it would be wrong to run it on the database server rather than on the application server, since they could be on different machines with different filesystems).
You have to execute many UPDATEs, not just one.  Furthermore, the UPDATE queries are dynamic — the table and column names vary.

This is a strong indication that your database schema is poorly designed.  There should not be a proliferation of tables!  You should also not be doing version control or backups by creating tables named …backup.  I encourage you to follow up with these issues on http://dba.stackexchange.com.

A solution to the first problem is to create a temporary table that lists all images on your website's filesystem, then write the UPDATE with an anti-join.

$db = new mysqli(…);
$db->query('CREATE TEMPORARY TABLE fs_images ( filename VARCHAR(128) PRIMARY KEY );');
$ins = $db->prepare('INSERT INTO fs_images VALUES (?)');
$file_it = new RecursiveDirectoryIterator($img_root);
foreach (new RecursiveIteratorIterator($file_it) as $f) {
    $ins->bind_param('s', basename($f));
    $ins->execute();
}
$ins->close();

A solution to the second problem is to use a stored procedure with a loop that executes dynamically composed SQL queries with a cursor.

DELIMITER $$
CREATE PROCEDURE clear_images()
BEGIN
    DECLARE done INT DEFAULT FALSE;
    DECLARE table_name VARCHAR(64);
    DECLARE image_col VARCHAR(64);

    DECLARE schema_cursor CURSOR FOR
        SELECT TABLE_NAME AS table_name, COLUMN_NAME AS image_col
            FROM INFORMATION_SCHEMA.COLUMNS
            WHERE
                TABLE_SCHEMA = 'schemaname'
                AND TABLE_NAME NOT LIKE '%backup%'
                AND (COLUMN_NAME LIKE '%image%' OR COLUMN_NAME = 'video');
    DECLARE CONTINUE HANDLER FOR NOT FOUND SET done = TRUE;

    OPEN schema_cursor;
    schema_loop: LOOP
        FETCH schema_cursor INTO table_name, image_col;
        IF done THEN
            LEAVE schema_loop;
        END IF;

        PREPARE stmt FROM CONCAT(
            'UPDATE `', @table_name, '`',
            '    SET `', @image_col, '` = ''''',
            '    WHERE',
            '        `', @image_col, '` != '''' AND `', @image_col, '` IS NOT NULL',
            '        AND `', @image_col, '` NOT LIKE ''%facebook%''',
            '        AND `', @image_col, '` NOT IN (SELECT filename FROM fs_images);'
        );
        EXECUTE stmt;
        DEALLOCATE PREPARE stmt;
    END LOOP;
END$$
DELIMITER ;

Anyway, this solution is rather nasty (and untested!) — all as a result of your poor database schema design.


Function to find the minimum depth of a tree For a leetcode problem to find the minimum depth of the binary tree, I have written the below solutions. The solution is accepted, but can I do any other changes to make the code elegent?

   public class Solution {

        public int minDepth(TreeNode root) {
            if(root == null)
            {
                return 0;
            }

            //to handle the leaf node case...
            if(root.left == null && root.right == null)
            {
                return 1;
            }

            //To handle the case that has only right children..
            if(root.left == null)
            {
                return minDepth(root.right) + 1;
            }

            if(root.right == null)
            {
                return minDepth(root.left) + 1;
            }

            return Math.min( minDepth(root.left)+1,minDepth(root.right)+1);
        }
    }

This code will visit every node in the tree, but it's not always necessary to do all that work. Instead you can do a breadth-first traversal of the tree, and look for the first node that has no children.

Consider this tree:

    o
   / \
  o   o
       \
        o
         \
          o
          ...
            o
             \
              o

With a breadth-first traversal, you only need to look at two nodes to determine the minimum depth. With the current code, you will visit every node and use \$O(n)\$ stack space in the process.

In some cases you will have to traverse every node, but my point is that sometimes you can do better.

The only trick is that you also have to keep track of the depth of each node.

The idea is to:

Enqueue the root onto a queue
While the queue is not empty:

Dequeue from the queue
If the node has no children, return its depth; this is a node of minimum depth in the tree.
Otherwise, enqueue the children of the node onto the queue


BFS for modified shortest path Given K descriptions for bus line paths that exists to lead students between N campuses. What is the minimum cost that a student will have to goes from campus 1 to campus N ?
The itinerary of each path L is a sequence of |L|( ≥ 2 ) campuses {C1, C2, …, CL}, and each line has only one bus, which passes by all campuses of the line, following the itinerary order, stopping in each of them and making a U-turn whenever it reaches an endpoint of the itinerary, reversing the itinerary order. The transport pass costs 1$, and has to be paid by the passenger while getting onto the bus, no matter the time the passenger is going to stay in it.
Input:

The first input line consists of two integers N and K (2 ≤ N ≤ 10^4, 1 ≤ K ≤ 10^3), which represent respectively the number of campuses and the number of public transport lines created by UFFS. Each one of the K input lines following describes a transport line L and consists of the integer |L| (2 ≤ |L| ≤ 10^2) followed by the |L| identifiers Ci (1 ≤ Ci ≤ N, 1 ≤ i ≤ |L|) of the campuses by which the ship passes, wherein C1 and C|L| are the endpoints of L. For every campus A and every campus B it is guaranteed that it is possible to go from A to B.

Output:

Output the minimum cost to go from campus 1 to campus N.

To solve this i've tried a BFS. Each node has its distance updated. The algorithm takes the minimum distance found.
The code is easy to understand ?
I'm getting time limit exceeded on the judge that has this problem. How can i make this solution faster ?
#include <cstdio>
#include <vector>
#include <queue>
#include <string.h>

struct Edge{
    int d,t,c; //distance , target and color
    Edge(int _d,int _t ,int _c):d(_d),t(_t),c(_c){}
    Edge(int _t, int _c):d(0),t(_t),c(_c){}
};

using adj_list = std::vector<Edge>    ;
using graph    = std::vector<adj_list>;

unsigned int BFS(graph &g, int source, int target, int max_colors){

    //init visited as false for every node
    bool visited[g.size()][max_colors];
    memset(visited,false,sizeof(visited));

    //put the source with an unexistent color(forcing int to buy all possible colors) and distance=0
    std::queue<Edge> q;
    q.push(Edge(0,source,0));

    unsigned int shortest_path = -1; //infinity
    while (!q.empty()) {
        Edge e = q.front();
        q.pop();

        //this path is already bigger than the minimum one
        if(e.d > mini)
            continue;

        if(e.t == target){ //target node was found
            shortest_path = e.d < shortest_path ? e.d : shortest_path;
            continue;
        }

        if(visited[e.t][e.c])
            continue;

        visited[e.t][e.c] = true;

        adj_list &adj_u = g[e.t];
        for (auto &v : adj_u) {
            //distance from adjacent node increases if its color is
            //different from the visited node color...
            //the target and color keeps the same
            q.push(Edge(e.d + (v.c != e.c), v.t, v.c));
        }
    }

    return shortest_path;
}

int main(void) {
    int n,k;
    scanf("%d %d",&n,&k);

    graph g(n + 1);
    for (int i = 1; i <= k; ++i) {
        int l,u,v;
        scanf("%d %d %d",&l,&u,&v);

        g[u].push_back(Edge(v,i));
        g[v].push_back(Edge(u,i));
        for (int j = 2; j < l; ++j) {
            u = v;
            scanf("%d",&v);
            g[u].push_back(Edge(v,i));
            g[v].push_back(Edge(u,i));
        }
    }

    /*
     for (int i = 0; i < g.size(); ++i) {
     printf("%d : ",i);
     for (int j = 0; j < g[i].size(); ++j) {
     printf("%d -> ",g[i][j].t);
     }
     printf("\\ \n");
     }
     */

    printf("%u\n",BFS(g, 1, n, k + 1));
}

For example consider the following input:

9 4
6 2 3 4 6 7 9
4 1 3 4 5
3 8 3 4
2 9 8

For this case the minimum cost is 2;

Push bus lines, not edges
I feel like your breadth first search could be improved if instead of pushing single edges onto your queue, you pushed a whole bus line.  After all, the distance you are finding is the number of bus lines traversed, not the number of edges.  So a pseudocode algorithm would be:
foreach (busline b connected to vertex 0) {
    visited_busline[b] = true;
    q.push(b, 1);  // 1 = distance
}
visited_vertex[0] = true;

while (q.notEmpty()) {
    busline, distance = q.pop();

    foreach (vertex v in busline) {
        if (v == target) {
            return distance;
        }
        if (visited_vertex[v]) {
            continue;
        }
        visited_vertex[v] = true;
        foreach (busline b connected to v) {
            if (!visited_busline[b]) {
                visited_busline[b] = true;
                q.push(b, distance+1);
            }
        }
    }
}

Then all that is left it to parse the original input so that each vertex v contains a list of bus lines that it is connected to.
Typo
This code didn't compile for me:

    if(e.d > mini)
        continue;

I believe that at some point, you changed mini to shortest_path.  Also, in this line, I believe that you can use >= which can help a little in reducing your search depth by 1 level:
    if(e.d >= shortest_path)
        continue;


K&R (C): generate histogram of word-lengths From "The C Programming Language" (K&R):

  Exercise 1-13. Write a program to print a histogram of the lengths of words in its input. It is easy to draw the histogram with the bars horizontal; a vertical orientation is more challenging.

I wrote a program to generate a vertically-oriented histogram. It doesn't accurately handle punctuation (punctuation characters get counted as word characters), but given the limited tools covered in chapter 1 of the book, I felt this was reasonable. I've tried to limit myself to tools covered up until this point in the book (I had to resist the urge to use ternaries in a couple spots).

#include <stdio.h>

#define IN_WORD     1   /* point in word */
#define OUT_WORD    0   /* point out of word */
#define MAX_LEN     20  /* max word length to consider */

int main()
{
    int i, j;       /* iterators */
    int c;          /* character */
    int len;        /* word length */
    int state;      /* in/out of word */
    int top_count;
    int histogram[MAX_LEN];

    i = j = c = len = top_count = 0;
    state = OUT_WORD;

    for (i = 0; i < MAX_LEN; ++i)
        histogram[i] = 0;

    /* process input */
    while ((c = getchar()) != EOF) {

        if (c == ' ' || c == '\t' || c == '\n') {
            if (state == IN_WORD) {
                /* increment histogram entry for current word length */
                if (len < MAX_LEN)
                    j = len;
                else
                    j = MAX_LEN;
                ++histogram[j - 1];

                /* keep a record of the highest count */
                if (histogram[j-1] > top_count)
                    top_count = histogram[j-1];
            }
            /* reset state */
            len = 0;
            state = OUT_WORD;

        } else {
            /* keep track of current word length */
            ++len;
            state = IN_WORD;
        }
    }

    /* print histogram from top to bottom */
    for (i = top_count; i > 0; --i) {
        /* print Y-axis */
        printf("%3d|", i);

        /* print entries for each word length */
        for (j = 0; j < MAX_LEN; ++j) {
            if (histogram[j] < i)
                printf("  ");
            else
                printf(" .");
        }
        putchar('\n');
    }

    printf("    ");
    /* print X-axis */
    for (i = 0; i < MAX_LEN; ++i)
        printf("--");
    putchar('\n');

    return 0;
}

A small sample run against a quote by Benjamin Franklin:

  In wine there is wisdom, in beer there is Freedom, in water there is bacteria.

  6|   .
  5|   .
  4|   .     .
  3|   .     .
  2|   .   . .
  1|   .   . .   . . .
    ----------------------------------------

Initial reaction to your program is that it seems to be good and clean C code. So the overall impression is good, but there are things to review, even though some comments may be a bit of personal reference.

Punctuation char or not – The code includes check against single characters, so the extension to check against the ranges from c > 'a' && c < 'z', should be within reach
Split main method in multiple functions – This might not be covered yet, but I include it as it is good to start doing this as early as possible. However this does introduce the icky subject of passing arrays with/without size. Sorry about that!
Use of state vs simple length of current word – The naming of the states was a little unclear to me, and whilst reviewing the logic it hit me that if you are not counting the length of a word (i.e. the character is a legal word character), you can simply test if the length of the word, len, is larger than 0 to match your current IN_WORD state. As such, the state variable can be replaced with a test len > 0
Make the most common (shortest?) block come first – In your code you start of the input processing by verifying length of something and adding to the histogram. But of what?  Then comes the block actually increasing the length and reading the words. To me, it is more natural to switch these blocks, so that you know why len has a length
Stay consistent with bracing – For most of the code you've chosen to have start braces at preceding line, and that is a style choice. You've also chosen to let one-liners be without braces, this I would advise against. At some point in time this cause you some issue. My strong suggestion is to always enclose for or if (or similar) blocks with braces always. It will save you some grief further down the line.
Include number on both axes – Kind of strange not have numbers on the x-axis. To allow for two digits in the length of word, you can increase width of each column to three characters
Skip empty columns – No need to print the empty columns, is there?
Consider changing columnn marker – To me, the . is a little invisible, and I feel the # or * is more intuitive to use to mark the count
Consider labeling the axes – Consider adding labels like Count or Occurences, and Word length for the axes. It does however impose a smaller problem of where to put them...
Why the putchar, when printf works? – The usage of putchar('\n'); seems unmotivated, when you can do a printf("\n");. Is there a reason behind that?
Comment on comments – You've added suitable comments in most places, and when going to functions this becomes even more importantprocessing

Refactored code

That is enough rambling, and here is my suggestion to refactor code addressing most of the issues I've commented upon:

#include <stdio.h>

#define IN_WORD     1   /* point in word */
#define OUT_WORD    0   /* point out of word */
#define MAX_LEN     20  /* max word length to consider */

/*
 * Reset the histogram to all 0's too clear it out
 */
void init_histogram(int histogram[], size_t histogram_length)
{
    int i;

    for (i = 0; i < histogram_length; ++i) {
        histogram[i] = 0;
    }
}

/*
 * Read input, character by character, and divide into
 * words which are counted the length of (up until the max
 * max length specified in histogram_length). If word is
 * actually longer, the histogram records is at max length.
 *
 * Count characters, a-z and A-Z, and increase length of word,
 * and add to histogram if length>0 when we hit a non-character
 */
void create_histogram_from_input(int histogram[], size_t histogram_length)
{
    int len=0;       /* length of current word */
    int c;           /* current character */

    /* Keep reading character until end of file */
    while ((c = getchar()) != EOF) {

         // Check if it is an character ...
         if ( ( c >= 'a' && c <= 'z') ||
              ( c >= 'A' && c <= 'Z') ||
              c == '-' ) {

            ++len; /* Increase length of current word */

        } else {
            /* ... it was not a character, so check if we have
             * a positive word length
             */
            if (len > 0) {
                /* increment histogram entry for current word length */
                if (len >= histogram_length) {
                    len = histogram_length;
                }

                histogram[len - 1]++;

                len = 0; /* Reset length of current word */
            }
        }
    }
}

/* Pretty print the histogram vertically, with numbered x-
 * and y-axis. Only print as many columns as we have word lengths
 */
void print_histogram(int histogram[], size_t histogram_length)
{
    int i, j, c;
    const int y_axis_width = 4;

    int top_count = 0; /* Highest count of a given length */
    int max_length = 0; /* The longest word in histogram */

    /* Find the highest count, and longest word */
    for (i = histogram_length; i > 0; --i) {

       /* If max_length is not set, and we find a positive length
        * whilst going backwards, it is the longest word
        */
       if (max_length == 0 && histogram[i-1] > 0) {
           max_length = i;
       }

       /* Scan entire histogram to find the highest count of that length */
       if (histogram[i-1] > top_count) {
           top_count = histogram[i-1];
       }
    }

    /* print histogram from top to bottom */
    for (i = top_count; i > 0; i--) {

        /* print Y-axis */
        printf("%3d|", i); /* If this changes, change y_axis_width */

        /* print entries for each word length */
        for (j = 0; j < max_length; j++) {
            if (histogram[j] < i) {
                printf("   ");
            } else {
                printf(" # ");
            }
        }
        printf("\n");
    }

    /* print X-axis separator*/
    printf("%*s", y_axis_width, " ");
    for (i = 0; i < max_length; i++) {
        printf("---");
    }
    printf("\n");

    /* print X-axis numbers */
    printf("%*s", y_axis_width, " ");
    for (i = 0; i < max_length; i++) {
        printf("%2d ", i+1);
    }
    printf("\n");
}

int main()
{
    int histogram[MAX_LEN];

    /* Reset histogram */
    init_histogram(histogram, MAX_LEN);

    /* Read input, and fill out the histogram */
    create_histogram_from_input(histogram, MAX_LEN);

    /* Print out the histogram */
    print_histogram(histogram, MAX_LEN);

    return 0;
}

Some extra comments regarding my refactored code:

Location of top_count in functions – When all is gathered within main() your choice of doing the top_count in the input processing loop is not the worst. However when dividing into functions, it does rather belong in the print_histogram() instead of input processing. This also removes the extra setting of this multiple times during the processing
Size of arrays and functions – One of the major hickups with C is that when an array is sent as an parameter to a function, you loose the size information. It is a best practice to include this as a parameter. For more information see How do I determine the size of my array in C?
Some dislike printf("literal string") – This is sometimes frowned upon, as it is considered to be a security issue, especially in the form char *s; ... ; printf(s). The first parameter of printf is supposed to be a format string, and neither the literal string nor the string pointer are safe in that respect. They could be replaced by using printfs("%s", s), but often this is skipped by most people
The printf width as parameter trick – Using printf("%*d", width, number), i.e. that is including * in the format string, you can specify the width in the parameter. This allows in my code for setting the leading indent before the x-axis text using the constant y_axis_width
Finding number of columns – To ease the logic of finding the longest word, I reversed the scan for top_count and max_length. This allows me to use one single loop, since top_count doesn't care, but for max_length I can there simply test if is set or not, before checking if that histogram column has a length, which indicates that there is a list one word of that length.

A test run against the leading comment of create_histogram_from_input() gave the following output:

 15|       #
 14|       #
 13|       #
 12|    #  #
 11|    #  #
 10|    #  #
  9|    #  #
  8|    #  #        #
  7|    #  #        #
  6|    #  #        #        #
  5|    #  #  #  #  #        #
  4|    #  #  #  #  #        #
  3|    #  #  #  #  #        #
  2|    #  #  #  #  #  #  #  #
  1| #  #  #  #  #  #  #  #  #  #        #
    ---------------------------------------
     1  2  3  4  5  6  7  8  9 10 11 12 13


Drawing multiple Images Im trying to draw a lot of fields image for my world generation, all is perfect 64 fps, but if i began to draw all the time the grass it became laggy, like 59 fps (something that musn't happen! :( ), i need help and some tricks to draw things in c# with more optimization :)

int loadedChunks = 0;
        for (int chunkX = 0; chunkX < sizeX; chunkX++)
        {
            for (int chunkY = 0; chunkY < sizeY; chunkY++)
            {
                int xChunk = chunkX + CamXCh - 1,
                    yChunk = chunkY + CamYCh - 1;

                long idChunk = xChunk + ((long)yChunk << 32);

                if (allChunks.ContainsKey(idChunk))
                {
                    loadedChunks++;
                    Chunks currentChunk = allChunks[idChunk];
                    List<CeldsChunk> celds = currentChunk.MapChunk;

                    // Draw Ground
                    for(int celdIndex = 0; celdIndex < celds.Count; celdIndex++)
                    {
                        CeldsChunk celd = celds[celdIndex];
                        Point cord = celd.cords;
                        int sizeCL = cord.size,
                            CeldX = xChunk * sizeChunk + CameraX + cord.posX * sizeCL,
                            CeldY = yChunk * sizeChunk + CameraY + cord.posY * sizeCL,
                            sizeCeld = cord.size;

                        Color celdType = celd.type;
                        //SolidBrush brsh = new SolidBrush(celdType);
                        g.DrawImage(grass, new Rectangle(CeldX, CeldY, sizeCeld, sizeCeld));

                        //Image sk = celd.img;
                        //if (sk != null)
                        //    step2.Add(new Point(CeldX, CeldY, sizeCeld), sk);
                    }
                }
            }
        }

        // Step 2  Draw Forest
        foreach (KeyValuePair<Point, Image> s in step2)
        {
            Point kps = s.Key;
            Image ips = s.Value;
            int posX = kps.posX,
                posY = kps.posY,
                siz = s.Key.size,
                w = ips.Width,
                h = ips.Height;

            //g.DrawRectangle(Pens.Red, new Rectangle(posX, posY, siz, siz));
            //g.DrawRectangle(Pens.Blue, new Rectangle(posX - (h >> 2) + (siz >> 1), posY - h + siz, w, h));
            g.DrawImage(ips, new Rectangle(posX - (h >> 2) + (siz >> 1), posY - h + siz, w, h));
        }

Naming

What is the difference between xChunk and chunkX? yChunk and chunkY? Choose names that will help, not hinder you.

Don't skimp on letters. size is better than siz, point is better than kps. Almost anything is better than step2. On the other hand, chunks is better than allChunks, chunk is better than currentChunk.

Class names are usually singular -- see for instance Point, Color, Rectangle. Follow the pattern and rename Chunks to Chunk.

The names Chunk and CeldsChunk don't convey anything to me -- that might be because I have no domain knowledge of what you're doing, or it might be because their names can be improved.

Commented-out code

It's confusing. Is it a failed attempt? Commented out for performance testing? No-one knows. If you might need it later, save it on a new branch. Otherwise, delete it.

Conventions

Follow C# conventions. Install StyleCop and listen to its recommendations.

Performance

Profiling is your best bet. You can try things like moving repeated calculations out of loops, as I suggested in the comment, but it's possible you're just asking too much of the computer. You're drawing up to sizeX * sizeY * ? images, and the solution might be to make do with fewer images.


Validating javascript onChange event I'm looking to simplify the below code for validating an onChange event. The allowedInput is a regex that is passed (optional) from all the form components.
    const validatedOnChange = (event: ChangeEvent<HTMLInputElement>): void => {
        if (isDisabled) {
            return;
        }
        if (allowedInput) {
            if (allowedInput.test(event.target.value)) {
                onChange({ name, value: event.target.value });
            }
            return;
        }
        onChange({ name, value: event.target.value });
    };

This is a common utility method used for all my React form components. Hence looking to write a readable and consistent code.

Obviously the code does have some redundancy, which violates the Don't Repeat Yourself principle.
This could be simplified by calling onChange() if allowedInput is falsey or if that is not the case then only when the call to the test method on it returns a truethy value:
const validatedOnChange = (event: ChangeEvent<HTMLInputElement>): void => {
    if (isDisabled) {
        return;
    }
    if (!allowedInput || allowedInput.test(event.target.value)) {
        onChange({ name, value: event.target.value });
    }
};

One could opt to take the return early approach, which might be more readable and decrease the indentation on the call to onChange() but would have an extra line:
    if (allowedInput && !allowedInput.test(event.target.value)) {
        return;
    }
    onChange({ name, value: event.target.value });


Check divisibility of each line in a large dataset I have solved this assignment in online coding community, and it has run successfully. But it's showing 12.09s of execution.
How could this be optimized ?

The purpose of this problem is to verify whether the method you are using to read input data is sufficiently fast to handle problems branded with the enormous Input/Output warning. You are expected to be able to process at least 2.5MB of input data per second at runtime.
Input
The input begins with two positive integers n k (n, k<=10^7). The next n lines of input contain one positive integer t(i), not greater than 10^9, each.
Output
Write a single integer to output, denoting how many integers t(i) are divisible by k

import java.util.*;
import java.io.*;

class InputTest{

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

    BufferedReader br=new BufferedReader(new InputStreamReader(System.in));
    String arr[]=br.readLine().split(" ");
    int n=Integer.parseInt(arr[0]);
    int k=Integer.parseInt(arr[1]);
    int count=0;

    if(n<=0 || k<=0 || k>Math.pow(10,7)){
        throw new Exception();
    }
    else{

        for(int i=0;i<n;i++){
            int m=Integer.parseInt(br.readLine());
            if(m>Math.pow(10,9))
                throw new Exception();
            if(m%k==0)
                count++;

        }
        System.out.println(count);
    }

}
}

class InputTest{

Spacing.

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

Again. There should be a space before each "{".

BufferedReader br=new BufferedReader(new InputStreamReader(System.in));

You're using a platform dependent encoding (which is actually nearly always wrong). There are chances that it's UTF-8 and it probably takes quite some time.

Nearly any encoding would do, and e.g., LATIN-1 should be faster as it converts one byte to one char, which is way simpler than UTF-8.

But even faster would be no encoding at all. The whole input can be easily read directly from bytes.

BufferedReader br=new BufferedReader(new InputStreamReader(System.in));

Spaces are inexpensive, use them! No more comments on formatting.

if(m>Math.pow(10,9))

Gotcha! Why are you exercising such an expensive operation every time? And why do you wonder that it's slow? Maybe the JIT may be able to optimize it away, but

You could simply 1e9 and get rid of the computation.
You could simply write 10000000 or (since Java 7) even 10_000_000 and have an integer as it should be.
But you surely should define a constant instead. The it doesn't matter how complicated the computation is as it gets done just once.

throw new Exception();

That's ugly. Exception is too unspecific and there's no message there. What about a

throw new RuntimeException(String.format("Invalid (k, n) = (%s, %s)", k, n));

Or would maybe

assert n > 0 : n;
assert k > 0 : k;
assert k <= 10_000_000 : k;

do?

So again, if you want to speed it up,

remove the obvious inefficiency
don't use strings
read the file in as a byte[]
parse numbers from it manually

For really big inputs, it could make sense to avoid both the conversion to int and the modulo computation by creating a data structure working directly with the input byte[]. But this is rather complicated , surely not for everyone.


Parallel accumulated sum in OpenCL This is my first relevant code in OpenCL. Please, let me know if I'm doing something wrong in a logical level (i.e., I guess it can crash in situations?) or accounting for performance (bottlenecks, bad use of resources, misunderstanding some concepts, etc).

void accumulatedSum(
    volatile __global u32* completeBlocks,
    volatile __global u32* input,
    volatile __global u32* output,
    __local u32* loc) {
    // Receives an input such as:    [1, 3, 0, 0, 4,  7,  0,  0,  3,  4,  1,  1]
    // Computes the accumulated sum: [1, 4, 4, 4, 8, 15, 15, 15, 18, 22, 23, 24]
    // Always call it with global_work_size = num_of_elements / 2
    // Works like this: http://imgur.com/3lm3CAq
    unsigned int k, a;
    int lid          = get_local_id(0);
    int gid          = get_global_id(0);
    int groupId      = get_group_id(0);
    int localPasses  = get_local_size(0);
    int globalPasses = get_num_groups(0)*2;
    loc[lid*2]       = input[gid*2];
    loc[lid*2+1]     = input[gid*2+1];
    barrier(CLK_LOCAL_MEM_FENCE);
    for (a=0; localPasses>>=1; ++a){
        k = (lid >> a << (a+1)) + (1<<a);
        loc[k + (lid&((1<<a)-1))] = loc[k - 1] + loc[k + (lid&((1<<a)-1))];
        barrier(CLK_LOCAL_MEM_FENCE);
    };
    output[gid*2] = loc[lid*2];
    output[gid*2+1] = loc[lid*2+1];
    for (; globalPasses>>=1; ++a){
        barrier(CLK_GLOBAL_MEM_FENCE);
        while (*completeBlocks != 0);
        if (lid == 0) atomic_inc(completeBlocks);
        while (*completeBlocks < get_num_groups(0));
        if (lid == 0 && gid == 0) atomic_xchg(completeBlocks,0);
        k = (gid >> a << (a+1)) + (1<<a);
        output[k + (lid&((1<<a)-1))] = output[k - 1] + output[k + (gid&((1<<a)-1))];
    };
};

If you have clever ideas in how to make it more readable please let me know, although that is not the focus.

What are k and a?  It looks like a is a loop counter variable, in which case it can be renamed to i and moved to the front of the first for loop.  As for k, rename it such that it conveys its purpose.  With the exception of loop counter variables such as i, variables shouldn't be a single character.
Many of the lines in the for loops are difficult to read, especially with all the bit-shifting.  Depending on the type of shift, the optimizing compiler may be able to replace some arithmetic operation with an appropriate bitshift.  But for others, it would help to leave comments to explain what's being done for each complex operation.
This is quite crammed together:

for (; globalPasses>>=1; ++a){
    barrier(CLK_GLOBAL_MEM_FENCE);
    while (*completeBlocks != 0);
    if (lid == 0) atomic_inc(completeBlocks);
    while (*completeBlocks < get_num_groups(0));
    if (lid == 0 && gid == 0) atomic_xchg(completeBlocks,0);
    k = (gid >> a << (a+1)) + (1<<a);
    output[k + (lid&((1<<a)-1))] = output[k - 1] + output[k + (gid&((1<<a)-1))];
};

You should isolate the while loops so that it's easy to see that they're busy-waits.  You could also consider using curly braces for the if statements.  Finally, consider adding whitespace between the operators for more readability.

for (; globalPasses >>= 1; ++a) {
    barrier(CLK_GLOBAL_MEM_FENCE);

    while (*completeBlocks != 0);

    if (lid == 0) {
        atomic_inc(completeBlocks);
    }

    while (*completeBlocks < get_num_groups(0));

    if (lid == 0 && gid == 0) {
        atomic_xchg(completeBlocks, 0);
    }

    k = (gid >> a << (a + 1)) + (1 << a);

    output[k + (lid&((1 << a) - 1))] =
        output[k - 1] + output[k + (gid&((1 < <a) - 1))];
};


Dropping a row in a dataframe I have made a function to drop a row in a pandas.DataFrame. Since, in general, the input dataframe of a function is just a reference to the dataframe. So any change made in that function to the dataframe will be preserved outside that function. Therefore, there are two choices when designing the function:

simply make the change without returning the dataframe.
or, make the change and return the dataframe.

See my code below (function change_df_1 and change_df_2). Could you please let me know which one is better or do you prefer? My concern is readability/clarity of the code.
import pandas as pd

def get_df():
    df = pd.DataFrame({'A': [10, 20, 30], 'B':[100, 200, 300]})
    return df

def change_df_1(df: pd.DataFrame):
    df.drop(index=[1], inplace=True)

def change_df_2(df: pd.DataFrame) -> pd.DataFrame:
    df.drop(index=[1], inplace=True)
    return df

def main():
    df = get_df()
    print(f'Original {df=}')

    change_df_1(df=df)
    print(f'Changed_with 1: {df=}')

    # change_df_2(df=df)
    # print(f'Changed_with 2: {df=}')

if __name__ == '__main__':
    main()
```

The documentation for sort, to pick a common example, says:

You can also use the list.sort() method. It modifies the list in-place (and returns None to avoid confusion).

One benefit of returning the object is to chain calls. If consumers of your code expect to be able to chain calls, perhaps returning the object makes sense. Otherwise I would say the more common behavior in Python is to return None from functions that mutate their arguments.
(Other languages may have other conventions.)


Speeding Up JavaMail Sending I am creating a miniature bulk email sender. I am using Postfix on Ubuntu with JavaMail. Here is the key part of the code:

public static void main(final String[] args){
    final Connection selectConnection = DriverManager.getConnection(prop.getProperty("select_connection"));
    final Statement stmt = selectConnection.createStatement(ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY);
    stmt.setFetchSize(Integer.MIN_VALUE);
    ResultSet rs = stmt.executeQuery("SELECT email FROM emails");
    while(rs.next()){
        recipient = rs.getString("email");
        session = rs.getString("code");
        unsubscribeParameters = "s="+session+"&c="+campaign+"&r="+recipient;
        openedParameters = "s="+session+"&c="+campaign;
        sendEmail(recipient, campaign, html.replace("unsubscribeParameters", unsubscribeParameters).replace("openedParameters", openedParameters), properties);
    }
}
private static void sendEmail(final String recipient, final String campaign, final String html, final Properties properties){
    final String verp = campaign+"="+recipient.replace("@", "+") + "@example.com";
    properties.put("mail.smtp.from", verp);
    final Session session = Session.getDefaultInstance(properties);
    final MimeMessage message = new MimeMessage(session);
    try {
        message.setFrom(new InternetAddress(SENDER_ADDRESS, SENDER_NAME));
        message.setSubject(subject);
        final MimeBodyPart textPart = new MimeBodyPart();
        textPart.setContent(plainText, "text/plain");
        final MimeBodyPart htmlPart = new MimeBodyPart();
        htmlPart.setContent(html, "text/html");
        final Multipart mp = new MimeMultipart("alternative");
        mp.addBodyPart(textPart);
        mp.addBodyPart(htmlPart);
        message.setContent(mp);
        message.setRecipient(Message.RecipientType.TO, new InternetAddress(recipient));
        Transport.send(message);
    } catch (final Exception | OutOfMemoryError e) {
        //handle errors
    }
}

Right now, I'm getting about 7 emails a second on a 512 MB RAM box, 1 core processor, and a 20 GB SSD disk. Does anyone see any ways to speed this up? Is it possible?

It looks like you are sending almost the same message multiple times. I see at least two possible speed improvements.

The most likely bottleneck is the actual email sending process, which is outside of your program, an overhead for every single message sent. You could reduce that overhead by putting multiple recipients in BCC, if possible. As you noted in comments, ISPs might consider messages with a large number of recipients. If you stay within reasonable limits, and you're not in fact spamming, you can hopefully convince them to allow it. (This did happen to me once, and my ISP was reasonable.)
Another minor improvement can be, instead of creating a new message from scratch for each recipient, you could create a single message object once, and replace the recipient and other variable parts in it.

The code is too compactly written. I suggest to use your IDE to reformat it nicely.

The try block covers a lot of code. It seems you could move some code of it. That would be better, to clarify the sensitive parts where something might go wrong.

Catching Exception is not recommended. It's recommended to catch the most specific exception that might be thrown.

It's a bit strange to catch OutOfMemoryError. This is not something a program can recover from gracefully. You don't share the piece of code handling the exception, but I doubt it would be much different from not handling that at all.