Read CSV into 2D float array in Go Depending on how you count, this is my first Go program. I'm trying to read a CSV into a two-dimensional array of some numeric type, and then print it out.

(I want to use this to read "edge weights" to build a Graph; that is my next mission, unrelated to the code below.)

So the code below works. But particularly as I'm new to the language, I'd like to know:

Are there shorter ways to accomplish the same functionality?
Any ways to make this code more idiomatic?
float64 feels arbitrary, but Go is statically typed -- any way I can make this more dynamic, allowing other types?

Here 'tis; rip her apart if you want! Trying to learn.

package csfloat

import (
    "encoding/csv"
    "fmt"
    "os"
    "strconv"
    "strings"
)

// make2dFloatArray makes a new 2d array of float64s based on the
// rowCount and colCount provided as arguments
func make2dFloatArray(rowCount int, colCount int) [][]float64 {
    values := make([][]float64, rowCount)
    for rowIndex := range values {
        values[rowIndex] = make([]float64, colCount)
    }

    return values
}

// stringValuesToFloats converts a 2d array of strings into a 2d array
// of float64s.
func stringValuesToFloats(stringValues [][]string) ([][]float64, error) {
    values := make2dFloatArray(len(stringValues), len(stringValues[0]))

    for rowIndex, _ := range values {
        for colIndex, _ := range values[rowIndex] {
            var err error = nil

            trimString :=
                strings.TrimSpace(stringValues[rowIndex][colIndex])

            values[rowIndex][colIndex], err =
                strconv.ParseFloat(trimString, 64)

            if err != nil {
                fmt.Println(err)
                return values, err
            }
        }
    }

    return values, nil
}

// ReadFromCsv will read the csv file at filePath and return its
// contents as a 2d array of floats
func ReadFromCsv(filePath string) ([][]float64, error) {
    file, err := os.Open(filePath)
    if err != nil {
        return nil, err
    }

    reader := csv.NewReader(file)
    stringValues, err := reader.ReadAll()
    if err != nil {
        return nil, err
    }

    values, err := stringValuesToFloats(stringValues)
    if err != nil {
        return nil, err
    }

    return values, nil
}

You might want to check that the input of stringValuesToFloats is valid (that every row has the same number of columns).

If you do that, you should document this behavior in ReadFromCsv.

Replace all for foo, _ := range … by for foo := range …. When you're not using the second argument, you don't need to discard it explicitly.

But instead of iterating on values, you could iterate on stringValues, and the inner loop could be for colIndex, s := range …, and you could use s instead of stringValues[rowIndex][colIndex].

var err error = nil → var err error.
When a function returns something, error and you want to return an error, you usually return the default value of something along with the error. So in stringValuesToFloats, you would return nil, err rather than values, err. This helps you make sure that you're not going to use the return value when you return an error.
Instead of printing the error with fmt.Println, you probably want to use log.Errorf and add an error message. Or not print anything at all and let the callers deal with the error (after all, it's why you return it), possibly returning fmt.Errorf("stringValuesToFloat: couldn't parse value %s", err) instead of just err.
Close the file after usage. Add defer file.Close().
Use a csv.Reader with option TrimLeadingSpace instead of trimming whitespace by hand. Your CSV file shouldn't have space after values (arguably, it shouldn't have spaces anywhere).
On "does this look like Go", I'd say you have too much whitespace (empty lines in functions are seldom used), line returns (you never see them right after declarations (:=) or assigments (=)), and your variable names are Java-level verbose. In Go, you would use i instead of rowIndex, floatArray instead of make2dFloatArray, ConvertCSV instead of ReadFromCSV, etc.

And, no, there's no easy way to have something more dynamic that float64, unless you have a specific use case in mind (and then, you would use an interface, but this is out of scope for this question). See this for a detailed discussion on why Go doesn't have generics for now.


Session state wrapper, extending an existing application I'm adding additional functionality to an existing MVC .net application, and to help prevent or at least reduce repeated reads to the dB I'm dumping a few custom entities in session. I'm limiting what can be stored in session to say a max of 5 objects for now.

Given below is the code and it seems generic enough and works fine locally on my box; any suggestions to improve this and session is In-Proc but if it does get stored in a persistent medium say SQL server then I would need to customize this further - decorate custom object properties w/XML element attributes

public class QuestionModelSessionStateProvider : IQuestionModelSessionStateProvider
{
    const int MaxSize = 5;

    public T GetValue<T>(string key)
    {
        HttpSessionState session = GetSessionState();

        T returnValue = default(T);

        if (session == null) return returnValue;

        if (session["QuestionModelStore"] != null)
        {
            var questionModelStore = (List<KeyValuePair<string, T>>)session["QuestionModelStore"];
            foreach (var t in questionModelStore.Where(t => t.Key.Equals(key)))
            {
                returnValue = t.Value;
            }
        }

        return returnValue;
    }

    public void SetValue<T>(string key, T value)
    {
        HttpSessionState session = GetSessionState();

        if (session == null) return;

        List<KeyValuePair<string, T>> questionModelStore;

        if (session["QuestionModelStore"] == null)
        {
            questionModelStore = new List<KeyValuePair<string, T>> { new KeyValuePair<string, T>(key, value) };
            session["QuestionModelStore"] = questionModelStore;
        }
        else
        {
            questionModelStore = (List<KeyValuePair<string, T>>)session["QuestionModelStore"];
            questionModelStore.Add(new KeyValuePair<string, T>(key, value));
        }

        if (questionModelStore.Count > MaxSize)
        {
            questionModelStore.RemoveAt(0);
        }
    }

    public void RemoveValue<T>(string key)
    {
        HttpSessionState session = GetSessionState();
        int indexToRemoveAt = 0;
        var found = false;
        if (session != null && session["QuestionModelStore"] != null)
        {
            var questionModelStore = (List<KeyValuePair<string, T>>) session["QuestionModelStore"];
            for (var i = 0; i < questionModelStore.Count; i++)
            {
                if (questionModelStore[i].Key.Equals(key))
                {
                    indexToRemoveAt = i;
                    found = true;
                    break;
                }
            }

            if (found)
                questionModelStore.RemoveAt(indexToRemoveAt);
        }
    }

    public void Clear()
    {
        HttpSessionState session = GetSessionState();
        if (session != null)
        {
            session["QuestionModelStore"] = null;
        }
    }

    public int Count<T>()
    {

        HttpSessionState session = GetSessionState();
        if (session != null)
        {
            //return session.Count;
            if (session["QuestionModelStore"] != null)
            {
                var questionModelStore = (List<KeyValuePair<string, T>>)session["QuestionModelStore"];
                return questionModelStore.Count;
            }

        }

        return 0;

    }

    private static HttpSessionState GetSessionState()
    {
        if (HttpContext.Current != null)
            return HttpContext.Current.Session;
        return null;
    }
}

There are 4 places where you have code like,

