Legacy application unreachable code I am debugging one legacy J2EE application I and think I found some dead code and some redundant condition checks. Please give me your thoughts on my observations i.e. if they are valid. Please note that I am not permitted to rewrite this block, only to fix something if wrong or redundant.

...
else if ((emailList != null) && (emailTest != null)
                  && (emailTest.compareTo(IParameterNames.EMAIL_GO) == 0))  // #1 One of the AND conditions to enter this "else if" block is: "emailList != null",
              {                                                             // #2 so if "emailList != null" is not "true" we wouldn't even be in this "else if" block.

                response.getOutputStream().print("...blah blah blah...");
                response.getOutputStream().print("...blah blah blah...");
                response.getOutputStream().print("...blah blah blah...");

                if (emailList != null)  //#3 Here we check again if primary condition is fulfilled: "emailList != null".
                                        //#4 If "emailList != null" is not "true" we wouldn't even be in this "else if" block.This should be removed IMHO
                {
                  response.getOutputStream().print(emailList.size());
                }
                else //#5 And as "emailList != null" was true at the else if here it can't be different than that. This should be removed IMHO as it is DEAD CODE.
                {
                  response.getOutputStream().print("0 ");
                }

                response.getOutputStream().print("...blah blah blah...");
                response.getOutputStream().print("...blah blah blah...");

                if (emailList != null)  //#6 Here we check again if primary condition is fulfilled: "emailList != null".
                                        //#7 If "emailList != null" is not "true" we wouldn't even be in this "else if" block.This should be removed IMHO
                {
                  response
                      .getOutputStream()
                      .print("...blah blah blah...");

                  for (int i = 0; i < emailList.size(); i++)
                  {
                    email = (EmailMessage) emailList.get(i);

                    if (email != null)
                    {
                      response.getOutputStream().print(outputEmailAddressAsHTML(email, i).toString());
                    }
                  }
                }

                response.getOutputStream().print("..lots.of.blah blah blah...");
              }
...

Your assessment is correct, the emailList null checks are redundant in that block.

I would recommend that you use the "FindBugs" tool to perform static analysis on your code. It is available as a plugin to Eclipse, and other tools too. FindBugs would clearly indicate and mark all instances where these problems happen, and also a bunch of other things too.

Additionally, I would neaten things up by pulling just one final local variable for the response.getOutputStream() too. Additionally, you should be using generics, and enhanced-for (for-each) loops over your emailList.

Summarizing this would be:

...
else if ((emailList != null) && (emailTest != null)
              && (emailTest.compareTo(IParameterNames.EMAIL_GO) == 0))
         {

            PrintOutputStream respos = response.getOutputStream();
            respos.print("...blah blah blah...");
            respos.print("...blah blah blah...");
            respos.print("...blah blah blah...");

            respos.print(emailList.size());

            respos.print("...blah blah blah...");
            respos.print("...blah blah blah...");

            respos.print("...blah blah blah...");

            for (EmailMessage email : emailList) {
                if (email != null) {
                  respos.print(outputEmailAddressAsHTML(email, i).toString());
                }
            }

            respos.print("..lots.of.blah blah blah...");
          }


Iterating and updating 35000 dictionaries I'd like the function below reviewed. I'm especially interested in improvement ideas to process the API responses faster.

products_list is the API response in the form of list of dictionaries which returns all products for one supermarket only (out of many).

def encode_products_response_list(products_list, rows_list):
    """
    Convert unicode values to utf-8
    Convert all other values (e.g. Int, Float) to str
    Return list of unique rows

    :param products_list:  list of dictionaries
    :param rows_list:          empty list
    """
    for item in products_list:
        # extract name from list
        item['product_name'] = item['product_name'][0]

        for key, value in item.iteritems():
            if isinstance(value, unicode):
                item[key] = value.encode('utf-8')
            else:
                item[key] = str(value)

        if item not in rows_list:
            rows_list.append(item)

    return rows_list

if __name__ == '__main__':
    # all work in same product
    rows_list = []
    # the api products_list response is a list of dicts like the one below
    products_list = [
        {
            u'product_name': [u'Super Bleach 5'],
            u'product_description': 'Cleans like nothing you have ever seen',
            u'cost': 2.55,
        },
        {
            u'product_name': [u'Magic Breakfast'],
            u'product_description': 'Start your day with proper breakfast!',
            u'cost': 5,
        }
    ]
    products_list = products_list * 354342

    encode_products_response_list(products_list, rows_list)

The metrics below is just regarding one of many supermarkets (using line_profiler):

Timer unit: 1e-06 s

Total time: 108.972 s
File: example_1.py
Function: encode_products_response_list at line 2

Line #      Hits         Time  Per Hit   % Time  Line Contents
==============================================================
 1                                           @profile
 2                                           def encode_products_response_list(products_list, rows_list):
 3    354343       362103      1.0      0.3      for row in response:
 4    354342       539821      1.5      0.5          item['product_name'] = item['product_name'][0]
 5
 6   2480394      1392104      0.6      1.3          for key, value in item.iteritems():
 7   2126052      1460806      0.7      1.3              if isinstance(value, unicode):
 8    354342       661158      1.9      0.6                  item[key] = value.encode('utf-8')
 9                                                       else:
10   1771710      1728154      1.0      1.6                  item[key] = str(value)
11
12   354342    102436117    289.1     94.0          if item not in rows_list:
13     13634        28912      2.1      0.0              rows_list.append(item)
14
15         1            1      1.0      0.0      return rows_list

Not surprisingly, almost all time is spent on the repeated if item not in rows_list, as that takes linear time every time. Instead of doing the duplicate-check with a list, you'd better do it with a set, where that check averages constant time. That would require hashable elements, though, which your items, being dicts, aren't. But you can probably extract something hashable correctly identifying each item, no? Maybe even just item['product_name']? If not, then the tuple (item['product_name'], item['product_description'], item['cost']).


JavaScript program to count lines of Java code I've implemented a small program in JavaScript to count the number of lines of a string of java source code.

This was done for one of the Code Kata exercise: http://codekata.com/kata/kata13-counting-code-lines/

Overall, the goal of this exercise is summarised as follow:

  The mixture of line-based things (single line comments, blank lines,
  and so on) with the stream-based block comments can make solutions
  slightly ugly. While coding your solution, consider the structure of
  your code, and see how well it fits the structure of the problem. As
  with most of these kata, consider coding multiple alternative
  implementations. Does what you learned on the first tries affect your
  approach to subsequent ones?

To be honest, I've submitted this for an interview and was told that this "lacked fundamentals by allowing errors in the code, there were issues in code construct and there were issues in choice of techniques".

Here's my simple app's source (also available in CodePen):

/**
 * Return the number of lines of code of a Java source code.
 * {@link http://codekata.com/kata/kata13-counting-code-lines/}
 * @param {string} Java source code
 * @return {number}
 */
function getJavaLineCount(source) {
    var count = 0;
    var patterns = {
        spaces: /^\s+/,
        singleLineComment: /^\/\/.*/,
        multiLineComment: /^\/\*((?!\*\/)[\s\S])*\*\//,
        code: /^.*\s*/
    };

    var readToken = function () {
        var m, type;
        if (m = source.match(patterns.spaces)) {
            type = 'spaces';
        } else if (m = source.match(patterns.singleLineComment)) {
            type = 'singleLineComment';
        } else if (m = source.match(patterns.multiLineComment)) {
            type = 'multiLineComment';
        } else if (m = source.match(patterns.code)) {
            type = 'code';
        } else {
            throw new Error("Unknown code pattern!");
        }
        var token = m[0];
        source = source.substr(token.length);

        //DEBUG
        console.log('Found token (%s): %s', type, JSON.stringify(token));
        return type;
    };

    while (source) {
        if (readToken() === 'code') {
            count++;
        }
        console.log('new source = |%s|', source);
    }

    //DEBUG
    console.log('LOC = ', count);
    return count;
}

$(function () {
  var calculateLOC = function() {
   $('#linesOfCode').text(getJavaLineCount(
    $('#source').val()
   ));
  };

    $('#source').change(calculateLOC);
  calculateLOC();
});
textarea {
  width: 50%;
}

<textarea id="source" rows="15">
/*****
 * This is a test program with 5 lines of code
 *  \/* no nesting allowed!
 //*****//***/// Slightly pathological comment ending...
public class Hello {

 @cool
 private String test = "some /* commented */ 'and' \" asd other\" " +
  "string";

    public static final void main(String [] args) { // gotta love Java
        // Say hello
      System./*wait*/out./*for*/println/*it*/("Hello/*");
    }
}
</textarea>

  n/a lines of java code.

To improve this, I could imagine writing a simple character parser to avoid using regexp, that's assuming that we consider regexps to be too slow in JS.

I know that if you provide it broken java code, it'll not work properly - but I assumed that this case would not be considered a normal scenario in this exercise.

Apart from this, I'm not sure what other kind of errors it might have.
Any suggestions?

Sorry, don't really have the time to do a comprehensive review (maybe later I'll edit!) but for now:

Your code actually fails on some cases.

The way that you're finding whether a line counts as a LOC or not is too greedy. (Alternatively, your comment regexes are too strict.) You're reading in basically the whole line as a line of code. This fails for relatively simple cases:

a/*
b*/

