Recursive function, high performance critical def uniop_nested(func,o_list):
    def inner(i_list):
        if isinstance(i_list[0],np.ndarray):
            return map(func, i_list)
        else:
            return map(inner, i_list)
    return inner(o_list)

def binop_nested(func, o1, o2):
    if not isinstance(o1,np.ndarray):
        return [binop_nested(func, i1, i2)  for (i1,i2) in zip(o1,o2)]
    else:
        return func(o1,o2)

def add_nested(s1,s2):
    return binop_nested(np.add,s1,s2)

My code need to work with lists of ndarrays and list of lists of ndarrays.  Profiling shows this is some of the most performance critical code in my project.

How can I optimise it?
Can I rewrite the recursion as a loop?
Is there nice way to rewrite, them as Cython or a C extention? (I have no experience with this)

My Stack Overflow question here indicates that changing datatypes is probably not going to the solution.

More info:

Operands (o1 o2, s1, s2) are short lists. Profiling has shown me that using it.izip is slower.
Function return results are unlikely to be repeated. As the ndarrays are full of floats being tweaked with mathematical operations based of floats. (We are talking a large segment of Rn possible values)
Functions being applied are simple, the add_nested is the most common op by far, but there are a few onthers like uniop_nested(np.zeros_like, o_list).
ndarrays are of different sizes/shapes. (so a multidimentional ndarray won't work)

Context:
This is being used for training Restricted Boltzmann Machines (RBMs) and Neural network.
I have a generic "Trainer" class,
that takes a Trainee class as a parameter.
the Trainee class exposes a few methods like:

Get_update_gradient - a function that returns  (for a RBM [Restricted Boltzmann Machine]) a list containing a ndarray of weight changes and 2 ndarrays of bias changes, or (for a multilayer neural net) a list containing a list of weight matrix changes and a list of bias changes
knowledge: a property exposing either a list containing (for a RBM) a 2 bias arrays and a weight matrix or (for a neural net) a list of weight matrixes and bias arrays

It may seem that the Trainer class is simple, and unnesc, however it is moderately complex, and its use is common between the RBM and neural net classes. (Both benefit from the use of momentum and minibatchs)
A typical use is:

trainee.knowledge  = binop_nested(lambda current_value,update: learning_rate*update+current_value, trainee.knowledge, updates)

For recursion, you can try adding an @memoize decorator to it, to speed it up.

import functools

def memoize(f):
    cache= {}
    @functools.wraps(f)
    def memf(*x):
        if x not in cache:
            cache[x] = f(*x)
        return cache[x]
    return memf

Not sure if in your case it will speed up a lot, but if you try with fibonacci, you will see that it vastly speeds up the recursions, since it caches previous results.

This is something that speeds up recursion things in general. For your specific case, we need a bit more info, on what your functions are, and what you want to achieve to get a more specific answer.


Manage list of subjects (show, delete, add) in Python This is my code. It loops a menu, from which you can select whether you want to add a subject, see all subjects or delete a subject. Is there anything style- or functionality-wise to add or correct?
def add_subject(subjects):

    # ask for subject to add
    add = input("\nWhat subject do you want to add? ")

    # if subject not already in list, add subject
    subjects.append(add) if not add in subjects else print("\nSubject already in list.")

def show_subjects(subjects):

    # show all subjects
    print("\nThese are your subjects: ", subjects)

def delete_subject(subjects):

    # ask for subject to delete
    delete = input("\nPlease enter subject to delete: ")

    # if subject is in list, delete subject
    subjects.remove(delete) if delete in subjects else print("\nSubject not found.")

def main():

    # initialise list of sample subjects
    subjects = ["Math","Science","English"]

    # loop action cycle
    while 1:
        try:
            # ask user for selection
            selection = int(input("\nPlease enter option: \n(1) Add subject\n(2) Show all subjects\n(3) Delete a subject\n(4) Quit\n"))

            # call associated function
            if selection == 1:
                add_subject(subjects)
            elif selection == 2:
                show_subjects(subjects)
            elif selection == 3:
                delete_subject(subjects)
            elif selection == 4:
                break
            else:
                raise ValueError

        except ValueError:
            print("\nNot a viable option. Please pick 1, 2 or 3.")

if __name__ == "__main__":
    main()

Clear the screen
Here is the terminal after three iterations

We can clean a lot of this up by simply doing
print(chr(27) + "[2J")

after every iteration.
def clear_scr():
    print(chr(27) + "[2J")

Note that on adding this, you should also do
input("Press any key to continue...")

After the user selects an option, this way the user can wait and see what is on the screen peacefully without the program interrupting
def wait():
    input("Press any key to continue...")
    print(chr(27) + "[2J")

Or a better version that also prints a confirmation message
def wait(message):
    input(f"{message}. Press any key to continue...")
    print(chr(27) + "[2J")

wait("Subject Added!")

>>> Subject Added! Press any key to coninue...

Unnecessary comments
# ask user for selection
selection = int(input("\nPlease enter option: \n(1) Add subject\n(2) Show all subjects\n(3) Delete a subject\n(4) Quit\n"))

A comment here is totally not required, it only makes the code look convoluted for no reason. What I do suggest is the usage of docstrings.
def wait():
    """
    Wait for the user to press any key then clear the screen
    """
    ...

Print the subjects in a better fashion
I added a few more subjects in your program. When I entered Show all subjects.
These are your subjects:  ['Math', 'Science', 'French', 'Computer', 'Hindi', 'Social Science', 'Spanish', 'English']

This isn't nice at all, I suggest you print one on each line, with numbering, something like
Subjects

1. Math
2. Science
3. French
3. Computer
...

def show_subjects(subjects):
    """
    print all the subjects line-by-line
    """
    print("\n\nSubjects\n")
    for i in range(len(subjects)):
        print(f"{i+1}. {subjects[i]}")

Don't use magic numbers
            if selection == 1:
                add_subject(subjects)
            elif selection == 2:
                show_subjects(subjects)
            elif selection == 3:
                delete_subject(subjects)
            elif selection == 4:
                break
            else:
                raise ValueError

1, 2, 3, and 4 are unnamed numeric constants here. They don't really really tell anything about what the branch does. What if it was instead
            if selection == Choices.add.value:
                add_subject(subjects)
            elif selection == Choices.show.value:
                show_subjects(subjects)
            elif selection == Choices.delete.value:
                delete_subject(subjects)
            elif selection == Choices.quit.value:
                break
            else:
                raise ValueError

Without reading anything inside the if block, I can tell exactly what its going to do. That is an example of clean code.
You can achieve this by using an enum
from enum import Enum

class Choices(Enum):
    add = 1
    show = 2
    delete = 3
    quit = 4

print(Choices.add.value)
>> 1

print(Choices.quit.value)
>> 4

Colored text
This one is optional, but it adds a lot of meaning to your code :).
You can use the coloroma module to color your text to different colors in an easy way. For example, when you find that the user has entered bad input, you can do

Isn't that great! The best part about it is that it is cross-platform
from colorama import Fore, Back, Style
print(Fore.RED + 'some red text')
print(Back.GREEN + 'and with a green background')
print(Style.DIM + 'and in dim text')
print(Style.RESET_ALL)
print('back to normal now')

You have a lot of options, it can help you customize the output a lot. Read the documentation
As suggested by @Reinderien, using the colorama library you can clear the screen too
import colorama as cr
cr.init()

print(cr.ansi.clear_screen()) # clears the screen

Re-written
from colorama import Fore, Back, Style
import colorama as cr
from enum import Enum

cr.init()

class Choices(Enum):
    add = 1
    show = 2
    remove = 3
    quit = 4

def wait(message = None):
    if message is None:
        input("Press any key to continue...")
    else:
        print(Fore.GREEN)
        print(Style.BRIGHT)

        print(message, end = '')

        print(Style.RESET_ALL)
        input("Press any key to continue...")

    print(cr.ansi.clear_screen())

def add_subject(subjects):
    add = input("\nWhat subject do you want to add? ")
    if add not in subjects:
        subjects.append(add)
        wait("Subject Added!")
    else:
        print("Subject already present")
        wait()

def show_subjects(subjects):
    print("Subjects\n")
    print(Fore.YELLOW)
    print(Style.BRIGHT)
    for i in range(len(subjects)):
        print(f"{i+1}. {subjects[i]}")
    wait()

def delete_subject(subjects):
    delete = input("\nPlease enter subject to delete: ")
    if delete not in subjects:
        print("Subject not found")
        wait()
    else:
        subjects.remove(delete)
        wait("Subject removed!")

def main():
    subjects = ["Math","Science","English"]
    while True:
        try:
            selection = int(input("\nPlease enter option: \n(1) Add subject\n(2) Show all subjects\n(3) Delete a subject\n(4) Quit\n"))
            if selection == Choices.add.value:
                add_subject(subjects)
            elif selection == Choices.show.value:
                show_subjects(subjects)
            elif selection == Choices.remove.value:
                delete_subject(subjects)
            elif selection == Choices.add.value:
                break
            else:
                raise ValueError

        except ValueError:
            print(Fore.RED)
            print("\nNot a viable option. Please pick 1, 2 or 3.")
            print(Style.RESET_ALL)
            wait()

if __name__ == "__main__":
    main()


A JavaScript class, which finalises the HTML of each page of my NodeJS (Express) website Background

A request (req in the code) is made to my server, and a response (res) returned. By the time the response has filtered through to the code below, most of the heavy lifting - looking up entries in a database, etc - has already been done. This code just puts some final tweaks on all responses before it's pushed out to the user's browser.

What This Code Does

More specifically, the code achieves three things:

A time stamp is added to the footer of the page.
The crude kind of apostrophes are transmuted into the prettier, curlier kind. (This is more important in some fonts than in others.)
-- and --- are transmuted into en and em dashes respectively.

Entry Point

This code is fired only when code in another file calls finaliser.protoRender(req, res, view, properties);. So the protoRender method is the only entry point; it's also the only exit.

The Code

/*
This code contains a class which handles any final, universal touches to the
page before it's passed to the browser.
*/

// The class in question.
class Finaliser
{
  constructor()
  {

  }

  // Ronseal.
  static fixApostrophes(input)
  {
    while(input.indexOf("``") >= 0)
    {
      input = input.replace("``", "&ldquo;");
    }
    while(input.indexOf("''") >= 0)
    {
      input = input.replace("''", "&rdquo;");
    }
    while(input.indexOf("`") >= 0)
    {
      input = input.replace("`", "&lsquo;");
    }
    while(input.indexOf("'") >= 0)
    {
      input = input.replace("'", "&rsquo;");
    }
    return input;
  }

  // Ronseal.
  static fixDashes(input)
  {
    while(input.indexOf("---") >= 0)
    {
      input = input.replace("---", "&mdash;");
    }
    while(input.indexOf("--") >= 0)
    {
      input = input.replace("--", "&ndash;");
    }
    return input;
  }