if (session != null)
{
    List<KeyValuePair<string, T>> questionModelStore;

    if (session["QuestionModelStore"] == null)
    {
        questionModelStore = (List<KeyValuePair<string, T>>)session["QuestionModelStore"];

It would be neater if these were a common subroutine:

List<KeyValuePair<string, T>> GetQuestionModelStore<T>()
{
    var session = HttpContext.Current.Session;
    if (!session)
        return null;
    return (List<KeyValuePair<string, T>>)session["QuestionModelStore"];
}

That would simplify code such as:

public T GetValue<T>(string key)
{
    var cached = GetQuestionModelStore();
    if (cached == null)
        return default(T);
    return cached.FirstOrDefault(t => t.Key.Equals(key));
}

It might be slightly faster to use a Queue than a List, because a Queue is designed to let you enqueue at one end and dequeue at the other.

You're not quite implementing a LRU algorithm: if you want to do that, when you find an item in the cache you should move the item to the most-recently-used end of the list.

There's one bug I see, which is that your methods are generic: the first time you call SetValue it creates a List<KeyValuePair<string, T>>; unless all your SetValue and GetValue methods use the same type of T this will cause a run-time error (casting between different types of T). If all your SetValue and GetValue methods do use the same type of T then you might as well specify what T is instead of making it generic.


Checking an array for duplicate values This function checks an array for duplicated values. If it finds a duplicate value, it increments the variable name repeatedTime and then if repeatedTime is greater than 1 it calls a randomize function which produce different values in the array that was passed to the function. It repeats these steps until a duplicated value free array is produced and then notDuplicate becomes true and the function ends.

Are there any improvements that can be made to the function? I am thinking about making it into a recursive function. Is that feasible?

private void CheckForDuplicates(int[] selectedWordIds, List<Integer> answersTempId) {
    boolean notDuplicate = false;

    while(notDuplicate != true){
        int repeatedTime = 0;
        for(int x = 0; x < selectedWordIds.length ; x++)
        {
            for(int y =0; x < selectedWordIds.length;x++)
            {
                if(selectedWordIds[x] == selectedWordIds[y])
                {
                    repeatedTime++;
                }
            }
        }
      if(repeatedTime > 1)
      {
          notDuplicate = false;
          RandomizeFunction(selectedWordIds, answersTempId);
      }
      else{
          notDuplicate    = true;
      }
    }
}

I suggest you use a Set when checking for duplicates:

Set<Integer> set = new HashSet<>();

for(int i : selectedWordIds) {
    if(!set.add(i)) {
        repeatedTime++;
    }
}

This is much simpler that what you have.

Here:

 if(repeatedTime > 1)
 {
     notDuplicate = false;
     RandomizeFunction(selectedWordIds, answersTempId);
 }

The assignment notDuplicate = false is not necessary because notDuplicate is already false.

About recursion: It is possible. With it, here is the final code:

private void CheckForDuplicates(int[] selectedWordIds,
        List<Integer> answersTempId) {

    int repeatedTime = 0;

    Set<Integer> set = new HashSet<>();

    for(int i : selectedWordIds) {
        if(!set.add(i)) {
            repeatedTime++;
        }
    }
    if (repeatedTime > 1) {
        RandomizeFunction(selectedWordIds, answersTempId);
        CheckForDuplicates(selectedWordIds, answersTempId);
    }

}


Check if a variable is nan with Python I want to check if a variable is nan with Python.

I have a working method value != value gives True if value is an nan. However, it is ugly and not so readable. Plus, sonarcloud considers it as a bug for the reason "identical expressions should not be used on both sides of a binary operator".

Do you have a better/elegant solution? Thanks.

import math
import numpy as np

v1 = float('nan')
v2 = math.nan
v3 = np.nan

def check_nan(value) -> bool:
    is_nan =  value != value
    return is_nan

v1_is_nan = check_nan(value=v1)
v2_is_nan = check_nan(value=v2)
v3_is_nan = check_nan(value=v3)

print(v1_is_nan)  # printed True
print(v2_is_nan)  # printed True
print(v3_is_nan)  # printed True

Methods for this already exist, particularly because of the weird properties of NaNs. One of them can be found in the math library, math.isnan and numpy already has a method implemented for this as well, numpy.isnan. They both deal with all three kinds of NaNs shown in your code (but the numpy version is vectorized):

import math
import numpy as np

list(map(math.isnan, [float("nan"), math.nan, np.nan]))
# [True, True, True]

np.isnan([float("nan"), math.nan, np.nan])
# array([ True,  True,  True])

If you are using pandas, pandas.isna is also interesting because it considers None and pandas.NaT (Not a Time) as missing values as well:

import pandas as pd

pd.isna([float("nan"), math.nan, np.nan, None, pd.NaT])
# array([ True,  True,  True,  True,  True])

Your code has a potential flaw, it does not return True only for NaNs (although it does return True for all NaNs), but for all objects which are not equal to themselves. In particular, it will think any instance of this class is NaN (probably true, but not necessarily):

class A:
    def __equal__(self, other):
        return False


Java 8 find array of countup sums I have an array of some arbitrary int values; I want to find the array containing the sum of the past elements in the current index.

Example:

int[] myArray = new int[5]{ 5, 7, 8, 2, 6 };

I would attempt to:

myArray[0] = 5
myArray[1] = myArray[0] + myArray[currentIndex] = 5 + 7
myArray[2] = 5 + 12 + 8 and so forth till my finished array is:
[5, 12, 25, 44, 92]

I have 2 solutions, one in Java 7, and the other in Java 8, and I was wondering if it was possible to get critiqued on both of them, specifically on how to make the code more terse and efficient i.e. cutting out unnecessary steps or operations.

Java 7 (or language agnostic) solution:

public static int[] sumifyIn7(int[] array) {
    int length = array.length;
    for(int i = 1; i < length; i++) {
        int sumSoFar = 0;
        for(int j = 0; j <= i; j ++) {
            sumSoFar += array[j];
        }
        array[i] = sumSoFar;
    }
    return array;
}

Side-note: Am I correct in saying this solution is \$O(nlog(n))\$?

Java 8 solution:

I'm very unfamiliar with Java 8, and plan on getting up to date with it during the school year, but this is what I could put together, but I'm sure it could be improved based on how redundant it seems.

public static int[] sumifyIn8(int[] array) {
    int length = array.length;
    for (int i = 1; i < length; i++) {
        array[i] = Arrays.stream(Arrays.copyOfRange(array, 0, i + 1)).sum();
    }
    return array;
}

Test:

public static void main(String[] args) {
    System.out.println(Arrays.toString(sumifyIn8(new int[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 })));
    System.out.println(Arrays.toString(sumifyIn7(new int[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 })));
}

Test Results:

[1, 3, 7, 15, 31, 63, 127, 255, 511, 1023]

[1, 3, 7, 15, 31, 63, 127, 255, 511, 1023]

Am I correct in saying this solution is O(nlog(n))?

Proof

Whenever there are nested loops,the complexity is measured by number of times inner most loop get executed.
Example

for (i = 1; i <=n; i ++) {
   for (j = 1; j <=i; j ++) {
      // some O(1) expressions
}

inner loop executed=1+2+3+4+...n=(n*(n+1))/2≈ n2.
Hence time complexity is O(n2).

In your case the inner loop is executed 2+3+4+..+n times≈n2.

Hence time complexity of your solution is  O(n2)

Your first approach in Java 7 can be improved by eliminating the inner loop.
Here is an approach which uses only one loop.

    array[1]=array[0]+array[1];
    int sum=array[0];
    int length=array.length;
    for(int i=2;i<length;i++)
    {
        array[i]+=array[i-1]+sum;
        sum+=array[i-1];//storing the sum up to i-1;
    }

Improvement

Eliminated the inner loop.
Added a new variable sum which keeps the summation of the numbers up to index i-2 when calculating array[i].
Time Complexity reduced from O(n2) to O(n).

Hope this helps.


Scanning an NSArray to produce an NSSet I have an NSArray called program and I need to scan it and then produce an NSSet of objects that fit the test isOperation:.  If none are found I should return nil.  I wrote the following code.  How would you make this better? (FYI - the fact the program is of type (id) comes directly from the assignment so I can't change that one thing)

Revised code:

+ (NSSet *) variablesUsedInProgram:(id)program {
    NSSet *programSet, *variablesUsed;
    if([program isKindOfClass:[NSArray class]]) {
        programSet = [NSSet setWithArray:program];
        variablesUsed = [programSet objectsPassingTest:^BOOL(id term, BOOL *stop) {
            return (! ([term isKindOfClass:[NSNumber class]] || [CalculatorBrain isOperation:term]));
        }];
    }
    return ([variablesUsed count] > 0) ? variablesUsed : nil;
}

There is no need to return a copy of the variablesUsed set. It's actually wrong to do so, because naming conventions for your method imply an autoreleased object to be returned. Thus I would rewrite your return as:

return [variablesUsed autorelease];
// or in case of ARC just
return variablesUsed;

See the Advanced memory management programming guide

Also set contains a useful method to filter using a block. In my opinion this results in slightly nicer code. You can convert your incoming array to a NSSet using + setWithArray:.

The if statement to return nil can be combined in your return.

Combining this results in the following implementation

+ (NSSet *) variablesUsedInProgram:(id)program {
    NSSet *programSet = [NSSet setWithArray:program];
    NSSet *variablesUsed = [NSSet objectsPassingTest:^(id object, BOOL *stop) {
        return !([object isKindOfClass:[NSNumber class]] || [self isOperation:term])
    }];
    return ([variablesUsed count] > 0) ? variablesUsed : nil;
}


Transform an object into another object structure I've got such an input object, which I should modify in order to use it with some charts library.

const input = {
  foo: {
    bar: 'biz',
  },
  statistic: {
    "2019-11": {
      A: 11,
      B: 11,
      C: 11,
      D: 11,
      E: 11,
    },
    "2019-12": {
      A: 12,
      B: 12,
      C: 12,
      D: 12,
      E: 12,
    },
    ....
  }
};

My library expects an input like this:

[
  { subject: 'A', '2019-11': 11, '2019-12': 12 },
  { subject: 'B', '2019-11': 11, '2019-12': 12 },
  { subject: 'C', '2019-11': 11, '2019-12': 12 },
  { subject: 'D', '2019-11': 11, '2019-12': 12 },
  { subject: 'E', '2019-11': 11, '2019-12': 12 }
]

And this is my current solution:

const transform = arg => {
  const keys = ['A', 'B', 'C', 'D', 'E'];
  return _.map(keys, key => {
    const data = _.reduce(arg.statistic, (result, value, k) => {
      result[k] = value[key];
      return result;
    }, {});
    return {
      subject: key,
      ...data
    }
  });
}

Any improvements would be very welcome!

So you have five data series here right (A through E)?  What happens when you have 4 or 6 data series?  Do you really want to rewrite your code?

Also, unless you have some reason to do so, lodash seems kind of unnecessary for these sort of simple data transforms these days.  I am guessing using ES6 Array.map, Array.reduce and similar would do the same exact thing while also yielding a more fluent, readable style,


The right way to hang a man Problem:
I have seen a few questions around hangman. Usually this is done in a very hackish way, which usually can not be generalized any further. My thought or question is about the creation of the gown, which is a central part in any hangman game.
I want to create the ASCII image below
 _____
 |   |
 O   |
/|\  |
/ \  |
     |
 ----------

Where one "limb" of the stick figure should appear each time the function is prompted.
I also wanted to be able to specify both the height and width of the gown. However I was not able to scale the stick figure relative to the gown size. The code is provided below and I have two simple questions about it =)
Question:

Is there a cleaner way to hang the man? I feel my method is very barbaric (I used some black voodo graduate mathematics to get it to look correct).
Is there a way to make the stick figure scale with the size of the gown? (essentially providing more guesses).

Code:
from math import ceil

def create_gown(width, height):
    gown = []
    gown.append('{:>{}s}'.format('_'*int(width/2), 10))
    gown.append('{:>{}s} {:>{}s}'.format(
        '|', 11 - int(width/2), '|', int(width/2)-2))
    for i in range(height-3):
        gown.append('{:^{}s}'.format('|', 20))
    gown.append('{:^{}s}'.format('-'*width, 20))
    return gown

def wrong_answer(gown, attempt=0):

    height, width = len(gown), len(gown[-1].strip())

    offset1 = int((-width+23)*0.5)+1
    offset3 = int(ceil(0.5*(width-7)))
    if attempt == 0:
        return gown
    elif attempt == 1:
        new_line = '{:>{}s} {:>{}s}'.format('O', offset1-1, '|', offset3+1)
        row = 2
    elif attempt == 2:
        new_line = '{:>{}s} {:>{}s}'.format('|', offset1-1, '|', offset3+1)
        row = 3
    elif attempt == 3:
        new_line = '{:>{}s} {:>{}s}'.format('/| ', offset1, '|', offset3)
        row = 3
    elif attempt == 4:
        new_line = '{:>{}s} {:>{}s}'.format('/|\\', offset1, '|', offset3)
        row = 3
    elif attempt == 5:
        new_line = '{:>{}s} {:>{}s}'.format('/  ', offset1, '|', offset3)
        row = 4
    elif attempt == 6:
        new_line = '{:>{}s} {:>{}s}'.format('/ \\', offset1, '|', offset3)
        row = 4
    else:
        raise Exception("Ops! The number of attempts must be an integer from 0 to 6.")
    gown[row] = new_line
    return gown

def print_gown(gown):
    for line in gown:
        print line

if __name__ == '__main__':
    gown = create_gown(10, 7)
    print len(gown)
    for i in range(7):
        gown = wrong_answer(gown, i)
        print_gown(gown)

Repetitions

All your elif share the same structure. You just need to ensure that the inserted pattern is always 3 wide and you can drop your offsets adjustments. The logical next step is to use a dictionary to store the pattern and the row for each attempt:

_GOWN_MODIFIER = {
    1: (' O ', 2),
    2: (' | ', 3),
    3: ('/| ', 3),
    4: ('/|\\', 3),
    5: ('/  ', 4),
    6: ('/ \\', 4),
}

def wrong_answer(gown, attempt=0):

    height, width = len(gown), len(gown[-1].strip())

    offset1 = int((-width+23)*0.5)+1
    offset3 = int(ceil(0.5*(width-7)))
    if not attempt:
        return gown

    try:
        pattern, row = _GOWN_MODIFIER[attempt]
    except KeyError:
        raise Exception("Ops! The number of attempts must be an integer from 0 to 6.")
    else:
        gown[row] = '{:>{}s} {:>{}s}'.format(pattern, offset1, '|', offset3)

    return gown

Calculus

You do not need ceil nor int to perform an integer division by 2. In Python 2, / is already the operator you need. You can also use // which is also the same operation. The advantage of the latter over the former is that the behaviour is the same in Python 3, whereas / will perform a decimal division in Python 3.

_GOWN_MODIFIER = {
    1: (' O ', 2),
    2: (' | ', 3),
    3: ('/| ', 3),
    4: ('/|\\', 3),
    5: ('/  ', 4),
    6: ('/ \\', 4),
}

def wrong_answer(gown, attempt=0):
    if not attempt:
        return gown

    height, width = len(gown), len(gown[-1].strip())
    offset1 = (23 - width) // 2 + 1
    offset3 = (width - 6) // 2

    try:
        pattern, row = _GOWN_MODIFIER[attempt]
    except KeyError:
        raise Exception("Ops! The number of attempts must be an integer from 0 to 6.")
    else:
        gown[row] = '{:>{}s} {:>{}s}'.format(pattern, offset1, '|', offset3)

    return gown

def create_gown(width, height):
    half_width = width // 2
    gown = []
    gown.append('{:>{}s}'.format('_'*half_width, 10))
    gown.append('{:>{}s} {:>{}s}'.format(
        '|', 11 - half_width, '|', half_width-2))
    for i in range(height-3):
        gown.append('{:^{}s}'.format('|', 20))
    gown.append('{:^{}s}'.format('-'*width, 20))
    return gown

Creation vs modifications

You're aware that you can create sequences of repeating elements using seq(element) * count. You can use that to create your gown variable by repeating the pole and then modifying the top two rows and the last one; it should be more memory friendly as the space needed to store the list will be allocated in one go:

def create_gown(width, height):
    half_width = width // 2
    gown = ['{:^{}s}'.format('|', 20)] * height
    gown[0] = '{:>{}s}'.format('_' * half_width, 10)
    gown[1] = '{:>{}s} {:>{}s}'.format(
        '|', 11 - half_width, '|', half_width-2)
    gown[-1] = '{:^{}s}'.format('-' * width, 20)
    return gown

On modifying in place and returning a value

and a bit about printing too

Your wrong_answer both modifies the gown in place and return the modified value. This is unnecessary as the caller still should hold a reference to the value (which was modified). When calling the code, you can have:

gown2 = wrong_answer(gown, attempt=3)

but

>>> gown2 is gown
True

So there is really no point in duplicating references to the same object. Instead, what you can do is preprocess the printing job, so you don't really need the print_gown function:

def wrong_answer(gown, attempt=0):
    height, width = len(gown), len(gown[-1].strip())
    offset1 = (23 - width) // 2 + 1
    offset3 = (width - 6) // 2

    if attempt:
        try:
            pattern, row = _GOWN_MODIFIER[attempt]
        except KeyError:
            raise Exception("Ops! The number of attempts must be an integer from 0 to 6.")
        else:
            gown[row] = '{:>{}s} {:>{}s}'.format(pattern, offset1, '|', offset3)

    return '\n'.join(gown)

Usage:

print(wrong_answer(gown, attempt=3))

Exceptions

Raising a generic purpose Exception is bad practice, as it makes the except clauses trying to handle your code being able to catch more than they should. You should at least use a more generic exception (such as ValueError) or define your own:

class AlreadyHangedError(ValueError):
    pass

def wrong_answer(gown, attempt):
    ...
    raise AlreadyHangedError('...')
    ...

However, there is something odd in the way you use your wrong_answer: the caller is expected to call this function using increasing attempts. This is the job for a generator. By turning wrong_answer into a generator, you can call it to create a generator instance and then call next on this instance (or let a for loop do it for you) to get the next gown to print:

def wrong_answer(width, height):
    gown = create_gown(width, height)
    offset1 = (23 - width) // 2 + 1
    offset3 = (width - 6) // 2

    yield '\n'.join(gown)
    for attempt in range(1, 7):
        pattern, row = _GOWN_MODIFIER[attempt]
        gown[row] = '{:>{}s} {:>{}s}'.format(pattern, offset1, '|', offset3)
        yield '\n'.join(gown)

Usage:

for gown in wrong_answer(10, 7):
    print(gown)

or, in a more real-case scenario:

def hangman_game():
    for gown in wrong_answer(10, 7):
        while True:
            status = manage_user_input()
            if status == 'FAILED':  # Whatever
                break
            if status == 'COMPLETED':  # Whatever
                return
        print gown
    print 'You lose'

Magic numbers

create_gown seems to center the gown in 20-sized strings, but what if width is greater than that? At first, you should define this value and give it a meaningful name, and same for all values that derive from it; and then, you might want to take max(20, width) as a base for your calculus.


JavaScript traits implementation I wanted to step up my JS game, and move on from mostly closure based scripting, so I decided to write an application in node. I don't have much experience with prototype based programming, and I guess it's a little too early for ES6 (well, maybe with traceur).

During planning of my app I saw that often I'll need container behaviour. So, I know I could implement it with a class, but I thought: why not use trait-like thingy instead?

Gamemode.js, concrete class

'use strict';

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

var Gamemode = module.exports = function () {
    Script.apply(this, arguments);
};

Gamemode.prototype = Object.create(Script.prototype);
Gamemode.prototype.constructor = Gamemode;

Script.js, abstract class

'use strict';

var helper = require("./helper");
var _ = require("lodash");

var Script = module.exports = function () {
    if (this.constructor === Script) {
        throw new Error("Can't initialize abstract class!");
    }

    var defaultDefinitions = {
        MAX_PLAYERS: '(500)'
    };

    _.merge(this, helper.collection("definition", defaultDefinitions));
};

Script.prototype.build = function () {
    return this.definitions;
};

helper.js

'use strict';

var _ = require("lodash");

var capitalize = function (string) {
    return string.charAt(0).toUpperCase() + string.slice(1).toLowerCase();
};

var container = function (singular, mutable) {
    if (undefined === mutable) {
        mutable = false;
    }

    var accessors = {},
        methodSingular = capitalize(singular),
        plural = singular + 's',
        methodPlural = capitalize(plural);

    accessors['get' + methodPlural] = function () {
        return this[plural];
    };

    accessors['set' + methodPlural] = function (value) {
        if (undefined === value) {
            delete this[plural];
        }

        this[plural] = value;

        return this;
    };

    if (mutable) {
        accessors['add' + methodSingular] = function (item) {
            _.assign(this[plural], item);
        };

        accessors['remove' + methodSingular] = function (item) {
            delete this[plural][item];
        };
    }

    return accessors;
};

var collection = function (name, defaults) {
    if (undefined === defaults) {
        defaults = {};
    };

    var all = {};
    all[name + 's'] = _.defaults({}, defaults);

    return _.merge(all, container(name, true));
};

var helper = module.exports = {
    capitalize: capitalize,
    container: container,
    collection: collection
};

It works properly, now every instance of Gamemode has its own "definitions" property, with getDefinitions, setDefinitions, addDefinition and removeDefinition methods.

Am I violating any rules, like encapsulation?
Am I adding property somewhere else than in the constructor?
Would creating a "container" object instead be better?

Interesting question,

this code pass JSHint, it is readable, well organized and except for return definitions; (where did you declare definitions?) holds few mysteries to me.

Still, when I take a step back, why not simple use a bog standard Object? It seems as if you just re-invented the Object with few added benefits.


Generic Entity Framework Query in CQRS I've been working on our application (CQRS and DDD) for awhile now. The application architecture is well layered and thought through.

However we are having difficulties in decide where to put the below code, in such a way that it makes sense from an architectural point of view.

What the code does: It's a generic method which executes a query against the dataset, specified by the generic parameter which inherits a particular interface.
Note: we are not using repositories, since the query is, as the name indicates, for our read layer.

The interface, used by the generic method:

public interface IOrganisationalDbEntry
{
    string OrganisationLevelId { get; set; }
    OrganisationLevelDbEntry OrganisationLevel { get; set; }
}

The generic method, which lives inside our Entity Framework DbContext:

public IList<TDbEntry> GetByOrganisationLevel<TDbEntry>(Guid? organisationLevelId,
    Func<TDbEntry, bool> filter = null)
    where TDbEntry : class, IOrganisationalDbEntry
{
    var organisationLevel = organisationLevelId.HasValue ? organisationLevelId.Value.ToString() : null;

    var query = filter == null ? Set<TDbEntry>() : Set<TDbEntry>().Where(filter);

    IList<TDbEntry> result = query.Where(x => x.OrganisationLevelId == organisationLevel).ToList();

    // If there are no results for the current organisation level
    // => recursive check for entries for parent organisation level
    if (!result.Any() && !string.IsNullOrWhiteSpace(organisationLevel))
    {
        // Get parentOrganisationLevel
        var parentOrganisationLevelId = Set<OrganisationLevelDbEntry>()
            .Where(x => x.Id == organisationLevel)
            .Select(x => x.ParentOrganisationLevelId)
            .SingleOrDefault();

        return
            GetByOrganisationLevel<TDbEntry>(
                string.IsNullOrWhiteSpace(parentOrganisationLevelId)
                    ? (Guid?)null
                    : new Guid(parentOrganisationLevelId), filter);
    }

    return result;
}

As you may be thinking, we have several EF Entities inheriting the IOrganisationalDbEntry. What comes to mind first, is a base Repository. But We do not want to use repositories in our query side, since we want this layer to be as flat as possible.

The next thing that comes to mind is a specific query and queryhandler for this query. Tho this is not acceptable since this method accepts a Func parameter to be able to add extra filter criteria while calling this method, to make it more flexible.

At this moment, this method is living as method on our IDbContext and DbContext implementation. However we are not satisfied with this.  We are wondering whether there is a specific pattern or approach for our case.  Imagine we have more of these methods, I do not want to keep expanding our context. Since this is not what the DbContext should be used for.

EDIT: Everywhere you read CQRS in the above question , replace it by CQS. I didn't want to modify the question since it would make some comments irrelavant.

I'll give it a shot since CQRS is something I'm still learning.

In CQRS we are supposed to separate our read model from our write model. Is this what you have done?

Then the read model should be designed to be a perfect fit for the thing that needs to read it:

Your query looks very complex. Is it derived from the write model, or do you indeed have separate tables for the queries?
Why a Func as a parameter? It seems strange to me that the criteria should be a negotiation between client and query.
Why do you put the query method onto the DbContext? That class will soon enough be cluttered with too many queries.

Here are some suggestions to simplify things:

Let the DbContext mimic the database tables only and nothing else. That means your DbContext will only have public properties for the tables you are going to query.
Don't call it a Repository, but a Query. Split the Query-class when it grows too big.
Let the Query class take the DbContext as a dependency. Query the DbContext-properties, don't use Set<T>.
Let each query-method accept one criteria object as parameter, then build the query expression from that inside the method.
Linq is tricky. While not entirely true, a rule of thumb is that a Func runs client side, while an Expression runs server side. That's also why I don't like the Func-parameter.
Consider using only non-generic query-methods. Yes, there will be some duplication, but you want your peers and the future you to understand the queries two months from now.

I meant to post some code examples, but Chrome crashed again and edits got lost :-(


Performing a "mean blur" I perform a mean blur on a number[][]-array (called grid in the code). I iterate the array, and calculate average values from adjacent "cells".
The kernel size defines how many adjacent values are used for average calculation.

/**
 * Performs a mean blur with a given kernel size.
 * @param {number[][]} grid - Data grid holding values to blur.
 * @param {number} kernelSize - Describes the size of the "blur"-cut for each cell.
 */
static MeanBlur(grid, kernelSize) {
    // Result
    let newGrid = [];

    // Iterate original grid
    for (let row = 0; row < grid.length; row++) {
        let newRow = [];
        for (let col = 0; col < grid[row].length; col++) {

            let adjacentValues = [];

            // Get all adjacent values
            for (let i = 1; i <= kernelSize; i++) {
                adjacentValues = adjacentValues.concat(this._getAdjacentValues(row, col, i, grid));
            }

            // Calculate average value
            const average = adjacentValues.reduce((a, b) => a + b, 0) / adjacentValues.length;

            // add average value to the current row
            newRow.push(average);
        }
        newGrid.push(newRow);
    }
    return newGrid;
}

/**
 * Return all adjacent values of a cell in a `number[][]`-array.
 * The amount of adjacent value depends on the kernel size.
 * @param {number} row - Describes the cell's row position.
 * @param {number} col - Describes the cell's column position.
 * @param {number} kernelSize - The kernel size.
 * @param {number[][]} grid - Original data grid.
 */
static _getAdjacentValues(row, col, kernelSize, grid) {

    if (kernelSize < 1) {
        throw "Kernel size value should be at least 1.";
    }

    let adjacentValues = [];

    // north
    if (row - kernelSize >= 0) {
        adjacentValues.push(grid[row - kernelSize][col]);
    }
    // north-east
    if (row - kernelSize >= 0 && col + kernelSize < grid[row].length) {
        adjacentValues.push(grid[row - kernelSize][col + kernelSize]);
    }
    // east
    if (col + kernelSize < grid[row].length) {
        adjacentValues.push(grid[row][col + kernelSize]);
    }

    // south-east
    if (row + kernelSize < grid.length && col + kernelSize < grid[row].length) {
        adjacentValues.push(grid[row + kernelSize][col + kernelSize]);
    }

    // south
    if (row + kernelSize < grid.length) {
        adjacentValues.push(grid[row + kernelSize][col]);
    }

    // south-west
    if (row + kernelSize < grid.length && col - kernelSize >= 0) {
        adjacentValues.push(grid[row + kernelSize][col - kernelSize]);
    }

    // west
    if (col - kernelSize >= 0) {
        adjacentValues.push(grid[row][col - kernelSize]);
    }

    // north-west
    if (row - kernelSize >= 0 && col - kernelSize >= 0) {
        adjacentValues.push(grid[row - kernelSize][col - kernelSize]);
    }

    return adjacentValues;
}

What I am most concerned with is _getAdjacentValues. I think there has to be a more convenient way to get adjacent values without including values at implausible indices.

What I mean is that i.e. at the grid position (0, 0) even with a kernel size of 1, values like (-1, -1) will be checked automatically by a loop.

Edit:
I was asked for clarification on how the kernel size actually is supposed to work.
Assuming that I want to calculate the value for the current position (black cell). With a kernel size of 1 I would include all green cells for average value calculation. With a kernel size of 2 I would include all green and blue cell values. And with a kernel size of 3 all orange, blue and green cell values will be used for average calculation.

Convolution Filter
In CG this type of processing is call a convolution filter and there are many strategies used to handle edges
As the previous answer points out for performance you are best to use typed arrays and avoid creating arrays for each cell you process.
In your example

for (let i = 1; i <= kernelSize; i++) {
    adjacentValues = adjacentValues.concat(this._getAdjacentValues(row, col, i, grid));
}
const average = adjacentValues.reduce((a, b) => a + b, 0) / adjacentValues.length;

You can do the reduce in the loop you create the sub array in, and avoid the 9 new arrays you create and the need to iterate again.
Example substitute value at edges.
const edgeVal = 0;
var val, sum = 0;
for (let i = 0; i < 9; i++) {
    val = grid[row + (i / 3 | 0) - 1];
    if (val !== undefined) {
         val = val[col + (i % 3) - 1];
         sum += val !== undefined ? val : edgeVal;
    } else {
         sum += edgeVal;
    }
}
const average = sum / 9;

Or ignore edges
var val;
var sum = 0;
var count = 0;
for (let i = 0; i < 9; i++) {
    val = grid[row + (i / 3 | 0) - 1];
    if (val !== undefined) {
         val = val[col + (i % 3) - 1];
         if (val !== undefined) {
            sum += val;
            count ++;
         }
    }
}
const average = sum / count;

Better yet, work on flattened arrays to avoid the double indexing for 2D arrays, and create the result array at the start rather than pushing to it each cell.
Workers
Convolution filters are ideally suited for parallel processing with an almost n / p performance increase (n is number of cells, p is number of processing nodes).
Using web workers on a i7 4 core (8 virtual cores) can give you a 8 times performance increase (On chrome (maybe others) you can get a core count at window.clientInformation.hardwareConcurrency)
Attempting to use more workers than available cores will result in slower processing.
GPU
For the ultimate performance the GPU, via webGL is available and will provide realtime processing of very large data sets (16M and above), A common non CG use is processing layers in convolutional neural network.
An example of a convolution filter (About halfway down the page) using webGL can easily be adapted to most data types, however doubles will only be available on a small number of hardware setups.


Largest LCM of partitions of a number Suppose there is a number N which can be divided into two positive non zero numbers X and Y i.e N = X + Y. There will be N/2 pairs of (X,Y).

I want to find the largest LCM(Least common multiple) of X & Y

  Example
  5 = (1 + 4) and 5 = (2 + 3)
  Since LCM(1,4) = 4 < LCM(2,3) = 6 the largest LCM will be LCM(2,3) = 6.

CODE

#include <stdio.h>
int lcm(int, int);
int main(void)
{
     int counter = 0;
     int lowerBound = 0, upperBound, input, result; // input is the n
     printf("input = ");
     scanf("%d",&input);
     int max = -1;
     upperBound = input; // at first upperBound will be the input
     for(counter=0; counter < input/2; counter++) // we will iterate till the input/2 terms
     {
         lowerBound++;
         upperBound--;

         // in every iteration we will compute the lcm of lowerBound and upperBound
         // so 1st iteration : compute lcm(1, n-1)
         // 2nd iteration    : compute lcm(2, n-2)
         // 3rd iteration    : compute lcm(3, n-3)
         // .......after ((n/2)-1)th iteration.....
         // if n is even     : compute lcm(n/2, n/2)
         // if n is odd      : compute lcm((n/2)-1, n/2)

         result = lcm(lowerBound,upperBound); // store the result

         if(result >= max) // if result is maximum update the maximum
         {
             max = result;
         }
     }

     printf("output = %d",max);
    return 0;
}

int lcm(int x, int y)
{
    int n1 = x, n2 = y;
    while(n1 != n2)
    {
        if(n1 > n2)
           n1 = n1 - n2;
        else
        n2 = n2 - n1;
    }
  return((x*y) / n1);
}

OUTPUT 1

input = 10
output = 21

OUTPUT 2

input = 11
output = 30

UPDATE 1

After Aseem Bansal's answer I try to find a more optimize way to solve the problem and i came up with this code

#include<stdio.h>
int main(void)
{
    int input, result = 0;
    printf("Enter the input = ");
    scanf("%d",&input);

    while(input <= 0) // if input is negative ask again
    {
        printf("Enter a positive non zero number :");
        scanf("%d",&input);
    }

    int middle = (input/2);
    if(input%2 != 0) // if the input is an odd number
    {
        result = (middle * (middle+1));
    }

    else // if input is an even number
    {
        result = middle%2 == 0 ? ((middle-1) * (middle+1))
                               : ((middle-2) * (middle+2));
    }

    printf("result = %d",result);
    return 0;
}

OUTPUT 1

Enter the input = 13
result = 42

OUTPUT 2

Enter the input = 12
result = 35

OUTPUT 3

Enter the input = 10
result = 21

EXPLANATION

As for any odd number the middle and middle+1 are highest coprime numbers, so the GCD will be 1. Therefore LCM will be product of middle and middle+1.

But for any even number the LCM of middle and middle+1 is 1. So we need to find the nearest coprime numbers. If middle is even then the nearest coprime numbers are middle-1 and middle+1 otherwise the nearest coprime numbers are middle-2 and middle+2. The product will be the result.

NOTE

here middle is floor of (middle).

Can it be more optimized? Any review is welcome.

I am not sure about the overall algorithm but there are better ways for finding LCM. You should use a different algorithm. Try Euclid's algorithm to find the GCD.
Then calculate result(your LCM) in main() as
result = (lowerbound * upperbound)/gcd(lowerbound upperbound);
The problem with your LCM function is that subtraction isn't the best way for finding the GCD which you are currently doing. It will take too long. Just consider the case of x = 1 and y = 100000. It should be easy to see that number of subtractions is much higher in this case and the program would be very slow.

If you don't want to change the algorithm(which you should) then the least optimization that I can think is to not declare n1 and n2 separately in the function lcm. Just use x and y. Do the final division by the product in the main function. It will at least save you 2 variables.

EDIT
I came up with a much better optimization. In your main loop instead of going from the outside pairs i.e (1, n-1) to the inside pairs you would do much better by going from the inside.
Example

5 = (1,4) and 5 = (2,3). The maximum is found at 5 = (2,3).
7 = (1,6) , 7 = (2,5) and 7 = (3,4). The maximum is at 7 = (3,4).

The answer is in the middle. You'll get it without a loop.
But there is a big problem. This reasoning works only for odd numbers. I tried writing down numbers on paper and solved it by hand. I'll need some more time to find out a pattern for even numbers.
But for odd numbers this is probably the best optimization.
EDIT2
In the main function instead of
if(result >= max) you should be using if(result > max). What is the use of replacing the number with the same number? One less operation.
You don't need to initialize counter at the beginning. You are doing that in the for loop.
EDIT3:
For the case of even numbers the maximum lcm will be either lcm((input/2)-2, (input/2)+2) or lcm((input/2)-1, (input/2)+1). It's easy to find which will be maximum without calculating both of them.
If input/2 is even then lcm((input/2)-1, (input/2)+1) should be maximum otheriwse it should be lcm((input/2)-2, (input/2)+2).

EDIT After OP's Update 1
How about this as a better code
#include<stdio.h>

int main(void)
{
    int input, result;
    printf("Enter the input = ");
    scanf("%d",&input);

    while (input <= 0)
    {
        printf("Enter a positive non zero number :");
        scanf("%d",&input);
    }

    // (x % 2) used as a condition in C means checking x for odd number
    int middle = input/2;
    result = input % 2  ? (middle % 2 ? middle * middle - 4 : middle * middle - 1)
                        :  middle * (middle + 1);

    printf("Result = %d",result);
    return 0;
}

Removed unnecessary braces, spaces, comparisons and comments. Comments are needed but commenting obvious things is just noise. I prefer single space on both sides of operators.


Key to Value / Value to Key - java I want to get the key from value / and value from key ,can anyone help to do in better way ? or can we implement the below code using java enum or spring or apache enumutils or java generic ?

Note : I used to get string from frond end(.jsp) and I want to store store this value into database table as integer.

Frond end  Database table
Admin   ---   1
Manager ---   2
Employee ---  3

import java.util.HashMap;
import java.util.Map;

public class   MyRole  {

    static HashMap<Integer,String> result= new HashMap<Integer,String>();
    static{
    result.put(1,"Admin");
    result.put(2,"Manager");
    }

    public static int getId(String role){
          Integer key=-1;
          for(Map.Entry entry: result.entrySet()){
                if(role.equals(entry.getValue())){
                    key = (Integer) entry.getKey();
                    break;
                }
            }

      return key;

    }
    public static String getRole(int id){
        return result.get(id);
    }

    public static void main(String args[]){

        String result=MyRole.getRole(1);
        System.out.println("role name"+result);
        int roleId=MyRole.getId("Admin");
        System.out.println("role id "+roleId);
    }
}

I want make sure , whether the above program is fine or can we achieve via Java enum or generic or spring api or apache enumutils api ?

Using an enum will only help if you know all the values/keys at compile time. If your data is coming from a database, then you can't know that, so you can't use an enum.

Note that your question assumes a double-key - the String value is unique, and the int value is also unique.

There are three common ways to do this problem, the easiest way is the one you have done, and then store the data in a HashMap with the most commonly used lookup as the key, and the less-common lookup as the value. This makes it quick to find the most common search on the key, and slower, but possible, to find the value.

I would improve your code by throwing an exception when the lookups fail to find a matching value. If someone asks getRole(-1) it should throw an exception... right?

The easiest solution for this type if problem is possible if you can assume that the key domain and value domain have no overlap (a value cannot also be a key). Since your keys and values are ints and Strings, it's clear there is no overlap.

To use this solution, create a "mapping" class, call it IDRole or something:

private static final class IDRole {
    private final int id;
    private final String role;
    IDRole(int id, String role) {
        this.id = id;
        this.role = role;
    }
}

Now your class can have just one HashMap<Object,IDRole> data store, and you can save each IDRole twice, once with the ID key, and once with the Role key.

private static final HashMap<Object, IDRole> mapping = new HashMap<>();

private static final addMapping(int id, String role) {
    IDRole idr = new IDRole(id, role);
    if (mapping.putIfAbsent(id, idr) != null) {
        throw new IllegalArgumentException("Cannot add duplicate mapping for " + id);
    }
    if (mapping.putIfAbsent(role, idr) != null) {
        throw new IllegalArgumentException("Cannot add duplicate mapping for " + role);
    }
}

OK, so, with the above, you can add a mapping that maps both the key, and value, in to the HashMap.

Your lookups are now easy too:

public static String getRole(int id){
    IDRole ird = mapping.get(id);
    if (idr == null) {
        throw new IllegalArgumentException("No mapping for input ID " + id);
    }
    return idr.role;
}

The getID would be similar.


multiple instances of maps and markers using object oriented javascript This is my first attempt at object oriented javascript code. I have been writing JavaScript code for few years now but this is first time I am trying to write OO javascript code. I have read about OO javascript, but I am not still very confident about it.

Could you please review it and provide your comments about this implementation of OO javascript?

"use strict";

var maps_1, maps_2, maps_3, maps_4;
var markers_array1 = [], markers_array2 = [], markers_array3 = [], markers_array4 = [];

var google_map = function (x,y,z) {
    this.map = new google.maps.Map(document.getElementById(x), {
        zoom : parseInt(z),
        center : y,
        mapnameId : google.maps.MapnameId.ROADMAP,
        disableDefaultUI : true,
        styles : {
            featurename : "poi",
            elementname : "labels",
            stylers : [{
                    visibility : "off"
                }
            ]
        }
    });
};

var google_marker = function(x,map) {
    var icon_url =  'icons/' + x.dname + '_' + ((x.dname2 === 0) ? '0.png' : '1.png');
    var lat = parseFloat(x.lat), lng = parseFloat(x.lng);

    if(lat*lng > 0) {
        this.marker = new google.maps.Marker({
                icon : icon_url,
                position: {"lat":lat,"lng":lng},
                description : ''+x.name+'
'+x.desc,
                title: x.dname,
                start: x.start,
                map: map
            });

        this.marker.addListener('click', infoWindow);
    }
}

function loadJson() {
    var xmlhttp = new XMLHttpRequest();
    xmlhttp.open("POST", "ajax.php", true);
    xmlhttp.setRequestHeader("Content-Type", "application/x-www-form-urlencoded");
    xmlhttp.send(data);

    xmlhttp.onreadystatechange = function () {
        if (xmlhttp.readyState == 4) {
            loadMarkers(JSON.parse(xmlhttp.response));
        }
    }
}

function init() {
    maps_1 = new google_map('maps_1',{"lat" : 32.93,"lng" : 85.06},12);
    maps_2 = new google_map('maps_2',{"lat" : 70.88,"lng" : 45.97},12);
    maps_3 = new google_map('maps_3',{"lat" : 60.49,"lng" : 38.00},12);
    maps_4 = new google_map('maps_4',{"lat" : 85.85,"lng" : 89.63},12);

    loadJson();
}

function loadMarkers(Markers) {

    for(var y in Markers) {
        var x  = Markers[y];
        markers_array1.push(new google_marker (x,maps_1.map));
        markers_array2.push(new google_marker (x,maps_2.map));
        markers_array3.push(new google_marker (x,maps_3.map));
        markers_array4.push(new google_marker (x,maps_4.map));
    }
}
init();

The problem I see on your code is that you try to be OO but still thinking in a procedural way.

for example, your code is full procedural, as you didn't design your class, instead you just wrote the code in the sequence it will be executed.

So far it is not so bad written for a procedural one.

Let's go on to explain the points.

What is the purpose to wrap a google map inside a function that contains only an attribute that is the map?

How do you use it:

maps_1 = new google_map('maps_1',{"lat" : 32.93,"lng" : 85.06},12);
maps_2 = new google_map('maps_2',{"lat" : 70.88,"lng" : 45.97},12);
maps_3 = new google_map('maps_3',{"lat" : 60.49,"lng" : 38.00},12);
maps_4 = new google_map('maps_4',{"lat" : 85.85,"lng" : 89.63},12);

So why you just don't directly create the google map instead?

What mean thinking in a object orientation way. you should first think about the object you use.

In your case you have a Map, that have reference to a DOM element, a configuration, and a list of markers.

// Please provide meaningful names for parameter
var Map = function (elementId, mapConf) {
    this.markers = []; //  Will use later
    this.elementId = elementId;
    var domElement = document.getElementById(this.elementId);
    this.map = new google.maps.Map(domElement, {
        zoom : parseInt(mapConf.zoom),
        center : mapConf.center,
        mapnameId : google.maps.MapnameId.ROADMAP,
        disableDefaultUI : true,
        styles : {
            featurename : "poi",
            elementname : "labels",
            stylers : [{
                    visibility : "off"
                }
            ]
        }
    });
};

// Now you need some methods
Map.prototype.addMarkers = function(markers) {
    markers.forEach(function (marker) {
        this.markers.push(new Marker(marker, this.map, infoWindow));
    }.bind(this));
};

Now you need a Marker class:

var Maker = function (markerConf, map, clickHandler) {
    var icon_url =  'icons/' + markerConf.dname + '_' + ((markerConf.dname2 === 0) ? '0.png' : '1.png');
    var lat = parseFloat(marketConf.lat), lng = parseFloat(marketConf.lng);

    if(lat*lng > 0) {
        this.marker = new google.maps.Marker({
            icon : icon_url,
            position: {"lat":lat,"lng":lng},
            description : ''+x.name+'
'+x.desc,
            title: marketConf.dname,
            start: marketConf.start,
            map: map
        });

        // You should handle this by your class (the Marker)
        // or by configuration, like shown here
        // so it's flexible
        if (clickHandler) {
            this.marker.addListener('click', clickHandler);
        }
    }
};

So your init function will be:

function init() {
    maps_1 = new Map('maps_1',{"lat" : 32.93,"lng" : 85.06},12);
    maps_2 = new Map('maps_2',{"lat" : 70.88,"lng" : 45.97},12);
    maps_3 = new Map('maps_3',{"lat" : 60.49,"lng" : 38.00},12);
    maps_4 = new Map('maps_4',{"lat" : 85.85,"lng" : 89.63},12);

    performPostAjaxRequest("ajax.php", function (data) {
        maps_1.addMarkers(data);
        maps_2.addMarkers(data);
        maps_3.addMarkers(data);
        maps_4.addMarkers(data);
    });
}

And loadJson will change to be more general purpose:

function performPostAjaxRequest(url, cb) {
    var xmlhttp = new XMLHttpRequest();
    xmlhttp.open("POST", url, true);
    xmlhttp.setRequestHeader("Content-Type", "application/x-www-form-urlencoded");
    xmlhttp.send(data);

    xmlhttp.onreadystatechange = function () {
        if (xmlhttp.readyState == 4) {
            cb(JSON.parse(xmlhttp.response));
        }
    }
}

This is just a first step, a lot more could be done, the purpose of OO is to design the objects you need as classes, and put all the logic that is connected with those objects inside the class.
You should not have a function that know how to attach a Marker object to a Map object. Instead, the Map object should know how to do that. You just need to know that Maps need Markers.
When you define a class it's important to start the name with a capital letter, to distinguish a class constructor from a normal function.


Checking whether a timestamp is 10 minutes old I have a timestamp (searchTimestamp) which I need to check to see whether it is less than 10 minutes old or not:

long currentTimestamp = System.currentTimeMillis();

long searchTimestamp = getTheTimestampFromURL();// this also gives me back timestamp in 13 digit (1425506040493)

long difference = Math.abs(currentTimestamp - searchTimestamp);

System.out.println(difference);

if (difference > 10 * 60 * 1000) {
    System.out.println("timestamp is greater than 5 minutes old");
}

I have got the above code which is working fine. Is this the right way to do this or is there any better way?

getTheTimestampFromURL will always be older than the current timestamp in milliseconds.

Your description says that you want to check whether the searchTimestamp is less than 10 minutes old. Your code does something different, though.

Your code checks whether there's less than 10 minutes between the times (the difference is <= 10 minutes .... If the searchTimestamp is 2 minutes in the future, it will pass the test, if it is 9 minutes in the future, it will pass the test, and if it is 11 minutes in the future, it will fail the test.

Of interest, the math you use does a 10 minute check, but the message says "greater than 5 minutes old."

Your message should say: "timestamp is older than 10 minutes, or more than 10 minutes in the future"

So, changing your code to be what I think it should be, is a lot simpler than you would think.

What you want is for the searchTimestamp to have happened sometime after 10 minutes ago.... this is the way to do it:

private static final int TEN_MINUTES = 10 * 60 * 1000;

then, in your method:

long tenAgo = System.currentTimeMillis() - TEN_MINUTES;
if (searchTimestamp < tenAgo) {
    System.out.println("searchTimestamp is older than 10 minutes");
}


Complex if statements in authentication method I had asked questions about my entire class which I'll flag for closing soon. It appeared to be a little too broad for 1 post so I chopped it up and will be asking a little more specific questions separately.

In my Application Layer I have a class AuthService containing the following method with a code structure that has gotten quite complex with if statements that I'm unable to re-factor a better way.

I'd appreciate anybody who could review this piece of code and give me suggestions on  improving / optimizing the design of it. Any other suggestions are welcome as well.

public function findLoggedInUser()
{
    if (($userData = $this->httpSession->getParameter('user')) && isset($userData['id'])) {
        // User found in current http session
        return $this->userMapper->findById($userData['id']);
    } elseif ($httpSessionData = $this->httpCookie->getParameter('usess')) {
        if (isset($httpSessionData['uid'], $httpSessionData['sno'], $httpSessionData['token'])) {
            if ($user = $this->userMapper->findById($httpSessionData['uid'])) {
                if ($httpSession = $this->httpSessionMapper->findByUserAndSeriesNo($user, $httpSessionData['sno'])) {
                    if (password_verify($httpSessionData['token'], (string) $httpSession->getTokenHash()) {
                        // Generate new http session entity tokens
                        $token['raw']  = new Token(CryptoCharGen::numeric(60));
                        $token['hash'] = new TokenHash(password_hash((string) $token['raw'], PASSWORD_BCRYPT, ['cost' => 4]));

                        // Update and persist http session entity with new token
                        $httpSession->setTokenHash($token['hash']);

                        $this->httpSessionMapper->update($httpSession);

                        // Issue new http cookie usess with updated http session recovery data
                        $this->httpCookie->setParameter('usess', [
                            'uid'   => (string) $httpSession->getUser()->getId(),
                            'sno'   => (string) $httpSession->getSeriesNo(),
                            'token' => (string) $token['raw']
                        ], 60 * 60 * 24 * 90);

                        // User has been found
                        return $httpSession->getUser();
                    } else {
                        // Token invalid, sabotage assumed
                        // Invalidate all persisted http sessions of this user
                        $this->httpSessionMapper->deleteAllByUser($user);

                        $this->httpCookie->destroy('usess');
                    }
                } else {
                    // Destroy invalid cookie
                    $this->httpCookie->destroy('usess');
                }
            } else {
                // Destroy invalid cookie
                $this->httpCookie->destroy('usess');
            }
        } else {
            // Destroy invalid cookie
            $this->httpCookie->destroy('usess');
        }
    }
}

Two suggestions on how to keep this code maintainable:

use early exits
keep conditions atomic (one if checks for one thing)

Example:

function findLoggedInUser()
{
    if (($userData = $this->httpSession->getParameter('user')) && isset($userData['id'])) {
        // User found in current http session
        return $this->userMapper->findById($userData['id']);
    }

    $httpSessionData = $this->httpCookie->getParameter('usess');
    if(!$httpSessionData) {
        $this->httpCookie->destroy('usess');
        return null;
    }

    if(!isset($httpSessionData['uid'], $httpSessionData['sno'], $httpSessionData['token'])) {
        $this->httpCookie->destroy('usess');
        return null;
    }

    $user = $this->userMapper->findById($httpSessionData['uid']);
    if(!$user) {
        $this->httpCookie->destroy('usess');
        return null;
    }

    $httpSession = $this->httpSessionMapper->findByUserAndSeriesNo($user, $httpSessionData['sno']));
    if(!$httpSessionData) {
        $this->httpCookie->destroy('usess');
        return null;
    }

    if (!password_verify($httpSessionData['token'], (string) $httpSession->getTokenHash()) {
        $this->httpSessionMapper->deleteAllByUser($user);
        $this->httpCookie->destroy('usess');
        return null;
    }

    // EVERYTHING'S FINE, DO THE JOB

    // Generate new http session entity tokens
    $token['raw']  = new Token(CryptoCharGen::numeric(60));
    $token['hash'] = new TokenHash(password_hash((string) $token['raw'], PASSWORD_BCRYPT, ['cost' => 4]));

    // Update and persist http session entity with new token
    $httpSession->setTokenHash($token['hash']);

    $this->httpSessionMapper->update($httpSession);

    // Issue new http cookie usess with updated http session recovery data
    $this->httpCookie->setParameter('usess', [
        'uid'   => (string) $httpSession->getUser()->getId(),
    'sno'   => (string) $httpSession->getSeriesNo(),
    'token' => (string) $token['raw']
        ], 60 * 60 * 24 * 90);

    // User has been found
    return $httpSession->getUser();
}

This way, the code remains readable and flexible for future modifications.


Get distinct of two columns I want to get distinct of two columns from an SQL table. Can I optimize this query?

create TABLE #Temporary_tbl
(
    ProductColour VARCHAR(50),
    ProductSize VARCHAR(20),
)

insert into #Temporary_tbl (ProductColour)
select distinct productcolour
from shoptransfer

insert into #Temporary_tbl (ProductSize)
select distinct ProductSize
from shoptransfer

select * from #Temporary_tbl

This will give you the same result as your sql.

select distinct ProductColour, null as ProductSize
from shoptransfer

union all

select distinct null as ProductColor, ProductSize
from shoptransfer

You don't actually need the "as" clause on the second select, but I find it good for readability.

Ultimatly, sql server will mostly be doing the same thing, but by removing the explicit temporary table and putting it all into one command, you give sql server a better chance of doing the two selects in parallel.


Work order spatial query (Part 1) Part 1: General Cleanup

(Edit: Part 2 is here.)

I have a script in a Work Order Management System (Maximo) that performs a spatial query.

Details:

Takes the X&Y coordinates of a work order in Maximo
Generates a URL from the coordinates (including other spatial query information)
Executes the URL via a separate script/library (LIB_HTTPCLIENT)
Performs a spatial query in an ESRI REST feature service (a separate GIS system)
JSON text is returned to Maximo with the attributes of the zone that the work order intersected
The zone number is parsed from the JSON text via a separate script/library (LIB_PARSE_JSON)
Inserts the zone number into the work order record

Jython main automation script

from psdi.mbo import MboConstants
from java.util import HashMap

#Get the y and x coordinates(UTM projection) from the WOSERVICEADDRESS table via the SERVICEADDRESS system relationship
#The datatype of the LatitydeY and LongitudeX is decimal: 1234567.8335815760, 123456.4075621164
#Convert the decimals to integers, and then strings, for the purpose of generating the URL (I don't think the URL can have decimal places)
laty = str(mbo.getInt("SERVICEADDRESS.LatitudeY"))
longx = str(mbo.getInt("SERVICEADDRESS.LONGITUDEX"))

#Verify if the numbers are legitimate UTM coordinates
if len(laty) == 7 and len(longx) == 6:
    #Assemble the URL (including the longx and the laty). Note: The coordinates are flipped in the url
    url="http://something.com/arcgis/rest/services/something/Zones/MapServer/15/query?geometry=" + longx + "%2C" + laty + "&geometryType=esriGeometryPoint&spatialRel=esriSpatialRelIntersects&outFields=*&returnGeometry=false&f=pjson"
    #Get the JSON text from the feature service (the JSON text contains the zone value)
    ctx = HashMap()
    ctx.put("url",url)
    service.invokeScript("LIB_HTTPCLIENT",ctx)
    json_text = str(ctx.get("response"))

    #Parse the zone value from the JSON text
    ctx = HashMap()
    ctx.put("json_text",json_text)
    service.invokeScript("LIB_PARSE_JSON",ctx)
    parsed_val = str(ctx.get("parsed_val"))

    #Enter the zone value into the zone field in the work order
    mbo.setValue("DESCRIPTION","Waste Zone: "+parsed_val,MboConstants.NOACCESSCHECK)

Jython library (LIB_HTTPCLIENT)

from psdi.iface.router import HTTPHandler
from java.util import HashMap
from java.lang import String
handler = HTTPHandler()
map = HashMap()
map.put("URL",url)
map.put("HTTPMETHOD","GET")
responseBytes = handler.invoke(map,None)
response = String(responseBytes,"utf-8")

JavaScript library (LIB_PARSE_JSON)

#The field name (ZONE) is hardcoded. I'm not sure if this is best practice or not.
var obj = JSON.parse(json_text);
parsed_val = obj.features[0].attributes.ZONE

How can this code be improved?

Functions are your friend

Even in Jython, functions are a valid construct. They allow for scope, so that you can better reason about temporary variables; give better stack traces when things go wrong; increase testability; etc. So you should move your code into some functions.

Invert your logic

This:

if len(laty) == 7 and len(longx) == 6:

should probably be inverted, and something done about it, i.e.

if len(laty) != 7 || len(longx) != 6:
    # throw, or at least print...
# continue on with the rest of the function

If you had requests...

then this:

url="http://something.com/arcgis/rest/services/something/Zones/MapServer/15/query?geometry=" + longx + "%2C" + laty + "&geometryType=esriGeometryPoint&spatialRel=esriSpatialRelIntersects&outFields=*&returnGeometry=false&f=pjson"

could be fairly reduced in insanity. The query params can be formed as a dictionary passed to the get method.

PEP8

Use any kind of linter or modern IDE, and it will give you suggestions on how to reformat this thing to follow Python formatting standards (PEP8). The most obvious thing it will notice is lack of spaces after commas.

Otherwise - perhaps you should provide some context about why this thing exists in Jython.

Suggested

I don't know whether this will work, because I don't have your setup; so you'll probably have to tweak it.

from psdi.mbo import MboConstants
from java.util import HashMap
from urllib import urlencode
from urlparse import urlunparse, ParseResult

def get_coords():
    """
    Get the y and x coordinates(UTM projection) from the WOSERVICEADDRESS table
    via the SERVICEADDRESS system relationship.
    The datatype of the LatitydeY and LongitudeX is decimal, i.e.
    1234567.8335815760, 123456.4075621164.
    """
    laty = mbo.getDouble("SERVICEADDRESS.LatitudeY")
    longx = mbo.getDouble("SERVICEADDRESS.LONGITUDEX")
    return laty, longx

def is_valid(laty, longx):
    """
    Verify if the numbers are legitimate UTM coordinates
    """
    return (0 <= laty <= 10e6 and
            167e3 <= longx <= 833e3)

def make_url(laty, longx):
    """
    Assemble the URL (including the longx and the laty). Note: The coordinates
    are flipped in the url
    """

    query = {
        'geometry': '%d,%d' % (laty, longx),
        'geometryType': 'esriGeometryPoint',
        'spatialRel': 'esriSpatialRelIntersects',
        'outFields': '*',  # You should narrow this if you only care about work zone.
        'returnGeometry': 'false',
        'f': 'pjson'
    }

    parts = ParseResult(scheme='http',
                        netloc='something.com',
                        path='/arcgis/rest/services/something/Zones/MapServer'
                             '/15/query',
                        query=urlencode(query),
                        fragment='')

    url = urlunparse(parts)
    return url

def fetch_waste_zone(url):
    # Get the JSON text from the feature service (the JSON text contains the
    # zone value)
    ctx = HashMap()
    ctx.put("url", url)
    service.invokeScript("LIB_HTTPCLIENT", ctx)
    json_text = str(ctx.get("response"))

    # Parse the zone value from the JSON text
    ctx = HashMap()
    ctx.put("json_text", json_text)
    service.invokeScript("LIB_PARSE_JSON", ctx)
    parsed_val = str(ctx.get("parsed_val"))

    return parsed_val

def main():
    laty, longx = get_coords()
    if not is_valid(laty, longx):
        print('Invalid coordinates')
        return

    url = make_url(laty, longx)
    waste_zone = fetch_waste_zone(url)

    # Enter the zone value into the zone field in the work order
    mbo.setValue("DESCRIPTION", "Waste Zone: " + waste_zone,
                 MboConstants.NOACCESSCHECK)

main()

Note:

There are functions
The functions contain """docstrings""" at the top
You should preserve the UTM coordinates as doubles until it's necessary to format them as integers in the URL string
UTM coordinates have real limits that you should use - not just string length
Even if you don't have requests, you can still do somewhat saner query string formation via dictionary
If the coordinates are invalid, do something - maybe print, definitely quit


Get nth node from end in a linked list Description:

  You’re given the pointer to the head node of a linked list and a
  specific position. Counting backwards from the tail node of the linked
  list, get the value of the node at the given position. A position of 0
  corresponds to the tail, 1 corresponds to the node before the tail and
  so on.

Code:

int GetNode(Node head,int n) {
     // This is a "method-only" submission.
     // You only need to complete this method.
    Node current = head;
    int count = 0;
    while (current != null) {
        count++;
        current = current.next;
    }
    current = head;
    for (int i = 0; i < count - n - 1; i++) { // extra -1 to avoid going out of linked list.
        current = current.next;
    }
    return current.data;
}

Instead of a count variable and reusing current,
I think it's neater to use two pointers:

Node runner = head;
for (int i = 0; i < n; i++) {
    runner = runner.next;
}
Node current = head;
while (runner.next != null) {
    runner = runner.next;
    current = current.next;
}
return current.data;

Other than that, your implementation is fine, except for a few tiny style points that are barely worth mentioning, but here we go anyway:

The commented out instructions about method-only submissions are unnecessary
Add a space after commas in parameter list


Drawing to a canvas based on user input The following is a code written in Javascript. It let's me change the appearance of a canvas depending on user input.
The question is, how can I make this code more CPU efficient?
I ask that because this code is just barely snappy enough on the iPad 2 and any improvement in speed would be really great to have.

With speed I am mainly focusing on that loop that iterates thousands of times (right now 3.600).

function repeat(){
//pc, input_mouse_x/y becomes input x/y.
if(device_type==0){
    ix=input_mouse_x;
    iy=input_mouse_y;
    //if pc's left mouse button isn't clicked then this isn't a valid input.
    if(input_mouse_button_left==0)  {ix='-';iy='-';}
    }
//ipad, input_touch becomes x/y.
if(device_type==1){
    ix=input_touch[0][0];
    iy=input_touch[0][1];
    //iy=trigonometry.flip({'number':iy,'around':screen_center});
    iy+=(screen_center-iy)*2;
    }

var pencil_x=ix;
var pencil_y=iy; pencil_y+=(screen_center-pencil_y)*2;
var pencil_width=30;
var pencil_height=30;
var data_quantity=(pencil_width*pencil_height*4);
var pencil_width_half=(pencil_width/2);
var pencil_height_half=(pencil_height/2);
var pencil_begin_x=(pencil_x-pencil_width_half);
var pencil_begin_y=(pencil_y-pencil_height_half);
var image_data=drawing_board_canvas_twod.getImageData(pencil_begin_x,pencil_begin_y,pencil_width,pencil_height);

//everything in this loop must be as cpu efficient as possible.
for(var cdata=3;cdata<3600;cdata+=4){
    //data to pixel, and then to x/y.
    var cpixel=Math.floor(cdata/4);
    var cx=(cpixel%pencil_width);
    var cy=parseInt(cpixel/pencil_width);

    //find distance to center of pencil.
    var to_center_x=Math.abs(pencil_width_half-cx);
    var to_center_y=Math.abs(pencil_height_half-cy);
    var to_center=Math.pow((Math.pow(to_center_x,2)+Math.pow(to_center_y,2)), 0.5);

    if(to_center<17){
        image_data.data[cdata]=0;//the higher the denser.
        }
    }
drawing_board_canvas_twod.putImageData(image_data,pencil_begin_x,pencil_begin_y);
}

The short answer is that you can't, not really anyways. You have no real control over how this code will be compiled. Since you mention iPad, the code is likely to be JIT-compiled to bytecode, prior to execution. How this is done depends (in part) on how the code is written, and how JS is implemented. The second factor, the implementation, is beyond your control.

You're also working with a canvas element. That's not JS that causes the bottleneck there, that's the DOM, which is gouverned by W3C, and is not part of ECMAScript in any way. JS merely has an API, via which it can request the DOM to change state. The DOM API isn't all that good: it's badly designed, slow, counter intuitive and one of the main reasons why many people claim JS is a terrible language. JS isn't, it's the DOM. But I'm going off course.

Inside that loop, which repeats 3600 times, you're calling a function which isn't part of your post (perhaps provide the source for that). But my guess is, that this function interacts with the dom (perhaps moves something about). If so: request a repaint, using the requestAnimationFrame method, it makes a huge difference!

After that, to further optimize your code, use jslint. As the page says, it will hurt your feelings, but it's the best way to teach yourself some good practices, like not declaring 10001 vars seperatly, but using:

var i, j, another = 123, someArray = [], andAnObject = {};

The main bottleneck, however is putImageData. this JSPerf demonstrates an alternative, up to x5000 more performant alternative!
It uses a neat little trick to buffer the states: it draws from one canvas onto the other, which is a lot faster. As is loading the image, then using:

drawing_board_canvas_twod.getContext('2d').drawImage(image_data, pencil_begin_x, pencil_begin_y);

But the way the image_data is gotten in your code still relies on the horrid getImageData, which is outpaced even by getting the img data pixel-by-pixed. Perhaps refer to this question on StackOverflow and all linked pages for details...

For now, the next to last thing I'd suggest is: look into how you call this repeat function of yours. If it's called in response to an event, check to see if you can't, perhaps, delegate that event, eliminating any excess event listeners is always good for performance. Are you using libs like jQuery? Get rid of them if you can, because they are slower than VanillaJS, if you know how to write good vanillaJS, of course.

The last, but certainly not least important thing I'd suggest: never ever use globals. Globals are risky, slow, and make for hellish debugging: your code relies on global variables, because the repeat function starts of with:

if(device_type==0)

device_type is not declared in the scope of repeat, so that implies it being a global variable. Learn about closures, scopes and how to use them. It's what makes JS worth your while.


Using pointers and type cast to break up integers into byte array Usually I don't start using pointers for a few microseconds here and there.

But with this implementation I started getting results of factor 2.

using System;
class BenchmarkPointers {
    public static void Main() {
#if ALLOW_UNSAFE
        Console.WriteLine("This is safe code");
#else
        Console.WriteLine("This is unsafe code");
#endif

        int bs = 0x100000;
        byte[] buffer = new byte[bs];
        Random rnd = new Random(DateTime.Now.GetHashCode());

        long total = 0;
        long avg = 0;

        int max = 100;

        DateTime start = DateTime.Now;

        for(int i = 0; i <= max; i++) {
            start = DateTime.Now;
            RandomizeBuffer(ref buffer, bs, rnd);
            TimeSpan ts = DateTime.Now - start;
            total += (long)ts.TotalMilliseconds;
            Console.WriteLine("Pass {0} took {1} ms", i, ts.TotalMilliseconds);
        }
        avg = total / max;
        Console.WriteLine("Avarage time for one pass was: {0}ms\nTotal time over {1} passes was {2}ms", avg, max, total);
    }

    public static
#if ALLOW_UNSAFE
    unsafe
#endif
    void RandomizeBuffer(ref byte[] buffer, int bufferSize, Random rnd) {

#if ALLOW_UNSAFE
        fixed(byte * pBuffer = buffer)
#endif
        for(int i = 0; i < bufferSize; i += 4) {
            int k = rnd.Next(int.MinValue, int.MaxValue);

#if ALLOW_UNSAFE
            // One of the rare moments when I like to use pointers (with type casting)
            *((int*)(pBuffer + i)) = k;
#else
            byte[] bits = BitConverter.GetBytes(k);
            buffer[i    ] = bits[0];
            buffer[i + 1] = bits[1];
            buffer[i + 2] = bits[2];
            buffer[i + 3] = bits[3];
#endif

        }
    }
}

Now essentially I need to put the 32bit integers I get from Random.Next into a byte array.

I started implementing the randomizer as

unsafe void RandomizeBuffer(byte **pBuffer, int bufferSize, Random rnd)

Thats why I got the size in the signature and thats also why I first used the 'pointery' approach. (Since then I removed one deref in the method body)

Then I remembered that C# has reference parameters so I refactored... but noticed my code took longer to execute.

Hence the little benchmark here.

I'm wondering whether there is a managed/safe way to break up lot's of ints into bytes.

Of course since results can vary from circumstance to circumstance here my latest results:

csc Benchmark.cs
...
Avarage time for one pass was: 31ms
Total time over 100 passes was 3197ms

and

csc -define:ALLOW_UNSAFE -unsafe benchmark.cs
...
Avarage time for one pass was: 19ms
Total time over 100 passes was 1931ms

Also I noticed with the unsafe execution there is quiet a fluctuation in execution times of the passes. i.E.:

 Pass 85 took 15.6097 ms
 Pass 86 took 15.6251 ms
 Pass 87 took 31.2502 ms
 Pass 88 took 15.626 ms
 Pass 89 took 15.6246 ms

I can only imagine it has something to do with the CPU Pipeline.
But I am no expert in CPU Architecture or Compiler Internals

Any thoughts on this are welcome!

(Yes also those that say: Don't waste your time with this kind of micro management)

Edit:

Using StopWatch and 1000 passes the results are similar, though the fluctuations have been eliminated.

safe -
Avarage time for one pass was: 24ms
Total time over 1000 passes was 24606ms
unsafe -
Avarage time for one pass was: 16ms
Total time over 1000 passes was 16965ms

If the buffer size is not divisible by 4, the remaining bytes are not set. This is a bug and should be corrected.

If you just need random values in the bytes, Random.NextBytes() does what you are trying to do and will likely be faster than both of your current implementation. It's up to you to decide if you care that the values were technically int32's before being written to the buffer.

When writing a benchmark, Stopwatch will provide more precision than DateTime.Now. I also thing 100 is not a large enough number of iterations to get a good average. As you pointed out, there were outliers that took almost twice as long as the majority of values.

It is probably best to take the seed from a command line argument. This will allow you to remove one more variable between the two implementation. While I don't expect Random's execution time to change significantly based on the seed, it is best to keep everything else equal when comparing two things.

Arrays have a Length property, so you shouldn't be dealing with the buffer size independently.

As for if it is worth it to make this optimization, that isn't something we can tell you. Have you profiled the execution of the actual program? Is this where your code if spending the majority of it's time? If it's actually IO bound, ~12ms doesn't mean anything. Where as, if this chunk of code is running millions of times each time your program does a single task, ~12ms is huge and well worth optimizing.


Using toString() as a hack to generate keys for react-select component While trying to integrate react-select component on my codebase on documentation it's explained that you can pass an array of options with label and value attributes eg :

import Select from 'react-select';

let options = [
 {
  label: 'Color',
  value: 'Blue'
 }
]

<Select options={options} />

And it works fine, but in my case I need value to be an object as simply a String won't do the job how I want. I know I can keep an array of objects separately and do a find based on their ID on that array but this lacks on performance and increases chances for side effects.

So what I tried was using options with value as objects :

let options = [
  {
    label: 'Color',
    value: {
      r: 123,
      g: 78,
      b: 198
    }
  }
]

But this way I had errors on my console, telling me that you have to use unique keys for each child and this was because the component takes an option value and uses that for key and that key when converted to string became "[object Object]" so finally the key was always option-[object Object] .

To solve this out I overrode the value attribute toString() method and it worked :

let options = [
  {
    label: 'Color',
    value: {
      r: 123,
      g: 78,
      b: 198,
      toString: function(){
        return this.r + this.g + this.b;
      }
    }
  }
]

So what could be downsides of this, as obviously this is a hack.

One possible down-site of this could be a performance issue and this depends on how toString() is implemented.

On my case I did it like this :

let options = [
  {
    label: 'Color',
    value: {
      r: 123,
      g: 78,
      b: 198,
      toString: function(){
        return uuid.v1();
      }
    }
  }
]

Whenever the state or props are updated React uses a diff. algorithm to check if it's VirtualDOM and DOM corresponds, which means every time React looped through options they had a different key value and React re-rendered every option no matter if their value changed or not.

In my case a fix was to hash the value object using this library;

let options = [
  {
    label: 'Color',
    value: {
      r: 123,
      g: 78,
      b: 198,
      toString: function(){
        return hash(this);
      }
    }
  }
]


Exercise for ordering pizzas with user input I've only been learning Java for two weeks, and I had to complete this task as part of my course. My instructor could not give specific feedback as answer file is already provided, the code works but I am just wondering what some of you might think of my code if it can be refined. It's a basic pizza ordering program.
The requirements were:
able to specify pizzas one at a time using an loop.
loop can end or not based on input.
order viauser input using the Scanner class.
format currency with NumberFormat
Thank you for your time and wisdom.
import java.util.Scanner;
import java.text.NumberFormat;

public class PizzaOrder {

    public static void main(String[] args) {
        //defining and initialising variables.
        NumberFormat nf = NumberFormat.getCurrencyInstance();
        Scanner keyboard = new Scanner(System.in);
        int pizzaquantity=0;
        int numberoftoppings = 0;
        double toppingtotalprice =0;
        char ordermore = 'n';
        double runningtotal = 0;
        final double stp = 1.0, ltp = 1.5, ftp = 2.0;
        final double s = 8.0, l = 11.0, f = 14.0;

        //using a do while loop, because condition to stop is at the end.
        do{

            System.out.println("Please place the order for your pizza.");
            System.out.println("Size (s = small, l = large, f = family):");
            char pizzasize = keyboard.next().charAt(0);
            //if else-else if nested inside loop
            //one iteration of the conditional statement for pizza size type.
            //number format is applied to every iteration of cost.
            if (pizzasize == 's')
            {
                System.out.println("Number of toppings?:");
                numberoftoppings = keyboard.nextInt();
                toppingtotalprice = (numberoftoppings * stp);
                System.out.println("Pizza Quantity:");
                pizzaquantity = keyboard.nextInt();
                runningtotal = (pizzaquantity * toppingtotalprice * s);
                System.out.println("Cost is: " + nf.format(runningtotal));

            }
            else if (pizzasize == 'l')
            {
                System.out.println("Number of toppings?:");
                numberoftoppings = keyboard.nextInt();
                toppingtotalprice = (numberoftoppings * ltp);
                System.out.println("Pizza Quantity:");
                pizzaquantity = keyboard.nextInt();
                runningtotal = (pizzaquantity * toppingtotalprice * l);
                System.out.println("Cost is:" + nf.format(runningtotal));

            }
            else if   (pizzasize == 'f')
            {
                System.out.println("Number of toppings?:");
                numberoftoppings = keyboard.nextInt();
                toppingtotalprice = (numberoftoppings * ftp);
                System.out.println("Pizza Quantity:");
                pizzaquantity = keyboard.nextInt();
                runningtotal = (pizzaquantity * toppingtotalprice * l);
                System.out.println("Cost is:" + nf.format(runningtotal));

            }
            else
            {
            }
                System.out.println("Order more? (y/n):");
                ordermore = keyboard.next().charAt(0);
        }
        while (ordermore != 'n');

        System.out.println("Total cost is: " + nf.format(runningtotal += runningtotal));
        System.out.println("Thank you for your order!");
    }

}

NumberFormat nf = NumberFormat.getCurrencyInstance();

Don't shorten names just because you can, it makes the code harder to read and maintain.
final double s = 8.0, l = 11.0, f = 14.0;

Perfect example of that. The more descriptive the names are, the better. Length should not be of any concern.

double toppingtotalprice =0;

A double is still a floating point. For money, you always want to use BigDecimal, or an int as the smallest unit your money has (Cents for example).

char ordermore = 'n';

Should mot likely be a boolean.

        //using a do while loop, because condition to stop is at the end.
        do{

You can use a while here, though.

I'll skip the rest of your code and directly skip to the part that you've been waiting for: That works, but is bad design.
Note that in the following examples I'm still using double for simplicity, you should use something else.
Let's start with the fact that you have a lot of repetition, extract common parts into a function which accepts parameters to parameterize it. For example:
private static final double inputPizza(double pizzaSizePrice, double toppingPricel) {
    System.out.println("Number of toppings?:");
    numberoftoppings = keyboard.nextInt();
    toppingtotalprice = (numberoftoppings * toppingPrice);
    System.out.println("Pizza Quantity:");
    pizzaquantity = keyboard.nextInt();
    return (pizzaquantity * toppingtotalprice * pizzaSizePrice);
}

That already reduces repetition a lot.
Now, we're going to encapsulate our information in a class of their own:
public class PizzaCosts {
    protected double price;
    protected double toppingPrice;

    public PizzaPriceValues(double price, double toppingPrice) {
        super();

        this.price = price;
        this.toppingPrice = toppingPrice;
    }

    public double getPrice() {
        return price;
    }

    public double getToppingPrice() {
        return toppingPrice;
    }
}

And in our main function:
PizzaCosts smallPizzaCosts = new PizzaCosts(8.0d, 1.0d);
PizzaCosts mediumPizzaCosts = new PizzaCosts(11.0d, 1.5d);
PizzaCosts largePizzaCosts = new PizzaCosts(14.0d, 2.d);

That groups the values nicely together and makes it easier to read. Now let's go one step further, we'll put them into a Map to access them dynamically based on user-input:
Map<String, PizzaCosts> pizzaCosts = new HashMap<>();
pizzaCosts.put("s", new PizzaCosts(8.0d, 1.0d));
pizzaCosts.put("m", new PizzaCosts(11.0d, 1.5d));
pizzaCosts.put("l", new PizzaCosts(14.0d, 2.0d));

PizzaCosts selectedPizzaCosts = pizzaCosts.get(keyboard.next());

if (selectedPizzaCosts != null) {
    runningTotal = runningTotal + inputPizza(pizzaCosts);
} else {
    // Invalid selection
}

Now that's a lot nicer, and should be enough to get you started further.


C++ binary search tree I am learning C++ and today I've implemented a version of the binary search tree. I would like your thoughts about it.

node.h

#pragma once

struct Node
{
   int val;
   Node *left;
   Node *right;
};

bTree.h

#include "node.h"

#pragma once

class BTree
{
private:

    Node *root;
    void addValue(int n, Node *node);
    void traverse(Node *node);
    Node* search(int n, Node *node);

public:

    BTree();

    void addValue(int n);
    void traverse();
    void search(int n);
};

bTree.cpp

#include <iostream>
#include "bTree.h"

BTree::BTree()
{
    root = nullptr;
}

void BTree::addValue(int n, Node *node) {
    if (n < node->val)
    {
        if (node->left != nullptr)
        {
            addValue(n, node->left);
        }
        else
        {
            node->left = new Node;
            node->left->val = n;
            node->left->left = nullptr;
            node->left->right = nullptr;
        }
    }
    else if (n >= node->val)
    {
        if (node->right != nullptr)
        {
            addValue(n, node->right);
        }
        else
        {
            node->right = new Node;
            node->right->val = n;
            node->right->left = nullptr;
            node->right->right = nullptr;
        }

    }
}

void BTree::traverse(Node *node) {
    if (node->left != nullptr)
        traverse(node->left);
    std::cout << node->val << ' ';
    if (node->right != nullptr)
        traverse(node->right);
}

Node* BTree::search(int n, Node *node) {
    if (node != nullptr)
    {
        if (node->val == n)
            return node;
        else if (n < node->val)
            return search(n, node->left);
        else if (n > node->val)
            return search(n, node->right);
    }
    else
        return nullptr;
}

void BTree::addValue(int n) {
    if (root == nullptr)
    {
        root = new Node;
        root->val = n;
        root->left = nullptr;
        root->right = nullptr;
    }
    else
    {
        addValue(n, root);
    }
}

void BTree::traverse() {
    if (root == nullptr)
    {
        std::cout << "No nodes in the tree" << std::endl;
    }
    else
    {
        traverse(root);
    }
}
void BTree::search(int n) {
    if (root == nullptr)
    {
        std::cout << "No nodes in the tree" << std::endl;
    }
    else
    {
        if (search(n, root))
            std::cout << n << " found" << std::endl;
        else
            std::cout << n << " not found" << std::endl;
    }
}

main.cpp

#include <iostream>
#include "bTree.h"
#include <ctime>

int main()
{
    srand(static_cast<unsigned int>(time(0)));
    rand();

    BTree bTree{};

    for (int i = 0; i < 50; i++)
    {
        int n = rand() % 100;
        std::cout << n  << ' ';
        bTree.addValue(n);
    }

    std::cout << std::endl << std::endl;

    bTree.traverse();

    std::cout << std::endl << std::endl;

    for (int i = 0; i < 10; i++)
    {
        int n = rand() % 100;
        bTree.search(n);
    }

    std::cout << std::endl << std::endl;

    system("pause");

    return 0;
}

Use constructors to initialize your objects.

        root = new Node;
        root->val = n;
        root->left = nullptr;
        root->right = nullptr;

This should be replaced by:

        root = new Node(n);

Design

I think you will find that your code becomes a lot simpler if you move your test for null out of the public members. Then make the test for null the first thing you do in the private member (Don't test before you call the private member test as the first action in the call). This will simplify all the situations were you currently have to test for null.

For Example:
I would write addValue like this:

void BTree::addValue(int n) {
    root = addValue(n, root);
}
Node* BTree::addValue(int n, Node* current) {
    if (current == nullptr) {
        return new Node(n);
    }
    if (n < current->val) {
        current->left  = addValue(n, current->left);
    else if (n > current->val) {
        current->right = addValue(n, current->right);
    }
    return current;
}