Optimizing program to output all numbers summing to 100 I just finished my program which calculates the solutions to this problem:

  Outputs all possibilities to put + or - or nothing between the numbers 1,2,…,9 (in this order) such that the result is 100.

Code in question:

package defaultpackage;

import javax.script.ScriptEngine;
import javax.script.ScriptEngineManager;
import javax.script.ScriptException;

class Main {
    static int[] pos_counter = {0,0,0,0,0,0,0,0};

    static Boolean running = true;

    public static void main(String[] args) {
        int counter = 0;

        while(running) {
            increment();
            oobcheck();
            try {
                counter = (int) calculate(printeq());
            } catch (ScriptException e2) {
                e2.printStackTrace();
            }
            if(counter==100) {
                System.out.print(printeq());
                System.out.print("=");
                System.out.println(100);
            }
        }
    }

    public static int[] increment(){
        int stepcounter = 7;
        pos_counter[7]++;
        while(stepcounter>= 0&&running){

            if(pos_counter[stepcounter]==3&&running){
                if(stepcounter-1 == -1){
                    running = false;
                } else{
                    pos_counter[stepcounter-1]++;
                    pos_counter[stepcounter]=0;
                }
            }
            stepcounter--;
        }
        return pos_counter;
    }

    public static double calculate(String s) throws ScriptException{
        ScriptEngineManager mgr = new ScriptEngineManager();
        ScriptEngine engine = mgr.getEngineByName("JavaScript");
        return (double) engine.eval(s);
    }

    public static void readout() {
        int stepcounter = 0;
        while(stepcounter <= 7) {
            System.out.print(pos_counter[stepcounter]);
            stepcounter++;
        }
        System.out.println("");
    }

    public static String printeq() {
        int stepcounter = 0;
        String read = "";
        while(stepcounter <= 7) {
            read = "" + read + (stepcounter+1);
            if(pos_counter[stepcounter] == 0){
            }else if(pos_counter[stepcounter] == 1) {
                read = read + "+";
            }else if(pos_counter[stepcounter] == 2) {
                read = read + "-";
            }
            stepcounter++;
        }
        read = read + "9";
        return read;
    }

    public static void oobcheck(){
        int stepcounter = 0;
        Boolean checker = true;
        while(checker&&stepcounter <= 7) {
            if(pos_counter[stepcounter] == 2) {
                stepcounter++;
                checker = true;
            }else{
                checker = false;
            }
            if(stepcounter==7){
                if(pos_counter[7] == 2) {
                    running = false;
                }
            }
        }
    }
}

My program works fine and spits out all the answers correctly but I'm looking to improve it. I've seen solutions to similar problems and they mostly take up less space or seem more organized.

Concrete questions:

Where can I learn to improve code efficiency or organization?
What could be used specifically to make my code faster and more efficient?

General remark

Short code (as in less space in code) is not necessarily easier to read, nor does it need to be faster. You can try to keep removing unnecessary stuff until it is as short as possible, but please keep it readable. Writing code is hard, reading code is twice as hard.

Short answer

Online, everywhere, in books. "Effective Java" for example. Read about algorithms and data structures. Look at how others solved problems. Read many code reviews.
Use a profiler and measure where your code is slow.

Longer answer

You need to expose yourself to a lot of code, and start with simple problems. See how you would solve it, then look how others solved it and try to improve upon it.
For this case: try to represent your operators and operands in such a way that you can evaluate them in Java.  Generally: use the correct data structures and algorithms. That is most important. Then, try to use the right types. String concatenation is generally a lot slower in execution time than integer math.

Example

For example to evaluate and generate the permutations (using recursion just because we can :):

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

public class MakeHundred {

    private static int LENGTH = 8;

    // Encode the operators in a enum. We use the factor to implement
    // easier operation later on, and the display String as well.

    enum Operator {

        PLUS(1, "+"), MINUS(-1, "-"), NONE(0, "");

        private int factor;
        private String display;

        public String getDisplay() {
            return display;
        }

        Operator(int factor, String display) {
            this.factor = factor;
            this.display = display;
        }

        public int getFactor() {
            return factor;
        }

    }

    /**
     * Class for keeping a permutation, with is an list of operators which will be
     * used to create a sum, eg. 1 op1 2 op2 3.
     *
     * @author RobAu
     *
     */
    public static class Permutation {

        Operator[] operators;

        // Just one operator
        public Permutation(Operator operator) {
            this.operators = new Operator[] { operator };
        }

        // Copy the given permutation, append the given operator
        public Permutation(Permutation sourcePermutation, Operator operatorToAppend) {
            this.operators = Arrays.copyOf(sourcePermutation.operators, sourcePermutation.operators.length + 1);
            this.operators[sourcePermutation.operators.length] = operatorToAppend;
        }

        @Override
        public String toString() {
            StringBuilder sb = new StringBuilder();
            for (int i = 0; i < operators.length; i++) {
                sb.append(i + 1);
                sb.append(operators[i].getDisplay());
            }
            // append the last digit
            sb.append(operators.length + 1);
            return sb.toString();
        }

        public int evaluate() {
            int index = 0;
            int currentNumber = 0;
            int total = 0;

            // We start with a previous operator (+),
            // So the code becomes simpler.
            // 1+2 = 0 + 1 + 2.
            Operator operator = Operator.PLUS;

            while (index < LENGTH) {

                currentNumber += (index + 1);

                // If there is no operator, we are making a bigger number.
                if (operators[index] == Operator.NONE) {
                    currentNumber *= 10;
                } else {

                    // We increate the total by adding the current Number
                    // and using the previously seen operator
                    total += currentNumber * operator.getFactor();
                    currentNumber = 0;
                    operator = operators[index];
                }
                index++;
            }

            // add the last number.
            currentNumber += index + 1;
            total += currentNumber * operator.getFactor();

            return total;
        }

        /**
         * Generate all permutation with length length
         */
        public static List<Permutation> generate(int length) {
            return generate(null, length);
        }

        private static List<Permutation> generate(List<Permutation> permutations, int length) {

            List<Permutation> perms = new ArrayList<>();
            if (length == 1) {
                for (Operator o : Operator.values()) {
                    perms.add(new Permutation(o));
                }
            } else {
                for (Permutation p : generate(permutations, length - 1)) {
                    for (Operator o : Operator.values()) {
                        perms.add(new Permutation(p, o));
                    }
                }
            }
            return perms;
        }
    }

    public static void main(String[] args) {
        for (Permutation p : Permutation.generate(null, LENGTH)) {
            if (p.evaluate() == 100) {
                System.out.println(p);
            }
        }
    }
}

Including JVM startup this runs in:

real    0m0,088s
user    0m0,088s
sys 0m0,012s


Function and method debugging decorator - Part 2 This is a follow-up to this question.

I've refactored my previous debugging decorator, and added a couple new features, and changed a few things. Here's a complete list of things that have changed:

There is only one decorator, Debug, and it now supports functions, and class methods.
Each debug message is prefixed with [debug] to help distinguish it from normal output.
The output now tells you what it's outputting, rather than just outputting unreadable data.
The decorator will now output local variables names, along with argument and keyword argument names as well.

I'm wondering the following:

Is there a way to get the values of local variables in the function, or is that just not possible?
Is there a shorter way to get the names of local variables than function.__code__.co_varnames?
Is it a good idea to create an empty string, and then add to, and re-assign it to build an output string?
Is this Python 3, and Python 2.7 compatible?
How's my documentation?
Is this code "pythonic"?

debug.py

from pprint import pformat
from inspect import getargspec

class Debug(object):
    """Decorator for debugging functions.

    This decorator is used to debug a function, or
    a class method. If this is applied to a normal
    function, it will print out the arguments of

    Keyword arguments:
    debug -- Whether or not you want to output debug info. Generally, a global DEBUG variable is passed in here.
    """
    def __init__(self, debug=True):
        self.debug = debug

    def __format_debug_string(self, function, *args, **kwargs):
        """Return a formatted debug string.

        This is a small private helper function that will
        return a string value with certain debug information.

        Keyword arguments:
        function -- The function to debug.
        *args    -- The normal arguments of the function.
        **kwargs -- The keyword arguments of the function.
        """
        debug_string = ""
        debug_string += "[debug] {}\n".format(pformat(function))
        debug_string += "[debug] Passed args: {}\n".format(pformat(args))
        debug_string += "[debug] Passed kwargs: {}\n".format(pformat(kwargs))
        debug_string += "[debug] Locals: {}".format(pformat(function.__code__.co_varnames))
        return debug_string

    def __call__(self, function):
        def wrapper(*args, **kwargs):
            if self.debug:
                if getargspec(function).args[0] != "self":
                    print(self.__format_debug_string(function, *args, **kwargs))
                else:
                    print(self.__format_debug_string(function, *args, **kwargs))
                    print("[debug] Parent attributes: {}".format(pformat(args[0].__dict__)))

            return function(*args, **kwargs)
        return wrapper

Here are a few small, albeit unreadable tests, but it's good enough to get the point across:

from debug import Debug

@Debug(debug=True)
def a(a, b):
    d = 10
    return a * b

print(a(10, 10))

class B(object):
    def __init__(self, a, b):
        self.a = a
        self.b = b

    @Debug(debug=True)
    def e(self, c):
        return self.a * self.b * c

c = B(10, 10)
print(c.e(10))

Here's the output of these tests:

[debug] <function a at 0x1bf9d38>
[debug] Passed args: (10, 10)
[debug] Passed kwargs: {}
[debug] Locals: ('a', 'b', 'd')
100
[debug] <function B.e at 0x1944ce8>
[debug] Passed args: (<B object at 0x1bfc838>, 10)
[debug] Passed kwargs: {}
[debug] Locals: ('self', 'c')
[debug] Parent attributes: {'a': 10, 'b': 10}
1000

You can improve the following:

            if getargspec(function).args[0] != "self":
                print(self.__format_debug_string(function, *args, **kwargs))
            else:
                print(self.__format_debug_string(function, *args, **kwargs))
                print("[debug] Parent attributes: {}".format(pformat(args[0].__dict__)))