should be one line of code, but you treat it as two, since your code regex absorbs the /*. In fact, I'd argue that the way that you choose to count lines of code is odd; it took me a few reads to see what you were doing and find that error. The more natural way perhaps would be to strip spacing and comments, then count non-empty lines.

That also has the advantage of being way faster: you're calling String#substr() a lot, which is pretty expensive.

Aside from that, I only have a few comments:

Keep spacing consistent. Choose either function() or function (); don't use both.
Be consistent in your choice of indentation character. This is easier if you enable showing whitespace in your editor.
Be consistent in string delimiters; don't switch between ' and " without a good reason.
Since you're using a bare object, you can iterate over it with a for..in. If you're really concerned that somebody will put properties into the prototype of Object (which, by the way, should NEVER EVER happen), then add the hasOwnProperty check:

    for (var tokenType in patterns) {
    //  if (patterns.hasOwnProperty(tokenType)) ...
        if (m = source.match(patterns[tokenType])) {
            var token = m[0];
            source = source.substr(token.length);
            //DEBUG
            console.log('Found token (%s): %s', type, JSON.stringify(token));
            return type;
        }
    }
    throw new Error('Unknown code pattern!');


A package for sort algorithms - v3 Addressed both issues in this codereview post.

Fixed the indexing issue and the call mechanism of the function expression.

Looking for perfect code.  Hope this is the last rev.

    /***************************************************************************************************
    **ALGORITHMS
    ***************************************************************************************************/

    // self used to hold client or server side global
    (function () {

        "use strict";

        // holds (Pub)lic properties
        var Pub = {},

            // holds (Priv)ate properties
            Priv = {},

            // holds "imported" library properties
            $A;

        (function manageGlobal() {

            // Priv.g holds the single global variable, used to hold all packages
            Priv.g = '$A';

            if (this[Priv.g] && this[Priv.g].pack && this[Priv.g].pack.utility) {
                this[Priv.g].pack.algo = true;
                $A = this[Priv.g];
            } else {
                throw new Error("algo requires utility module");
            }
        }());

        Pub.swap = function (arr, i, j) {
            var temp = arr[i];
            arr[i] = arr[j];
            arr[j] = temp;
        };

        // checks to see if sorted
        Pub.isSorted = function (arr) {
            var i,
                length = arr.length;
            for (i = 1; i < length; i++) {
                if (arr[i] < arr[i - 1]) {
                    return false;
                }
            }
            return true;
        };

        // repeatedly orders two items ( a bubble ) at a time
        Pub.bubbleSort = function (arr) {
            var index_outer,
                index_inner,
                swapped = false,
                length = arr.length;
            for (index_outer = 0; index_outer < length; index_outer++) {
                swapped = false;
                for (index_inner = 0; index_inner < length - index_outer; index_inner++) {
                    if (arr[index_inner] > arr[index_inner + 1]) {
                        Pub.swap(arr, index_inner, index_inner + 1);
                        swapped = true;
                    }
                }
                if (!swapped) {
                    break;
                }
            }
            return arr;
        };

        // repeatedly finds minimum and places it the next index
        Pub.selectionSort = function (arr) {
            var index_outer,
                index_inner,
                index_min,
                length = arr.length;
            for (index_outer = 0; index_outer < length; index_outer++) {
                index_min = index_outer;
                for (index_inner = index_outer + 1; index_inner < length; index_inner++) {
                    if (arr[index_inner] < arr[index_min]) {
                        index_min = index_inner;
                    }
                }
                if (index_outer !== index_min) {
                    Pub.swap(arr, index_outer, index_min);
                }
            }
            return arr;
        };

        // repeatedly places next item in correct spot using a "shift"
        Pub.insertionSort = function (arr) {
            var index_outer,
                index_inner,
                value,
                length = arr.length;
            for (index_outer = 1; index_outer < length; index_outer++) {
                value = arr[index_outer];

                // JavaScript optimization - index_inner >=0 is removed
                // as the array index will return undefined for a negative index
                for (index_inner = index_outer - 1; (arr[index_inner] > value);
                        index_inner--) {
                    arr[index_inner + 1] = arr[index_inner];
                }
                arr[index_inner + 1] = value;
            }
            return arr;
        };

        // module complete, release to outer scope
        this[Priv.g] = $A.extendSafe(this[Priv.g], Pub);
    }).call(this);

I would remove Pub.swap and copy its code where you need it.

That's because function calls are expensive, so better avoid calling a function at each iteration if possible.

And try to cache the expressions in loop's condition. For example,