  // Render, and deliver the page to the browser.
  protoRender(req, res, view, properties)
  {
    var date = new Date();

    properties.footstamp = date.toISOString();
    res.render(view, properties, function(err, html){
      if(html === undefined)
      {
        res.render(view, properties);
      }
      else
      {
        html = Finaliser.fixApostrophes(html);
        html = Finaliser.fixDashes(html);
        res.send(html);
      }
    });
  }
}

// Exports.
module.exports = Finaliser;

Ways of improving/optimizations:

What fixApostrophes and fixDashes functions actually try to do is replacing a specific punctuation chars with respective HTML entities.
Instead of those numerous horrifying while loops - a more optimized, concise and extendable approach would be to:

compose a predefined replacement map (where keys are search patterns and values - respective entities values):

const replaceMap = {"``": "&ldquo;", "''": "&rdquo;", "`": "&lsquo;",
                    "'": "&rsquo;", "---": "&mdash;", "--": "&ndash;"};

perform all the replacements at once
with String.replace function based on combined regex pattern:

input = input.replace(new RegExp(Object.keys(replaceMap).join('|'), 'g'), function(m){
    return replaceMap[m] || m;
});

where Object.keys(replaceMap).join('|') is used to compose regex alternation group from replaceMap keys like ''|'|---|--
the former 2 functions can be conceptually combined into a single function called, say punctToEntities ("punctuations to entities")

Eventually, the Finaliser class would look as:

const replaceMap = {"``": "&ldquo;", "''": "&rdquo;", "`": "&lsquo;",
                    "'": "&rsquo;", "---": "&mdash;", "--": "&ndash;"};

class Finaliser
{
    constructor()
    {

    }

    static punctToEntities(input) {
    /** Converts punctuation chars to respective HTML entities **/
        input = input.replace(new RegExp(Object.keys(replaceMap).join('|'), 'g'), function(m){
            return replaceMap[m] || m;
        });

        return input;
    }

    // Render, and deliver the page to the browser.
    protoRender(req, res, view, properties) {
        var date = new Date();

        properties.footstamp = date.toISOString();
        res.render(view, properties, function(err, html){
            if (html === undefined) {
                res.render(view, properties);
            } else {
                html = Finaliser.punctToEntities(html);
                res.send(html);
            }
        });
    }
}


Advent Of Code 2017 Day 4 (part 1) in Functional Programming (FP) I wanted to practice functional programming (fp) without using any library but using vanilla JS only. So I took a problem from Advent of Code (the 1st part of Day 4):

  A new system policy has been put in place that requires all accounts
  to use a passphrase instead of simply a password. A passphrase
  consists of a series of words (lowercase letters) separated by spaces.

  To ensure security, a valid passphrase must contain no duplicate
  words.

  For example:

  aa bb cc dd ee is valid.

  aa bb cc dd aa is not valid - the word aa appears more than once.

  aa bb cc dd aaa is valid - aa and aaa count as different words.

  The system's full passphrase list is available as
  your puzzle input. How many passphrases are valid?

My solution in FP:

const INPUT =
`pphsv ojtou brvhsj cer ntfhlra udeh ccgtyzc zoyzmh jum lugbnk
vxjnf fzqitnj uyfck blnl impo kxoow nngd worcm bdesehw
...
caibh nfuk kfnu llfdbz uxjty yxjut jcea`;

const get = input => input.split('\n');
const countDuplicate = words => words.reduce((acc, word) => {
  return Object.assign(acc, {[word]: (acc[word] || 0) + 1});
}, {});
const onlyUniqueWords = phrases => {
  const words = phrases.split(' ');
  const duplicateWords = countDuplicate(words);
  return !Object.values(duplicateWords).some(w => w > 1);
};
const phrasesWithUniqueWords = get(INPUT).filter(onlyUniqueWords);
console.log("solution ", phrasesWithUniqueWords.length);

Is there a better way to write it in FP with pure JavaScript, i.e. no additional FP library? Any other improvement suggestions are welcomed.

I don't remember where I saw it (maybe one of your other posts) but I recently saw a technique for ensuring uniqueness of array elements - iterate over the elements and just return the equality of the current index to the value returned by calling Array.indexOf(). Using that technique here, Array.every() can be used to ensure each word is unique.

With that technique, there is no need to count the number of occurrences of each word and hence the reduction can be removed. Thus countDuplicate can be eliminated and onlyUniqueWords can be simplified like below:

const onlyUniqueWords = phrases => {
    const words = phrases.split(' ');
    return words.every((word, index) => words.indexOf(word) === index);
};

Here is a jsPerf comparing this technique with the original. The original appears to be ~83-86% slower.

In the snippet below (and the jsPerf mentioned above), I replaced the 3rd line (i.e. ...) with two lines similar to those in the problem description. The first inserted line has unique words and the second does not.

const INPUT =
`pphsv ojtou brvhsj cer ntfhlra udeh ccgtyzc zoyzmh jum lugbnk
vxjnf fzqitnj uyfck blnl impo kxoow nngd worcm bdesehw
aa bb cc dd
aa bb cc dd ee cc
caibh nfuk kfnu llfdbz uxjty yxjut jcea`;

const get = input => input.split('\n');
const onlyUniqueWords = phrases => {
  const words = phrases.split(' ');
  return words.every((word, index) => words.indexOf(word) === index);
};

const phrasesWithUniqueWords = get(INPUT).filter(onlyUniqueWords);
console.log("solution ", phrasesWithUniqueWords.length);

P.S. I feel like this technique could very easily apply to your subsequent post for the 2nd part of this...,
 but perhaps for the sake of varying ideas, I shall refrain from mentioning this in that post...


Compare data within an interval and draw rectangles accordingly I have an interval array like below:

var interval = [[1], [1,3], [3,5], [5,7], [7,9], [9]];

I want to compare an input value say 1.5 with in the interval, and create an array drawData = [0,1,0,0,0]. To do this, i'm using a if loop like below:

            if(attr.freq < interval[0]){
                freqData = [0,0,0,0,0];
            }
            if(attr.freq >= interval[1][0] && attr.freq < interval[1][1]){
                freqData = [1,0,0,0,0];
            }
            if(attr.freq >= interval[2][0] && attr.freq < interval[2][1]){
                freqData = [1,1,0,0,0];
            }
            if(attr.freq >= interval[3][0] && attr.freq < interval[3][1]){
                freqData = [1,1,1,0,0];
            }
            if(attr.freq >= interval[4][0] && attr.freq < interval[4][1]){
                freqData = [1,1,1,1,0];
            }
            if(attr.freq >= interval[5]){
                freqData = [1,1,1,1,1];
            }

but the interval may change and I also want to know if there is a better way to to compare within the intervals.

This can be rewritten using a single map statement.

We just have normalize the data by making the first and last intervals consistent with the other intervals, which are defined by two points.  We use Infinity to do this.

The final solution will work with an arbitrary number of test intervals.

Rewrite:

function inIntervals(val, intervals) {
  var intervals = intervals.slice(0),    // this makes a copy, so we don't disturb the original array
      last      = intervals.length - 1;

  intervals[0]    = [-Infinity      , intervals[0]];  // here we normalize the first and last intervals
  intervals[last] = [intervals[last], Infinity    ];

  return intervals.map(x => val >= x[0] && val < x[1] ? 1 : 0)
}

var test = [[1], [1,3], [3,5], [5,7], [7,9], [9]];
console.log(inIntervals(1.5, test));
// [ 0, 1, 0, 0, 0, 0 ]

Update

To transform from a single element array to the format in your example:

x = [1,3,5,7,9]
var first = [x[0]];
x = x.map((x,i,a) => a[i+1] === undefined ? [x] : [x, a[i+1]])
x = [first].concat(x);


String join function C char* join(char** strings, size_t num_strings, char* seperator)
{
    int cache = 0;
    int count = 0;
    for(int i = 0; i < num_strings; i++){
        cache += strlen(strings[i]);
    }
    char * joined = malloc((cache + num_strings * strlen(seperator) + 1) * sizeof(char));
    for(int j = 0; j < num_strings; j++){ //big loop
        if(j != 0){
            int j = 0;
            while(j < strlen(seperator)){ //inserting seperators
                joined[count] = seperator[j];
                count++;
                j++;
            }
        }
        int insert_string = 0;
        while( insert_string < strlen(strings[j])){ //inserting strings
            joined[count] = strings[j][insert_string];
            insert_string++;
            count++;
        }
    }
    joined[count] = 0; //Zero Termination
    return joined;
}

I wrote this join() function, which creates a string from num_strings with specified separator, for example *. Is there a way to make this more concise?  Also is the termination correct?

char* join(char** strings, size_t num_strings, char* seperator)

The inputs can be const as they should not be modified by this function: const char* join(const char** strings, size_t num_strings, const char* seperator)

int cache = 0;

cache is not a very descriptive name. Prefer input_strings_length or similar