If the code is executed no matter the statement, and it always goes first, move it above the condition: (and don't forget to reverse the condition)

            print(self.__format_debug_string(function, *args, **kwargs))
            if getargspec(function).args[0] == "self":
                print("[debug] Parent attributes: {}".format(pformat(args[0].__dict__)))

As for this:

    debug_string = ""
    debug_string += "[debug] {}\n".format(pformat(function))
    debug_string += "[debug] Passed args: {}\n".format(pformat(args))
    debug_string += "[debug] Passed kwargs: {}\n".format(pformat(kwargs))
    debug_string += "[debug] Locals: {}".format(pformat(function.__code__.co_varnames))
    return debug_string

You can remove the = "" entirely:

    debug_string =  "[debug] {}\n".format(pformat(function))
    debug_string += "[debug] Passed args: {}\n".format(pformat(args))
    debug_string += "[debug] Passed kwargs: {}\n".format(pformat(kwargs))
    return debug_string + "[debug] Locals: {}".format(pformat(function.__code__.co_varnames))

It may not look as visually stimulating, but, it's not as redundant.

  Is it a good idea to create an empty string, and then add to, and re-assign it to build an output string?

If you were directly printing these then it would be a bad idea, but in this case, not really. However, I suppose you could move them to an object, or an array and return the result of a join function.

You could even return it as an array, and print each [debug] result. Which would remove the need for the \ns at the end, and DRY up the [debug] at the beginning of the strings (put it in the loop, not altogether)

You've got a few too long lines, by PEP8 standard:

debug -- Whether or not you want to output debug info. Generally, a global DEBUG variable is passed in here.
debug_string += "[debug] Locals: {}".format(pformat(function.__code__.co_varnames))
print(self.__format_debug_string(function, *args, **kwargs))
print("[debug] Parent attributes: {}".format(pformat(args[0].__dict__)))

As for your documentation:

function, it will print out the arguments of

Keyword arguments:

I'm a bit confused by that, grammatically.

__call__ is a more complex function (in my mind, at least) than __format_debug_string, but it has no documentation.

  Is this Python 3, and Python 2.7 compatible?

It ran fine when I tested it in Python 2.7.9 and 3.1.1


Hello Functional World: Counting occurrences of each letter Coming from a OO (C#) background, I am trying to learn some FP. To help me transition, I am trying to learn F#. I am taking baby steps.

I set myself a simple challenge to count the instances of each letter in a sentence, ordered alphabetically. ("Hello World" -> "[D, 1]; [E, 1]; [H, 1]; [L, 3]; [O, 2]; [R, 1]; [W, 1]; ").

My solution works but I can't help but thinking that I am not able to break out of an imperative style (my solution is based on Linq type thinking).

open System

let getLetterCount xs =
    xs |>
    Seq.filter Char.IsLetter |>
    Seq.map Char.ToUpper |>
    Seq.groupBy id |>
    Seq.sortBy fst |>
    Seq.map (fun (k,v) -> (k, Seq.length v))

let tuplesToString = Seq.fold (fun a (k,v) -> a + sprintf "[%c, %i]; " k v) ""

printfn "%s" (tuplesToString (getLetterCount "Hello World"))

Does anyone know of a good source of challenges to help me get my teeth into learning this?

As @Caridorc says, this is functional code, not imperative.  It's basically pretty good.

It would be a good idea to include type annotations — for example, (xs:string).  If you wanted to (and I'm not saying it would be better to do so), you could drop xs as an explicit parameter and change the pipeline into a composition of functions.  Also, you could sort before grouping for a slight simplification.

I find the fold in tuplesToString a bit hard to follow.  In my opinion, using String.concat would be clearer, and it would also eliminate the extra space at the end of the line.

I'd rename both functions to have "counts" in their name.  You can see the coherence in getLetterCounts |> countsToString.

open System

let getLetterCounts:(string -> seq<char * int>) =
    Seq.filter Char.IsLetter >>
    Seq.map Char.ToUpper >>
    Seq.sort >>
    Seq.groupBy id >>
    Seq.map (fun (k, v) -> (k, Seq.length v))

let countsToString (counts:seq<char * int>) =
    [for k, v in counts -> sprintf "[%c, %i];" k v] |> String.concat " "

"Hello World" |> getLetterCounts |> countsToString |> printfn "%s"


Optimizing string match algorithm I was trying to solve this problem.  I thought of any possible relation between the number of matches among subsequent substrings, but could not find one. So, the probably naive solution that I drafted was:

#include <iostream>
#include <cstring>

using namespace std;

int n;
char str[1000000];

int sim(int i, int len)
{
    int j=0;
    for(; i<len; i++, j++)
    {
        if(str[i]!=str[j])
            return j;
    }
    return j;
}

void solve()
{
    int len=strlen(str);
    int count=len;

    for(int i=1; i<len; i++)
    {
        count+=sim(i, len);
    }
    cout<<count<<endl;
}

int main()
{
    cin>>n;
    for(int i=0; i<n; i++)
    {
        cin>>str;
        solve();
    }

    return 0;
}

Now submission to online judge at interviewstreet says that I did pass 7/10 testcases but the time limit exceeded for the 8th testcase! It means that my solution isn't even that naive, but it also indicates a need for some optimization. Can you people please suggest any way to optimize it?

I think what you need is a suffix tree: http://en.wikipedia.org/wiki/Suffix_tree. I don't really remember much about it, except that it exists and seems relevant to the problem at hand.

Style wise, I recommend spaces around operators like < and << and +=


Find all possible sumation numbers to a specified sum This is a problem that I had in mind for a loong time and today I decided to give it a go, the basic idea is you enter a number/sum and the program outputs all the possible ways this sum can be formed let's say we have 3 as input the output will be (1, 1, 1),(1, 2) I don't think that the code is really efficient so any tips are appreciated.

    static void Main(string[] args)
    {
        int sum = int.Parse(Console.ReadLine());
        List<List<int>> subsets = GetSubsets(sum);
        for (int i = 0; i < subsets.Count; i++)
        {
            Console.WriteLine(string.Join(",", subsets[i]));
        }
        Console.ReadKey();
    }

    private static List<List<int>> GetSubsets(int sum)
    {
        List<List<int>> subsets = new List<List<int>>();
        int[] allNumbers = new int[sum - 1];
        List<int> baseSubset = PopulateWith(1, allNumbers.Length + 1);
        int baseNumberIndex = 0;
        int additiveNumberIndex = 1;
        for (int i = 0; i < allNumbers.Length; i++)
        {
            allNumbers[i] = i + 1;
        }
        if (sum > 1)
        {
            // if 1 is entered as a sum we dont want to return 1 as result because that's not a sum of numbers it's just 1.
            subsets.Add(baseSubset);
        }
        while (baseNumberIndex < allNumbers.Length && additiveNumberIndex < allNumbers.Length)
        {
            List<int> currentSubset = PopulateWith(allNumbers[baseNumberIndex], (int)Math.Round((double)sum / allNumbers[baseNumberIndex]));
            int currentSum = currentSubset.Sum();
            while (true)
            {
                if (currentSum + allNumbers[additiveNumberIndex] > sum)
                {
                    currentSubset.Remove(allNumbers[baseNumberIndex]);
                }
                else
                {
                    currentSubset.Add(allNumbers[additiveNumberIndex]);
                }
                currentSum = currentSubset.Sum();
                if (currentSum == sum && !subsets.Any(seq => seq.SequenceEqual(currentSubset)))
                {
                    subsets.Add(currentSubset.ToList());
                }
                if (currentSum + allNumbers[additiveNumberIndex] > sum && !currentSubset.Contains(allNumbers[baseNumberIndex]))
                {
                    break;
                }
            }
            additiveNumberIndex++;
            if (additiveNumberIndex == allNumbers.Length)
            {
                baseNumberIndex++;
                additiveNumberIndex = baseNumberIndex + 1;
            }
        }
        return subsets;
    }

    private static List<int> PopulateWith(int number, int size)
    {
        List<int> collection = new List<int>();
        for (int i = 0; i < size; i++)
        {
            collection.Add(number);
        }
        return collection;
    }

Rather than go through a line-by-line critique of your program, let me just say:

It is way too long and too complicated
You are getting hung up on all the adds and removes.  This problem is much easier to solve if you never add or remove anything. Treat sequences as immutable data structures.

You should be able to do this in about four lines of code in your method body, and some little helper methods.  Here's an example:

using System;
using System.Collections.Generic;
using System.Linq;
public class Program
{
    static IEnumerable<T> Singleton<T>(T t)
    {
        yield return t;
    }
    static IEnumerable<IEnumerable<int>> AllSums(int n, int min = 1)
    {
        for (int i = min; i <= n / 2; ++i)
            foreach(var seq in AllSums(n - i, i))
                yield return Singleton(i).Concat(seq);
        yield return Singleton(n);
    }
    public static void Main()
    {
        foreach(var result in AllSums(7))
            Console.WriteLine(string.Join(",", result));
    }
}

The key here is to make a clear specification for the AllSums method.  It takes positive integers n and min and returns a sequence of sequences that all have the following properties:

They sum to n
The smallest number in the sequence is not smaller than min
The sequence is non-decreasing

Once you have a method that has these properties, the recursion becomes much easier to reason about.

Go through this implementation very carefully and annotate each line of code with its meaning and purpose in the algorithm.

For another application of this general principle of generating a sequence of data structures that all have a sum property, see my series of articles which begins here:

https://blogs.msdn.microsoft.com/ericlippert/2010/04/19/every-binary-tree-there-is/


Inserting a DBNull value into a database I have code which works pretty well. I just need some of your opinions on how to write it better with fewer line of code. I want to use a ternary operator but I couldn't make it work so I did an if else instead.

Basically, I want to insert a price on my database. The price column is a nullable decimal. Even in my C# code, it's a nullable decimal as well.

if (items.Price == null)
{
    SqlParameter[] p =
    {
        new SqlParameter("@purchaseorder", items.PurchaseOrderItems),
        new SqlParameter("@modelnumber", items.ModelNumber),
        new SqlParameter("@quantity", items.Quantity),
        new SqlParameter("@Price",  DBNull.Value),
        new SqlParameter("@Description", items.Description),
        new SqlParameter("@OrderNumber", items.OrderNumber),
        new SqlParameter("@Unit", items.Unit)
    };

    i = Dal.ExecuteNonQuery(sql,p);
}
else
{
    SqlParameter[] p =
    {
        new SqlParameter("@purchaseorder", items.PurchaseOrderItems),
        new SqlParameter("@modelnumber", items.ModelNumber),
        new SqlParameter("@quantity", items.Quantity),
        new SqlParameter("@Price",  items.Price),
        new SqlParameter("@Description", items.Description),
        new SqlParameter("@OrderNumber", items.OrderNumber),
        new SqlParameter("@Unit", items.Unit)
    };

    i = Dal.ExecuteNonQuery(sql,p);
}
return i;

Can this be written without the else block?

The most terse and declarative way to write this is to use C#'s ?? operator. Your line becomes:

new SqlParameter("@Price",  (object)items.Price ?? DbNull.Value),

The expression here will either return the non-null value of the left operand (items.Price), or return the operand to the right instead.

If you hadn't guessed, the ?? operator is basically syntactic sugar for if operl  != null ? operl : operr.


Test if a string is a number In need for testing if a string is a number which could be a double or integer, I wrote a simple skeleton, which parses a string like this:

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

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

char* to_convert = argv[1];
char* p = NULL;
errno = 0;
long val = strtol(argv[1], &p, 0);

if (errno != 0)
return 1;// conversion failed (EINVAL, ERANGE)

if (to_convert == p){
// conversion to int failed (no characters consumed)
double val2 = strtod(p, &p);
if (*p){
printf("Not a number!\n");
return 1;
}
printf("Double %lf\n" , val2);
return 0;
}

if (*p != 0){
// conversion to int failed (trailing data)
double val2 = strtod(argv[1], &p);
if (*p){
printf("Not a number!\n");
return 1;
}
printf("Double %lf\n" , val2);
return 0;
}

printf("Int %ld\n" , val);
return 0;
}

/*
$ ./a.out 123
Int 123
$ ./a.out -123
Int -123
$ ./a.out 31.1
Double 31.100000
$ ./a.out -231
Int -231
$ ./a.out NaN
Double nan
$ ./a.out -Nan
Double nan
$ ./a.out -INF
Double -inf
$ ./a.out INF
Double inf
$ ./a.out 12.3
Double 12.300000
$ ./a.out -12,3  # Although some european localels (like German, would consider this a number)
Not a number!
$ ./a.out -12.3
Double -12.300000
$ ./a.out foo
Not a number!
*/

I would like to know if this could be done better, without iterating on all the characters like here.

Is there any case I didn't think of which breaks this code?
or causes undefined behaviour (except numbers larger than long on my architechture)?

Another thing is the problem with decimal seperator. How do you convert a number where the decimal separator is comma instead of a point like in the German language? This would be a nice to have, but not a must.

For example, my code says 12,3 is not a number, for a German person this would be a number (12.3 for English speakers).

int test and double test are incompletely done.

errno = 0;
long val = strtol(argv[1], &p, 0);
if (errno) Fail();         // Range error (or possible other reasons)
if (argv[1] == p) Fail();  // no conversion
while (isspace((unsigned char) *p)) p++;
if (*p) Fail();            // Extra junk afterwards

if (*p) Fail(); Needs to be applied to both integer and FP test.

while (isspace((unsigned char) *p)) p++; allows trailing white-space.  Both strtol() and strtod() allow leading white-space.

Double is more complex as in the case of underflow like "1e-10000", the errno may set and that is usually not considered an error in many uses.  The returned value will be either +/-0.0 or a small double value.

double val2 = strtod(argv[1], &p);
if (errno == ERANGE && fabs(val2) <= DBL_MIN) NoFail();

Keep in mind a string may nicely translate to a long as well as a double.  Should "-0" be along 0 or double -0.0?  Aside from this case, I would favor long, especially if the test for extra junk is used.

Your test cases do not include big values "123...thousand_more...456", values like LONG_MIN (-1, +0, +1) and  LONG_MAX (-1, +0, +1), DBL_MAX, next_after(DBL_MAX,0), strings with trailing garbage "123xyz", -0.0.  BTW: a harsh check it to use (char) -1 in various places in the string. Or lead the string with (char) (x80 + ' ')

Nor does it test odd things like "++123", "0x0xABC", "08".  (multiple sign, 2-prefix, octal 8) and new emerging "0b0101"  (C14?)

Extreme FP strings include "0.000...0001e320", "" and "  " (white-space only strings.)

FP also has a hexadecimal format, see scanf("%a")/printf("%a") for additional strings.

Original request was for "double or integer".  Code conveniently uses long, but what if int was meant or intmax_t?  Example additional test for int

long val = strtol(argv[1], &p, 0);
if (val < INT_MIN || val > INT_MAX) RangeError();
int i = (int) val;
...

To cope with decimal separator as , or ., use setlocale() and that will adjust strtol() and strtod().  But that is a weak aspect of C.

Lastly, a given locale could have "additional subject sequences" that convert into numbers.


Haskell-like function composition function Much like this question I'm attempting to write in C++ something which can resemble Haskell's (.) function as much as possible, which is basically Chapter 1's second exercise from Bartosz Milewski's Category Theory for Programmers.
This similar question about writing the identity function served as a reference too.
This answer on StackOverflow reassured me that decltype used on lambdas' auto parameters is equivalent to decltype used on template parameters.
And this is what I came up with:
static constexpr struct compose_t {
    template <typename F, typename G>
    constexpr decltype(auto) operator()(F&& f, G&& g) const noexcept {
        return [&f,&g](auto&& x){
            return f(g(std::forward<decltype(x)>(x)));
        };
    }
} compose;

Some thoughts:

operator() takes f and g by reference, and the returned lambda captures them by reference; and I made this choice thinking it should make passing non-stateless callables cheaper, but I'm not sure I made the right choice.  Another reference is this.
Since I'm passing them by reference, I had to choose between const& and &&, and I've basically chosen randomly.
I've not concerned myself with variadics, as I wanted a binary function composition "operator", in line with Haskell's (.) :: (b -> c) -> (a -> b) -> a -> c.
I cannot compose compose with something else, because it is binary, whereas the implementation implicitly assumes that F and G be unary. I guess making something like compose(compose,compose) (which is (.).(.) in Haskell) possible would be up to a partial application "facility".

This is a code where I tried to test it:
#include <cassert>
#include <utility>

// ... the code from above

static constexpr struct changesign_t {
    template<typename T>
    constexpr decltype(auto) operator()(T&& t) const noexcept { return -t; };
} changesign;

int square(int x) { return x*x; }
int twice(int x) { return 2*x; }

int main() {
    assert(compose(square,twice)(3) == 36);
    assert(compose([](int x){ return x*x; },[](int x){ return 2*x; })(3) == 36);
    assert(compose(changesign, compose([](auto x){ return x*x; }, [](auto x){ return 2*x; }))(3) == -36);
}

There's no need to use a struct compose_t to wrap operator(), you can just move operator() out of it and rename it compose():
template <typename F, typename G>
constexpr decltype(auto) compose(F&& f, G&& g) noexcept {
    return [&f,&g](auto&& x){
        return f(g(std::forward<decltype(x)>(x)));
    };
}

Now it is also much closer to the code written in this post you mentioned, except of course you are limited to composing only two unary functions.
Other than that, if you can live with the restrictions you have, I see nothing wrong with your implementation.


Python script for sorting a huge log file based on timestamps I want to write a python script to sort a huge file, say 2 GB in size, which contains logs in the following format -

Jan 1 02:32:40 other strings but may or may not unique in all those lines
Jan 1 02:32:40 other strings but may or may not unique in all those lines
Mar 31 23:31:55 other strings but may or may not unique in all those lines
Mar 31 23:31:55 other strings but may or may not unique in all those lines
Mar 31 23:31:55 other strings but may or may not unique in all those lines
Mar 31 23:31:56 other strings but may or may not unique in all those lines
Mar 31 23:31:56 other strings but may or may not unique in all those lines
Mar 31 23:31:56 other strings but may or may not unique in all those lines
Mar 31 23:31:57 other strings but may or may not unique in all those lines
Mar 31 23:31:57 other strings but may or may not unique in all those lines
Mar 31 23:31:57 other strings but may or may not unique in all those lines
Mar 31 23:31:57 other strings but may or may not unique in all those lines
Feb 1 03:52:26 other strings but may or may not unique in all those lines
Feb 1 03:52:26 other strings but may or may not unique in all those lines
Jan 1 02:46:40 other strings but may or may not unique in all those lines
Jan 1 02:44:40 other strings but may or may not unique in all those lines
Jan 1 02:40:40 other strings but may or may not unique in all those lines
Feb 10 03:52:26 other strings but may or may not unique in all those lines

I want to sort them basted on the timestamp.

I was able to get this working but for my code to succeed, I need to load the WHOLE file into a list.. this means it will be extremely inefficient from a memory utilization point of view.

Can you please suggest if there is a more efficient way where I can sort this by reading the file line by line or perhaps some other approach that I am not aware of ?

Here is my code -

# convert the log into a list of strings
with open("log.txt", 'r') as f:
    lines = f.read().splitlines()

# writing the method which will be fed as a key for sorting
def convert_time(logline):
    # extracting hour, minute and second from each log entry
    h, m, s = map(int, logline.split()[2].split(':'))
    time_in_seconds = h * 3600 + m * 60 + s
    return time_in_seconds

sorted_log_list = sorted(lines, key=convert_time)

''' sorted_log_list is a "list of lists". Each list within it is a representation of one log entry. We will use print and join to print it out as a readable log entry'''
for lines in sorted_log_list:
    print lines

You're ignoring the Date-part of the timestamps; it doesn't sound like that's on purpose.
(Also, the year is missing altogether, which should make us quite nervous.)
Also, let's use explicit datetime utilities and regexes.

import datetime
import re

timestamp_regex = re.compile("[^:]+:\d\d:\d\d")

def convert_time(logline):
    stamp = timestamp_regex.match(logline).group() #this will error if there's no match.
    d = datetime.strptime(stamp, "%b %e %H:%M:%S")
    return int(d.timestamp())

As for the rest, the comments are right that we can't do much unless we know exactly what it would mean for the solution to be improved.

If the concern is just to handle the biggest file with the least ram, something like this might work:

def save_where_we_can_find_it(line, temp_file):
    retval = temp_file.tell()
    temp_file.write(line)
    return retval

def fetch_line(location, temp_file):
    temp_file.seek(location)
    return temp_file.readline()

items = []

with open("log.txt", 'r') as original, open(".temp.log.txt", 'w') as temp:
    for line in original:
        items.append((convert_time(line), save_where_we_can_find_it(line, temp)))

items.sort(key = lambda pair: pair[0]) #sort-in-place isn't necessarily a good idea; whatever.

with open(".temp.log.txt", 'r') as temp:
    for (stamp, location) in items:
        print(fetch_line(location, temp))

import os
os.remove(".temp.log.txt")

But this is just a really inefficient way using a scratch-file. Better to register scratch-space in the OS, and then do your file manipulation "in memory".


Remove all vowels from a string except an initial character I've written the following code where all vowels are removed from a string except if the string starts with a vowel then this vowel is maintained.

def removeVowels(string):
    output = string[0]

    for char in string[1:]:
        if char.lower() not in 'aeuio':
            output += char
    return output

I was wondering if it's possible to use a list comprehension for this?

removeVowels is not quite an accurate name.  Furthermore, by PEP 8, the official Python style guide, function names should be lower_case_with_underscores unless you have a good reason to deviate.  Therefore, I recommend renaming the function to remove_non_initial_vowels.

Your function crashes on string[0] if the input is an empty string.

I don't recommend writing string[1:], since that would entail making a temporary copy of nearly the entire string.

'aeuio' is a bit weird.  Why not 'aeiou'?  (I assume that for this exercise, you don't care that y is sometimes a vowel, w is a semivowel, and assume that the string contains no diacritics.)

Fundamentally, this operation is a fancy string substitution.  Typically, such substitutions are best done using regular expressions.

import re

def remove_non_initial_vowels(string):
    return re.sub('(?<!^)[aeiou]', '', string, flags=re.I)

The (?<!^) part of the expression is a negative lookbehind assertion that means "not at the beginning of the string".


Calculating the amount of cubes needed to form a sum Since I've never done any performance programming (Aside from the better choices such as array vs list etc. The real basics.), I should probably read up on it. But I had to start somewhere, so, someone I know - which is a much better programmer than I am - tasked me with making this "assignment":

  You are given the total volume m of the building. Being given m can you find the number n of cubes you will have to build?

  The parameter of the function findNb (find_nb, find-nb) will be an integer m and you have to return the integer n such as \$n^3 + (n-1)^3 + \dots + 1^3 = m\$ if such a n exists or -1 if there is no such n.

He gave me this template with it:

using System;

public class Program
{
    public static void Main()
    {
        Console.WriteLine(findNb(4183059834009));
        Console.WriteLine(findNb(24723578342962));
        Console.WriteLine(findNb(135440716410000));
        Console.WriteLine(findNb(40539911473216));
    }

    public static long findNb(long m)
    {

    }
}

In which I inserted:

for(long n = 1; n < (m / 3); n++)
{
    double vol = 0;
    for(long i = 0; i < n; i++)
        vol += Math.Pow((n - i), 3);

    if(vol > m)
        return -1;

    if(vol == m)
        return n;
}
return -1;

This code works, but it takes incredibly long for the larger numbers, as is my main problem.

What can I do to shorten the time it takes for this code takes to complete?

When there's math to be done, always check if some formula exists

$$1^3+2^3 + \dots+n^3 = m = (1+2 + \dots+n)^2$$

in conjunction with some other formula (interesting wikipedia article title)

$$\sum^n_{k=1}k = \frac{n(n+1)}{2}$$

you get some equation for the result

$$\sqrt{m} = 1+2 + \dots+n = \sum^n_{k=1}k = \frac{n(n+1)}{2}$$
and maybe some closed formula

$$
\begin{align}
\sqrt{m} &= \frac{n(n+1)}{2}\\
2\sqrt{m} &= n(n+1)\\
2\sqrt{m} &= n^2+n\\
0&= n^2+n - 2\sqrt{m}
\end{align}
$$

Which has the 2 solutions

$$n_{1,2} = -\frac12 \pm\sqrt{\frac14+2\sqrt{m}}$$

As \$n\$ should be a positive number, only the first solution makes sense.

Return \$ -\frac12 +\sqrt{\frac14+2\sqrt{m}}\$ if it is an integer or \$-1\$ otherwise.

The results are:

$$
\begin{array}{r|r}
m & n\\
\hline
4183059834009 & 2022\\
24723578342962 & -1\\
135440716410000 & 4824\\
40539911473216 & 3568\\
\end{array}
$$

24723578342962 is a very close call, because

$$\sum^{3153}_{k=1}k^3=24723578342961$$

is only 1 off. The floating point result is \$n=3153.00000000003\$. If you don't want to get false positives due to floating point precision (lack thereof), you can do the check by converting the result to some integer type and see if you get the exact number by doing the calculations with that.


Chunk Generation Performance in C# I'm working on a game in C#, this game generate new chunk (infinite) when you move around. If I have > 1000 chunks in my list then it become laggy even though I only load the necessary chunks on my screen. Tried all things to improve the code but it did not improve.

Stopwatch ChunkDetect = new Stopwatch();
List<Chunks> allChunks = new List<Chunks>();
public readonly int sizeChunk = 800;

private void loadChunks()
{
    ChunkDetect.Restart();
    ChunkDetect.Start();
    int CamXCh = Convert.ToInt32(Math.Ceiling(-Convert.ToDouble(CameraX) / sizeChunk)),
        CamYCh = Convert.ToInt32(Math.Ceiling(-Convert.ToDouble(CameraY) / sizeChunk));

    int sizeX = Convert.ToInt32(Math.Ceiling(Convert.ToDouble(gameBox.Width) / sizeChunk)) + 4,
        sizeY = Convert.ToInt32(Math.Ceiling(Convert.ToDouble(gameBox.Height) / sizeChunk)) + 4;

    Random generation = new Random();
    // Load Chunks or Create
    for (int chunkX = 0; chunkX < sizeX; chunkX++)
    {
        for (int chunkY = 0; chunkY < sizeY; chunkY++)
        {
            int xChunk = chunkX + CamXCh - 2,
                yChunk = chunkY + CamYCh - 2;

            Chunks currentChunk = allChunks.Where(i => i.positionX == xChunk && i.positionY == yChunk).FirstOrDefault();

            bool isLoad = true;
            if (chunkX == 0 || chunkX == (sizeX - 1) || chunkY == 0 || chunkY == (sizeY - 1))
                isLoad = false;

            if (currentChunk == null)
            {
                Color ColorBiome = Color.FromArgb(64, 233, 56);
                string BiomeName = "Grass";
                allChunks.Add(new Chunks(xChunk, yChunk, sizeChunk, ColorBiome, BiomeName, isLoad, 40));
            }
            else
            {
                bool isCurrentLoaded = currentChunk.isLoaded;
                if (!isLoad)
                    currentChunk.isLoaded = false;
                else if (!isCurrentLoaded)
                    currentChunk.isLoaded = true;
            }
        }
    }
    ChunkDetect.Stop();
}

class Chunks
{
    public Color biomeType;
    public Guid uid;
    public bool isLoaded;
    public int positionX, positionY, Size;
    public string biomeName;
    public List<CeldsChunk> MapChunk;

    public Chunks(int x, int y, int size, Color biome, string BiomeName, bool loaded, int sizeCeld)
    {

        // Identificador
        this.uid = new Guid();

        // Posición y tamaño
        this.positionX = x;         // Position
        this.positionY = y;         // Position
        int cSize = this.Size = size;         // Size

        // Bioma
        this.biomeType = biome;      // Biome Color
        this.biomeName = BiomeName;  // Name of Biome

        // Propiedades
        this.isLoaded = loaded;      // If chunk is loaded

        int sizeChunkInCeld = (cSize / sizeCeld);

        List<CeldsChunk> celdsToChunk = new List<CeldsChunk>();

        Random rd = new Random();
        for (int CeldX = 0; CeldX < sizeChunkInCeld; CeldX++)
        {
            for (int CeldY = 0; CeldY < sizeChunkInCeld; CeldY++)
            {
                celdsToChunk.Add(new CeldsChunk(new Point(CeldX, CeldY, sizeCeld), this.biomeType));
            }
        }

        this.MapChunk = celdsToChunk;
    }

    public void changeLoaded(bool g)
    {
        this.isLoaded = g;
    }

}

Your allChunks is a List. The Linq query to get the chunk with the right coordinates is going to scan the entire list, and check each chunk. This is a linear process, and the performance depends on the number of chunks in the list. As the list grows, the performance decreases.

The solution is to index the chunks in a way that makes the lookup performance a constant-time operation (not dependant on the number of chunks).

The logical data structure for this is to use a Dictionary, and key the Dictionary on a value that is the coordinate. Thus, given a coordinate, you can easily find the Chunk.

Your chunks are indexed using int values for the X and Y coordinate. I would recommend using a long-value as the key, and to simply shift the X and Y coordinates in to the Dictionary... for example:

Dictionary<long, Chunks> indexChunks = ....

// merge both coordinates in to one unique key
// the y-coordinate is in the high-32-bits, the x-coord in the low 32 bits.
// this should probably be in a function.
long key = xChunk + ((long)yChunk << 32);

if (!indexChunks.containsKey(key)) {
    indexChunks.Add(key, new Chunks(.....));
}

Chunks currentChunk = indexChunks[key];

The Dictionary will convert the lookup process to a faster, scalable mechanism.

If you want to, you can create a more complicated key mechanism, but I would recommend that you keep the key as simple as possible.


React onDrop function for a task manager This a working function as part of a drag and drop task manager im playing around with. The function is working fine, but it just looks messy and long to me. I haven't added all the code as I'm looking just to tidy up the function itself. I have added the state though as guidance.

function

onDrop = () => {

  let people = [...this.state.people]
  let newPeopleArray = []
  let newChoresArray = []
  let personToUpdate = {}

  people.forEach( person => {
    if ( person.name !== this.state.targetPerson ) {
      newPeopleArray.push(person)
     } else {
      personToUpdate = person
      personToUpdate.chores.forEach( chore => {
          if ( chore !== this.state.targetChore ) {
          newChoresArray.push(chore)
        }})
      personToUpdate.chores = newChoresArray
    }})
 newPeopleArray.push(personToUpdate)
 this.setState ({ people: newPeopleArray})
}

state

class App extends Component {
  state = {

    targetChore: '',
    targetPerson: '',

    people: [
    { name: 'Grace', chores: ['clean kitchen', 'wash dog', 'laundry']  },
    { name: 'Sam', chores: ['walk dog', 'wash car', 'go shopping']  },
    { name: 'Rose', chores: ['wash windows', 'vaccum', 'clean bathroom'] }
    ]
}

A Person class encapsulates the attributes/methods and logic for the Person object. Currently inherits from Object, but you can at any time extend this logic by creating an new super class or children and distribute the logic between those hierarchies.

class Person extends Object {
    constructor(props) {
       super
       this.name = props.name
       this.chores = props.chores
    }

    addSubject() {
      if (this.isFound) { return new People().push(person) }
      // your choice to either create a chore class or write some subroutine
      else { person.chores.forEach( chore => chore.addChore ) }
    }

    // either pass targetPerson as param or add it to the Person object as attribute
    isFound() { this.name !== this.state.targetPerson }

}

people = [ new Person('Grace', 'clean kitchen', 'wash dog'..), etc..]

I also believe the onDrop subroutine should just have the responsibility of dropping person or returning a new people object. It would be an instance method of the People class

class People extends Object {
    drop = () => {
      this.each( person => person.addSubject() )
      // etc...
    }

}


Get _m_s time format from milliseconds value I could have any value in milliseconds like:

  1040370

I must alert a specific converted time format : _m_s (ie:17m20s)

Here is how I do it:

function ms(a){
var m = a/1000/60,
s = Math.floor(('0.'+(''+m).split('.')[1])*60);
m = (''+m).split('.')[0];
alert(m+'m'+s+'s');
}

ms(1040370);

What do you think?
Is it satisfactory or is there a better way to accomplish the same?

Seems overly complicated.  Let the builtin math and parsing functions do the work for you.  And name time values something like t.

const ms = t => alert( `${parseInt( t/1000 / 60 )}m${parseInt( t/1000 % 60 )}s` )
ms(1040370);


Mergesort vs Quicksort Timing in Python I have implemented Mergesort and Quicksort in Python and have noticed that Quicksort runs two times as fast as Mergesort. Is this a property of the algorithm or is Mergesort coded inefficiently? Are there any other suggestions?

(I am aware that the timing includes the creation of the random list.)

Quicksort:

import time, random

def quicksort_benchmark(n=2**24):
    def quicksort(array, low, high):
        if low < high:
            p = partition(array, low, high)
            quicksort(array, low, p)
            quicksort(array, p+1, high)

    def partition(array, low, high):
        pivot_index = random.randint(low, high)
        pivot = array[pivot_index]
        i = low - 1
        j = high + 1

        while True:
            while True: # do while
                i += 1
                if array[i] >= pivot:
                    break

            while True:
                j -= 1
                if array[j] <= pivot:
                    break

            if i >= j:
                return j

            temp = array[i]
            array[i] = array[j]
            array[j] = temp

    start = time.time()

    # initialize array
    array = [random.randint(0, n) for i in range(0,n)]
    quicksort(array, 0, len(array)-1)

    end = time.time()
    elapsed = end - start
    return(elapsed)

Mergesort:

def mergesort_benchmark(n=2**24):
    def mergesort(m):
        if len(m) <= 1:
            return m

        left, right = [], []
        for i in range(0, len(m)):
            if i < len(m)//2:
                left.append(m[i])
            else:
                right.append(m[i])

        left = mergesort(left)
        right = mergesort(right)
        return(merge(left,right))

    def merge(left,right):
        result = []

        i, j = 0, 0

        while i < len(left) and j < len(right):
            if left[i] <= right[j]:
                result.append(left[i])
                i += 1
            else:
                result.append(right[j])
                j += 1

        for i in range(i, len(left)):
            result.append(left[i])

        for i in range(j, len(right)):
            result.append(right[i])

        return result

    start = time.time()
    m = [random.randint(0,n) for i in range(0,n)]
    m = mergesort(m)
    end = time.time()
    elapsed = end - start
    return(elapsed)

The tl;dr answer to your questions are: Timing differences are related to your coding, and not the algorithm itself. Mostly due to how you handle arrays in-place, or methods for copying.

With that out of the question, let us review the code and in particular the code smells:

In general nicely written code – Let us start with the positive. Your code is mostly clean and easy to read. Good spacing, and reasonable names for variables and methods.
Functions within functions – Hiding the actual sorting methods within the benchmark methods, removes the potential for using your methods outside of the actual benchmark methods. Not wise. I could understand hiding the partition() method as that is intrinsic to the quicksort() implementation, and similar for the merge() to be included within mergesort(). As that would still expose the main sorting functions.
In-place vs new arrays – Your quicksort() changes the array in-place, and doesn't use extra storage, whilst the mergesort() creates a bunch of extra arrays whilst it is dividing and conquering. This gives the quicksort() and unfair advantage, and renders the quicksort() with a nasty sideeffect of changing the array. This could be a wanted effect in some cases, but mostly you do not want to change the array passed into the function.
Try to avoid While True: – In general, a lot of these can be avoided by flipping and turning the statements a little. Especially when used to loop to a certain point, where you use break to exit the loop, it is a lot nicer to flip it around. Instead of this:

i = low - 1
while True:
  i += 1
  if array[i] >= pivot
    break

do this:

i = low
while array[i] < pivot:
  i += 1

Note that the initial values has changed, and the condition has been flipped around. But the end result is the same, and the latter is easier to read and understand.
Swapping of variables – The technique you use for swapping the array elements, can be replaced by a neater statement in Python:

array[i], array[j] = array[j], array[i]

Behind the scenes, this will work exactly like your swapping, but it slightly easier on the eye, and avoids the unnecessary temp variable.
Avoid multiple len(m) on the same array – There is actually time to spare by not doing this operation multiple times when the array in question doesn't change.
range() vs xrange() – In Python 2, there is a bit of a difference between these two. The first generates the list, whilst the latter is a generator giving each number as we move on. This has a memory and performance impact when the range is getting large, like for 2**24...
Looping vs slicing – Python has some really nice slice operators which means that stuff like:

for i in range(j, len(left)):
  result.append(left[i])

can be replaced with:

result.extend(left[j:len(left)]

Note that I've also switched to extend, and not append, to proper handle the case if the slice is empty, and not actually add the slice to result.

Speed implications

When trying to compare the various implementations I had to make some slight changes to original code, so here are the simple changes with the new names of the main sorting method:

quicksort – Your original code but with a duplication of the array to be sorted, to even out the playing field, and giving all sorters the same array to sort.
hquicksort – My implementation flipping the while True statements around, and implementing the slightly simpler swap method. I also removed references to i and j, with low and right. Code included at end.
sort – An quicksort implementation as implemented in Quicksort with Python. A rather fast recursive, new array, no indices implementation.
SOquicksort – The in-place solution of the same thread (linked above).
mergesort – Your implementation of mergesort.
hmergesort – Your implementation with replacing of the len(m) with precalculated values, and using xrange() instead of range(). Code not included.
imergesort – Taking it even one step further, using slicing instead of the loop appending of the results. Code included at end.
sorted – Using the Python default sorting methods of sorted().

The timing of these using timeit for n=2**14 with 10 testruns gave the following (when run using repl.it (Do note that the code given there are messy and more of a sandbox rather than a good example of coding :-)) result:

quicksort  : 0.645922899246
hquicksort : 0.563888788223
sort       : 0.527070045471
SOquicksort: 0.455115795135

mergesort  : 1.35985612869
hmergesort : 0.970368146896
imergesort : 0.714356899261
sorted     : 0.0575048923492

As can be seen the quicksort was improved quite a bit with even relatively small changes in your code, but better implementations does exist. For the mergesort I didn't look for alternative implementations, but the changes I made it ran in half the time, resulting in a faster time compared to your implementation of quicksort.

Do also note that the timings varied slightly between runs, but the relation between the order was usually kept as given in the listing. As seen by the timing on sorted, we've not reinvented the wheel in any of these implementations, and for practical purposes we're better of using the default sort methods which runs in 1/10th of the time!

Code inclusion

hquicksort

def hquicksort(array, low, high):
  if low < high:
    p = hpartition(array, low, high)
    hquicksort(array, low, p)
    hquicksort(array, p+1, high)

def hpartition(array, low, high):
  pivot_index = random.randint(low, high)

  pivot = array[pivot_index]

  while True:
    while array[low] < pivot:
      low += 1

    while array[high] > pivot:
      high -= 1

    if low >= high:
      return high

    array[low], array[high] = array[high], array[low]

    low += 1
    high -= 1

I could possibly also get rid of the remaining While True:, but the revision does show my point. Note that to get it to work exactly like your code, I also had to add the two last lines to move the index pointers forward.

imergesort

def imergesort(m):
  len_m = len(m)
  middle = len_m//2

  if len_m <= 1:
    return m

  return imerge(imergesort(m[:middle]), imergesort(m[middle:]))

def imerge(left,right):
  result = []
  len_left = len(left)
  len_right = len(right)
  i, j = 0, 0

  while i < len_left and j < len_right:
    if left[i] <= right[j]:
      result.append(left[i])
      i += 1
    else:
      result.append(right[j])
      j += 1

  result.extend(left[i:len_left])
  result.extend(right[j:len_right])

  return result


Transfer the cell formatting in a legend to a dataset in another sheet On Sheet 1 is a legend that contains unique values in Column A every ten rows. To the right of each value in the next five columns over are a series of color filled cells that extend down ten rows to the start of the next unique value. The color fill of each 5x10 area is what I want transferred to the dataset sheet.

The dataset is on Sheet2. The entirety of the paste area is filled with a dummy value so the macro works correctly. In the defined “paste to” columns in the macro I have, every so often, included a value already present in the legend where I want my color fill to be transferred. The “paste to” columns are within the area of paste, so it is possible for highlighted areas to overlap, which is necessary.

Using a macro, I want to copy and paste the 5x10 areas from the legend to the corresponding areas in the dataset sheet. So if a value is present in the dataset sheet and matches one of the unique values in the legend, I want the unique value 5x10 area to be pasted there in order from left to right, top to bottom, overlapping.

I’ve tried using the Find Method to do a lookup against the values in the dataset, and with that an offset function to tell the macro where to paste by cell. While this does work it is process intensive even with only a few columns and a limited legend.

How can I make it more efficient? Can I accomplish what I've explained via a better route? An array? I've toyed with using hex values and converting them into cell fills, if that would in anyway be more advantageous. A user by the name of gizlmeier has already cut my code down in size, however I've noticed the procedure time to be around the same.

Any ideas? Here’s what I have so far:

Sub LegTra3()

Dim i As Integer, j As Integer
Dim RngMap As Range, RngLeg As Range, RngCom As Range, RngTar As Range

Application.ScreenUpdating = False
Application.CutCopyMode = False

Set RngMap = Sheet2.Range("$A$1:$A$100,$D$1:$D$100,$G$1:$G$100") 'Columns spaced closer than width of legend fill to provide for overlap
Set RngLeg = Sheet1.Range("$A$1:$F$41")

For Each RngCom In RngMap
    Set RngTar = RngLeg.Find(What:=RngCom, LookIn:=xlValues, LookAt:=xlWhole, SearchOrder:=xlByRows, SearchDirection:=xlNext, MatchCase:=False, SearchFormat:=False)
    If Not RngTar Is Nothing Then
        For i = 0 To 9 Step 1
            For j = 0 To 4 Step 1
                RngTar.Offset(i, j + 1).Copy
                RngCom.Offset(i, j).PasteSpecial Paste:=xlPasteFormats, Operation:=xlNone, SkipBlanks:=True, Transpose:=False
            Next j
        Next i
    End If
Next
End Sub

Your naming isn't great, you should use variable names to describe what they do

Dim i As Integer
Dim j As Integer
Dim mapToFill As Range
Dim legend As Range
Dim targetDestination As Range
Dim targetSource As Range

What is LegTra3 - I can't tell what that's supposed to mean at all.

Standard VBA naming conventions have camelCase for local variables and PascalCase for other variables and names.

Integers - integers are obsolete. According to msdn VBA silently converts all integers to long.

Instead of hard-coding cells, if they are static, assign them a named range property. e.g. instead of Sheets("mySheet").Range("A1:A10") you can have mysheet.Range("MyNamedRange").


First go at my own form validation, and suggestions? Well I just finished my first go at my own form validation (I always used validate plugin) for simple front end.  It's definitely small potatoes as it's only meant for a form with 4 fields; 3 required text and one required e-mail.

I'm new to Jquery so I am doing little exercises like these to help me learn and I was hoping someone could take a look and see if what I did was ok.  Did I make any newbie mistakes? Can I improve this? Perhaps I should use js objects (new to those too) to organize code? I know there is no one answer, in fact I'm sure there are countless ways to validate a form...But I was just hoping some JS expert could offer a little insight on better coding practices.

*EDIT - forgot to mention...I'm aware that I should probably use regex for better email validation, however, just threw the indexOf() method for the time being *

$(function() {

        var err = "Required field.";
        var errEmail = "Please enter valid email.";
        var errors = false;

        $('.submit').click(function() {
            $('.error').remove();

            $('.req').each(function() {
                if($(this).val() == '') {
                    $(this).after(err);
                    errors = true;
                }
            });

            if ($('.reqe').val().indexOf('@') === -1) {
                $('.reqe').after(errEmail);
                errors = true;
            }

            if (errors == true) {
                return false;
            }
            else {
                var errors = false;
                alert("Submitted");
                return true;
            }

        });
});

Any feedback would be greatly appreciated.  Thanks

A couple of small changes:

near the end:

if (errors == true) {
    return false;
}
else {
    var errors = false;
    alert("Submitted");
    return true;
}

Could be:

if(!errors)
{
    alert("Submitted");
}
return !errors;

the .req

if($(this).val() == ''){

could be:

if(!$(this).val()){

This would actually be a problem because

 " " != ""

so a space would validate (if that's what you intended then ignore this one ;) )
var errors= false; needs to be inside the click event:

 $('.submit').click(function() {
     var errors = false;

otherwise when you click the submit a second time after an invalid attempt it won't reset the value.

Alternatively:-

http://jsfiddle.net/ssTPm/

var validationConfig = [
{
    "selector": ".req",
    "hasErrors": function() {
        return !$(this).val()
    },
    "errorMessage": "Required field."},
{
    "selector": '.reqe',
    "hasErrors": function() {
        return $('.reqe').val().indexOf('@') === -1
    },
    "errorMessage": "Please enter valid email."
}];

then a nice loop like:

for (var i = 0, validation = null; validation = validationConfig[i]; i++) {
    $(validation.selector).each(validateEach);
}

and validateEach would look like:

 function validateEach () {
    var hasErrors = validation.hasErrors.call(this);
    errors |= hasErrors;
    if (hasErrors) {
        $(this).after(validation.errorMessage);
    }
};

This would mean you could have the validation function in you js files and then when you initialise the page from the server you just output the validationConfig:

validationConfig = validationConfig || [];
validationConfig.concat([
    {
        "selector": ...
    }
];

as many times as you like. Although from the looks of things it might be over-engineering things a bit.

Explanations:-

so the || is a coalesce or OR operator.

myVariable = myVariable || [];

means if myVariable results in a true value (not false, undefined, null etc etc) then it does nothing otherwise it assigns it a defualt value.

so if I had the following:

myList = myList|| [];
myList.push(1);
myList = myList|| [];
myList.push(2);
myList = myList|| [];
myList.push(3);

it would result in [1,2,3]. The first time it sets the value as [] the next times they just assign itself to itself. This way it doesn't need to know if it has been initialised or not.

the |= is a bitwise | (BITWISE OR) and an = so

value |= otherValue;

is the equivalent to

value = value | otherValue;

In this example

errors |= hasErrors;

is the exact same as

errors = errors || hasErrors;

Errors is assigned true as long as either it was already true or hasErrors is true;


Display the n terms of even natural numbers(between 1 and 100) and their sum This is a C# console application that displays the n terms of even natural numbers between (1 and 100) and their sum

Is this good? Or is it bug riddled ?

int numberOfEvenNumbers;
int currentNumber;
currentNumber = 0;
int counter;
counter = 1;
int sum;
sum = 0;

Console.Write("How many even numbers :");

numberOfEvenNumbers = Convert.ToInt32(Console.ReadLine());

while (counter <= 100)
{

    if (counter % 2 == 0)
    {
        Console.WriteLine(counter);
        sum += counter;
        currentNumber++;
    }

    if (currentNumber == numberOfEvenNumbers)
    {
        break;
    }

    counter++;
}

Console.Write("Sum : ");
Console.WriteLine(sum);

Console.ReadLine();

It is neither good nor bad. It does the job: it shows a list of even numbers up to a limit lesser than or equal to 100. But the design or architecture of the program is ugly, even for a small program like this.

Previous it was common to declare all variables at the top of the scope (function or other bracketed section) (and some languages require that), but now in C# it is considered standard to declare the variable when they are used the first time.

  numberOfEvenNumbers = Convert.ToInt32(Console.ReadLine());

This is quick and dirty and will crash the program if the user enters a non numeric string.

So you'll have to check the input and react to an invalid such:

  int numberCount = 0;
  string prompt = "How many even numbers [Enter a number lesser than or equal to 50]: ";
  string errorMessage = "";
  do
  {
    Console.Write($"{errorMessage}{prompt}");
    errorMessage = "Invalid input. ";
  }
  while (!int.TryParse(Console.ReadLine(), out numberCount) || numberCount <= 1 || numberCount > 50);

Here I validate (> 1 and < 100) the input in the UI which is preferable to do compared to validating while running the algorithm.

The header of the question says

  numbers between 1 and 100

(excluding 100) but you iterate potentially up to and including 100:

  while (counter <= 100) { ... }

so what do you really mean?

if (counter % 2 == 0)
{

Even numbers can be found by checking if a number is divisible by 2 as you do, but the sequence of even numbers from 2 and upwards can be found like this:

  for (int i = 2; i <= limit; i += 2)
  {
    Console.WriteLine(i);
  }

in this way you are almost done with one variable (i) and a limit, you just need a sum to sum up with:

// The 6 first even numbers:
int numberCount = 6 * 2;
int sum = 0;

for (int i = 2; i <= numberCount; i += 2)
{
  Console.WriteLine(i);
  sum += i;
}
Console.WriteLine(sum);

So all in all it ends up like this:

static void EvenNumberCount()
{
  int numberCount = 0;
  string prompt = "How many even numbers [Enter a number lesser than 50]: ";
  string errorMessage = "";
  do
  {
    Console.Write($"{errorMessage}{prompt}");
    errorMessage = "Invalid input. ";
  }
  while (!int.TryParse(Console.ReadLine(), out numberCount) || numberCount <= 1 || numberCount > 50);

  numberCount *= 2;
  int sum = 0;

  for (int i = 2; i <= numberCount; i += 2)
  {
    Console.WriteLine(i);
    sum += i;
  }

  Console.WriteLine();
  Console.WriteLine($"Sum: {sum}");
}

But here I do too much in the same method:

The program has 3 steps:

User input
The algorithm
Display of result

This calls for 3 separate methods or functions:

limit = GetCountOfNumbers();
result = EnumerateAndEvenNumbers(limit);
ShowResult(result)

Which could result in something like:

static int GetCountOfNumbers()
{
  int numberCount = 0;
  string prompt = "How many even numbers [Enter a number lesser than or equal to 50]: ";
  string errorMessage = "";
  do
  {
    Console.Write($"{errorMessage}{prompt}");
    errorMessage = "Invalid input. ";
  }
  while (!int.TryParse(Console.ReadLine(), out numberCount) || numberCount <= 1 || numberCount > 50);

  return numberCount;
}

static (int Sum, IEnumerable<int> EvenNumbers) EnumerateAndSumEvenNumbers(int numberCount)
{
  int sum = 0;
  List<int> evenNumbers = new List<int>();
  numberCount *= 2;

  for (int i = 2; i <= numberCount; i += 2)
  {
    evenNumbers.Add(i);
    sum += i;
  }

  return (sum, evenNumbers);
}

static void ShowResult(int limit, int sum, IEnumerable<int> evenNumbers)
{
  Console.WriteLine();
  Console.WriteLine($"The first {limit} even numbers are: ");
  foreach (int number in evenNumbers)
  {
    Console.WriteLine(number);
  }

  Console.WriteLine();
  Console.WriteLine($"Sum: {sum}");
}

static void TestNumberCount()
{
  int numberCount = GetCountOfNumbers();
  (int Sum, IEnumerable<int> EvenNumbers) = EnumerateAndSumEvenNumbers(numberCount);
  ShowResult(numberCount, Sum, EvenNumbers);
}

By the way: The sum of any number n of even numbers from 2 and upwards can be found as: sum = n * (n + 1). This could simplify the above "algorithm" a bit.


Hackerrank Sparse Arrays in Java 8 I have recently started trying Java 8 and I am trying to solve problems as much possible using Java 8 lingo.

I wrote the code for the Sparse Arrays problem on HackerRank and needed to know a few pointers regarding my code:

Is it the right way to code in Java 8?
How much more efficient is it than legacy Java programs?
Are there any improvements or misconceptions regarding the association of this code to Java 8?

public class SparseArrays {

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

        Scanner sc = new Scanner(System.in);

        int N = sc.nextInt();
        List<String> corpus = new ArrayList<String>();
        IntStream.range(0, N).forEach(n -> corpus.add(sc.next()));

        int Q = sc.nextInt();
        List<String> queries = new ArrayList<String>();
        IntStream.range(0, Q).forEach(n -> queries.add(sc.next()));

        queries.stream().forEach(query -> System.out.println(corpus.stream().filter(p -> p.equals(query)).count()));
    }
}

Your code uses streams quite neatly and well. The algorithm employed is not necessarily the best, but the logic is reasonably clear.

It's common with streams to use vertical white-space to make the steps in code more readable. For example, the line:

queries.stream().forEach(query -> System.out.println(corpus.stream().filter(p -> p.equals(query)).count()));

could be re-formatted as:

queries.stream()
       .forEach(query -> System.out.println(
              corpus.stream()
                    .filter(p -> p.equals(query))
                    .count()
               ));

Now, while dealing with that stream, it's not really streaming things "right".... The System.out.println is out-of-place. You should be mapping the query in to a count in one step, and then outputting the count in a different step....

queries.stream()
       .mapToInt(query -> corpus.stream()
                    .filter(p -> p.equals(query))
                    .count()
               )
       .forEach(System.out::println);

Now, there are ways to improve that performance still, for example, lots of println statements could be grouped in to just one print...

String output = queries.stream()
       .mapToInt(query -> corpus.stream()
                    .filter(p -> p.equals(query))
                    .count()
               )
        .mapToObj(String::valueOf)
        .collect(Collectors.joining("\n"));
System.out.println(output);

Finally, note that you are doing an \$O(n)\$ operation for each query. You should consider a way of improving that time-cost by front-loading the creation of a Map when loading up the words will improve the performance of querying things.

For example, consider this:

IntStream.range(0, N).forEach(n -> corpus.add(sc.next()));

First up, that inner part n -> corpus.add(sc.next()) is doint a "side-effect" operation that can be avoided. Functional-programming purists try to avoid side-effects in streasm as much as possible. SOmetimes it is unavoidable, but you could easily re-write that line as:

List<String> corpus = IntStream.range(0, N)
      .mapToObj(n -> sc.next())
      .collect(Collectors.toList());

That adds each word to a list. How about doing it this way:

Map<String, List<String>> data = IntStream.range(0, N)
         .mapToObj(i -> sc.next())
         .collect(Collectors.groupingBy(v -> v));

That code takes each input word, and returns a Map keyed by each unique word, and each word's value is a list containing each instance of that word in the source.

Now, you can use the length (size) of the value list to return your data with:

String output = queries.stream()
        .mapToInt(query -> data.get(query).size())
        .mapToObj(String::valueOf)
        .collect(Collectors.joining("\n"));
System.out.println(output);


Lattice path from Project Euler with Python solution I was trying to solve this problem called lattice path from Project Euler:

  Count the number of unique paths to travel from the top left to the bottom right of a lattice of squares.

How many such routes are there through a 20×20 grid? It takes me forever to get an answer...

(source: projecteuler.net)

    #  Input:     An interger N (which is the size of the lattice)
    #  Output:    An interger (which represents the number of unique paths)
    #
    #  Example:   input: 2
    #
    #             (2 x 2 lattice of squares)
    #              __ __
    #             |__|__|
    #             |__|__|
    #

When moving through the lattice, you can only move either down or to the right.

I currently have 1 solution, and I am not sure how much better if we memoize it. But I wanted to ask for code review for my solution below:

def lattice_paths(m, n):
    if m == 0 and n == 0:
        return 1
    if m < 0 or n < 0:
        return 0
    return lattice_paths(m - 1, n) + lattice_paths(m, n - 1)

Here is the time complexity and space complexity of my first solution.
# Time Complexity: O(2^(M+N))
# Auxiliary Space Complexity: O(M+N)

Dynamic approach with the memorized solution:

def lattice_paths(m, n): # m rows and n columns

    def helper(x, y):

      if x > helper.m or y > helper.n:
        return 0
      if helper.cache.get((x, y)) is not None:
        return helper.cache[(x, y)]

      helper.cache[(x, y)] = helper(x + 1, y) + helper(x, y + 1)
      return helper.cache[(x, y)]
    helper.cache = {}

    helper.m = m
    helper.n = n
    helper(0,0)

    if m  ==  0 and n == 0:
      return 1
    elif m < 0 or n < 0:
      return 0
    else:
      return helper.cache[(0, 0)]

All of your approaches are very brute force.  The answer can be computed with a bit of mathematical reasoning.  For an m × n lattice, all paths will have m + n segments, of which m segments will go down and n segments will go right.  So, out of m + n slots, count the ways to pick m downward moves; the rest of them will be rightward moves.  The number of paths is:

$${m + n \choose m} = {m + n \choose n} = \frac{(m+n)!}{m! \, n!}$$

So, the Python solution is simply:

from math import factorial

def lattice_paths(m, n):
    return factorial(m + n) // factorial(m) // factorial(n)

Even though the calculation involves large factorials, Python supports big integers seamlessly, and arrives at the answer nearly instantaneously.


Menu-based Caesar Cipher with encryption and decryption I am about to start a very large project in Ruby. To get myself in the Ruby mindset I have been doing all of my homework and other side work in Ruby. I am hoping that some of you who know Ruby very well can look at this small program I have written and point out areas that aren't written in a Ruby-like fashion. Any and all tips and criticisms would be appreciated.

This program is a menu-based Caesar Cipher with encryption and decryption.

KEY = 3
$caesar_map = {}

#initializes the caesar_map and reads input from the user
#starting point of the program
def init()
  #create empty hashes
  plain_alphabet  = {}
  cipher_alphabet = {}

  #fill in the alphabet hashes with the letter and its position
  #in the english alphabet
  "a".upto("z") {|x| plain_alphabet[x]  = ((x[0] - "a"[0]) % 26) }
  "A".upto("Z") {|x| cipher_alphabet[x] = ((x[0] - "A"[0]) % 26) }

  #fill in the caesar_map, maps every letter KEY positions
  #to the right in the alphabet
  plain_alphabet.each do |k,v|
    $caesar_map[k] = cipher_alphabet.index((v+KEY) % 26)
    #puts "key: #{k} mapped value: #{cipher_alphabet.index((v+3) % 26)}"
  end
  #special case, I want spaces to be maintained in the messages
  $caesar_map[" "] = " "

  #print menu
  choice = -1
  while(choice != 3)
    puts "What would you like to do?"
    puts "\n1. Encrypt a message"
    puts "\n2. Decrypt a message"
    puts "\n3. Exit\n"

    choice = Integer(gets.chomp)

    if(choice == 1)
      puts "Please enter a message you would like to Encrypt\n"
      message = gets.chomp
      encrypt(message)
    elsif(choice == 2)
      puts "Please enter a message you would like to Decrypt\n"
      message = gets.chomp
      decrypt(message)
    elsif(choice == 3)
      abort("Exiting...")
    else
      puts "You didn't enter a valid choice. Enter a number between 1..3\n"
    end
  end
end

#encrypts a plaintext message using the caesar cipher
def encrypt(plaintext)
  #make sure the message is in lowercase
  plaintext.downcase!
  ciphertext = ""

  #fill in the ciphertext with mapping information in the caesar_map
  plaintext.split("").each do |c|
    ciphertext += $caesar_map[c]
  end

  #print the encrypted ciphertext
    puts "\n plaintext: #{plaintext}\nciphertext: #{ciphertext}\n\n"
end

#decrypts a ciphertext message using the caesar cipher
def decrypt(ciphertext)
  #make sure the ciphertext is in uppercase
  ciphertext.upcase!
  plaintext = ""

  #fill in the plaintext with mapping information in the caesar_map
  ciphertext.split("").each do |c|
    plaintext += $caesar_map.index c
  end

  #print the decrypted plaintext
  puts "\n ciphertext: #{ciphertext}\n plaintext: #{plaintext}\n\n"
end

#start the program
init()

I'd initialize with something closer to this (although I might inline the two arrays):

plain_alphabet = ('a'..'z').to_a
cipher_alphabet = ('A'..'Z').to_a.rotate KEY
$caesar_map = Hash[plain_alphabet.zip cipher_alphabet]

The en/decryption (which could likely be handled by the built-in tr method) might look more like:

plaintext.split("").inject("") {|s, c| s << $caesar_map[c]}

I'd put the whole thing in a class, too; skip the global.


Stylish underline when input is focused, modern browsers only I've got a text input element which is decorated with an underline. The underline goes up half the height of the input on either side of it and turns blue when the input is focused. It's similar to how inputs look on Android devices.

Here's a JSFiddle and some accompanying code.

    <input placeholder="Search..." type="text" value="">

//  Start:normalize
*, *:before, *:after {
    box-sizing: border-box;
}

input:focus {
    outline: none
}
//  End:normalize

.input-underline-wrapper {
    display: inline-block;
    width: 100%;
}

input {
    width: calc(100% - 2px);
    margin-left: 1px;
    padding-bottom: 2px;
}

input[type=text] {
    border: 0;
    cursor: text;
}

.underline {
    background-color: #ebebeb;
    transition: color 0.3s, background 0.3s, border 0.3s, text-shadow 0.3s;
    z-index: 0;
    height: 5px;
    margin-top: -4px;
    width: 100%;
}

input:focus ~ .underline {
    background-color: #4EA6EA;
}

This all works well and good, but it feels a bit verbose. I'm wondering if I've missed something simple here. Is it possible to achieve the same effect using a white box-shadow which covers the upper-half of the input and omit using a wrapper and a second, underlining div? I only need to support the latest version of Google Chrome.

Using box-shadows on the input can let you have the same underlining effect without any wrapper or unsemantic markup + strip your CSS a lot :

input[type=text] {
  height:17px;
  border: 0;
  width: calc(100% - 2px);
  margin-left:1px;
  box-shadow: -8px 10px 0px -7px #ebebeb, 8px 10px 0px -7px #ebebeb;
  -webkit-transition: box-shadow 0.3s;
  transition: box-shadow 0.3s;
}
input[type=text]:focus {
  outline: none;
  box-shadow: -8px 10px 0px -7px #4EA6EA, 8px 10px 0px -7px #4EA6EA;
}
<input placeholder="Search..." type="text" value=""/>

For browser support :

box-shadows : IE9+
transitions : IE10+
calc() : IE9+


"Stack Exchange page visits" alike implementation The objective is to increment the page visits by one. The user should not be able to do this by refreshing. The number of visits can be incremented only if the user's last visit was later than 10 minutes or none at all.  This should work regardless of multiple users behind the same IP or router.

Does this need any improving?

Stack Exchange page views works like this.

//Is data storage available?
if(typeof(Storage)!=="undefined")
  {
    var tenMinutes = 3600 * 10; //10 minutes
    var now = new Date();
    var visits = JSON.parse(localStorage.getItem('visits')); //Get visits array from local storage
    var currentVisit = {id:$scope.ad.id, datetime:now};
    var duplicate = false;

    //There is no older visits, user is new here
    if(!visits){
        visits = [currentVisit];
        addViewForID(currentVisit.id);
    }
    else{
        //Check for duplicate visit
        angular.forEach(visits, function(visit){
            if (visit.id == currentVisit.id) {

                duplicate = true;
                var visitTime = new Date(visit.datetime);
                var deltaTime = Math.abs(now.getTime() - visitTime.getTime());
                var deltaDays = Math.ceil(deltaTime / (1000 * 3600 * 24));

                if (deltaDays > 1 || (deltaDays == 1 && deltaTime > tenMinutes))
                    visit.datetime = now;
            }

        })

        //First time visiting this page
        if (!duplicate){
            visits.push(currentVisit);
            addViewForID(currentVisit.id);
        }

    }

    //Save visits array in local storage
    localStorage.setItem('visits', JSON.stringify(visits));
}

function addViewForID(id){
    FData.incrementVisitsForID(id);
}

My 2 cents:

I would not use an array of objects to represent the pages, I would use an object where each $scope.ad.id is a property and where the value is the last time visited. This would take out your loop and dupe management
Any enterprising individual is able to artificially inflate the counter, either by clearing the localStorage or by calling FData.incrementVisitsForID(id); in a loop from the console.
Are you sure that 10 minutes = 3600 * 10, it seems wrong to me

I would counter propose something like this:

//Is data storage available?
if(typeof(Storage)!=="undefined")
{
  var tenMinutes =  1000 * 60 * 10; //10 minutes
  var now = (new Date()).getTime();
  //Get visits array from local storage or if that fails, an empty object
  var visits = JSON.parse(localStorage.getItem('visits')) || {};
  //Get time of last visit, or 0
  var lastVisit = visits[ $scope.ad.id ] || 0;
  //Does it count?
  if( now - lastVisit > tenMinutes)
  {
    FData.incrementVisitsForID( $scope.ad.id );
  }
  //Update visits for the next time
  visits[ $scope.ad.id ] = now;
  //Save visits array in local storage
  localStorage.setItem('visits', JSON.stringify(visits));
}


RSS Feed Calendar Plugin This plugin is initialized like so:

$("#calendar").ksdCalendar({
    feedUrl: "http://www.kent.k12.wa.us/site/RSS.aspx?DomainID=275&ModuleInstanceID=4937&PageID=4334",
    elemHeight: 750
});

I would like a review on the code, structure, or anything else that comes to mind. What this plugin does is take an RSS feed from JGFeed, parse the entries, then pass them into another calendar plugin which in turn builds and displays the calendar.

(function ( $, window, document, undefined ) {
    "use strict";
    var Calendar = {

        init: function(options, elem) {

            this.options = $.extend( {}, this.options, options );
            this.elem = $(elem);

            this.setupAjax();
            this.getFeed();

            return this;
        },

        options: {
            feedUrl: "",
            elemHeight: 750
        },

        entries: [],

        getFeed: function() {
            var self = this;
            $.jGFeed(this.options.feedUrl, function (feeds) {

                if (!feeds) {
                    return false;
                }

                $.extend(self.entries, feeds.entries);

                self.parseEntries();
            }, 100);
        },

        parseEntries: function() {
            //Rename to fit plugin requirements
            for (var i = 0; i < this.entries.length; i++) {
                var entry = this.entries[i];
                    entry["allDay"] = false;

                //Rename
                entry["url"] = entry["link"];
                delete entry["link"];

                var position = entry.title.indexOf(' - ');

                if (position === -1) {
                    //All day event
                    entry.allDay = true;
                    var space = entry.title.indexOf(" "),
                        title = entry.title.substring(space + 1),
                        firstHalf = entry.title.slice(0, space); //Start date, no time because it's all day event
                } else {
                    var firstHalf = entry.title.slice(0, position), //Start date/time
                        secondHalf = entry.title.substring(position + 3);

                    if (secondHalf.indexOf("AM") !== -1) {
                        var title = secondHalf.substring(secondHalf.indexOf("AM") + 3); //Title if has AM
                    } else {
                        var title = secondHalf.substring(secondHalf.indexOf("PM") + 3); //Title if has PM
                    }

                    secondHalf = secondHalf.slice(0, -(title.length + 1)); //End date/time
                }

                entry["start"] = Date.parse(firstHalf);
                entry["end"] = Date.parse(secondHalf);
                entry.title = title;
            };

            this.setUpCalendar();
        },

        setUpCalendar: function() {
            this.elem.fullCalendar({
                editable:   false,
                weekends:   true,
                header: {
                    left:       'month basicDay',
                    center:     'title',
                    right:      'today prev, next'
                },
                height:     this.options.elemHeight,
                events:     this.entries
            });
        }
    };

    if ( typeof Object.create !== 'function' ) {
        Object.create = function (o) {
            function F() {}
            F.prototype = o;
            return new F();
        };
    }

    $.fn.ksdCalendar = function( options ) {
        if (this.length) {
            return this.each(function() {
                var myCalendar = Object.create(Calendar);
                myCalendar.init(options, this);
                $.data(this, 'ksdCalendar', myCalendar);
            });
        }
    };

})( jQuery, window, document );

This code looks good, I only have 2 minor items from a once over;

I would have a constant for -1 meaning an all day event

var ALL_DAY_EVENT = -1; // <-- Declared somewhere on top
if (position === ALL_DAY_EVENT) {
    ...

This:

if (secondHalf.indexOf("AM") !== -1) {
    var title = secondHalf.substring(secondHalf.indexOf("AM") + 3); //Title if has AM
} else {
    var title = secondHalf.substring(secondHalf.indexOf("PM") + 3); //Title if has PM
}

Declares title with var twice, you should declare title only once, ideally on top before your loop. Also, there is some serious code repetition there. I would suggest something like:

//Declare meridiem and title somewhere higher prior to the loop
meridiem = ~secondHalf.indexOf("AM") ? "AM" : "PM";
title = secondHalf.substring(secondHalf.indexOf(meridiem) + 3);


C# AES Encryption random IV per file I've made some improvments on the code from : Csharp-AES-bits-Encryption-Library-with-Salt

saltBytes is now the SHA512 of the password.
Random IV for each encryption call. ( IV length 16 is added to the encrypted file , removed from file before decryption)

Do you see any flows, something that needs optimization ?

Is generateIV() safe and secure ?

The Code bellow is working, I've made a test to encrypt 2 text files with exact same text inside each. the result is both have different data, and decrypt succeed.

also made test before the random IV, both files had same encrypted text, results in same data.

Encryption code :

    private static int IV_LENGTH = 16;

    public static byte[] AES_Encrypt(byte[] bytesToBeEncrypted, byte[] passwordBytes)
    {

        byte[] encryptedBytes = null;
        byte[] encryptedBytesAndIV = null;
        byte[] saltBytes = SHA512.Create().ComputeHash(passwordBytes);
        using (System.IO.MemoryStream ms = new System.IO.MemoryStream())
        {
            using (AesCryptoServiceProvider AES = new AesCryptoServiceProvider())
            {
                AES.KeySize = 256;
                //AES.BlockSize = 128;

                var key = new Rfc2898DeriveBytes(passwordBytes, saltBytes, 100);
                AES.Key = key.GetBytes(AES.KeySize / 8);
                AES.IV = generateIV();

                AES.Mode = CipherMode.CBC;

                using (var cs = new CryptoStream(ms, AES.CreateEncryptor(), CryptoStreamMode.Write))
                {
                    cs.Write(bytesToBeEncrypted, 0, bytesToBeEncrypted.Length);
                    cs.Close();
                }
                encryptedBytes = ms.ToArray();
                encryptedBytesAndIV = new byte[encryptedBytes.Length + AES.IV.Length];
                AES.IV.CopyTo(encryptedBytesAndIV, 0);
                encryptedBytes.CopyTo(encryptedBytesAndIV, IV_LENGTH);
            }
        }

        return encryptedBytesAndIV;
    }

    private static byte[] generateIV()
    {
        using (RNGCryptoServiceProvider rng = new RNGCryptoServiceProvider())
        {
            byte[] nonce = new byte[IV_LENGTH];
            rng.GetBytes(nonce);
            return nonce;
        }
    }

Decryption code :

    public static byte[] AES_Decrypt(byte[] bytesToBeDecrypted, byte[] passwordBytes)
    {
        byte[] decryptedBytes = null;

        byte[] saltBytes = SHA512.Create().ComputeHash(passwordBytes);

        using (MemoryStream ms = new MemoryStream())
        {
            using (AesCryptoServiceProvider AES = new AesCryptoServiceProvider())
            {
                AES.KeySize = 256;
                //AES.BlockSize = 128;

                var key = new Rfc2898DeriveBytes(passwordBytes, saltBytes, 100);
                AES.Key = key.GetBytes(AES.KeySize / 8);
                AES.IV = getIV(bytesToBeDecrypted);
                bytesToBeDecrypted = removeTagAndIV(bytesToBeDecrypted);
                AES.Mode = CipherMode.CBC;

                using (var cs = new CryptoStream(ms, AES.CreateDecryptor(), CryptoStreamMode.Write))
                {
                    cs.Write(bytesToBeDecrypted, 0, bytesToBeDecrypted.Length);
                    cs.Close();
                }
                decryptedBytes = ms.ToArray();

            }
        }

        return decryptedBytes;
    }

    private static byte[] removeTagAndIV(byte[] arr)
    {
        byte[] enc = new byte[arr.Length - IV_LENGTH];
        Array.Copy(arr, IV_LENGTH, enc, 0, arr.Length - IV_LENGTH);
        return enc;
    }

    private static byte[] getIV(byte[] arr)
    {
        byte[] IV = new byte[IV_LENGTH];
        Array.Copy(arr, 0, IV, 0, IV_LENGTH);
        return IV;
    }

There are three magic numbers in here:

AES.KeySize = 256;
//AES.BlockSize = 128;

var key = new Rfc2898DeriveBytes(passwordBytes, saltBytes, 100);
AES.Key = key.GetBytes(AES.KeySize / 8);

Also, that comment should go. Dead code is dead.

As stated in the comments:

byte[] saltBytes = SHA512.Create().ComputeHash(passwordBytes);

Generating the saltBytes based on the password is a bad idea. The salt should be random and if two users with the exact same password have the same salt, well now either of them can attack the other.
The salt does not need to be a secret. You can keep it in plain-text. Many time, in the case of a database, it will have two (or three) columns: EncryptedPassword and Salt (maybe IV if it's not based on the Salt). (Or in the case of ASP.NET Entity Framework / Simple Membership this is all stored in one column with separators.)
The attacker can know the Salt and IV, you can even use the Salt to generate the IV (or initialize the RNG for it), but you cannot use the password to generate either. That defeats the purpose of the password. If I know Salt is generated from the password, then I don't need the password at all, I can just brute force everything to find that Salt, or I can use a rainbow table lookup, et. al.


Review of status bar code Simplification of code would be appreciated.

var textarea = document.getElementById("textarea"),
  statusBar = document.getElementById("status-bar"),
  inputFile = document.getElementById("input-file"),
  appname = "notepad",
  isModified = false,
  showStatusBar = true,
  filename;

function changeDocTitle(newFilename) { // Change doc title
  filename = newFilename;
  document.title = filename + " - " + appname;
}

function dontSave() { // Confirm dont save
  if (confirm("You have unsaved changes that will be lost.")) {
    isModified = false;
    return true;
  }
}

function newNote(text, name) { // New
  if (!isModified || dontSave()) {
    textarea.value = text || "";
    changeDocTitle(name || "untitled.txt");
  }
  textarea.focus();
}

function openNote() { // Open
  if (!isModified || dontSave()) {
    inputFile.click();
  }
  textarea.focus();
}

function rename() { // Rename
  var newFilename = prompt("Name this note:", filename);
  if (newFilename !== null) {
    if (newFilename === "") {
      changeDocTitle("untitled.txt");
    } else {
      changeDocTitle(newFilename.lastIndexOf(".txt") == -1 ? newFilename + ".txt" : newFilename);
    }
    return true;
  }
}

function saveNote() { // Save
  if (rename()) {
    var blob = new Blob([textarea.value.replace(/\n/g, "\r\n")], {
      type: "text/plain;charset=utf-8"
    });
    saveAs(blob, filename);
    isModified = false;
  }
  textarea.focus();
}

function updatestatusBar() { // Update statusBar
  var text = textarea.value,
    chars = text.length,
    words = text.split(/\S+/g).length - 1,
    lines = text.split("\n").length;
  statusBar.value = lines + " lines, " + words + " words, " + chars + " characters";
}

function showStatusBarFunc() {
  textarea.style.height = "calc(100% - 18px)";
  statusBar.style.display = "";
  showStatusBar = true;
  updatestatusBar();
}

function hideStatusBarFunc() {
  textarea.style.height = "";
  statusBar.style.display = "none";
  showStatusBar = false;
}

function toggleStatusBar() {
  if (showStatusBar) {
    hideStatusBarFunc();
  } else {
    showStatusBarFunc();
  }
}

textarea.addEventListener("input", function() {
  isModified = true;
  if (showStatusBar) {
    updatestatusBar();
  }
});

inputFile.addEventListener("change", function() { // Load file
  var file = inputFile.files[0],
    reader = new FileReader();
  reader.onload = function() {
    newNote(reader.result, file.name);
  };
  reader.readAsText(file);
});

document.addEventListener("keydown", function(e) { // Shortcuts
  if (e.altKey && e.shiftKey) { // Alt+Shift
    newNote();
  }
  if (e.ctrlKey) { // Ctrl+
    switch (e.keyCode) {
      case 79: // O
        e.preventDefault();
        openNote();
        break;
      case 83: // S
        e.preventDefault();
        saveNote();
        break;
      case 66: // B
        e.preventDefault();
        toggleStatusBar();
        break;
      case 191: // /
        e.preventDefault();
        alert("Help note for " + appname + " will be added soon!");
        break;
    }
  }
  if (e.keyCode == 9) { // Tab
    e.preventDefault();
    var sStart = textarea.selectionStart,
      text = textarea.value;
    textarea.value = text.substring(0, sStart) + "\t" + text.substring(textarea.selectionEnd);
    textarea.selectionEnd = sStart + 1;
  }
});

onunload = function() { // Save localStorage
  if (isModified) {
    localStorage.setItem("text", textarea.value);
    localStorage.setItem("name", filename);
  } else {
    // localStorage.clear();
    localStorage.removeItem("text");
  }
  localStorage.setItem("showStatusBar", showStatusBar);
};

onload = function() { // Load localStorage
  if (localStorage.getItem("text")) {
    newNote(localStorage.getItem("text"), localStorage.getItem("name"));
    isModified = true;
  } else {
    newNote();
  }
  showStatusBar = JSON.parse(localStorage.getItem("showStatusBar"));
  if (showStatusBar) {
    showStatusBarFunc();
  } else {
    hideStatusBarFunc();
  }
};

The code is good. The only comment I have is that the data and its representation are highly coupled so it may be good idea to invest some time in separating them. But it will make sense only if this application will grow and evolve. Now it is very neat and good looking.


Rotating color table object (for easy and consistent labeling) I wanted to be able to consistently label an arbitrary number of objects on my site. It's pretty simple, but I if there's a more clever way...

// Usage:
// ColorTable.getColor('my link name');
ColorTable = {
  '_colors' : [
    '#F4BB2E',
    '#2DE656',
    '#66CCFF',
    '#6666FF',
    '#CC66FF',
    '#FF6FCF',
    '#FC4048'
   ],
  'tableSize' : function() { return this['_colors'].length; },
  'nextIndex' : function() {
    if (localStorage["ColorTable:_index"]===undefined) {
        localStorage["ColorTable:_index"] = -1;
    }
    rv = (parseInt(localStorage["ColorTable:_index"]) + 1) % this.tableSize();
    localStorage["ColorTable:_index"] = rv;
    return rv;
  },
  'getIndex' : function(key) {
    if (!localStorage["ColorTable:" + key]) {
      localStorage["ColorTable:" + key] = this.nextIndex();
    }
    return localStorage["ColorTable:" + key];
  },
  'setIndex' : function(key, index) {
    if (index >= 0 && index < this.tableLength()) {
      localStorage["ColorTable:" + key] = index;
      return localStorage["ColorTable:" + key];
    } else {
      return false;
    }
  },
  'getColor' : function(key) {
    return this['_colors'][this.getIndex(key)];
  }
};

Minor nitpicks:

None of you keywords are reserved words (especially if it starts with an underscore!) so encasing them in single-quotes is superfluous.

ColorTable.tableSize is an example of the smurf-naming convention (see item 21). Drop the second instance of table to become ColorTable.size

For setIndex you return two different datatypes: index on success, and false for failure. Since index is an arguements known to external code, what purpose does this serve? For consistency, you should return true/false for pass/fail if you have to. If you have to, how about a single point-of-return? You'd have to introduce a variable, and two more LOCs, but... single point-of-return!

But why not throw an exception? The existing mixed-type return suggests that calling code doesn't use the return value in any meaningful way.