var lengthMinusIndexOuter = length - index_outer;
for (index_inner = 0; index_inner < lengthMinusIndexOuter; index_inner++) {

instead of

for (index_inner = 0; index_inner < length - index_outer; index_inner++)


ASCII code decode to string I am working on this question and have my answer as following?

A string is encoded by performing the following sequence of actions:

Replace each character with its ASCII value representation.
Reverse the string.

For example, the table below shows the conversion from the string  "HelloWorld" to the ASCII string "7210110810811187111114108100":
Character
H  e   l   l   o    W  o   r   l   d
ASCII Value
72 101 108 108 111  87 111 114 108 100

The ASCII string is then reversed to get the encoded string "0018014111117811180180110127".
The characters in encoded string are within the range 10 - 126 which include special characters.

Example:
asciidecode('111111') => '\x0b\x0b\x0b' or 'oo'
because we can split the
input 111111 (after reverse back: '111111') like 11,11,11 or 111,111
from typing import List

def asciidecode(self, s: str) -> List:
    if not s: return ''
    s = s[::-1]
    n = len(s)
    ch_map = {str(i): chr(i) for i in range(10, 127)}
    dp = [[''] for _ in range(n + 1)]
    dp[2] = [ch_map[s[:2]]] if s[:2] in ch_map else ''
    dp[3] = [ch_map[s[:3]]] if s[:3] in ch_map else ''
    dp[4] = [dp[2][0] + ch_map[s[2:4]]] if s[2:4] in ch_map else ''
    for i in range(5, n + 1):
        p1 = s[i - 2: i]
        p2 = s[i - 3: i]
        tmp = []
        if p1 in ch_map:
            tmp += [i + ch_map[p1] for i in dp[i - 2]]
        if p2 in ch_map:
            tmp += [i + ch_map[p2] for i in dp[i - 3]]
        dp[i] = tmp
    return dp[-1]

return '' doesn't have the type List.
List means List[Any], you should use List[str].
Your variable names are garbage.

s -> string
n -> length
ch_map -> char_map or character_map
dp -> what does this even mean?
p1 -> position_1?

Don't put return statements on the same line as an if.
Again strings shouldn't be assigend to lists, yes you can iterate over them because they're both sequences. But they're not the same type. [ch_map[s[:2]]] if s[:2] in char_map else ''
ch_map should be a constant outside the function.
It's far easier to understand your code if it's written using recursion.
Recursion has some problems, and so it should be written in a way that allows you to easily convert it to a while loop.

CHAR_MAP = {str(i): chr(i) for i in range(10, 127)}

def asciidecode(string: str) -> List[str]:
    if not string:
        return []
    string = string[::-1]
    length = len(string)

    def inner(index):
        if index == length:
            yield ''
        else:
            for size in (2, 3):
                if length < index + size:
                    break
                letter = CHAR_MAP.get(string[index:index + size])
                if letter is not None:
                    for word in inner(index + size):
                        yield letter + word
    return list(inner(0))

def asciidecode_pre_while(string: str) -> List[str]:
    if not string:
        return []
    string = string[::-1]
    length = len(string)
    output = []

    def inner(index, word):
        if index == length:
            output.append(word)
            return

        for size in (2, 3):
            if length < index + size:
                break
            letter = CHAR_MAP.get(string[index:index + size])
            if letter is not None:
                inner(index + size, word + letter)
    inner(0, '')
    return output

From the second one it's easy to convert it to a while loop:

CHAR_MAP = {str(i): chr(i) for i in range(10, 127)}

def asciidecode(string: str) -> List[str]:
    if not string:
        return []
    string = string[::-1]
    length = len(string)
    output = []

    stack = [(0, '')]
    while stack:
        index, word = stack.pop()
        if index == length:
            output.append(word)
            continue

        for size in (2, 3):
            if length < index + size:
                break
            letter = CHAR_MAP.get(string[index:index + size])
            if letter is not None:
                stack.append((index + size, word + letter))
    return output


Node.js callback error pattern The errors are passed through every callback, but in fact, it might be better to throw an error at higher levels. For example, look at the mongodb.js database function. It passes through an error in the callback. However, if there was an error at that level, it might be better to deal with it at that point and just fall back on procces.on('uncaughtException')

It is a lot of extra code each time, dealing with errors in callbacks. I am wondering if there is a better way around it.

controllers/users.js

var usersModel = require('../models/users.js');

users.create = function(req, res){
  usersModel.create(req.data.username, req.data.password, function(error, user){
    if(error){
      res.statusCode = 500;
      res.end('server-error');
      return;
    }

    res.statusCode = 201;
    res.end(user._id);
  });
};

models/users.js

var db = require('../mongodb.js');

module.exports.create = function(username, password, callback){
  db.collection('main', 'users', function(error, collection){
    if(error){
      callback(error);
      return;
    }

    collection.insert({
      username: username,
      password: password,
      createdOn: new Date()
    }, callback);
  });
};

mongodb.js

var mongo = require('mongodb')
  , mongodb = module.exports
  , connections = {}
  , config = {
    main: {
      name: 'application-main',
      server: new mongo.Server('127.0.0.1', 27017, { auto_reconnect: true })
    }
  };

mongodb.database = function(key, callback){
  if(connections[key]){
    callback(null, connections[key]);
    return;
  }

  var db = mongo.Db(config[key].name, config[key].server);

  db.open(function(error, connection){
    if(error){
      callback(error);
      return;
    }

    connection.on('close', function(){
      delete(connections[key]);
    });

    connections[key] = connection;
    callback(null, connection);
  }
};

mongodb.collection = function(dbKey, collName, callback){
  mongodb.database(dbKey, function(error, connection){
    if(error){
      callback(error);
      return;
    }

    connection.collection(collName, function(error, collection){
      if(error){
        callback(error);
        return;
      }

      callback(null, collection);
    });
  });
};

First, there is a principle you should be reminded:
Never deal with the error you don't know, just pop it up.

Which means, if you can't deal with or don't know about the error passed in the callback, you just pass it to next callback, or throw it(A pattern of CHAIN OF RESPONSIBILITY)!

For the errors you know how to deal with it, you can define a module for error handling and deal with it according to the type of error and log the error for debugging, like the following way.

error-handler.js

var log = require('./log'); // Log module(local or third-party);

function handler(error){
  if(error instanceof error1){
    // log it
    // handler for error1
  }else if(error instanceof error2){
    // log it
    // handler for error2
  }else if ....
};

module.exports = handler;

logic.js

var errorHandler = require('./error-handler');

db.open(function(err, connection){
  if(err){ return errorHandler(err); }
});

For good modules like mongoose will throw their customized Errors.


Passing bitmap from C# to C++ via a struct First, I've read the a few questions regarding this issue, the most helpful being:
https://stackoverflow.com/questions/27463876/passing-bitmap-from-c-sharp-to-c

I was unable to get the provided solutions to work as I kept getting an AccessViolationException.

What I'm attempting to do is pass bitmap data to an unmanaged c++ dll. To do this I created a struct which holds a pointer to the image data as well as its length. I'm using a struct as I plan on passing in multiple images (in a single call) to the unmanaged API.

What I implemented works but I have a feeling there is probably some serious drawbacks so I'm curious as to what those drawbacks could be.

My current solution uses a generic pointer to hold the image data. This of course would be a drawback as I lose type safety. Anyway here is the relevant code.

C++ dll

raw_image.h

struct raw_image
{
   void* data;
   int size;
};

alignment.cpp (exports)

ALIGNMENT_API void submit( raw_image& img )
{
    cv::Mat mat = cv::imdecode( cv::_InputArray(
        static_cast<uchar*>( img.data ), img.size ), cv::IMREAD_COLOR );
    cv::imshow( "image", mat );
    cv::waitKey( );
    cv::destroyWindow( "image" );
}

C# dll

RawImage.cs

[StructLayout( LayoutKind.Sequential )]
internal unsafe struct RawImage
{
    internal void* ImageData;
    internal int Size;
}

Aligner.cs (import)

[DllImport( "alignment-vc141-mtd-x64.dll", CallingConvention =
 CallingConvention.Cdecl )]
    static extern void submit( RawImage img );

And this is where I pass the image to the unmanaged API.

using( var bitmap = new Bitmap( "AlignmentCenter.jpg" ) )
using( var stream = new MemoryStream( ) )
{
    bitmap.Save( stream, ImageFormat.Jpeg );
    var source = stream.ToArray( );
    fixed( void* ptr = source )
    {
        var raw = new RawImage
        {
            ImageData = ptr,
            Size = source.Length
        };
        submit( raw );
     }
  }

Is what I'm doing unsafe? Am I copying more than I should?
One last thing, I know about EmguCv and I've used it in the past but I won't be using it here.

There is not much to review. The only thing I can contribute with is that you don't have to run in unsafe mode:

If you define the data structure as:

  [StructLayout(LayoutKind.Sequential)]
  internal struct RawImage
  {
    internal IntPtr ImageData;
    internal int Size;
  }

Then you can run the .NET side as:

  using (var bitmap = new Bitmap(@"fileName.Jpeg"))
  using (var stream = new MemoryStream())
  {
    bitmap.Save(stream, ImageFormat.Jpeg);
    var source = stream.ToArray();

    IntPtr handle = IntPtr.Zero;

    try
    {
      handle = Marshal.AllocHGlobal(source.Length);
      Marshal.Copy(source, 0, handle, source.Length);

      var raw = new RawImage
      {
        ImageData = handle,
        Size = source.Length
      };
      submit(raw);
    }
    catch (Exception ex)
    {
      if (handle != IntPtr.Zero)
      {
        Marshal.FreeHGlobal(handle);
      }
    }
  }

The above works with the same signature for submit(RawImage img).

It's a little more code, but you are not limiting the use of your module.


Credit card validation in C I am relatively new to C programming and I am currently working through the CS50 EDX course. The problem I have solved below is for week 1 (credit).

Any suggestions on how to improve this code to further my learning would be most appreciated. I feel like my solution is very clunky although it does the job!

#include <cs50.h>
#include <stdio.h>
#include <math.h>

int main(void)
{
    long number = get_long("Number: ");

    // get the individual intergers of number
    // *2 odd digits

    int i1 = ((number / 1000000000000000) % 10);
    int t1 = i1 * 2;
    int i2 = ((number / 100000000000000) % 10);
    int i3 = ((number / 10000000000000) % 10);
        int t3 = i3 * 2;
    int i4 = ((number / 1000000000000) % 10);
    int i5 = ((number / 100000000000) % 10);
        int t5 = i5 * 2;
    int i6 = ((number / 10000000000) % 10);
    int i7 = ((number / 1000000000) % 10);
        int t7 = i7 * 2;
    int i8 = ((number / 100000000) % 10);
    int i9 = ((number / 10000000) % 10);
        int t9 = i9 * 2;
    int i10 = ((number / 1000000) % 10);
    int i11 = ((number / 100000) % 10);
        int t11 = i11 * 2;
    int i12 = ((number / 10000) % 10);
    int i13 = ((number / 1000) % 10);
        int t13 = i13 * 2;
    int i14 = ((number / 100) % 10);
    int i15 = ((number / 10) % 10);
        int t15 = i15 * 2;
    int i16 = (number % 10);

    // Luhns Alg
    // calculate sum of variable digits if > 9

    if (t1>9)
    {t1 = t1 - 9;}
    if (t3>9)
    {t3 = t3 - 9;}
    if (t5>9)
    {t5 = t5 - 9;}
    if (t7>9)
    {t7 = t7 - 9;}
    if (t9>9)
    {t9 = t9 - 9;}
    if (t11>9)
    {t11 = t11 - 9;}
    if (t13>9)
    {t13 = t13 - 9;}
    if (t15>9)
    {t15 = t15 - 9;}

    // check lunghs algo = true (0)
    // print card type

    int sum = (t1+t3+t5+t7+t9+t11+t13+t15+i2+i4+i6+i8+i10+i12+i14+i16);
    int check = (sum % 10);

    if (check != 0)
    printf("INVALID\n");
    else
    {
    // check type of card
        if(i1 == 0 && i2 == 3 && i3 == 4)
        {
            printf("AMEX\n");
        }
        else if(i1 == 0 && i2 == 3 && i3 == 7)
        {
            printf("AMEX\n");
        }
        else if (i1 == 5 && (i2 == 1 || i2 == 2 || i2 == 3 || i2 == 4 || i2 == 5))
        {
            printf("MASTERCARD\n");
        }
        else if (i1 == 4)
        {
            printf("VISA\n");
        }
        else
            printf("INVALID\n");
    }
}

The code currently outputs the correct results below.

378282246310005 = AMEX
5555555555554444 = MASTERCARD
5105105105105100 = MASTERCARD
4111111111111111 = VISA
5673598276138003 = INVALID
4062901840 = INVALID

It might be better to treat the credit card number as a string. In the C programming language a string is a null terminated array of type char or character. This would remove all the division in the program to get each character. It would also allow the program to detect if any non-numeric characters were entered.

To get the actual numeric value of a character you would subtract '0' from the numeric character. For a single character this would always give you a value between zero and nine.

Variable Names
Having variables i1 through i16 is a very good indication that i should be an array, this is also true of t.

Having single character variable names is generally frowned upon except for loop control values. A single character really doesn't tell anyone reading or modifying the code what the variable really is or does. It isn't really clear in the program what i or t represents. While number is longer, it might be better if it was credit_card_number.

Basic Principles When Writing Code
One of the earliest principles all programmers learn is Don't Repeat Yourself, usually shortened to DRY code. Basically anytime you have code that repeats it would be better to put it into a loop or a function where the code is reused.

One example of repeating code from the question is :

    if (tN > 9)
    {
        tN = tN - 9;
    }

This code can be made into a function:

int adjust_t_value(int tN)
{
    if (tN > 9)
    {
        return tN - 9;
    }
    else
    {
        return tN;
    }
}

If the variable t was an array, then code in the program could be reduced to

    for (int t_count = 0; t_count < N; t_count++)
    {
        t[t_count] = adjust_t_value(t[t_count]);
    }

There is a second form of the if statement that could also make the code shorter, it is generally covered in the later part of any C programming course

    tN = (tN > 9)? tN - 9 : tN;

This single statement is equivalent to the function above.

A second example of repeating code is the division to reduce each digit in the credit card number to a single number, this could also be put into a loop. The divisor could be reduced in each iteration of the loop if the algorithm sticks with using numbers.

A second principle that should be taught early but is part of more complex programming is the Single Responsibility Principle which states that every module, class, or function should have responsibility over a single part of the functionality provided by the software, and that responsibility should be entirely encapsulated within the function, module or class. This reduces the size of functions which allows functions to be more readable, maintainable and easier to debug. This would mean breaking the main() function into two or three sub functions to reduce the complexity of main.

Use Vertical Spacing to Make the Code More Readable
The code in the question has the if (tN > 9) on only two lines, it might be more readable if it was 4 lines as shown above.


Calculating the number of rounds required to complete a tournament We're running an heads-up tournament on our website, where the goal is to keep playing until a single winner remains. This works much the same as e.g. a tennis tournament bracket.

However, it only works perfectly if the number of participants is a power of 2, and this happens only rarely, so most of the time the first step is to play a round where some number of participants get a free win (a "bye") into the next round, in order to get the number of players down to the closest power of 2.

Everything works just fine, but I have a nagging suspicion that the part that calculates the number of rounds required to complete the tournament can be heavily improved. Here's the code I'm using to calculate it:

function totalRounds($participants)
{
    $closestLowerPow2 = pow(2, (strlen(decbin($participants)) - 1));

    $roundsWithoutByes = (int) log($closestLowerPow2, 2);

    return $participants == $closestLowerPow2
        ? $roundsWithoutByes
        : $roundsWithoutByes + 1;
}

So the length of the binary minus 1 is the closest power. Use log to find out how many rounds that requires, since if e.g. there are 8 participants, then there are 3 rounds to play, as 2³ = 8. Finally, add a round if we need to have a round with byes before that.

Is there a smarter way I'm overlooking? :)

Well, I feel like an idiot but I finally figured out why it seemed like there should be an obvious solution - one hour after posting, and three after starting to think about it. If I'm not allowed to post the answer myself, please let me know and I'll remove it.

function totalRounds($participants)
{
    return strlen(decbin($participants - 1));
}

The logic goes as follows: each bit in a string representation of the number of participants as a binary can be seen as a doubling of the number of participants, as long as the total number is a power of 2.

Then, by the same logic, each bit (except for the left-most one) can be seen as the reverse - a halving of the number of participants. And this is exactly what we want: each round, half of the players should be eliminated.

If we need to assign byes, that basically means we need to do one more round than that - so we can just use the string length straight away. But if we don't, then we need to disregard the left-most bit.

So if we start by subtracting one from the number of participants, that means that e.g. 8 participants = 1000 in binary turns into 7 = 111, which requires three rounds. For 9-15 participants, the string length would not change, so it's still correct. And for 16, the same logic applies as did to 8 - and so on.


Calculate total, minimum, maximum and mean from a set of distances I have only begun to learn C++ and am reading through the book, "Programming Principles and Practices Using C++". This bit of code is my attempt to complete exercise 3 at the end of chapter 4.

The actual task is:

  Read a sequence of double values into a vector. Think of each value as the distance between two cities along a given route. Compute and print the total distance (the sum of all distances). Find and print the smallest and greatest distance between two neighboring cities. Find and print the mean distance between two neighboring cities.

I am looking for any constructive criticism of my code. Keep in mind some things have not been covered. Even saying that, I still wouldn't mind hearing about what could be. Some things have been covered and I just don't implement them well.

    #include "std_lib_facilities.h"

int main()
{
    double ddistance = 0;
    double dtotaldist = 0;
    double dgreatest = 0;
    double dshortest = 0;
    char cgoagain;
    bool bgoagain = true;
    vector<double>vdistance;

    while (bgoagain) {
        cout << "Enter a valid 'double' ";
        cin >> ddistance;

    if (cin) {
        vdistance.push_back(ddistance);
        dtotaldist += ddistance;
        dshortest = ddistance;

        for (int i = 0; i < vdistance.size(); ++i)
            if ((vdistance[i]) > dgreatest) {
                dgreatest = vdistance[i];
            }
        for (int i = 0; i < vdistance.size(); ++i)
            if (vdistance[i] < dshortest) {
                dshortest = vdistance[i];
            }

        std::cout << "vdistance size is\t" << vdistance.size() << "\n";
        std::cout << "dtotaldist size is\t" << dtotaldist << "\n";
        std::cout << "The mean distance is\t" << dtotaldist / vdistance.size() << "\n";
        std::cout << "The long distance is\t" << dgreatest << "\n";
        std::cout << "The short distance is\t" << dshortest << "\n";
    }

    else {
        std::cout << "That is not a valid value. Would you like to try again? 'y' / 'n' \n";
        std::cin.clear();
        std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
        cin >> cgoagain;
        if (cgoagain == 'y' || cgoagain == 'Y') {
            bgoagain = true;
        }
        else{
            bgoagain = false;
        }
    }

    }
}

I guess std_lib_facilities is a header from the book? It's best to get used to including the standard library headers directly as soon as possible.

If std_lib_facilities contains using namespace std; note that this is bad practice.

In this case, we only need:

#include <vector>
#include <iostream>

"Hungarian" notation (adding type information to the names of variables), is not generally recommended. As such, the variable names should not be ddistance, cgoagain etc., just distance and goagain.

Using camel case or underscores may make them easier to read, and it's generally better to use whole words instead of abbreviations, e.g. totalDistance.

While C once had a requirement for all variables to be declared at the start of a block, C++ does not. Variables should be declared as close to their point of use as practical. As such, ddistance and cgoagain can be moved to just above where they are populated from std::cin.

Since you're storing the greatest and shortest distances, there's no need to iterate the whole vector of distances when a new value is entered. We can simply compare against the existing value. However, this won't work when initializing the greatest and shortest values to zero!

We can use another part of the standard library to initialize the greatest value to something that will always be smaller than a valid double input, and the shortest value to something that will always be larger. That way the first input will override both values successfully:

#include <limits>
...
double greatest = std::numeric_limits<double>::lowest();
double shortest = std::numeric_limits<double>::max();

...
        if (distance > greatest)
            greatest = distance;

        if (distance < shortest)
            shortest = distance;

Note that with this change, there's actually no need to keep a vector containing all the distances. We can just keep a count of the number of values entered and use that to calculate the mean.

Altered code:

#include <vector>
#include <iostream>
#include <limits>

int main()
{
    double totalDistance = 0;
    double greatest = std::numeric_limits<double>::lowest();
    double shortest = std::numeric_limits<double>::max();
    unsigned int distancesEntered = 0;

    while (true) {

        std::cout << "Enter a valid 'double' ";

        double distance = 0;
        std::cin >> distance;

        if (std::cin) {

            ++distancesEntered;

            totalDistance += distance;

            if (distance > greatest)
                greatest = distance;

            if (distance < shortest)
                shortest = distance;

            std::cout << "distancesEntered is\t" << distancesEntered << "\n";
            std::cout << "totalDistance is\t" << totalDistance << "\n";
            std::cout << "The mean distance is\t" << totalDistance / distancesEntered << "\n";
            std::cout << "The greatest distance is\t" << greatest << "\n";
            std::cout << "The shortest distance is\t" << shortest << "\n";
        }

        else {
            std::cout << "That is not a valid value. Would you like to try again? 'y' / 'n' \n";
            std::cin.clear();
            std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');

            char goAgain = 0;
            std::cin >> goAgain;

            if (!(goAgain == 'y' || goAgain == 'Y')) {
                break;
            }
        }
    }
}


WPF Localisation - using resx I have a small WPF application which I'm not exactly localizing, but not hard-coding resources either. I know WPF doesn't use .resx files for that purpose, but I exposed the resource strings as public ViewModel properties which the view's controls bind to:

using resx = MyApp.Properties.Resources;
...

public string InterferingProcessesText { get { return resx.InterferingProcessesText; } }
public string SettingsText { get { return resx.SettingsText; } }
public string ExecuteInstallerCommandText { get { return resx.ExecuteInstallerCommandText; } }
public string SaveSettingsCommandText { get { return resx.SaveSettingsCommandText; } }
public string RefreshProcessesCommandText { get { return resx.RefreshProcessesCommandText; } }
public string KillInterferingProcessesCommandText { get { return resx.KillInterferingProcessesCommandText; } }

And then if I ever decide to translate the strings I can copy my .resx file and rename it with a culture suffix just like I would have done in WinForms or in any other Web app.

I know I'm supposed to put x:Uid all over my XAML and localize my app the way WPF has it, but just how bad is that, and why?

Yes, it clutters up my ViewModels with public string properties (#region to the rescue!). But does it violate some MVVM principle that will come and bite me in the *** later?

I have had the same thought when implementing localisation in WPF, and came to the conclusion that .resx files are fine. I came to that conclusion because alternatives quickly became more complicated than I could be bothered with, and a RESX-based solution works for me.

So, in my various View.xaml files, I use bindings like this;

<MenuItem Name="Exit" Header="{x:Static Resources:Strings.MENU_HEADER_EXIT}" />

Changing languages is simply a matter of creating language-specific versions of the RESX entries. (http://msdn.microsoft.com/en-us/library/vstudio/aa992030%28v=vs.100%29.aspx).

Now in regards to your actual question; I do not think this sort of data belongs in a ViewModel. There is no 'modelling' going on with these strings; they are just label text, and as such are view-only data. If we follow your above suggestion to an overzealous conclusion, we might as well add ViewModel properties (and Bindings) like;

Font InterferingProcessesFont { get { return FontFamily.Arial; }
Color InterferingProcessesFontColor { get { return Color.Black; }
FontStyle InterferingProcessesFontStyle { get { return FontStyle.Normal; }
.
etc.
.

For my mind, localised labels like this are fixed display settings that will not change for the life of an application, and in most cases, will never change for a particular user. Why clutter a ViewModel with static text?


100 Doors simulation 100 Doors

  100 doors is a relatively simple mathematical problem. If you have 100 doors that are initially closed, and follow this procedure:

  Walk past the doors 100 times, the first time you toggle every door (if the door is closed, you open it, and vice versa). The second time, you only visit every second door, the third time every third door. This continues until you only visit the 100th door.

  .. what doors are remaining open?

class Door {
  state {_state}
  construct new() {
    _state = false
  }
  changeState() {
    _state = !_state
  }
}

class Doors {
  get {_array}
  construct new(size) {
    _array = []
    for (i in 1..size) {
      _array.add(Door.new())
    }
  }
}

var doors = Doors.new(100)
var step = 1
var result = 1

while (step != 100) {
  var count = 1
  var stepNextPoint = step

  for (door in doors.get) {
    if (count >= stepNextPoint) {
      door.changeState()
      stepNextPoint = stepNextPoint + step
    }
    count = count + 1
  }

  step = step + 1
}

for (door in doors.get) {
  if (door.state) {
    System.print("%(result) door is open")
  }
  result = result + 1
}

Your code is clean, and (based on my "literature research") your code is right. There are parts of your code though, that should be simplified, and an algorithmic change that should be applied to the way you iterate the doors. Let's go through these in detail...

Your primary loop is incrementing the step variable. You've implemented it like:

var step = 1

while (step != 100) {
  .....
  step = step + 1
}

That is OK, but it would be better to implement it using the range operator (semi-inclusive ... instead of inclusive ..) and a for-loop:

for (step in [1...100]) {
    ......
}

The next issue is algorithmic - you're looping over every door even when you only need to toggle every step doors. This code:

  var stepNextPoint = step
  for (door in doors.get) {
    if (count >= stepNextPoint) {
      door.changeState()
      stepNextPoint = stepNextPoint + step
    }
    count = count + 1
  }

should be inverted, and should become:

  var stepNextPoint = step
  while (stepNextPoint < 100) {
      doors.get[stepNextPoint].changeState()
      stepNextPoint = stepNextPoint + step
  }

These changes all make me realize that the value 100 is repeated in many places... the 100-value should be encoded as a variable, and the whole logic block should probably be expressed as a function call taking 100 as a parameter, and then just called from the main routine.


How do I add JDBC and Queue functionality to a simple server? I would like to add some capabilities to the server.  Firstly, it should accept and handle connections with multiple clients, so that there are no mixups between clients.  Secondly, there should be some very basic JDBC connectivity: serialize a result set as a List, and then, as requested pop from the list and send that instance to a client for updates.  When the client sends back an updated record, update the database accordingly.

package net.bounceme.dur.driver;

import java.io.IOException;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.net.ServerSocket;
import java.net.Socket;
import java.util.Properties;
import java.util.logging.Level;
import java.util.logging.Logger;

public class Server {

    private static final Logger log = Logger.getLogger(Server.class.getName());
    private final RecordQueue recordsQueue = new RecordQueue();

    public static void main(String[] args) {
        Properties props = PropertiesReader.getProps();
        int portNumber = Integer.parseInt(props.getProperty("port"));
        while (true) {
            try {
                new Server().inOut(portNumber);
            } catch (java.net.SocketException se) {
                Logger.getLogger(Server.class.getName()).log(Level.FINE, "spammy", se);
            } catch (IOException ioe) {
                Logger.getLogger(Server.class.getName()).log(Level.SEVERE, null, ioe);
            } catch (ClassNotFoundException cnf) {
                Logger.getLogger(Server.class.getName()).log(Level.INFO, null, cnf);
            }
        }
    }

    public void inOut(int portNumber) throws IOException, ClassNotFoundException, java.net.SocketException {
        ServerSocket serverSocket = new ServerSocket(portNumber);
        Socket socket = serverSocket.accept();
        ObjectOutputStream objectOutputStream = null;
        MyRecord recordFromClient = null;
        try (ObjectInputStream objectInputStream = new ObjectInputStream(socket.getInputStream())) {
            objectOutputStream = new ObjectOutputStream(socket.getOutputStream());
            recordFromClient = (MyRecord) objectInputStream.readObject();
        }
        objectOutputStream.flush();
        objectOutputStream.close();
        log.info(recordFromClient.toString());
    }
}

Is that a reasonable progression from this server code?  Obviously, I would only add a single feature at a time.  While I would love to learn Log4J or a similar testing framework, my immediate concern is adding functionality to server code.

What are some pitfalls I might run into?  What would be the most pragmatic approach to increasing the functionality of the server side operations?

For example, I might start with a Queue, and then only later tie that into a database.  Each client will only have access to a single record, so I'm not concerned about corrupt data.  Will there be a problem when multiple clients are trying to access the Queue, however?  The clients will only need pop and add, nothing more.  The client will update, or modify, each record instance it receives.

Using imports:

catch(java.net.SocketException se) { /* [...] */
throws java.net.SocketException { /* [...] */

fully qualifying these is unnecessary:
import java.net.SocketException;

catch(SocketException se) { /* [...] */
throws SocketException { /* [...] */

I feel that this code is much more concise, while containing the same information.
Using try-with-resources:
First: You are using try-with-resources. Good. That's the way to go.
Second: You're doing it wrong.

    try (ObjectInputStream objectInputStream = new ObjectInputStream(socket.getInputStream())) {
        objectOutputStream = new ObjectOutputStream(socket.getOutputStream());
        recordFromClient = (MyRecord) objectInputStream.readObject();
    }
    objectOutputStream.flush();
    objectOutputStream.close();

try-with resources does the last two things for you. You achieve the exact same result when you leave those out. The last two statments become useless clutter if you used try-with-resources:
try (ObjectInputStream objectInputStream = new ObjectInputStream(socket.getInputStream());
    ObjectOutputStream objectOutputStream = new ObjectOutputStream(socket.getOutputStream())) {
     recordFromClient = (MyRecord) objectInputStream.readObject();
}

Apart from that... You don't use your objectOutputStream anywhere. Why do you have it?
Using exceptions / logging:
Why is your ClassNotFoundException only logged as INFO? I'd expect a ClassNotFoundException to be minimum ERROR, if not FATAL! If you "expect" ClassNotFoundExceptions, then your design might be flawed.


Managing fault types I have a FaultType enum with more than 100 members:

public enum FaultType
{
    FaultType1,
    FaultType2,
    FaultType3,
    FaultType4,
    FaultType5,
}

And I have a FaultTypeConstants class corresponding to FaultType:

public class FaultTypeConstants
{
    public const int FaultType1 = 600;
    public const int FaultType2 = 100;
    public const int FaultType3 = 453;
    public const int FaultType4 = 200;
    public const int FaultType5 = 300;
}

Now, I want to populate the List<FaultType> based on FaultTypeConstants values:

public static List<FaultType> GetFaults(List<int> FaultConstants)
{
    var faults = new List<FaultType>();
    FaultConstants.ForEach(fc => {
        switch (fc)
        {
            case FaultTypeConstants.FaultType1:
                faults.Add(FaultType.FaultType1);
                break;
            case FaultTypeConstants.FaultType2:
                faults.Add(FaultType.FaultType2);
                break;
            case FaultTypeConstants.FaultType3:
                faults.Add(FaultType.FaultType3);
                break;
            case FaultTypeConstants.FaultType4:
                faults.Add(FaultType.FaultType4);
                break;
            case FaultTypeConstants.FaultType5:
                faults.Add(FaultType.FaultType5);
                break;
            default:
                break;
        }
    });

    return faults;
}

Is there a better way to do this?

Get rid of the constants entirely and assign those values to the enum members themselves.
You can then use the enum everywhere.

You can cast an enum member to int to get its numeric value.


Matching script tags with regexes Like anything that shouldn't be done, I decided to see if it is possible to match <script> tags robustly using regexes in PHP. Since there is no arbitrary nesting, I figured it should at least be possible.

This is what I came up with. It is designed to handle every edge case I could think of, including:

arbitrary attributes in the opening script tag
single and multiline comments and single and double-quoted strings (which might include arbitrary escape sequences) in the javascript which may contain the characters </script>
Captures the smallest script tag it finds.

Did I miss anything? Ideally, I want it to match exclusively anything a browser would consider a script element (might not be possible), but at the very least, I would like it to match only well-formed script tags with well-formed javascript.

Here is the string for the regex that I am passing to preg_match:

'#<script(?:[^>"]*(?:"[^"]*")?)*>((?:"(?:[^\\\\\\n"]*(?:\\\\.)*)*"|\'(?:[^\\\\\\n\']*(?:\\\\.)*)*\'|<[^/]?|/\\*(?:[^*]|\\*[^/]?)*\\*/|//.*|/[^/*]|[^\'"</])*)</script>#';

Note: I am not using this in production.

Instead of writing a long uncommented regex, compose it out of multiple parts. This makes it easier to understand what you're trying to do.

Let's go through the spec for a start tag:

  The first character of a start tag must be a "<" (U+003C) character.
  The next few characters of a start tag must be the element's tag name.
  If there are to be any attributes in the next step, there must first be one or more space characters.
  Then, the start tag may have a number of attributes, the syntax for which is described below. Attributes must be separated from each other by one or more space characters.
  After the attributes, or after the tag name if there are no attributes, there may be one or more space characters. (Some attributes are required to be followed by a space. See the attributes section below.)
  Then, if the element is one of the void elements, or if the element is a foreign element, then there may be a single "/" (U+002F) character. This character has no effect on void elements, but on foreign elements it marks the start tag as self-closing.
  Finally, start tags must be closed by a ">" (U+003E) character.

Expressed as a regex fragment, with insignificant whitespace:

$start_tag = "(?: [<] script (?: $ws+ (?:$attribute $ws+)* $attribute)? $ws* [>] )"

Attributes themselves are rather complex. They can either be empty, unquoted, single-quoted or double-quoted.

$attribute = "(?: $attr_name
              |   $attr_name $ws* [=] $ws* (?:[^${space_characters}\"'<>`&]+|$character_reference)+
              |   $attr_name $ws* [=] $ws* [\"] (?:[^\"&]|$character_reference)* [\"]
              |   $attr_name $ws* [=] $ws* ['] (?:[^'&]|$character_reference)* [']
              )"

Now once we fill in the appropriate values for character references and the space character recognized by HTML5 and possible attribute names, we have finally correctly matched the <script> start tag.

How does your solution hold up? I am led to believe that this part is supposed to match the start tag (I added whitespace for clarity):

<script (?: [^>"]* (?:"[^"]*")? )* >

Err, no. This fails to match single-quoted strings <script title='<You Fail>'>. This will also match some strings that do not contain valid HTML at all, like <script <<<<<<<>.

What is the point of this exercise? It's absolutely possible to correctly match HTML with the PCRE (it's not possible with a regular language, which has a specific computer science meaning. Too many people confuse the theoretical concept with a similarly named practical tool which happens to be more powerful). However, if you do want to do this, you have to follow the spec. Don't fudge it, read it. (Actually, I fudged it as well, but to a lesser degree. Do as I say, not as I do).

It is absolutely possible to write readable regexes. Compose them from multiple reusable parts. Use the /x option to include insignificant whitespace. But do not ram them onto a single line. That's just obfuscation, and you wouldn't do that in other languages.


Ruby wrapper for Instagram API I've created my first gem as Instagram API wrapper https://github.com/agilie/instagram_api_gem. I would be very pleased if you give me some feedback on code organization, methods elegancy and so on.

The thing I concerned about the most is next. I have a module Resource

  module Resource
    include Client

    private

    def show(id, options = {})
      raise 'Resource ID is required' unless id
      make_request resource_path(id), query: options
    end

    def index(id, resource_name, options = {})
      raise 'Resource ID is required' unless id
      make_request resource_path("#{id}/#{resource_name}"), query: options
    end

    def create(id, resource_name, options = {})
      raise 'Resource ID is required' unless id
      make_request(resource_path("#{id}/#{resource_name}"), { body: options }, :post)
    end

    def destroy(id, resource_name, resource_id = nil, options = {})
      raise 'Resource ID is required' unless id
      path = "#{id}/#{resource_name}"
      path = "#{path}/#{resource_id}" if resource_id
      make_request(resource_path(path), { query: options }, :delete)
    end
  end

And I use methods from it like this

  class Tag
    include Search
    include Resource

    def initialize(tag_id = nil)
      @tag_id = tag_id
    end

    def recent_media(options = {})
      index @tag_id, 'media/recent', options
    end
  end

Does this implementation look good or are there more elegant solutions?
I would be very pleased if you give me some feedback regarding the whole code organization and implementation, which could be found here https://github.com/agilie/instagram_api_gem

You are welcome to contribute if you prefer.

I find your code to be very good and readable :)

In the code you present here, I only see one mishaps :

raise 'Resource ID is required' unless id

is repeated multiple time and actually don't test for much.
I'd consider making a private method that check id and also check if id is not empty (or even if it isn't strip.empty? to consider only-spaces strings).

For the create method, can the option parameters really can be an empty hash ? If not I'd consider at least removing the default value for this.

I haven't read your whole repo, so I'm not sure but does the Tag class really need to include both Search and Ressource ?

Code like

@user_id = user_id ? user_id : 'self'

should be replaced with @user_id = user_id || 'self'

One important point : your code must be hard to unit test as it is coupled with HTTParty, you should consider using a Strategy pattern here.

Finally, I'd make some simple class that have only one or two fields (such as Tag, Location, User...) immutable by duping and freezing the parameter and freezing the whole object : (note that it won't work if you expect a boolean, a number, a Symbol or nil as dup raise an exception in such case :()

def initialize(user_id)
  @user_id = (user_id || 'self').dup
  @user_id.freeze
  freeze
end

I haven't done Ruby in some time so I'm a bit rusty but I hope my review helped you at least a bit :)


Can I optimize two for loops that look for the closest zip code based on lat/lon? I am new to python and I had the task to find the US zip code based on latitude and longitude. After messing with arcgis I realized that this was giving me empty values for certain locations. I ended up coding something that accomplishes my task by taking a dataset containing all US codes and using Euclidean distance to determine the closest zip code based on their lat/lon. However, this takes approximately 1.3 seconds on average to compute which for my nearly million records will take a while as a need a zip code for each entry. I looked that vectorization is a thing on python to speed up tasks. But, I cannot find a way to apply it to my code. Here is my code and any feedback would be appreciated:
for j in range(len(myFile)):
    p1=0
    p1=0
    point1 = np.array((myFile["Latitude"][j], myFile["Longitude"][j]))  # This is the reference point
    i = 0
    resultZip = str(usZips["Zip"][0])
    dist = np.linalg.norm(point1 - np.array((float(usZips["Latitude"][0]), float(usZips["Longitude"][0]))))
    for i in range(0, len(usZips)):
        lat = float(usZips["Latitude"][i])
        lon = float(usZips["Longitude"][i])
        point2 = np.array((lat, lon))  # This will serve as the comparison from the dataset
        temp = np.linalg.norm(point1 - point2)
        if (temp <= dist):  # IF the temp euclidean distance is lower than the alread set it will:
            dist = temp  # set the new distance to temp and...
            resultZip = str(usZips["Zip"][i])  # will save the zip that has the same index as the new temp
            # p1=float(myFile["Latitude"][58435])
            # p2=float(myFile["Longitude"][58435])
        i += 1

I am aware Google also has a reverse geocoder API but it has a request limit per day.
The file called myFile is a csv file with the attributes userId, latitude, longitude, timestamp with about a million entries. The file usZips is public dataset with information about the city, lat, lon, zip and timezone with about 43k records of zips across the US.

So a better approach may be to :

covert your myFile to a Numpy array [[lat0, long0], [lat1, long10], ...].

lat_long = np.asarray(myFile)

Sort this array on latitude

lat_long = np.sort(lat_long, axis=0)

find the closest western points to the target point

closest_point_to_west = lat_long[lat_long[:, 0] >= myPoint[0]]

Do the same for the other wind directions, this will give you the four closest points using Numpy's efficient slicing operations and you can do a distance measurement on only 4 of the points and not all of them.


Find two numbers that sum closest to a given number I'm working on the problem of finding 2 numbers that their sum is closest to a specific number. Input numbers are not sorted.

My major idea is to sort the numbers and find from two ends. I'm wondering if there are any better ideas in terms of algorithm time complexity. Any advice of code bug and code style is appreciated.

import sys
def find_closese_sum(numbers, target):
    start = 0
    end = len(numbers) - 1
    result = sys.maxint
    result_tuple = None
    while start < end:
        if numbers[start] + numbers[end] == target:
            print 0, (numbers[start], numbers[end])
            return
        elif numbers[start] + numbers[end] > target:
            if abs(numbers[start] + numbers[end] - target) < result:
                result = abs(numbers[start] + numbers[end] - target)
                result_tuple = (numbers[start], numbers[end])
            end -= 1
        else:
            if abs(numbers[start] + numbers[end] - target) < result:
                result = abs(numbers[start] + numbers[end] - target)
                result_tuple = (numbers[start], numbers[end])
            start += 1

    return result, result_tuple

if __name__ == "__main__":
    numbers = [2,1,4,7,8,10]
    target = 16
    numbers = sorted(numbers)
    print find_closese_sum(numbers, target)

Your approach is good. Improvement you can do is use a better sorting technique and rest the approach you are using seems best one.

Sort all the input numbers (Use better sorting techniques Eg. Quicksort).
Use two index variables l and r to traverse from left and right ends respectively. Initialize l as 0 and r as n-1.
sum = a[l] + a[r]
If sum is less than number, then l++
If sum is greater than number, then r–-.
Keep track of min sum
Repeat steps 3, 4, 5 and 6 while l < r.

Time complexity: complexity of quick sort + complexity of finding the optimum pair

= O(nlogn) + O(n) = O(nlogn)


Make To and From date range picker using jQuery UI I implemented date range picker logic using the jQUery UI datepicker component:

var onDocumentReady = function () {
        var datepickerConfiguration = {
            dateFormat: "dd/mm/yy"
        };

        var startDatepickerHandler = function () {
            var selectedDate = $(this).val();
            var newConfiguration = Object.create(datepickerConfiguration);
            newConfiguration.minDate = moment(selectedDate, "DD/MM/YYYY").toDate();
            $("#End").datepicker('destroy');
            $("#End").datepicker(newConfiguration);
        };

        var endDatepickerHandler = function () {
            var selectedDate = $(this).val();
            var newConfiguration = Object.create(datepickerConfiguration);
            newConfiguration.maxDate = moment(selectedDate, "DD/MM/YYYY").toDate();
            $("#Start").datepicker('destroy');
            $("#Start").datepicker(newConfiguration);
        };

        ///--- Component Binding ---///
        $('#Start').datepicker(datepickerConfiguration);
        $('#End').datepicker(datepickerConfiguration);

        $('#Start').on('change', startDatepickerHandler);
        $('#End').on('change', endDatepickerHandler);

    };
    $(document).ready(onDocumentReady);

But I am not satisfied with this approach of setting the min and max values of datepicker, because in this case, it always destroys and creates a new datepicker on every date change. It also creates an object for configuration which may cause lot of memory waste. Please tell what would be a better approach of setting the min and max values of datepicker.

Instead of destroying and recreating the datepickers, use the (3rd) option method (e.g. .option('maxDate', selectedDate)) to set the minDate/maxDate options when appropriate.

Also, by utilizing the onSelect option, which receives as arguments the selected date and the datepicker instance, instead of using an onChange handler, MomentJS can be eliminated.

Lastly, the two id selectors can be combined in the same jQuery call:

$('#Start, #End').datepicker(datepickerConfiguration);

See the simplified code below.

Another option might be to consider a daterange picker UI widget like this one but perhaps the current UI is desired.

//use this one handler for both Start and End Date pickers
var onDateSelect = function(selectedDate, input) {
  if (input.id === 'Start') { //Start date selected - update End Date picker
    $("#End").datepicker('option', 'minDate', selectedDate);
  } else { //End date selected - update Start Date picker
    $("#Start").datepicker('option', 'maxDate', selectedDate);
  }
};
var onDocumentReady = function() {
  var datepickerConfiguration = {
    dateFormat: "dd/mm/yy",
    onSelect: onDateSelect
  };
  ///--- Component Binding ---///
  $('#Start, #End').datepicker(datepickerConfiguration);
};
$(onDocumentReady);  // jQuery DOM ready callback registration
<link rel="stylesheet" href="//code.jquery.com/ui/1.12.1/themes/base/jquery-ui.css">
<link rel="stylesheet" href="/resources/demos/style.css">

Start Date: <input type="text" id="Start">

End Date: <input type="text" id="End">


Calculate average spacing per direction I am interested in finding the average spacing between objects on a directional basis. I did my best to minimally implement this in Python and to conform to PEP8.

I try to explain what I'm doing in the code comments. The problem I want to solve is to find the average spacing between circles of known radius and locations (in my application, these locations are irregular) on a directional basis. For example, what is the average spacing between these circles in the x, diagonal, and y directions?

import numpy as np

# I have a set of x and y coordinates describing the
# locations of objects
px = np.array([0, 0, 0, 10, 10, 10, 20, 20, 20])
py = np.array([0, 10, 20, 0, 10, 20, 0, 10, 20])

# Each object is a circle with known radius size
point_diameter = 2

# I'll put my answers here
spacing = {}

# The challenge is to calculate the average spacing per point on a directional
#  basis, e.g., what is the average distance between points in the direction
#  of 45 degrees?

# To do this, I define a function to check if points are alligned in a
#  direction
def check_alligned(direction, p1x, p1y, p2x, p2y):

    # Since the points have some wisdth, I need to check the "edge cases"
    # as well as the centerlines.
    tolerance = 0.2
    pr = point_diameter / 2
    for offsetx1 in [-1, 0, 1]:
        for offsetx2 in [-1, 0, 1]:
            for offsety1 in [-1, 0, 1]:
                for offsety2 in [-1, 0, 1]:

                    # The scheme is to check if the direction leads to the object x or y location.
                    # I chose or to handle cases where the x or y difference is zero.
                    distance = distance_between([p1x + pr*offsetx1,
                                                     p1y + pr*offsety1],
                                                    [p2x + pr*offsetx2,
                                                     p2y + pr*offsety2])
                    if abs(p1x + distance * np.cos(np.radians(direction)) - p2x) <= tolerance or abs(p1y + distance * np.sin(np.radians(direction)) - p2y) <= tolerance:
                         return True
    return False

def distance_between(v1, v2):
    return np.sqrt( (v1[0] - v2[0])**2 + (v1[1] - v2[1])**2)

# Initial Smallest Spacing - use to find if we get to a spacing in the loop
# and record the smallest one
ISP = 9e4

# Iterate through directions to calculate average distance in each direction.
# Directions go counterclockwise and x points in the direction of 0 degrees.
for direction in [0, 60, 45, 90]:

    # These are a running sum and counter of 1st nearest neighbor spacings
    # that are less than an initial smallest spacing value. They are used
    # to calculate the average spacing in each direction.
    total_spacing = 0.
    space_count = 0

    # Look through every point
    for ii in range(px.size):

        smallest_spacing = ISP
        # and every other point except ones we've already checked
        for jj in range(px.size - ii - 1):

            # If the jj point is not in the same line as the
            # ii point along this diretion, skip it
            idx2 = ii + jj + 1

            if check_alligned(direction, px[ii], py[ii], px[idx2], py[idx2]):
                space = np.sqrt((px[ii]-px[idx2])**2 +
                                          (py[ii] - py[idx2])**2)
                if space < smallest_spacing:
                    smallest_spacing = space

        # Record a distance if it is less than the initial
        # smallest spacing
        if smallest_spacing != ISP:
            total_spacing += smallest_spacing
            space_count += 1

    # record the average spacing in this direction
    if space_count:
        spacing[direction] = total_spacing / space_count
    else:
        spacing[direction] = 0.

print spacing

I don't fully understand your problem statement, could you elaborate? I asked about your definition of distance because unless the distance you are interested in is some projection of the euclidian distance, it should be constant regardless which direction you are looking.

In any case, I did your problem manually and found a different result than your program above reports. So here's my implementation with inline comments:

#!/usr/local/bin/python3

from sklearn.neighbors import NearestNeighbors
import numpy as np
import matplotlib.pyplot as plt

class Turbine():

    def __init__(self, x, y):
        self.initialx = x
        self.initialy = y
        self.x = x
        self.y = y

    def rotateBy(self, theta):
        newX = self.x*np.cos(theta)-self.y*np.sin(theta)
        newY = self.x*np.sin(theta)+self.y*np.cos(theta)
        self.x = newX
        self.y = newY

    def point(self):
        return [self.x, self.y]

class Farm():
    def __init__(self, turbineList):
        self.turbineList = turbineList

    def rotateFarmBy(self, theta):
        for turbine in self.turbineList:
            turbine.rotateBy(theta)

    def turbineCoordinates(self):
        return np.array([turbine.point() for turbine in self.turbineList])

    def turbineCount(self):
        return len(self.turbineList)

for theta in [0, np.pi/4., np.pi/3., np.pi/2.]:

    # establish the coordinates,
    # but first test with a small subset and compare with hand calculations
    coordinates = [[0, 0], [0, 10], [0, 20]]
    # coordinates = [[0, 0], [10, 10], [20, 20]]
    # coordinates = [[0, 0], [0, 10], [0, 20], [10, 0], [10, 10], [10, 20], [20, 0], [20, 10], [20, 20]]

    # instantiate the turbines and the container farm
    turbines = [Turbine(coord[0], coord[1]) for coord in coordinates]
    farm = Farm(turbines)
    farm.rotateFarmBy(theta)

    # get the N nearest neighbors, where N in this case is the number of turbines in the farm
    coordinates = farm.turbineCoordinates()
    neighbors = NearestNeighbors(n_neighbors=farm.turbineCount(), algorithm='ball_tree').fit(coordinates)
    distances, indices = neighbors.kneighbors(coordinates)

    # remove any zero distances so that you dont count each turbine twice
    nonzeros = distances[distances > 0]

    print("at {0:0.1f} degrees, the average distance is {1:0.3f}".format(theta*180/np.pi, sum(nonzeros)/farm.turbineCount()))

    # print(indices)
    # print(distances)
    # plt.scatter([c[0] for c in coordinates], [c[1] for c in coordinates])
    # plt.show()


Multi-threaded Winforms Conventions for Main() I'm trying to find a good pattern for new applications (and for refactoring old ones).

Maybe it's just me, but I dislike spawning my main logic thread from inside my main form thread (so far I always have 2+ threads in my apps and beefy hardware, also .NET 4.5).  I feel it conflicts with the separation of concerns principle.

So is this a good pattern?

Program.cs

using System;
using System.Collections.Generic;
using System.Linq;
using System.Windows.Forms;
using System.Threading.Tasks;

namespace MyApp
{
    static class Program
    {
        /// <summary>
        /// The main entry point for the application.
        /// </summary>
        [STAThread]
        static void Main()
        {
            try
            {
                // Prevent multiple instances of the application from opening
                using (new SingleGlobalInstance(1000))
                {
                    // Run Main Logic/Sequence Task
                    Task.Factory.StartNew(() =>
                        {
                            new MainTask()
                                .Run();
                        });

                    // Display GUI
                    Application.EnableVisualStyles();
                    Application.SetCompatibleTextRenderingDefault(false);
                    Application.Run(new MainForm());    // Blocks until Form Closes
                }
            }
            catch (TimeoutException)
            {
                MessageBox.Show("An instance of the application is already open.");
            }
            catch (Exception ex)
            {
                // Log Error
                // ... log to file...

                // Display Error to User
                MessageBox.Show(ex.Message);
            }
        }
    }
}

MainTask.cs

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading;

namespace MyApp
{
    public class MainTask
    {
        // Do Work
        public void Run()
        {
            Console.WriteLine("MainTask - Running");
            Thread.Sleep(10000);
        }

        // Constructor
        public MainTask()
        {
            Console.WriteLine("MainTask - Constructed");
        }

        //// Destructor
        //~MainTask()
        //{
        //    // Destructor defined for purposes of debugging
        //    Console.WriteLine("MainTask - Destructed");
        //}
    }
}

MainForm

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Windows.Forms;

namespace MyApp
{
    public partial class MainForm : Form
    {
        // Constructor
        public MainForm()
        {
            InitializeComponent();
        }
    }
}

I would second Steven Doggart's comment about it being pointless to spawn a thread that does nothing.

To address your separation of concerns issue, though: If you are using one of a number of design patterns for your UI (e.g., MVC, MVVM, MVP, etc.), your MainForm class will merely be binding to one or more data objects and raising events from the user.  The real work and asynchrony will be done in another class.

For example, in the MVP pattern, you might have MainForm, MainPresenter, and MainViewModel.

MainForm binds various controls to properties exposed by MainViewModel
MainViewModel implements INotifyPropertyChanged and raises PropertyChangedEvents when those properties change.
MainForm updates those properties as the result of users manipulating those controls.
MainPresenter responds to events raised by MainForm and updates MainViewModel properties as well, causing MainForm to respond through the PropertyChangedEvents

It is then up to MainPresenter (or more appropriately, other classes MainPresenter uses) to figure out how to handle threading.  MainForm itself never has to know or care that you are using separate threads.


colorprint - An ANSI Escape sequence console writer mini-library I wrote this a long time ago. I came back to it and decided to clean it up a little.

It uses the preprocessor and C99 anon structs to make a flexible python-like API for calling its functions.

colorprint.c

#include <stdio.h>
#include <string.h>

#include "colorprint.h"

static const char *numch = "0123456789";

static void
apply_effect(char *out, enum fx fx)
{/*{{{*/
    char buf[3] = { numch[fx - 1], ';' };

    strcat(out, buf);
}/*}}}*/

static void
apply_fg(char *out, enum color c)
{/*{{{*/
    char buf[4] = { '3', numch[c - 1], 'm' };

    strcat(out, buf);
}/*}}}*/

static void
apply_bg(char *out, enum color c)
{/*{{{*/
    char buf[4] = { '4', numch[c - 1], ';' };

    strcat(out, buf);
}/*}}}*/

static void
apply_fx(char *out, enum fx *fx)
{/*{{{*/
    for(int i = 0; fx[i]; i++)
        apply_effect(out, fx[i]);
}/*}}}*/

static void
apply_colors(char *out, struct cpoptions *cpo)
{/*{{{*/
    if(cpo->bg)
        apply_bg(out, cpo->bg);

    if(cpo->fg)
        apply_fg(out, cpo->fg);
}/*}}}*/

void
_cprint(struct cpoptions *cpo)
{/*{{{*/
    char options[30] = "";

    apply_fx(options, cpo->fx);
    apply_colors(options, cpo);

    if(!cpo->fg && !cpo->bg && !cpo->fx[0])
         fputs(cpo->string, stdout);
    else
        printf("\e[%s%s\e[0m", options, cpo->string);
}/*}}}*/

void
_cputs(struct cpoptions *cpo)
{/*{{{*/
    _cprint(cpo);
    putchar('\n');
}/*}}}*/

colorprint.h

#define func(name, type, ...) \
    name(&(type){.header = 0, __VA_ARGS__})

enum color {
    color_invalid,
    color_black,
    color_red,
    color_green,
    color_yellow,
    color_blue,
    color_purple,
    color_aqua,
    color_white
};

enum fx {
    fx_invalid,
    fx_plain,
    fx_bold,
    fx_dark,
    fx_italic,
    fx_underline,
    fx_light,
    fx_light2,
    fx_reverse,
    fx_blink,
    fx_strike
};

struct cpoptions {
    char header;
    char *string;
    enum fx fx[10];
    enum color fg;
    enum color bg;
};

void _cprint(struct cpoptions *cpo);
void _cputs(struct cpoptions *cpo);

#define cprint(...) func(_cprint, struct cpoptions, __VA_ARGS__)
#define cputs(...) func(_cputs, struct cpoptions, __VA_ARGS__)

/* Example:
 *      cprint("Hello World", .fg = color_green, .fx = {fx_bold, fx_italic, fx_strike});
 * Or:
 *      struct cpoptions cpo = {
 *          .string = "Hello World",
 *          .fg = color_green,
 *          .fx = {
 *              fx_bold,
 *              fx_italic,
 *              fx_strike
 *          }
 *      };
 *      _cprint(&cpo);
 */

Its usage ranges in complexity from

cputs("Hello World"); // prints "Hello World\n"

to

cputs( .string = "Hello World",
       .fg = color_green,
       .bg = color_red,
       .fx = {
           fx_bold,
           fx_strike,
           fx_italic
        }
); // prints "\e[1;9;3;41;32mHello World\e[0m\n", which appears as "Hello World" with a red background, green foreground, with bold+strikethrough+italic effects

In my opinion, this method of function calling is far more type-safe and versatile than the horrid va_args. It's also nice because it allows colorful output without the overhead of ncurses or termcap.

Architecture: IMO, the time for trusting that char * buffers will not overflow is past.  Pass in a size_t size and check for valid sizes in function calls.
static const char *numch = "0123456789"; serves little value and its application can cause UB on unruly input.  Rather than use it convert to digit character, just add '0', offset as needed.  numch[fx - 1] --> fx - 1 + '0'
Global functions that being with _ are reserve for the implementation. Use a different name.

// void _cprint(struct cpoptions *cpo)
void cp_print(struct cpoptions *cpo)

\e is a non-standard escape sequence.  Use \033 or ...

// printf("\e[%s%s\e[0m"
#define ESC 27
printf("%c[%s%s\e[0m", ESC

colorprint.h should have a protective #ifndef  to cope with re-entrantcy.

#ifndef colorprint_h
#define colorprint_h
   ... // existing content
#endif

I found useful enumerating the Nth item

enum color {
    color_invalid,
    color_white,
    color_N  // add
};

if (value >= color_N) Handle_InvalidValue(value);

Nice to have example usage in .h file.  Suggest #if 0 bracketing instead as it does not stop syntax highlighting.
Pedantic: size_t is the type to use for array indexing rather than int.  Code accordingly, remember that size_t is some unsigned type.
Namespace colorprint.h abuses namespace with color_..., fx_..., cp....  Suggest a more uniform prefix.  cp... is not too bad, but then use cp.h.
Avoid magic number - why 30?  Numbers other than -1,0,1,2 and maybe 42 deserve a macro or const object.

char options[30] = "";

Do not include files before "colorprint.h". "colorprint.c" should do this to insure "colorprint.h" is not dependant on some <*.h> file
tidy up, remove /*{{{*/, etc.
Good use of static functions.
Print-like functions should return int.

/// void _cprint(
int  _cprint(
  ...
  int i = printf("\e[%s%s\e[0m", options, cpo->string);
  return i;
}

func is much too common to use as a macro name

// #define func(name, type, ...) \
#define cp_func(name, type, ...) \


Quadratic equation solver in JavaScript The task is to implement a solveEquation function, which solves the Quadratic equation. Each equality has exact 2 integer solutions. Return those numbers as an ordered array.

Example:

const solutions = solveEquation('2 * x^2 - 10 * x + 12');
console.log(solutions); // [2, 3]

Solution:

function solveEquation(equation) {
    // Your solution here
    let a, b, c;
    // Using ES6 destructuring
    [a, b, c] = getValues(equation);
    return getXList(a, b, c);
}

// Determine a,b,c and return in an array
function getValues(equation) {
    // ax^2 + bx + c
    let a = equation.split(' * x^2')[0];
    let aSign = equation.split(a + ' * x^2')[0];
    // Assigns +- a
    a = parseInt(aSign + a);
    let bSign = equation.split(a + ' * x^2 ')[1].charAt(0);
    let b = equation.split(' * x^2 ' + bSign + ' ')[1].split(' * x ')[0];
    // Assigns +- b
    b = parseInt(bSign + b);
    // Assigns +- b
    let cSign = equation.split(' * x ')[1].charAt(0);
    let c = equation.split(' * x ' + cSign + ' ')[1];
    // Assigns +- c
    c = parseInt(cSign + c);
    return [a, b, c];
}
// Returns ordered Array
function getXList(a, b, c) {
    let list = [];
    list.push(getX('+', a, b, c));
    list.push(getX('-', a, b, c));
    list.sort((a, b) => a - b);
    return list;
}
// Returns X
function getX(sign, a, b, c) {
    // represent x = (- b +- √b^2-4ac)/2a as x = (b1 +- c1)/a1
    let x, b1, a1, c1;
    b1 = parseInt(- + b);
    a1 = 2 * a;
    c1 = Math.sqrt((b * b) - 4 * a * c);
    return x = Math.round((sign === '+') ? ((b1 + c1) / a1) : ((b1 - c1) / a1));
}

console.log(solveEquation('2 * x^2 - 10 * x + 12'));// [2, 3]
console.log(solveEquation('-20 * x^2 - 108797540 * x - 130011773690520'));// [-3667291, -1772586]
console.log(solveEquation('294 * x^2 - 141195558 * x - 1600964090384736'));// [-2105744, 2586001]
console.log(solveEquation('-267 * x^2 + 296412186 * x + 4722715166392080'));// [-3687112, 4797270]
console.log(solveEquation('-381 * x^2 + 2871374115 * x - 5385906614046864'));// [3516911, 4019504]
console.log(solveEquation('-154 * x^2 + 645219652 * x - 602658645850800'));// [1405588, 2784150]

The main reason I post this program is to get critical feedback and perhaps improve my coding style.

Buggy as.
Unusable I made a single change and then could not get it to work (turned out I added an extra space)
Bugs
Any spaces in the wrong spot, and deviations from the simplified equation, any fractions and more will cause the code to throw a variety of errors.
Expectation
When you create a function people that use it have idiomatic expectations. You must attempt to conform at least to common variations on these expectations, and or ensure that your code does not throw errors if the input is not as expected.
One would expect a equation solver to be robust and handle all the standard input variations, like be

invariant to white spaces,
invariant to equation layout (1 * x ^ 2 + 1 * x ^ 2) === ( 2 * x ^ 2)
invariant to form of power (x * x) === (x ^ 2)
invariant to implied constants (x + 0) === (x), (1*x) === (x) and (2*x^2+0*x) === (2*x^2)
invariant to equation form (x + 1) ^ 2 === ((x + 1) * (x + 1)) === (x ^ 2 + 2 * x + 1)
I invariant to implied multiplier (2x === 2 * x) Note the number (or variable) if before x and has no operator, the multiplication * is always implied.

Other problems.
Not all quadratics have solutions.

A quadratic can have zero solutions as it is above of below the x axis. Eg x ^ 2 + 1
A quadratic can have one solution if it just touches the x axis eg x ^ 2
A quadratic can have two solution eg x ^ 2 - 1 * x

Quadratics can also include fractions  1/2 or 0.5. You should also be able to add the equations strings together, two or more 2nd order polynomials added together (joined with a "+" or "-") should equate into a valid quadratic.
Naming
The function naming is rather bad. To name a few examples

getValues maybe parseEquation
getXList mabe solveQuadratic
list maybe coefficients

Constants
You should use const for constants. Some examples...
const [a, b, c] = getValues(equation);

const list = [];

const cSign = equation.split(' * x ')[1].charAt(0);

And most likely more problems but as the code is so hard to use I have not looked any deeper into your algorithm.
Conclusion.
I would say "Try again." as it is not at all what I would expect from a function that solves quadratics.


ASN.1 BER Encoding and Decoding I have a project (ldaplib) I am working on that needs to do ASN1-BER decoding and encoding.  The function I have for the decoding portion is slightly more complex but neither are all that complicated.  I would like to get some feedback on the overall approach as well as the code/exceptions/inline comments.

The decoding specifically uses a named tuple defined much higher in the code as a return type which is then used in many places throughout the code:

BER = namedtuple('BER', 'cls pc tag value')

The encoding portion:

def ber_encode(ber_cls, ber_pc, ber_tag, message):
    """ Encode a message into ASN1-BER (TLV) bytes.
    The ber_cls, ber_pc, and ber_tag should be numbers, the message can be
    bytes, str, or int. """
    if type(message) not in (str, int, bytes):
        raise TypeError("Requires str, int, or bytes object.")

    encoded = (ber_cls + ber_pc + ber_tag).to_bytes(1, byteorder='big')

    if type(message) is int:
        bytelength = (message.bit_length() // 8) + 1
        message = message.to_bytes(bytelength, byteorder='big')
    elif type(message) is str:
        message = message.encode(encoding='ascii')

    if len(message) == 0:
        return encoded + int(0).to_bytes(1, byteorder='big')

    # Short form or Long form?
    length = len(message)
    if length < 0x80:
        encoded += length.to_bytes(1, byteorder='big')
    else:
        bytelength = (length.bit_length() // 8) + 1
        encoded += (0x80 + bytelength).to_bytes(1, byteorder='big')
        encoded += length.to_bytes(bytelength, byteorder='big')

    # Add the message
    encoded += message

    return encoded

The decoding portion:

def ber_decode(message, result=None):
    """ Decode a message from ASN1-BER (TLV) bytes into a list of byte values.
    Values that contain embedded types will need to call this function to
    break the objects into their individual type/value components. """
    if type(message) is not bytes:
        raise TypeError("Requires bytes object.")

    if not result:
        result = []

    if len(message) == 0:
        raise Exception("No message")

    # Use masks to isolate the binary parts
    ber_type = message[0]  # NOQA
    ber_cls = ber_type & 0b11000000
    ber_pc  = ber_type & 0b00100000
    ber_tag = ber_type & 0b00011111

    ber_len = message[1]

    # Assume that the ber_value is offset 2 or is blank
    offset = 2

    # Only the definite form of length encoding is used.
    if ber_len > 0x80:
        nbytes = ber_len & 0x7f
        ber_len = int.from_bytes(
                message[offset:offset + nbytes], byteorder='big')
        offset = offset + nbytes
        ber_value = message[offset: offset + ber_len]
    else:
        ber_value = message[offset: offset + ber_len]

    # Append the decoded message part.
    result.append(BER(ber_cls, ber_pc, ber_tag, ber_value))
    message = message[ber_len + offset:]

    # If the message had only one type then we are done, if not recurse.
    if len(message) == 0:
        return result
    else:
        return ber_decode(message, result)

Some ideas:

String concatenation

The method you're using for string concatenation (successively using the += operator) is the slowest method in Python for constructing strings.  Consider instead creating a list of strings and concatenating them at the end using return ''.join(created_strings).  Because your BER encoder is likely to be used with a protocol, speed may be important.

Checking of inputs

Your cls, pc and tag elements are not checked on input to the ber_encode routine and would allow constructs such as ber_encode(0xc3,7,3,"bongo") which gets coded with a tag of 0xcd.  That isn't necessarily wrong but it's not what I might have expected.

Orthogonality of ber_encode and ber_decode

It's not unreasonable for a user of your functions to assume that the two complementary functions can each be fed the output of the other, but that's not the case for these functions.  While ber_decode will happily digest the output of ber_encode, what ber_decode produces is a named tuple rather than an output that's compatible with ber_encode's input requirements.  Again, this isn't necessarily wrong, but it is a potential impediment to users of your functions.

Error checking

Generally, the code does a pretty good job of validating inputs and throwing appropriate errors, but there is at least one case which is accepted that should probably throw an error.  Specifically,

ber_decode(b'\x86\xff'+('a'*256).encode(encoding='ascii'))

results in a BER value which is 129 bytes long.  It's probably worth double checking ITU X.690 section 8.1.3.5 which says that an encoded length value of 11111111b shall not be used.

Similarly,

ber_decode(b'\x86\x80'+('a'*256).encode(encoding='ascii'))

is decoded as three BER encodings, the first of which is 128 bytes long (which is suspect), the second is 97 bytes long (which is OK) and the third is 27 bytes long, which may or may not be OK, but shouldn't a truncated message throw an exception?

Test vectors

You may simply have omitted them from the posted code, but I'd highly recommend including test vectors with the code. Python has a number of ways to do testing including doctest and unittest. Not only do they help you make sure you've considered many different types of input (both good and bad), but they also serve as documentation for users of the functions.


Nested values copies from one object to another Here is a nodejs code that copies values from one object (submitted from a firm) to a model object.

Model:

var directory_model = {

    "link_title": {
        "value": 1, // This needs to be filled with object data
        "validation": null
    },

    "link_desc": {
        "value": 2, // This needs to be filled with object data
        "validation": null
    },
};

Object values that must get into the model:

   { link_title: 'test1', link_desc: 'test2' }

Here is the code that does, it with lodash. This code does the job:

_.forOwn(directory_model, function(directory_model_value, directory_model_key) {
    _.forOwn(fields, function(fields_value, fields_key) {
        "use strict";
        if (directory_model_key === fields_key)  {
            directory_model[directory_model_key].value = fields_value;
        }
    });

});

I don't like the nested loop, and would like to know if you know another way to do this.
I found several ways, but none of them works, because of the nested properties.

Why bother with libraries? This doesn't need anything more than pure JS. Assuming the incoming data is stored in fields:

var keys = Object.keys(fields);
for(var i = 0; i < keys.length; i++) {
    var key = keys[i];
    if(directory_model[key]) {
        directory_model[key].value = fields[key];
    }
}


GetHashCode of a Dictionary There is a huge documentation about using and generating hashcodes for objects that will go into a dictionary, really few more if you try to look about hashcode of a dictionary.

No, the default Dotnet dictionary doesn't override the Object.GetHashCode() method, so two different dictionary with same keys and same values will have different hashcodes.

Taking inspiration from this Stack Overflow question, I made this implementation in my object.

The object is a simple wrapper over a dictionary, which takes different values and save them with the number of times they have been added (values are keys of the dictionary, occurrences are values).

The starting value for the hashcode is zero (for empty dictionary) and it's calculated and stored every time a value is added, to avoid the necessity of enumerate the whole dictionary every time the hashcode is requested.

What do you think about it?

private int hascode = 0;

public bool Add(T item)
{
    values[item] = values.ContainsKey(item) ? values[item]++ : 1;

    unchecked
    {
        hashcode += 486187739 * item.GetHashCode();
        hashcode += 982451653 * values[item];
    }
    return true;
}

Hashes does not need to be unique, a good hash will have a good distribution and as little as possible collisions.

Which are the implications of this? Given two dictionaries \$a\$ and \$b\$ (with \$a = b\$) and an hash function \$\operatorname{f_{hash}}(D)\$ you can assert that \$\operatorname{f_{hash}}(a) = \operatorname{f_{hash}}(b)\$; similarly for \$a \neq b\$ the same condition may also be true. With this in mind default behavior may be considered wrong but simply they can't provide an effective generic-enough function. Also note that they explicitly suggest to do not override GetHashCode() if object is mutable (a dictionary is) because its hash may be used in an outer container.)

Which hash function is better may be not a simple choice because it has to be choosen to balance complexity, performance and distribution. Some knowledge of the elements you want to hash will also help you to pick the best one (in your case...where those numbers come from? Is there any rationale behind them to reach a better distribution?)

Let's start with a very simple case: hash is the number of items in the collection \$\operatorname{f_{hash}}(D) = \#(D)\$. It's better than default implementation and it's pretty fast but it's still a terrible hash function (huge number of collisions.) More often than not I just needed this.

Any other implementation depends on context: are items immutable (or you hash only keys)? In this case you may keep a running hash when inserting/removing items (assuming hash function you use can be reverted to subtract a now removed hashed item.)

What I sometimes do for dictionary of immutable items (unless your dictionary is made by thousands items, or you need hash in a very performance critical function) is to calculate hash only when required and cache it, cache will be invalidated when you add/remove an element. Proof of concept (here I override just Add() but you have to do it also for Remove() and Clear()):

public override int GetHashCode() {
    if (_hash == null)
        _hash = CalculateHashCode();

    return _hash;
}

public override void Add(TKey key, TValue value) {
    base.Add(key, value);

    _hash = null;
}

Note that if items are mutable then you can't cache calculated hash and it must be recomputed each time it is required (unless they're also observable objects...)

Which hash function? That's an hard choice, your magic numbers must be chosen to have a good distribution (if you know anything about objects stored in the dictionary.) As starting point you may use the simplest solution (unless you have some knowledge about distribution of hashes of elements): even \$\operatorname{xor}(\{d_1,\ldots,d_n\})\$ is good enough (you will use as much bits as used by hashes of items/keys in your dictionary.) It has the advantage to be fast, if you just need a quick comparison of two dictionaries (or lists) then you probably want the fastest algorithm (not the best - what a vague concept - one.)

Next step may be to use a popular general purpose (non-cryptographic!) hash function to balance between performance and quality, you may take a look to CityHash (developed by Google). It has good performance and it's easy to implement (plus: it may have different sizes then you may even decide to devote 16 bit for keys and 16 bits for values.) Its inputs are hashes from keys/values (then don't forget that also those hashes have to be of good quality)

To summarize: too many ifs, right? That's why to-do-nothing path was a good choice...