for(int i = 0; i < num_strings; i++){

Format as for (int i = 0; i < num_strings; i++) { (added spaces for readability)

char * joined

Why does the * have spaces around both side, but the rest are char* name? (space only on right side)

for(int j = 0; j < num_strings; j++){
    if(j != 0){
        int j = 0;

Prefer not using shadowed variables (same variable name in both inside and outside)

int j = 0;
while(j < strlen(seperator)){ //inserting seperators
    joined[count] = seperator[j];
    count++;
    j++;
}

This is same as:
strcpy(joined + count, seperator);
count += strlen(seperator);

which I find more readable

// big loop

This comment is not descriptive at all. What does it mean?
I would rename variable in for(int j = 0; j < num_strings; j++){ to be for (int string_index = 0; string_index < num_strings; string_index++) {

The whole program:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

const char * array[] = {
    "First entry",
    "Second entry",
    "Third entry",
};

const char* join(const char** strings, size_t num_strings, const char* seperator) {
    int input_strings_length = 0;
    for (int i = 0; i < num_strings; i++) {
        input_strings_length += strlen(strings[i]);
    }

    size_t string_length = input_strings_length + num_strings * strlen(seperator);
    char* joined = malloc((string_length + 1) * sizeof(char));
    joined[string_length] = '\0';

    int array_offset = 0;

    for (int string_index = 0; string_index < num_strings; string_index++) {
        if (string_index != 0) {
            strcpy(joined + array_offset, seperator);
            array_offset += strlen(seperator);
        }

        strcpy(joined + array_offset, strings[string_index]);
        array_offset += strlen(strings[string_index]);
    }

    return joined;
}

int main() {
    printf("%s\n", join(array, 3, "SEP"));
    return 0;
}


Valid Parentheses Algorithm Effeciency What do you think of my Algorithm?

I start to eliminate the nearest valid parentheses

and if string.length == 0 so it's valid

/**
 * @param {string} s
 * @return {boolean}
 */
var isValid = function(s) {
    while (s.length != 0 && s.includes("[]") || s.includes("()") || s.includes("{}")) {
        s = s.replace("[]", "");
        s = s.replace("()", "");
        s = s.replace("{}", "");
    }
    return s.length < 1
};

console.log(isValid("({((}{}))})"));
console.log(isValid("({(())})"));

It's correct, but it's ineffiecent. It uses O(n^2) time in the worst case (for instance, if the input's ((((((..)))))))).

You can do better than that by iterating over the input string and keeping a stack of brackets, pushing opening brackets to it and popping when you encounter closign brackets, checking that it's not empty and that the top bracket matches the current one.


Data analysis code (finds coincidences in three sets of data) I've written the following data analysis code (using just a couple lines from CERN's ROOT data analysis framework). It's designed to find coincidences in three sets of data (timestamps from an experiment for a given year) by pairing up each timestamp in one dataset with the closest possible timestamp in both the second and third datasets, then sorting those pairings by maxval - minval, and keeping only the closest "unique" coincidences (no two coincidences may share a timestamp, and each coincidence is a triplet, with one timestamp contributed by each dataset). The code works perfectly, however it's a bit messy and could probably use some cleaning up. Perhaps there are some clever tricks for simplifying some of this that I'm unaware of as well. Please do be very critical, I appreciate the feedback.
My code:
int binary_search(const vector<double> &vec, const double key) {

    int high = vec.size();
    int low  = 0;
    unsigned int mid;

    while(low <= high){
        mid = low + (high-low)/2;

        if(vec[mid] == key) return mid;
        else if(vec[mid] < key)  low = mid + 1;
        else high = mid - 1;
    }

    return mid;

}

void binary(){

    ROOT::RDataFrame statn1("D", "./path/to/data");
    ROOT::RDataFrame statn2("D", "./path/to/data");
    ROOT::RDataFrame statn3("D", "./path/to/data");

    vector<double> vec2, vec3;
    statn2.Foreach([&](double tstamp){vec2.push_back(tstamp);},{"UNIX"});
    statn3.Foreach([&](double tstamp){vec3.push_back(tstamp);},{"UNIX"});

    vector<vector<double> > coincidences;
    statn1.Foreach([&](double tstamp){},

        int res_i = binary_search(vec2, tstamp);

        double p = dabs(tstamp-vec2[res_i]);
        double q = dabs(tstamp-vec2[res_i+1]);
        double r = dabs(tstamp-vec2[res_i-1]);

        if(r<q && r<p) --res_i;
        else if(q<r && q<p) ++res_i;

        int res_j = binary_search(vec3, tstamp);

        p = dabs(tstamp-vec3[res_j]);
        q = dabs(tstamp-vec3[res_j+1]);
        r = dabs(tstamp-vec3[res_j-1]);

        if(r<q && r<p) --res_j;
        else if(q<r && q<p) ++res_j;

        double first = tstamp-vec2[res_i];
        double second = tstamp-vec3[res_j];
        double third = vec2[res_i] - vec3[res_j];
        double fourth = std::min(std::min(first, second), third);

        coincidences.push_back({tstamp, vec2[res_i], vec3[res_j], first, second, third, fourth});

    {"UNIX"});

    std::sort(coincidences.begin(), coincidences.end(),
        [](const vector<double>& A, const vector<double>& B){
            return A[6] < B[6];
    });

        std::set<double> cache;

    for(auto pair : coincidences){

        if(cache.find(pair[1]) == cache.end() && cache.find(pair[2]) == cache.end())    {
            std::cout << "Coincidence found!\n";
            cache.insert(pair[1]); cache.insert(pair[2]);
        }
    }
}

EDIT: CERN's ROOT framework can be found at https://root.cern.ch

Undefined behavior
There's UB in case when empty vector passed to the binary_search.
unsigned int mid;

it is uninitialised and will pop whatever compiler think he want in release mode. To solve this issue it is require initialise value with some error value (usually it is ~0 but you can choose your own), or use std::optional (c++17) and handle error case in lambda.
probably it is not an issue if vectors guaranteed to be non-empty and there's at least one proper value inside them.
mix of signed and unsigned types
Return type of binary_search is int, but branches can be unsigned int.
As it is actually return position in vector best way to use special unsigned integral type macro size_t meant to be used like this. So final check will be like.
...
constexpr size_t ERR = ~0;
siz_t res_i = binary_search(vec2, tstamp);
if (ERR == res_i) return; // early exit from lambda
...

But that kind of handling raise question about how to handle res_i-- if binary search return position 0. It probably will return INT_MAX for according integral type, but that kind of subtraction is implementation defined behavior so result defined by compiler.
potential out of bounds
if binary search return 0 position access to -1 will crash your program.
double r = dabs(tstamp-vec2[res_i-1]);

tightly related to previous issues.
probably it is not an issue if found position never meant to be 0
unnecessary variable reusing
If your program don't meant to be run on some embed device this kind of optimization basically do nothing - compilers usually smart enough to get rid of it by themselves. Probably good idea create some small function and p,q,r manipulations in it. Also it will prevent chance to make copy-paste errors
size_t someRoutine(double tstamp, const auto& vec, size_t res) {
  double p = dabs(tstamp-vec[res]);
  double q = dabs(tstamp-vec[res+1]);
  double r = dabs(tstamp-vec[res-1]);

  if(r<q && r<p) return res-1;
  if(q<r && q<p) return res+1;
  return res;
}
//...
res_i = someRoutine(tstamp, vec2, res_i);
res_j = someRoutine(tstamp, vec3, res_j);

structs clearer than vector
it is good idea to create a struct instead of using a vector - less memory print, clearer usage.
struct SomData{
  double tstamp, vec2,vec3, first, second, third, fourth;
};
//...
coincidences.push_back({tstamp, vec2[res_i], vec3[res_j], first, second, third, fourth});
// OR
// coincidences.emplace_back(tstamp, vec2[res_i], vec3[res_j], first, second, third, fourth); // construct struct in-place,
//...
std::sort(coincidences.begin(), coincidences.end(),
   [](const SomData& A, const SomData>& B){
            return A.fourth < B.fourth;
});
///...
cache.insert(pair.vec2);cache.insert(pair.vec3);

no need to use find if you don't need an iterator
maps ands sets have convenient method count which represents amount of items found by the key. You just need to check whether its zero or one
if(cache.count(pair.vec2) == 0 && cache.count(pair.vec3) == 0) {
  std::cout << "Coincidence found!\n";
  cache.insert(pair.vec2);
  cache.insert(pair.vec3);
}

cycle copy elements
probably just missed &
for(const auto& pair : coincidences){


Wrapper to mount partition before launching executable Solved task is pretty straightforward: mounting partition if not mounted already, to start executable.
There is a couple of checks and notifications because it is supposed to be launched from X and there are no way to see stdout/stderr stream. I have started with much larger version and a lot of error handling, that is why it has a function despite being so simple. On the other hand, it is much more cleaner of what is going on, when you read it and it will be easier to come back to it later on.
#!/usr/bin/env bash
DEVICE="/dev/sda5"
PARTITION="/mnt/Games"
EXECUTABLE="$PARTITION/MultiMC/MultiMC"

launch() {
    if ! findmnt "$PARTITION"; then
        DETAILS=`SUDO_ASKPASS="$(which ssh-askpass)" sudo --askpass mount "$DEVICE" "$PARTITION" 2>&1`
        [ $? -ne 0 ] && return 1
    fi
    notify-send "Launching.."
    exec "$EXECUTABLE"
}

launch || notify-send "Launching failed" "$DETAILS"

[ $? -ne 0 ] is a well-known over-complication - you can replace that with a simple || in the pipeline:
DETAILS=`SUDO_ASKPASS="$(which ssh-askpass)" sudo --askpass mount "$DEVICE" "$PARTITION" 2>&1` \
    || return $?

And in fact we can have a whole chain here:
launch() {
    findmnt "$PARTITION" \
        || DETAILS=`SUDO_ASKPASS="$(which ssh-askpass)" sudo --askpass mount "$DEVICE" "$PARTITION" 2>&1` \
        || return $?
    notify-send "Launching.."
    exec "$EXECUTABLE"
}

If you have chronic installed, that could replace the tedious saving of output into DETAILS:
NAME
       chronic - runs a command quietly unless it fails

SYNOPSIS
       chronic [-ev] COMMAND...

DESCRIPTION
       chronic runs a command, and arranges for its standard out and standard
       error to only be displayed if the command fails (exits nonzero or
       crashes).  If the command succeeds, any extraneous output will be
       hidden.

So,
launch() {
    findmnt "$PARTITION" \
        || SUDO_ASKPASS="$(which ssh-askpass)" \
           chronic sudo --askpass mount "$DEVICE" "$PARTITION" \
        || return $?
    notify-send "Launching.."
    exec "$EXECUTABLE"
}


Finding duplicates in arrays I'm given two sorted arrays, arr1 and arr2. They can either be of the same length or arr2 will be longer than arr1. I implemented a method findDuplicates that returns an array of all numbers that are both in arr1 and arr2. Note that the output array should be sorted in an ascending order.

I know there are better solutions than the one I came up with. What I'm trying to understand is the space complexity of the solution I came up with.

  static int[] findDuplicates(int[] arr1, int[] arr2) {
   // your code goes here
   HashMap<Integer, Integer> table = new HashMap<>();
   ArrayList<Integer> returnlist = new ArrayList<Integer>();
   int arr1Length = arr1.length;
   int arr2Length = arr2.length;

   // this is for the first array
   for(int i=0; i<arr1Length; i++){
      int n = arr1[i];
      table.put(n, 1);
   }

   for(int i=0; i<arr2Length; i++){
     int n = arr2[i];

     if(table.containsKey(n)){
       returnlist.add(n);
    }
  }
  int[] arr = new int[returnlist.size()];

  for(int i=0; i<=returnlist.size()-1; i++){
    arr[i] = returnlist.get(i);
  }
  return arr;
} // end of method

Since I created a temporary Hashtable and an ArrayList, and then another array to return the duplicates is my space complexity \$O(n^3)\$. I know its greater than \$O(n)\$. If someone can help me understand this better, I would really appreciate it.

static int[] findDuplicates(int[] arr1, int[] arr2) {

"Duplicates" is probably not the best name here. You would expect a duplicate to occur twice in the same collection of values or objects, not at least once in two collections. Maybe findPairs would be better, however still not perfect.

You might consider using an interface like List or even Iterable for the parameter types, to be able to support different types of collections instead of just arrays.

HashMap<Integer, Integer> table = new HashMap<>();

Do you really need a HashMap? It seems like you always use 1 as value and actually just want the keys. Use HashSet in that case. Call it uniqueValuesFirstArray or similar, instead of table. Use a name that tells you what you use it for.

ArrayList<Integer> returnlist = new ArrayList<Integer>();

Call the list result or better pairwiseOccuring or duplicates, because that is what you are saving in it. The name should not imply what the code will be doing with it later, e. g. returning it.

int arr1Length = arr1.length;
int arr2Length = arr2.length;

This is obsolete. There is no reason to save these values, instead just access them directly.

// this is for the first array
for(int i=0; i<arr1Length; i++){
   int n = arr1[i];
   table.put(n, 1);
}

This comment is obsolete. The code tells you that it is for array1, even better if you improve your variable names and access the array's length directly, like for (int i=0; i < array1.length; i++). Note the spaces that I added, to make it more readable.

Saving the value in a variable an then accessing it is obsolete. Just do table.put(arr1[i], 1);.

If you use HashSet instead of HashMap, you can replace the whole loop with HashSet<Integer> uniqueValuesFirstArray = new HashSet<>(Arrays.asList(array1));

for (int i=0; i < arr2Length; i++) {
    int n = arr2[i];

    if (table.containsKey(n)) {
        returnlist.add(n);
    }
}

This is what the indentation and whitespaces in this part of your code should look like.

As above, you do not need the variable n. Just use the value from the array directly.

If you change HashMap to HashSet, replace containsKey with contains.

int[] arr = new int[returnlist.size()];

for(int i=0; i<=returnlist.size()-1; i++){
    arr[i] = returnlist.get(i);
}

This can be done much easier. Instead of the for-loop, just use returnlist.toArray(arr);. It is probably faster, and definitely shorter and more readable.


Generate line chart from SQL database output using Morris <script>
<?php
unset($data0);
unset($data1);
unset($data2);

$sth = $dbh->prepare("
SELECT a, b, c
FROM   t
");
$sth->execute();
?>

Morris.Line({
  element: 'morris-line-chart',
  data: [
  <?php
  while ($row = $sth->fetch()) { ?>
    { m: '<?php echo $row['0']; ?>', a: <?php echo $row['1']; ?>, b: <?php echo $row['2']; ?> },
  <?php } ?>
  ],
  xkey: 'm',
  xLabels: 'month',
  ykeys: ['a', 'b'],
  labels: ['2014', '2015']
});
</script>

Is there a cleaner way to write it, especially the PHP echoing to create the JavaScript?

I like to keep the Javascript in one echo at the bottom, like this:

<?php

unset($data0);
unset($data1);
unset($data2);

$sth = $dbh->prepare("SELECT a, b, c FROM t");
$sth->execute();

$data = '';
while ($row = $sth->fetch(FETCH_ASSOC)) {
  extract($row);
  $data .= "{ m: '$c', a: $a, b: $b },".PHP_EOL;
}

echo "<script>
Morris.Line({
  element: 'morris-line-chart',
  data: [$data],
  xkey: 'm',
  xLabels: 'month',
  ykeys: ['a', 'b'],
  labels: ['2014', '2015']
});
</script>";

I had to make the assumptions that you're using PDO and that $c is your m in the morris line.


Get multiple minimal number from array I have script for get three values (minimal numbers) from array :

$(document).ready(function(){
    var _array = [1,2,3,4];
    var save = 0;
    var _new_array = [];
    var nearDistance = [];
    $.each(new Array(3),function(n){
        if (_new_array.length < 1){
            save = Math.min.apply(Math,_array);
            _new_array = jQuery.grep(_array, function(value) {
              return value != save;
            });
            nearDistance.push(save);
        } else {
            save = Math.min.apply(Math,_new_array);
            _new_array = jQuery.grep(_new_array, function(value) {
              return value != save;
            });
            nearDistance.push(save);
        }
    });
    console.log(nearDistance);
});

This script works to get 3 minimal values,

scenario script : Do each 3 times, first each get minimal value (save) from first array (_array), delete minimal value from the first array (_array) and then put resudial values to new array (_new_array), minimal value push to nearDistance, next each get minimal value from new array.

I think my script too less performance if I want to get more values from an array, what do you think about my script?

If I understand your question correctly, you're trying to:

Get n smallest numbers from your array.
The original array doesn't get mutated.

First off:

Underscores have a special meaning in programming. They're used to denote pseudo-private variables. Since JS has no concept of private (unless emulated with closures), prefixing an underscore to a publicly accessible variable means not to access that variable directly.
For this operation, you don't need jQuery. Vanilla JS has it's own forEach for $.each and filter for $.grep.
Just so you know, $ and jQuery are the same. The $ is an alias made for convenience. jQuery is the global of jQuery to prevent issue with clobbering. IIRC, MooTools uses the $ global. Anyone can alias their library as $, but who names their library "jQuery" besides jQuery?
$(document).ready(fn) can be shortened to $(fn) or jQuery(fn).
This operation is best moved out to a function so that you can call this out, pass in an array, and get the results you want.
Another problem is that $.grep return a new array every call. That means on each iteration of $.each, you're creating a new array - which is bad.
Math.min in your code will always return the smallest number. This means on every push, the numbers run from smallest to largest. Why not sort the array first and slice the first n?

Running with the above assumptions and review, you can do this:

function getSmallestNumbers(array, n) {
  // We slice first to prevent mutation of the original array when we
  // do `sort`. Then we slice again to get n items.
  return array.slice().sort().slice(0, n);
}

var distances = [4, 3, 1, 2];
var nearDistances = getSmallestNumbers(distances, 3);

// SE should seriously have a console view
document.write(JSON.stringify(nearDistances));


Is using the result of cmp() as an index "too cryptic"? This is a very subjective question, but I'm curious as to other people's opinions.

Would you find this bit of code distasteful? Is it cryptic and unreadable, or is it using features of the language in an elegant way?

def winning_side(home_goals, away_goals):
    """Takes a home score and an away score, and returns the winner

    Return DRAW, HOME or AWAY depending on the outcome
    """
    return (DRAW, HOME, AWAY)[cmp(home_goals, away_goals)]

The alternative obviously being explicit if-else statements:

def winning_side(home_goals, away_goals):
    """Takes a home score and an away score, and returns the winner

    Return DRAW, HOME or AWAY depending on the outcome
    """
    if home_goals > away_goals:
        return HOME
    elif away_goals > home_goals:
        return AWAY
    else:
        return DRAW

I personally find the verbose if-else logic in the latter solution, and the assumed equality in the 3rd branch, less appealing.

The documentation to cmp says nothing about the return value except it being negative, zero or positive, so you rely on implementation-dependent behaviour.  This exact function will also not work in Python 3, as there is no cmp function anymore.

I'd say use the second option or shorten it a bit:

def winning_side(home_goals, away_goals):
    if home_goals == away_goals:
        return DRAW

    return HOME if home_goals > away_goals else AWAY


Radix sort with integer divisions Here's a perhaps naive but simple implementation of Radix sort in Java.
It works with any radix, and both positive and negative numbers.

import java.util.ArrayList;
import java.util.Arrays;
import java.util.LinkedList;
import java.util.List;

public class RadixSortWithDivision {

    public static final int DEFAULT_RADIX = 10;

    public static void sort(int[] arr) {
        sort(arr, DEFAULT_RADIX);
    }

    public static void sort(int[] arr, int radix) {
        int maxDigits = 1 + (int) (Math.log(max(arr)) / Math.log(radix));
        int divisor = 1;
        for (int pos = 0; pos < maxDigits; ++pos) {
            List<List<Integer>> buckets = splitToBuckets(arr, divisor, radix);
            flattenBuckets(arr, buckets);
            divisor *= radix;
        }
        List<List<Integer>> buckets = splitBySign(arr);
        flattenBuckets(arr, buckets);
    }

    private static List<List<Integer>> splitToBuckets(int[] arr, int divisor, int radix) {
        List<List<Integer>> buckets = new ArrayList<>();
        for (int i = 0; i < radix; ++i) {
            buckets.add(new LinkedList<>());
        }
        for (int num : arr) {
            int bucketIndex = Math.abs(num) / divisor % radix;
            buckets.get(bucketIndex).add(num);
        }
        return buckets;
    }

    private static List<List<Integer>> splitBySign(int[] arr) {
        List<Integer> positive = new LinkedList<>();
        List<Integer> negative = new LinkedList<>();
        for (int num : arr) {
            if (num >= 0) {
                positive.add(num);
            } else {
                negative.add(0, num);
            }
        }
        return Arrays.asList(negative, positive);
    }

    private static void flattenBuckets(int[] arr, List<? extends List<Integer>> buckets) {
        int i = 0;
        for (List<Integer> bucket : buckets) {
            for (int num : bucket) {
                arr[i++] = num;
            }
        }
    }

    private static int max(int[] arr) {
        int max = Integer.MIN_VALUE;
        for (int num : arr) {
            if (num > max) {
                max = num;
            }
        }
        return max;
    }
}

Unit tests:

import org.junit.Test;

import java.util.Arrays;
import java.util.Random;

import static org.junit.Assert.assertArrayEquals;

public abstract class SortTest {

    abstract void sort(int[] arr);

    private int[] newRandomArray(int num) {
        Random random = new Random(0);
        int[] arr = new int[num];
        for (int i = 0; i < arr.length; ++i) {
            arr[i] = random.nextInt();
        }
        return arr;
    }

    private void sortAndVerify(int[] arr) {
        int[] copy = arr.clone();
        Arrays.sort(copy);
        sort(arr);
        assertArrayEquals(copy, arr);
    }

    @Test
    public void test_empty() {
        sortAndVerify(new int[0]);
    }

    @Test
    public void test_1() {
        sortAndVerify(new int[]{1});
    }

    @Test
    public void test_1_2() {
        sortAndVerify(new int[]{1, 2});
    }

    @Test
    public void test_2_1() {
        sortAndVerify(new int[]{2, 1});
    }

    @Test
    public void test_1_2_3() {
        sortAndVerify(new int[]{1, 2, 3});
    }

    @Test
    public void test_3_2_1() {
        sortAndVerify(new int[]{3, 2, 1});
    }

    @Test
    public void test_random_10() {
        sortAndVerify(newRandomArray(10));
    }

    @Test
    public void test_random_1000() {
        sortAndVerify(newRandomArray(1000));
    }

}

public class RadixSortWithDivisionTest extends SortTest {
    @Override
    void sort(int[] arr) {
        RadixSortWithDivision.sort(arr);
    }
}

How would you improve this?

How would you improve the unit test cases?

Bugs

@JS1's answer was spot-on:
the counting of digits this way was buggy:

    int maxDigits = 1 + (int) (Math.log(max(arr)) / Math.log(radix));

Because Math.log(...) gives NaN for negative numbers,
and negative infinity for 0,
which breaks the digit count and the sorting.

This unit test exposed the bug:

@Test
public void test_m300_m200_m999() {
    sortAndVerify(new int[]{-30, -200, -999});
}

I could fix this by finding the max absolute value.

private static int getMaxAbs(int[] arr) {
    int maxAbs = 0;
    for (int num : arr) {
        int absNum = Math.abs(num);
        if (absNum > maxAbs) {
            maxAbs = absNum;
        }
    }
    return maxAbs;
}

There is another bug, related to floating point arithmetics.
Even after taking the max of absolute values for counting digits,
this test still fails:

@Test
public void test_m300_m200_m1000() {
    sortAndVerify(new int[]{-300, -200, -1000});
}

The problem is that Math.log(1000) / Math.log(10) doesn't give 3 as expected,
but 2.9999999999999996, which then gets truncated to 2 in the line that calculates maxDigits.

The fix for this is to count the digits in a simple loop instead of using divisions with Math.log that is prone to floating point arithmetic issues:

private static int countDigits(int num, int radix) {
    if (num == 0) {
        return 1;
    }
    int work = num;
    int count = 0;
    while (work > 0) {
        work /= radix;
        ++count;
    }
    return count;
}

Optimize

This step is a bit wasteful:

    List<List<Integer>> buckets = splitBySign(arr);
    flattenBuckets(arr, buckets);

The splitting step will visit all elements to create the buckets,
and then the flatting step will visit all elements again to write back to the array.

A more efficient approach is possible:

Create a helper array to store the negative numbers in the right order
Iterate from the back of the target array

If the current item is negative, write it to the next position in the helper array
If the current item is positive, write it to the next position from the back of the target array

After one pass, the positive numbers will be clustered at the end of the target array, in the correct order, and the negative numbers will be at the beginning of the helper array
Copy the beginning of the helper array to target

Implementation:

private static void reverseAndShiftNegatives(int[] arr) {
    int len = arr.length;
    int[] negative = new int[len];
    int negativeFront = 0;
    int back = len - 1;
    for (int i = back; i >= 0; --i) {
        int num = arr[i];
        if (num > 0) {
            arr[back--] = num;
        } else {
            negative[negativeFront++] = num;
        }
    }
    System.arraycopy(negative, 0, arr, 0, negativeFront);
}


Perf wrapper for Excel VBA Spoiler

This code is actually BAD for performances! So don't use it! ;)

For a while now, I've been using this wrapper to avoid retyping most of it :

Public Sub PerfWrap(SubNameToRun As String, _
                    Optional ByVal DispStatusBar As Boolean = False, _
                    Optional ArgumentsToPass As String = vbNullString)
    Dim aWB As Workbook, _
        ActiveSH As Worksheet, _
        ScreenUpdateState As Boolean, _
        StatusBarState As Boolean, _
        CalcState As XlCalculation, _
        EventsState As Boolean, _
        DisplayPageBreakState As Boolean
    Set aWB = ActiveWorkbook
    Set ActiveSH = aWB.ActiveSheet
    DoEvents
    With Application
        ScreenUpdateState = .ScreenUpdating
        StatusBarState = .DisplayStatusBar
        CalcState = .Calculation
        EventsState = .EnableEvents
        DoEvents
        .ScreenUpdating = False
        .DisplayStatusBar = DispStatusBar
        .Calculation = xlCalculationManual
        .EnableEvents = False
    End With
    DisplayPageBreakState = ActiveSH.DisplayPageBreaks
    ActiveSH.DisplayPageBreaks = False

    If ArgumentsToPass <> vbNullString Then
        Application.Run SubNameToRun, ArgumentsToPass
    Else
        Application.Run SubNameToRun
    End If

    DoEvents
    ActiveSH.Activate
    ActiveSH.DisplayPageBreaks = DisplayPageBreakState
    With Application
        .ScreenUpdating = ScreenUpdateState
        .DisplayStatusBar = StatusBarState
        .StatusBar = vbNullString
        .Calculation = CalcState
        .EnableEvents = EventsState
    End With
End Sub

I use it like this :

Public Sub Launcher_CopyAndUpdate()
    Call PerfWrap("CopyAndUpdate_Templates", True)
End Sub

Or to pass arguments to the runned procedure :

Public Sub Launcher_Update()
    Call PerfWrap("Update_Templates", True, "Arg1, Arg2")
End Sub

I was wondering if :

it can be improved regarding performances
converted as a function to return the value sent by the runned function

Anyhow, any input or opinion is very welcome! :)

First, I'd like to echo @Mat'sMug's distaste for your multi-lined Dim statements.  There's absolutely no reason to do this at all having them prefixed with Dim makes it completely obvious that I'm looking at a declaration block.  It took me a good minute to realize that it wasn't a continuation of Sub declaration, because the Dim literally disappears into a huge wall of code. You aren't saving any time typing it either - last time I checked, Dim and , _ were both 3 characters, and the former is a lot easier to type on a qwerty keyboard.

The other answers hint around the performance, but I'll just come right out and say it.  Your Sub should really be named the AntiPerfWrap.  First, every single call that Application.Run makes is late bound - even for Excel objects.  It has to be, because like CallByName, it is forced to call IDispactch.GetIDsOfNames on itself.  This is always slower than an early bound call - period, end of discussion.

You always cache the ActiveWorkbook and ActiveSheet regardless of whether it is necessary or not.  I would guess that roughly 1% of all the code I've written requires this.  Same thing with .Calculation.  Same thing with .EnableEvents.  Do I write another PerfWrapWithEvents if I need that turned on?  Or maybe PerfWrapWithEventsAllowCalculation?  None of this work is free, and it seems like a jumbled collection of "this will make your code go faster anecdotes" collected from SO comments.  The cargo cult has found thier messiah in this wrapper.

DoEvents is not free.  In fact it's the complete opposite.  It yields the processor to every other thread that needs to clear it's event queue.  It also makes absolutely no sense to call DoEvents when you've explicitly disabled 95% of the functionality that would make the call useful.

Brass tacks time - let's see how much overhead this wrapper really adds.  This is the called routine:

Public Sub UnderTest()
    Dim i As Long
    For i = 1 To 10
        Debug.Print i
    Next
End Sub

And the baseline benchmark:

Public Sub BenchmarkOne()
    Dim starting As Single
    starting = Timer

    Dim i As Long
    For i = 1 To 100
        Sheet1.UnderTest
    Next

    Debug.Print "Unwrapped: " & Timer - starting
End Sub

  Unwrapped: 0.8515625

Public Sub BenchmarkTwo()
    Dim starting As Single
    starting = Timer

    Dim i As Long
    For i = 1 To 100
        PerfWrap "Sheet1.UnderTest"
    Next

    Debug.Print "Wrapped: " & Timer - starting
End Sub

  Wrapped: 6.492188

Ouch.

Conclusion - Burn it.  Burn it with fire.  The premise behind the code is that there is a common set of operations that you can do to "speed up" code.  In fact, this is never true.  In reality, you are turning off Excel functionality that can slow down code.  There's a big difference.  If you approach performance issues by looking outside of code that you've written, you're starting out looking in the wrong direction.  A catch-all "solution" for performance simply doesn't exist, so I wouldn't pretend to be providing it.  The best way to look at it is to imagine that any code under performance review to be copied and pasted in place of this...

If ArgumentsToPass <> vbNullString Then
    Application.Run SubNameToRun, ArgumentsToPass
Else
    Application.Run SubNameToRun
End If

...then do a code review on the entire thing.  Ask yourself, "In the context of the full Sub, what is the reason for every single line of code other than part above"? I'm guessing that in roughly 99.99% of cases, there won't be a good answer for all of them.


Determine if an HTML tag name is valid We've developed an ESLint rule to determine if a tag name used inside the Protractor's by.tagName() locator is valid.

  A Tag Name is considered valid when:

  It can contain alphanumeric characters.
  It can contain the dash(-) symbol.
  It cannot start with dash or a number.
  It cannot end with dash.

The Code:

'use strict'

var isTagName = /^[A-Za-z][A-Za-z0-9-]*$/

module.exports = {
  meta: {
    schema: []
  },

  create: function (context) {
    return {
      'CallExpression': function (node) {
        var object = node.callee.object
        var property = node.callee.property

        var insideByTagName = object && property && object.name === 'by' && property.name === 'tagName'
        var argumentExists = node.arguments && node.arguments.length && node.arguments[0].value

        if (insideByTagName && argumentExists) {
          var tagName = node.arguments[0].value
          if (!tagName.match(isTagName) || tagName.endsWith('-')) {
            context.report({
              node: node,
              message: 'Invalid TagName value: "' + tagName + '"'
            })
          }
        }
      }
    }
  }
}

Is it possible to decrease nested-ness and improve on readability?

FYI, using standard JS code style.

The original code is fine IMO, but technically it's possible to flatten using a complete regexp

var isTagName = /^[A-Za-z]([A-Za-z0-9-]*[A-Za-z0-9])?$/

plus dubious/stinky value fallbacks and ES2015 arrow function:

  create: context => ({
    CallExpression: node => {
      if ((node.callee.object || {}).name !== 'by' ||
          (node.callee.property || {}).name !== 'tagName') {
        return
      }
      var tagName = ((node.arguments || [])[0] || {}).value
      if (tagName && !tagName.match(isTagName)) {
        context.report({
          node,
          message: `Invalid tag name: "${tagName}"`
        })
      }
    }
  })


Querying .NET DataSet and ignore rows that contain NULL I've written some code to read a column of data from a dataset called AltIds.  The values are nullable.  My code is written to find all the values in the column that are not null and write them to an array.

Guid[] altIds = myDataSet.Tables[0].AsEnumerable()
  .Select(row => row.Field<Guid?>("AltId"))
  .Where(x => x.HasValue)
  .Select(x=>x.Value)
  .ToArray();

The code works, but it seems a bit complicated.  Is there a better way to do this?

What you have is fine. You could restructure it to eliminate one of the selects, but then you would repeat yourself with the row.

var altIds = myDataSet.Tables[0].AsEnumerable()
                 .Where(row => row.Field<Guid?>("AltId").HasValue)
                 .Select(row => row.Field<Guid?>("AltId").Value)
                 .ToArray();

You could rewrite it in query expression syntax, but that's only less complicated if you find fluent syntax more so.

var altIds = (from row in myDataSet.Tables[0].AsEnumerable()
             let x = row.Field<Guid?>("AltId")
             where x.HasValue
             select x.Value).ToArray();

Either way, it's six in one hand, half a dozen in the other. I find your code readable, and it passes the "does what it is supposed to" test, so I wouldn't worry about this snippet.


Enumerating a boolean in C This is one way to make a boolean in C.

typedef enum {
    false = 1 == 0,
    true = 0 == 0
} bool;

Recently I made some like

typedef enum {
    true = ~(int)0, //true is the opposite of false
    false = (int)0  //the smallest enum is equal to sizeof int
} bool;

int main(void) {
    return false;
}

Can this be improved upon?

Well, according to the ANSI book:

  The identifiers in an enumerator list are declared as constants of type int

So the cast is redundant:

typedef enum {
    true = ~0,
    false = 0
} bool;

Also, in C, a "true" value is anything other than zero, so ~0 is no more "true" than 1 - might as well keep it simple:

typedef enum {
    false = 0,
    true = 1
} bool;

In fact, 0 and 1 are the standard choice for representing boolean values where there's no native boolean type (for example, the bit type in SQL uses 0/1).

Now you might ask yourself, "so - did we just redefine zero and one?" - well, we certainly did. As an experiment in using typedef/enum it's okay. But don't use it in actual C code, because it'll make if clauses unnecessarily complex.

For example, let's say we want to compare str and str2 and to do something if they're not equal. We'll use strcmp which compares two strings and returns 0 if they're equal.

// without bool
if(strcmp(str1,str2))
{
    ...
}

// with bool
if(strcmp(str1,str2) == 0 ? false : true)
{
    ...
}

In conclusion, as an experiment your code is OK and can be slightly improved. Generally, the lack of bool type in C seems scary at first, but it's actually quite convenient when you get used to it.


Improving GUI update call from a worker thread in winforms I have a winforms app and I've been using the following approach to update controls on the main form from worker threads:

GamepadManager class has:

public static event EventHandler OnGamepadConnected;

Main app class has:

GamepadManager.OnGamepadConnected += OnGamepadConnected;
private void OnGamepadConnected()
{
    Invoke((Action)(() => { gamepadStatusLabel.Text = "Connected"; }));
}

That looks kinda bad, but without the Invoke thing I get a cross-thread operation exception because I can't update controls from worker threads (from where the call comes from).

Is there a way to reduce the number of parentheses used? Maybe there's a whole other, better way to do this?

Previously I've been using timers to check for every small thing like that, which ran on GUI thread (apparently?) and there was no trouble simply calling controlName.Text = "something"; without all the Invoke() stuff. But that means even more code and more processing cost due to constant checks.

You could create a helper method like the following:

void ChangeUI(Action action)
{
  if (InvokeRequired)
  {
     Invoke(action);
  }
  else
  {
     action();
  }
}

Now you can use it from any place where you need to modify the UI of that form and let the method check if it actually needs to call Invoke or simply execute the delegate.

ChangeUI(() => gamepadStatusLabel.Text = "Connected");


Comparing two strings to see if string 2 is inside string 1 Here is what I have to do:

  Write a function scramble(str1,str2) that returns true if a portion of str1 characters can be rearranged to match str2, otherwise returns false.

  For example:

  str1 is 'rkqodlw' and str2 is 'world' the output should return true.
  str1 is 'cedewaraaossoqqyt' and str2 is 'codewars' should return true.
  str1 is 'katas' and str2 is 'steak' should return false.

  Only lower case letters will be used (a-z). No punctuation or digits will be included.
  Performance needs to be considered

public class Scramblies {
    public static boolean scramble(String str1, String str2) {
        String temp = str1;
        int count = 0;
        boolean result = true;
        for(int i=0 ; i<str2.length() ; i++){
            char c = str2.charAt(i);
            if(temp.contains(String.valueOf(c))){
                temp = temp.replaceFirst(String.valueOf(c), "");
                count++;
            }
        }
        if (count == str2.length()){
            result = true;
        } else {
            result = false;
        }
        return result;
    }
 }

My code works perfectly, but the only problem now is efficiency. I am tested against how long it takes to finish the final test class, which times out. I get this:

  Process was terminated. It took longer than 10000ms to complete

How can this code be made more efficient to achieve the same result?

The problem with your approach is that you're traversing the second String as many times as there are elements in the first String with the replaceFirst call (which also involves a regular expression).

Also, this has nothing to do with performance, but you could just use

return count == str2.length();

instead of the long, and so more difficult to read,

boolean result = true;
if (count == str2.length()){
    result = true;
} else {
    result = false;
}
return result;

You can tackle this problem a lot more effectively by taking advantage of the fact that only lower case letters will be used (a-z). Consider the following approach:

We know that there are 26 lower case characters. Let us create an array of 26 values int[] array = new int[26];.
For each character c of the second String, we increment the value stored in the array at the position c - 'a': this corresponds to the alphabetical position of the character (based from 0: 'a' -> 0, 'b' -> 1, ..., 'z' -> 25).
Then for each character c of the first String, we decrement the value stored at the alphabetical position.
Finally, if at the end of this process, we end up with at least one value that is strictly positive, we know that the second String wasn't contained in the first one. This is because it means that we encountered a character in str2 more times than it was in str1 (it was incremented more times than it was decremented).

A sample implementation of this approach would be:

public static boolean scramble(String str1, String str2) {
    int[] array = new int[26];
    for (char c : str2.toCharArray()) {
        array[c - 'a']++;
    }
    for (char c : str1.toCharArray()) {
        array[c - 'a']--;
    }
    for (int value : array) {
        if (value > 0) {
            return false;
        }
    }
    return true;
}

You can test it with

public static void main(String[] args) {
    System.out.println(scramble("rkqodlw", "world")); // prints "true"
    System.out.println(scramble("cedewaraaossoqqyt", "codewars")); // prints "true"
    System.out.println(scramble("katas", "steak")); // prints "false"
}


Simple CoffeeScript navigation menu I'm using simple script to handle tab menu like this:

HTML:

Info

Orders

Models

Balance

and CoffeeScript:

jQuery ($) ->
 class Nav
  constructor: (@elements) ->
    self = @
    wHash = document.location.hash.replace("#", "") or "info"

    # activate element by location hash
    @.activate document.getElementById "personal-#{wHash}"

    # bind element activation on click
    elements.click ->
      self.activate @

  activate: (elem) ->
    @elements.removeClass "active"
    $(elem).addClass "active"

    # set a proper hash
    wHash = elem.id.split("-")[1]
    document.location.hash = "##{wHash}"

    # load proper content into an inner section
    # it works very fast and is the reason i use this type of navigation in the first place

    $("#personal-content").load "/user/#{wHash}"

 new Nav $("#personal").find("li")

This code works, but i'm pretty sure that it's very far away from best practices, and can be greatly improved, as these are my first tries in CoffeeScript. So the question is, can this code be beautified in some way?

You're building a navigation, however there are no links. It's highly recommended to use anchor tags for linking purposes.

Your naming is a bit loose. What is personal in your context? Please specify that. Even something like personal-navigation is quite loose, but still better.

I've changed your markup a bit to reflect my small suggestions. Note that I changed the ID's from your list items to classes. ID's are only for elements which will occur only once a page in any situation. You can't say that for sure for single navigation items.

Info

Orders

Models

Balance


Print a double as a decimal with a specified precision How could I make this code more effective?

static void     print_decimal(double decimal, int precision)
{
    size_t  integer;

    while (precision--)
    {
        decimal *= 100.;
        integer = (int)decimal;
        if (!precision  && (integer % 10) >= 5 && decimal != (double)integer)
            integer += 10;
        decimal *= .10;
        integer /= 10;
        printf("%i", integer);
        decimal -= integer;
    }
}

Example of calling that show me the decimals of a double with precision

print_decimal(65.226, 4)

Well, there's the obvious stuff, like whitespace and declaring variables at their first point of use. For example:

static void     print_decimal(double decimal, int precision)
{
    size_t  integer;

    while (precision--)
    {
        decimal *= 100.;
        integer = (int)decimal;

could usefully be rewritten as

static void print_decimal(double decimal, int precision)
{
    while (precision--) {
        decimal *= 100.;
        size_t integer = (int)decimal;

This spots a bug: you fail to handle negative inputs. If you want integer to hold a signed int, you should define it as int (not size_t); and vice versa, if you want it to hold an unsigned size_t, you should be casting (size_t)decimal (not (int)decimal).

    if (!precision  && (integer % 10) >= 5 && decimal != (double)integer)
        integer += 10;

This line could use some comments. I imagine it has something to do with rounding?

    printf("%i", integer);

It's more idiomatic to use printf("%d", integer) to print a decimal integer in C (and C++). True, %i exists, but it's deservedly obscure.

(Also, because of the above bug where you made integer a size_t, down here you should be using %zu to print it. But once you make it an int, you can go back to using %d.)

Every compiler will warn you about the format-string mismatch between %i and size_t. Are you compiling with -Wall? (No.) Why not? (There's no reason not to.) So, start compiling with -Wall today! It'll find your bugs so that you don't have to.

Performance-wise... I conclude from context that you expect this line to only ever print one digit, is that right? That is, your code could benefit from some "design by contract":

    assert(0 <= integer && integer <= 9);
    printf("%i", integer);

If the assertion is correct, then we can speed up the code by writing simply

    assert(0 <= integer && integer <= 9);
    putchar('0' + integer);

However, by adding the assertion, we have made the code a little bit testable. We can write some simple tests — just loop over all the floating-point numbers in the world and see if any of them fail the assertion — and indeed it doesn't take long to find one that fails.

print_decimal(0.097, 2);

This prints

010

when it should actually have printed

10

according to my understanding of your intent.

Which reminds me: print_decimal is a weird name for this function, since it literally does not have the ability to print a decimal point! Even your example print_decimal(65.226, 4) actually prints 652260, not 65.2260. This seems problematic for your prospective users.

So in conclusion:

Make your code testable.
Test your code.
Compile your code with -Wall.


Simple Fibonacci using recursion The sole purpose of this post is to see how a production quality developer would approach this very simple problem. I am a fresh graduate and do not have a professional development experience. What are the cases and approaches that I should start considering when developing in a production environment? Please feel free to provide any suggestion.

public class RecursiveFinonacci {

    public static void main(String[] args) {
        // TODO Auto-generated method stub
        System.out.println(getFibonacci(10));
    }

    public static int getFibonacci(int n){
        if(n==0 || n ==1)
            return 1;
        else
            return getFibonacci(n-2) + getFibonacci(n-1);
    }
}

Since you are asking what are the cases to consider, I'll give you a very simple one. Following that, there is a further refinement to achieve a final solution as well as a few comments to consider.

Trading space for speed using memoization

Definition: https://en.wikipedia.org/wiki/Memoization

Basically, we want to keep a look-up table of Fibonacci numbers that we've already computed in order to avoid computing them again. We can also obtain newer Fibonacci numbers by simply using the pre-computed values that we've already evaluated.

package codereview;

import java.util.HashMap;

public final class FibonacciNumber {
    /**
     * A O(1) look-up table to store Fibonacci numbers
     */
    private static HashMap<Integer, Integer> computedFibonacciNumbers = new HashMap<>();

    /**
     * Calculates the n-th Fibonacci number by using memoization
     *
     * @param n
     *            n-th Fibonacci number to calculate
     * @return n-th Fibonacci number
     */
    public static int getFibonacci( final int n ) {
        // check for Fibonacci numbers that have already been computed
        if ( computedFibonacciNumbers.containsKey( n ) ) {
            return computedFibonacciNumbers.get( n ).intValue();
        }

        if ( n == 0 || n == 1 ) {
            return 1;
        } else {
            // calculate it
            Integer nFibonacciNumber = getFibonacci( n - 2 ) + getFibonacci( n - 1 );

            // insert it into our look-up table
            computedFibonacciNumbers.put( n, nFibonacciNumber );

            return nFibonacciNumber.intValue();
        }
    }
}

For example, this will greatly increase the speed of computing successive Fibonacci numbers.

Here is a timed test that should show you the difference:

package codereview;

import java.lang.Runnable;

public class Main {
    public static void main( String[] args ) {
        int loops = 40;

        Runnable memoizationCompute = () -> {
            int last = 0;
            for ( int i = 0; i < loops; ++i )
                last = FibonacciNumber.getFibonacci( i );
            System.out.println( "fibonacci = " + last );
        };

        Runnable alwaysCompute = () -> {
            int last = 0;
            for ( int i = 0; i < loops; ++i )
                last = getFibonacci( i );
            System.out.println( "fibonacci = " + last );
        };

        System.out.println( timeExecutionMilliseconds( memoizationCompute ) );
        System.out.println( timeExecutionMilliseconds( alwaysCompute ) );
    }

    public static long timeExecutionMilliseconds( Runnable r ) {
        long timeBegin = System.nanoTime();
        r.run();
        long timeEnd = System.nanoTime();
        return ( timeEnd - timeBegin ) / 1000000;
    }

    public static int getFibonacci( int n ) {
        if ( n == 0 || n == 1 )
            return 1;
        else
            return getFibonacci( n - 2 ) + getFibonacci( n - 1 );
    }
}

The problems of recursiveness

However, while this highlights an optimization to your current solution, there are better alternatives. You should usually consider implementing recursive algorithms as non-recursive because this will result in better speed and will also allow for higher values of n, since you will get a java.lang.StackOverflowError when you try to compute it recursively for modest values of n.

A final solution?

Knowing all this, we can combine both approaches in order to get the benefits of the iterative solution and the benefits of storing precomputed values. This is good if you will be constantly accessing Fibonacci numbers in a range.

Source of iterative algorithm: http://en.literateprograms.org/Fibonacci_numbers_(Java)#Iteration

package codereview;

import java.util.HashMap;

public final class FibonacciNumber {
    /**
     * A O(1) look-up table to store Fibonacci numbers
     */
    private static HashMap<Integer, Integer> computedFibonacciNumbers = new HashMap<>();

    /**
     * Calculates the n-th Fibonacci number by using memoization
     *
     * @param n
     *            n-th Fibonacci number to calculate
     * @return n-th Fibonacci number
     */
    public static int getFibonacci( final int n ) {
        // check for Fibonacci numbers that have already been computed
        if ( computedFibonacciNumbers.containsKey( n ) ) {
            return computedFibonacciNumbers.get( n ).intValue();
        }

        int prev1 = 0, prev2 = 1;
        for ( int i = 0, hi = n + 1; i < hi; i++ ) {
            int savePrev1 = prev1;
            prev1 = prev2;
            prev2 = savePrev1 + prev2;
        }
        computedFibonacciNumbers.put( n, prev1 );
        return prev1;
    }
}

Implementation remarks

While this is working code, there is still work to be done in order to make this more efficient. I'll leave the implementation of most comments in the following section to you.

Other comments

Can you get even better performance by using a simpler data structure? See this comment for a suggestion. It entails using n as the index of the array.
Is the int type really sufficient for computing Fibonacci numbers? We note that it starts to overflow for n >= 46 (See this comment in another answer). You might want to use BigInteger.
What happens with invalid values of n (negative values)? Do you assume users are responsible enough to use the method correctly (I would, but this is subjective).
Mark constant variables as final. n is never modified it should thus be marked as final to indicate so to users. It is clearer and less error-prone in the sense that you won't modify it by mistake.
You will most definitely want to provide Javadoc in production code. You should also do it for your own code, as it can clearly define what the operation does without having you read through the code. You can look at my Javadoc for a very basic documentation of the method.
Is there a more efficient way of doing the calculation? See this answer for a way and this comment for possibilities.


2-opt algorithm for the Traveling Salesman and/or SRO In this question I present a method to solve the Traveling Salesman Problem and/or the Single Route Optimization problem.

I am extracting 100 lat/long points from Google Maps and placing these into a text file. The program should be able to read in the text file, calculate the haversine distance between each point, and store in an adjacency matrix. The adjacency matrix will eventually be fed to a 2-opt algorithm.

Extracting an adjaceny matrix containing haversine distance from points on map has already been dealt with. This question tackles the 2-opt algorithm.

The 2-opt function is called from main as follows. route is a randomly generated list of 100 numbers, which is the path the 2-opt should follow.

def main():
    best = two_opt(connect_mat, route)  #connectivity/adjacency matrix

And here is the 2-opt function and a cost function that it utilizes. Can they be optimized in any way?

def cost(cost_mat, route):
    return cost_mat[np.roll(route, 1), route].sum()  # shifts route array by 1 in order to look at pairs of cities

def two_opt(cost_mat, route):
    best = route
    improved = True
    while improved:
        improved = False
        for i in range(1, len(route) - 2):
            for j in range(i + 1, len(route)):
                if j - i == 1: continue  # changes nothing, skip then
                new_route = route[:]  # Creates a copy of route
                new_route[i:j] = route[j - 1:i - 1:-1]  # this is the 2-optSwap since j >= i we use -1
                if cost(cost_mat, new_route) < cost(cost_mat, best):
                    best = new_route
                    improved = True
                    route = best
    return best

Sample Input:

35.905333, 14.471970
35.896389, 14.477780
35.901281, 14.518173
35.860491, 14.572245
35.807607, 14.535320
35.832267, 14.455894
35.882414, 14.373217
35.983794, 14.336096
35.974463, 14.351006
35.930951, 14.401137
.
.
.

Sample Output

I would like to be able to scale up the points being read to 5000. With the code above it would be painfully slow.

Timer Test:

Starting a timer at the beginning of the function and ending before the return statement gives an average of 1.5s per 100 points. If simple proportion can be used to test algorithm scaling performance, then:

100 points:  1.5s
1000 points: 15s
5000 points: 75s

Please correct me if my above assumption is wrong.

I was wondering whether it could be improved in any way. More information can be added if requested.

EDIT:

I noticed that I was using an extra variable best. This can be removed as such:

def two_opt(connect_mat, route):
    improved = True
    while improved:
        improved = False
        for i in range(1, len(route) - 2):
            for j in range(i + 1, len(route)):
                if j - i == 1:
                    continue  # changes nothing, skip then
                new_route = route[:]    # Creates a copy of route
                new_route[i:j] = route[j - 1:i - 1:-1]  # this is the 2-optSwap since j >= i we use -1
                if cost(connect_mat, new_route) < cost(connect_mat, route):
                    route = new_route    # change current route to best
                    improved = True

    return route

I doubt how much (if any) this increases efficiency, however it does sacrifice readability to some extent.

You repeatedly evaluate

cost(cost_mat, best)

There is an opportunity to memoize this, or at least store it in a temp var.

You should decide if you want "optimal" TSP, or "good enough" TSP
that is k-competitive with an optimal solution.

You suggest you're taking 15ms per city.
You didn't post profile / timing data, but
I have to believe that much of the time is taken up by roll + sum,
rather than by, say, creating route copies.
Could we pre-compute distances between points, and
then consider just next hops within some threshold distance?
Or order by distance, and consider only a fixed number of next hops,
adjusted upward each time a better route is found?

Could the cost() function be broken into "near" and "far" components,
with the advantage that "far" would be substantially constant?
Usually we do not find a better cost.
If we do find one, we could then fall back to "carefully"
computing the detailed "far" cost.


Observable timer sequence This question over on SO about how to call a function "later" using Rx led me to post the following ill-advised piece of code to solve the problem:

void Main() {
  getScheduleTimes()
    .Where(t => t >= DateTimeOffset.Now)
    .SelectMany(t => Observable.Timer(t).ToEnumerable())
    .ToObservable()
    .Subscribe(_ => {
      Console.WriteLine("Ding! It's now {0}", DateTimeOffset.Now);
    });
}

IEnumerable<DateTimeOffset> getScheduleTimes() {
  DateTimeOffset day = DateTimeOffset.Now.Date;
  while (true) {
    yield return day + new TimeSpan(09, 00, 00);
    yield return day + new TimeSpan(13, 00, 00);
    day = day.AddDays(1);
  }
}

This doesn't work. Or rather, it appears to work -- if you run this, you'll notice that the call to .Subscribe() never returns. I was lambasted (rightly or otherwise) for involving .ToEnumerable() and .ToObservable(), as using these can cause "subscription side effects, collapsed times and dead-locks", as well as the code being confusing overall. The accepted answer builds an Observable directly.

Have I learned my lesson? Hardly. I still like the idea of using an (infinite) IEnumerable for producing the schedule. My question to you is: is anything still wrong with this code (assuming we keep the same getScheduleTimes()):

getScheduleTimes()
  .Where(t => t >= DateTimeOffset.Now)
  .Select(t => Observable.Timer(t))
  .Concat()
  .Subscribe(_ => {
    Console.WriteLine("Ding! It's now {0}", DateTimeOffset.Now);
  });

I was originally fixated on .SelectMany(), but instead .Concat() seems to be what I'm looking for. The code works, in that the subscribe no longer blocks and, as far as I can tell, no unacceptable overhead or confusion is produced, but my experience with Rx is limited. Are there any problems with this I'm not seeing, perhaps when you take it out of the toy box (cancellation, multiple subscriptions, thread overhead, etc.)?

The accepted answer also mentions using .Generate():

Observable.Generate(
  getScheduleTimes().GetEnumerator(),
  e => e.MoveNext(),
  e => e,
  e => e.Current,
  e => e.Current);

Would you say this is better than .Select(...).Concat(), based on whatever people experienced in Rx use to judge such things?

Although you didn't explicitly ask this (and I should now, as I am you) it's worthwhile to first look at why the first piece of code doesn't work, and why the approach is ill-advised.

Observable.ToEnumerable() has a fundamental problem in that it doesn't (cannot) know when the consumer of the resulting IEnumerable is going to enumerate. As a result, it is forced to buffer all the values that the observable is producing, until the consumer is kind enough to fetch them. Essentially, this produces a hidden producer/consumer model involving two threads. If the consumer is "sufficiently fast", the overhead of this may not be noticeable, but it's always there. Whenever you find yourself using Observable.ToEnumerable(), you're probably doing something wrong.

The second approach, where we build an enumerable sequence of observables and then .Concat() works (and I can't see any potential problems with it in terms of multiple subscriptions or cancellation), but has more overhead than necessary in that it produces a separate Observable for every single point in time, with each of these producing a single value. This wouldn't be necessary if we had a single observable to produce the values in order, and that's essentially what the third piece of code achieves.

The third piece of code is a tad cryptic if you don't see what overload of .Generate() it's calling. We can make this clearer by adding the parameter names:

Observable.Generate(
  initialState: getScheduleTimes().GetEnumerator(),
  condition: e => e.MoveNext(),
  iterate: e => e,
  resultSelector: e => e.Current,
  timeSelector: e => e.Current
);

The timeSelector parameter is documented rather sparsely as "the time selector function to control the speed of values being produced each iteration". Paraphrasing the actual implementation and assuming an implicit scheduler, .Generate() produces an observable that does this:

var dueTime = default(DateTimeOffset);
bool first = true;
var state = initialState();
if (hasResult) {
    observer.OnNext(result);
}
if (first) {
    first = false;
} else {
    state = iterate(state);
}
hasResult = condition(state);
if (hasResult) {
    result = resultSelector(state);
    dueTime = timeSelector(state);
    scheduler.Schedule(dueTime, Generate(...));
} else {
    observer.OnCompleted();
}

In the actual implementation, scheduling the tail-recursive call is more involved than this. If we substitute in our actual arguments:

var enumerator = getScheduleTimes().GetEnumerator();
if (hasResult) {
    observer.OnNext(result);
}
if (first) {
    first = false;
} else {
    enumerator = enumerator;
}
hasResult = enumerator.MoveNext();
if (hasResult) {
    result = enumerator.Current;
    dueTime = enumerator.Current;
    scheduler.Schedule(dueTime, generate(...));
} else {
    observer.OnCompleted();
}

So this is almost like a regular enumeration, but with the twist that each value we produce is also used as the time at which it should be produced.

There's a slight niggle in that, unlike the foreach pattern, this application of .Generate() will not call IEnumerator.Dispose() like an enumeration is supposed to. In this case, that's not a problem since we're using an iterator and those always have do-nothing Dispose() implementations, but if you were getting the times from another source, you would need to dispose them. That's exactly what Observable.Using() is for. If we put this together in a little extension method that works like Observable.Timer(), but for more than one point in time:

public static class ObservableExtensions {
  public static IObservable<DateTimeOffset> Timers(
    IEnumerable<DateTimeOffset> sequence
  ) {
    return Timers(sequence, Scheduler.Default);
  }

  public static IObservable<DateTimeOffset> Timers(
    IEnumerable<DateTimeOffset> sequence,
    IScheduler scheduler
  ) {
    return Observable.Using(
      () => sequence.GetEnumerator(),
      e => Observable.Generate(
        initialState: e,
        condition: ee => ee.MoveNext(),
        iterate: ee => ee,
        resultSelector: ee => ee.Current,
        timeSelector: ee => ee.Current,
        scheduler: scheduler
      )
    );
  }
}

And this would be my preferred way of doing it: using an IEnumerable allows us to test the result of code that produces a schedule in a simple way, while providing an IScheduler overload still allows us to test observable sequences as a whole using the TestScheduler if we are so inclined.


Is there a better way to write this jQuery UI effect animation? I'm writing some code to perform some simple form validation and I would like to display error messages inside the actual relevant textarea by means of a text fadeout/fadein effect.

To achieve this I am using jQquery UI to do the following:

Step 1: Animate the color to match the background colour
Step 2: Modify the text to show the error message then fade it in
Step 3: Fade the error message out, change the text back to its
previous    content
Step 4: Fade the text back in
Step 5: Finally, Focus the    textarea    and put the cursor at the
end

This works fine, however to do this I have had to nest a lot of anonymous function which is something I really don't like to do.

Is there a better way of achieving the same effect?

if (replyto == body) {
    $textarea.prop("disabled", true).animate({ color: 'rgb(41, 41, 41)'},500, function(){
        $textarea.text('Please enter your reply here')
            .animate({ color: 'white'},500, function(){
                $textarea.delay('1000')
                    .animate({ color: 'rgb(41, 41, 41)'},500, function(){
                        $textarea.text(body)
                            .animate({ color: '#a7a7a7'},500)
                            .prop("disabled", false)
                            .focus()
                            .putCursorAtEnd();
                    });
            });
    });
    return false;
}

You could create a function that will set up the animation of text or color and then just call it a few times to set up your animation:

$.fn.animateIt = function(color, text) {
    var that = $(this);
    if (text) {
        that.queue(function(next) {
            that.text(text);
            next();
        });
    }
    if (color) {
        that.animate({
            color: color
        }, 500);
    }
    return that;
};

$textarea.animateIt('rgb(41, 41, 41)')
    .animateIt('#FFF', 'Please enter your reply here')
    .delay('1000')
    .animateIt('rgb(41, 41, 41)')
    .animateIt('#a7a7a7', body)
    .queue(function(next) {
        $textarea.focus().putCursorAtEnd();
        next();
    });

Gets rid of the nested functions.

http://jsfiddle.net/petersendidit/LYsQM/1/


Fetching the definition of a word using a REST API This is the code I'm using to GET the definition of a word from Wordnik's REST API. I don't want to ignore any error but this is causing the code to be ridiculously long. I'm new to Swift so I'd appreciate it if someone could explain how best to trim it down.

class func define(params: DefinitionParameters!, delegate: WordnikDictionaryDelegate!){

        if let url = url(definitionParams: params){

            NSLog("URL: \(url)")

            let session = NSURLSession.sharedSession()
            let task = session.dataTaskWithURL(url, completionHandler: { (data, response, error) -> Void in

                var error : NSError?
                if let json = NSJSONSerialization.JSONObjectWithData(data, options: nil, error: &error) as? NSMutableArray{

                    NSLog("Response: \(json)")

                    if json.count == 0 {

                        delegate.dictionaryDidLoad(
                            nil,
                            forWord: params.word,
                            error: self.buildError(404, message: "No definitions found")
                        )

                    }else if let definitionJSON = json[0] as? NSMutableDictionary{

                        delegate.dictionaryDidLoad(
                            definitionJSON.valueForKeyPath("text") as? String,
                            forWord: params.word,
                            error: nil
                        )

                    }else{

                        delegate.dictionaryDidLoad(
                            nil,
                            forWord: params.word,
                            error: self.buildError(401, message: "Unexpected Result Format")
                        )

                    }

                }else{

                    delegate.dictionaryDidLoad(
                        nil,
                        forWord: params.word,
                        error: self.buildError(401, message: "Unexpected results format")
                    )

                }

            })

            task.resume()

        }else{

            delegate.dictionaryDidLoad(
                nil,
                forWord: params.word,
                error: self.buildError(400, message: "Invalid URL")
            )

        }

    }

Inside the if let json = ... { ... }, all the execution branches will call delegate.dictionaryDidLoad,
and the value of forWord is always params.word.
Instead, you could initialize value and error to nil,
set them in the appropriate conditional branches,
and finally call the delegate.
Something like this:

if let json = NSJSONSerialization.JSONObjectWithData(data, options: nil, error: &error) as? NSMutableArray{

    NSLog("Response: \(json)")

    if json.count == 0 {
        error = self.buildError(404, message: "No definitions found")
    } else if let definitionJSON = json[0] as? NSMutableDictionary{
        value = definitionJSON.valueForKeyPath("text") as? String,
    } else {
        error = self.buildError(401, message: "Unexpected Result Format")
    }
} else {
    error = self.buildError(401, message: "Unexpected Result Format")
}

delegate.dictionaryDidLoad(
    value,
    forWord: params.word,
    error: error
)


Optimizing this Excel automation The createReport method takes around 30 seconds to execute and I was wondering how I could optimize it. I'm using the Interop.Excel class and the workbook I'm importing to is very formula intensive.

Dim UserDate As New Data_Entry_Form.UserDate
Dim xlApp As Excel.Application
Dim xlWorkBook As Excel.Workbook
Dim xlWorkSheet As Excel.Worksheet
Dim xlSourceRange, xlDestRange As Excel.Range
Dim path As String = Application.StartupPath
Dim dt As New DataTable
Public Sub New()

End Sub
Sub createReport(dt As DataTable) ' this procedure determines the flow of the excel manipulation'
    openfile("2012 Master.xlsx")
    debugging(False)
    putfilesIntoYTD(dt)
    save("test.xlsx")
    closexl()
    cleanup()
    MsgBox("fin")
End Sub
Sub closexl()
    xlWorkBook.Close()
    xlApp.Quit()
End Sub
Sub cleanup() ' Releases all of the com objects to object not have excel running as a process after this method finishes'
    GC.Collect()
    GC.WaitForPendingFinalizers()
    System.Runtime.InteropServices.Marshal.ReleaseComObject(xlSourceRange)
    System.Runtime.InteropServices.Marshal.ReleaseComObject(xlWorkSheet)
    System.Runtime.InteropServices.Marshal.ReleaseComObject(xlWorkBook)
    System.Runtime.InteropServices.Marshal.ReleaseComObject(xlApp)
    xlApp = Nothing
End Sub
Sub debugging(mode As Boolean) 'Determines if we want to display anything during the dubugging process'
    If mode = True Then
        xlApp.DisplayAlerts = True
        xlApp.Visible = True
    Else
        xlApp.DisplayAlerts = False
        xlApp.Visible = False

    End If
End Sub
Sub putfilesIntoYTD(dt As DataTable)
    'dt = CType(dbTools.getYTD(startDate, endDate), DataTable)'
    Dim fileNum() As String = convertColumntoArray(dt, 0)
    Dim dateRecorded() As String = convertColumntoArray(dt, 1)
    Dim closerAndTypeCode() As String = convertColumntoArray(dt, 2)
    xlSourceRange = xlWorkSheet.Range("N2:N" & fileNum.Length)
    xlSourceRange.Value = fileNum
    xlSourceRange = xlWorkSheet.Range("O2:O" & dateRecorded.Length)
    xlSourceRange.Value = dateRecorded
    xlSourceRange = xlWorkSheet.Range("P2:P" & closerAndTypeCode.Length)
    xlSourceRange.Value = closerAndTypeCode

End Sub

Function convertColumntoArray(dt As DataTable, columnIndex As Integer) As String() ' Converts a single column of an datatable into an array of strings'
    Dim array(dt.Rows.Count) As String
    If columnIndex = 1 Then
        For i As Integer = 0 To dt.Rows.Count - 1
            array(i) = CDate(dt.Rows(i)(columnIndex).ToString()).Date
        Next
    Else
        For i As Integer = 0 To dt.Rows.Count - 1
            array(i) = dt.Rows(i)(columnIndex).ToString()
        Next
    End If
    Return array
End Function

Sub save(filename As String) ' Saves the sheet under a specfic name'
    xlWorkSheet.SaveAs(path & "/" & filename)
End Sub

Sub openfile(filename As String) ' opens the excel file under a specfic type'
    xlApp = New Excel.Application()
    xlWorkBook = xlApp.Workbooks.Open(path & "/" & filename)
    xlWorkSheet = CType(xlWorkBook.Sheets("1"), Excel.Worksheet)
    'xlWorkSheet.Activate()
End Sub

There is definetely a way to improve that even though the excel sheet has a lot of formulas.
In VBA there is an:

Application.Calculation = xlManualCalculation

That can be used to help dramatically improve the time it takes, when the problem is many formulas in a sheet. And then in the end you should turn it back to automatic.

As you are using VB with a connection to the Excel Workbook through a COM, the way to do it is very similar. It is:

ExcelApp.Calculation = XlCalculation.xlCalculationManual

Where ExcelApp is the excel application object.