Most common occurrence of an int in an array I am prepping for a junior level interview in C#.  I am hoping I can get some feedback on how to improve my code to print the most common occurrence of an int in an array.

using System;
using System.Collections;

namespace FrequentInt
{
    class MainClass
    {

        static void CommomOcurrence (int [] array, Hashtable hs)
        {
            int mostCommom = array[0];
            int occurences = 0;
            foreach (int num in array)
            {
                if (!hs.ContainsKey (num)) {
                    hs.Add (num, 1);
                }
                else
                {
                    int tempOccurences = (int)hs [num];
                    tempOccurences++;
                    hs.Remove (num);
                    hs.Add (num, tempOccurences);

                    if (occurences < tempOccurences)
                    {
                        occurences = tempOccurences;
                        mostCommom = num;
                    }
                }
            }
            foreach(DictionaryEntry entry in hs)
            {
                Console.WriteLine("{0}, {1}", entry.Key, entry.Value);
            }
            Console.WriteLine ("The commmon numer is " + mostCommom + " And it appears " + occurences + " times");
        }

        public static void Main (string[] args)
        {

            int[] array = new int[20]{ 3,6,8,5,3,5,7,6,4,3,2,3,5,7,6,4,3,4,5,7};
            Hashtable hs = new Hashtable ();
            CommomOcurrence (array, hs);

        }
    }
}

Whenever you're dealing with collections in C#, there's often a simple Linq solution.

Instead of using a Hashtable or Dictionary to group together and count each occurrence, use the .GroupBy method on the integers. This makes each integer a key with a list of values for each occurrence.

Then all you need to do to find the largest is to order by occurrence and take the first result. Using Linq's .OrderByDescending and .First methods do this for you.

static void CommonOccurrence(int[] numbers)
{
    var groups = numbers.GroupBy(x => x);
    var largest = groups.OrderByDescending(x => x.Count()).First();
    Console.WriteLine("The most common number is {0} and it appears {1} times", largest.Key, largest.Count());
}

If you wanted to take the same approach without relying on Linq to group and sort for you, you can make separate methods for each action you need to perform, as per the Single Responsibility Principle:

static void CommonOccurrence(int[] numbers)
{
    var dictionary = GroupByOccurrence(numbers);
    var mostCommonNumber = KeyOfLargestValue(dictionary);
    var occurrences = dictionary[mostCommonNumber];
    Console.WriteLine("The most common number is {0} and it appears {1} times", mostCommonNumber, occurrences);
}

static Dictionary<int, int> GroupByOccurrence(int[] numbers)
{
    var result = new Dictionary<int, int>();
    foreach (int i in numbers)
    {
        if (!result.ContainsKey(i))
        {
            result[i] = 0;
        }
        result[i]++;
    }
    return result;
}

static int KeyOfLargestValue(Dictionary<int, int> dictionary)
{
    int result = dictionary.Keys.First();
    foreach (var entry in dictionary)
    {
        if (entry.Value > dictionary[result])
        {
            result = entry.Key;
        }
    }
    return result;
}


Cut and paste rows to new sheet based on different criteria in Column A I have a "working sheet" that contains 15000+ rows. In column A is an identifier for that row. There are over 20 different identifiers i.e 9W, AM, AV, BG, CY, HJ etc. etc.

My current code looks for each row on "Working Sheet" that has 9W in column A, cuts and pastes that row into a sheet called 9W. Once finished it moves to AM, finds am in Column A, cuts and pastes each row into a sheet called AM. Process repeats until all Identifiers have been done.

Here is a sample of the current code that I have created with my limited knowledge:

Sub Test()

'Do 9W

    Dim sht1 As Worksheet, sht2 As Worksheet
    Dim i As Long

    Set sht1 = ThisWorkbook.Worksheets("Working Sheet")
    Set sht2 = ThisWorkbook.Worksheets("9W")

    For i = 2 To sht1.Cells(sht1.Rows.Count, "A").End(xlUp).Row
        If sht1.Range("A" & i).Value = "9W" Then
            sht1.Range("A" & i).EntireRow.Cut sht2.Range("A" &
 sht2.Cells(sht2.Rows.Count, "A").End(xlUp).Row + 1)
        End If
    Next i

'Do AM

Dim sht3 As Worksheet, sht4 As Worksheet
Dim i1 As Long

Set sht3 = ThisWorkbook.Worksheets("Working Sheet")
Set sht4 = ThisWorkbook.Worksheets("AM")

For i1 = 2 To sht3.Cells(sht3.Rows.Count, "A").End(xlUp).Row
    If sht3.Range("A" & i1).Value = "AM" Then
        sht3.Range("A" & i1).EntireRow.Cut sht4.Range("A" &
sht4.Cells(sht4.Rows.Count, "A").End(xlUp).Row + 1)
    End If
Next i1

'DO AV

Dim sht5 As Worksheet, sht6 As Worksheet
Dim i2 As Long

Set sht5 = ThisWorkbook.Worksheets("Working Sheet")
Set sht6 = ThisWorkbook.Worksheets("AV")

For i2 = 2 To sht5.Cells(sht5.Rows.Count, "A").End(xlUp).Row
    If sht5.Range("A" & i2).Value = "AV" Then
        sht5.Range("A" & i2).EntireRow.Cut sht6.Range("A" &
sht6.Cells(sht6.Rows.Count, "A").End(xlUp).Row + 1)
    End If
Next i2

'DO BG

Dim sht7 As Worksheet, sht8 As Worksheet
Dim i3 As Long

Set sht7 = ThisWorkbook.Worksheets("Working Sheet")
Set sht8 = ThisWorkbook.Worksheets("BG")

For i3 = 2 To sht7.Cells(sht7.Rows.Count, "A").End(xlUp).Row
    If sht7.Range("A" & i3).Value = "BG" Then
        sht7.Range("A" & i3).EntireRow.Cut sht8.Range("A" &
    sht8.Cells(sht8.Rows.Count, "A").End(xlUp).Row + 1)
    End If
Next i3

End Sub

As long as all the rows in your working sheet have a valid identifier, you can simplify this process a lot by looking at each row and looking up the Sheet you're supposed to copy the row to.

This allows you to write a single loop instead of a loop for each ID there is:

Dim source As Worksheet
Dim target As Worksheet
Dim targetRow As Long

Set source = ThisWorkbook.Worksheets("Working Sheet")
' As long as there is a row to cut and paste
Do While source.Cells("A2").Value <> vbNullString
  ' select where the row is supposed to go
  Set target = ThisWorkbook.Worksheets(source.Cells("A2").Value)
  targetRow = target.Cells(target.Rows.Count, "A").End(xlUp).Row + 1
  ' and transfer it using copy & delete
  With source.Range("A2").EntireRow
    .Copy target.Range("A" & targetRow)
    .Delete xlShiftUp
  End With
Loop

Of course if that's not the case, this simplification still applies: Iterate the rows of the worksheet once and only Copy&Delete the rows where the identifier matches one of the allowed identifiers.

If deleting the rows from the working sheet is not correct, or you don't want to move every row, you will need to iterate using a For Loop.


3D metrics for segmentation evaluation I am new to programming and I am trying to develop a machine learning approach for image segmentation (https://en.wikipedia.org/wiki/Image_segmentation) and here I want to evaluate the predicted (output) segmentation. In order to do this, I am using multiple (common) metrics (dice score, jacquard index, surface distance Hausdorff distance, diameter error and volume error).

The code works, but it really looks bad! I heard that one of the best practices is to avoid repetition in the code. For my piece I have to create multiple arrays which will all have different usage, but generating a lot of repetitions, and I am sure there might be better way to get better performance for this piece of code.

Do you have an idea of how to optimize this for speed and readability?

The details of the code:

folder = glab() : gets the name of the different folders containing the arrays that I want to open (glab() = glob() with natural sorting)

prediction/ground_truth : loads the predicted segmentation and ground_truth to be compared

scores() : calculates the different metrics (dice score, jacquard index, sensibility and specificity) between the ground truth and prediction

surfd() : calculates the surface distance between ground truth and prediction

diam_vol : calculates the diameter/volume and error between ground truth and prediction

Since I am evaluating multiple cases I want to know the mean() metrics to assess my model that's why I call for np.mean() for all the values at the end.

def metrics(savedir):

    """3D metrics for segmentation evaluation:

    dice score : https://en.wikipedia.org/wiki/S%C3%B8rensen%E2%80%93Dice_coefficient
    jacquard index : https://en.wikipedia.org/wiki/Jaccard_index
    sensibility & specificity : https://en.wikipedia.org/wiki/Sensitivity_and_specificity
    diameter and volume error
    Surface distance : https://www.cs.ox.ac.uk/files/7732/CS-RR-15-08.pdf
    Hausdorff distance : https://en.wikipedia.org/wiki/Hausdorff_distance
    """

    folder = glab(savedir+"/OutputData/*/")

    # array initialization
    np_dice = np.zeros([len(folder)])
    np_jacq = np.zeros([len(folder)])
    np_sens = np.zeros([len(folder)])
    np_spec = np.zeros([len(folder)])
    np_surf = np.zeros([len(folder)])
    np_haus = np.zeros([len(folder)])

    np_diam_GT = np.zeros([len(folder)])
    np_diam_pred = np.zeros([len(folder)])
    np_diam_err = np.zeros([len(folder)])

    np_vol_GT = np.zeros([len(folder)])
    np_vol_pred = np.zeros([len(folder)])
    np_vol_err = np.zeros([len(folder)])

    for number, folder_name in enumerate(folder):

        prediction = np.load(folder_name+"prediction.npy")
        ground_truth = np.load(folder_name+"test_label.npy")

        scores = scoring(ground_truth, prediction)
        surface_distance = surfd(prediction, ground_truth, [0.625, 0.625, 0.625], 1)
        diam_vol = diam(prediction, ground_truth)

        np_dice[number] = scores[0]
        np_jacq[number] = scores[1]
        np_sens[number] = scores[2]
        np_spec[number] = scores[3]
        np_surf[number] = surface_distance.mean()
        np_haus[number] = surface_distance.max()

        np_diam_GT[number] = diam_vol[0]
        np_diam_pred[number] = diam_vol[1]
        np_diam_err[number] = diam_vol[2]

        np_vol_GT[number] = diam_vol[3]
        np_vol_pred[number] = diam_vol[4]
        np_vol_err[number] = diam_vol[5]

    mean_dice = np.mean(np_dice)
    mean_jacq = np.mean(np_jacq)
    mean_sens = np.mean(np_sens)
    mean_spec = np.mean(np_spec)
    mean_surf = np.mean(np_surf)
    mean_haus = np.mean(np_haus)
    mean_diam_GT = np.mean(np_diam_GT)
    mean_diam_pred = np.mean(np_diam_pred)
    mean_diam_err = np.mean(np_diam_err)
    mean_vol_GT = np.mean(np_vol_GT)
    mean_vol_pred = np.mean(np_vol_pred)
    mean_vol_err = np.mean(np_vol_err)

    return ([mean_dice, mean_jacq, mean_sens, mean_spec, mean_surf, mean_haus, mean_diam_GT, mean_diam_pred, mean_diam_err, mean_vol_GT, mean_vol_pred, mean_vol_err])

Realy, dud its childish!

only ones:

lenf = len(folder)

just one 2d array:

len_types = 12
arr = np.zeros([lenf ,  12])

learn how to work with numpy arrays, you will avoid the loop as well. And for example the mean can be done in one line for 2d array:

means = np.mean(arr, axis = 1)

https://docs.scipy.org/doc/numpy/reference/generated/numpy.mean.html


Running a script recursively in subdirectories I got tired of running the same command in multiple directories, so I thought "there has to be a way to make this easier". The commands I was running was mostly git status, git stash list, hg summary, mvn clean test, and so on and so on...

So what I did was to create a bash script that you can pass a parameter to. It will then run the same script or command in all subdirectories, until it finds a place where the script returns exit status 0 (meaning that it was run successfully), then it stops going deeper in those subdirectories.

As I am not that used to bash-scripting, I am wondering if there's anything I can improve here. I would also like you to comment on the usability of this script. You are also welcome to make feature-requests, bug reports, or pull requests (my favorite!) on my bash-recursive repository on GitHub.

The script is:

#!/bin/bash

RUN_SCRIPT=$1

recurseCheck() {
  local f
  for f in $1/* ; do
    local PREV_DIR=`pwd`
    if [ -d $f ]; then
      cd $f
      RESULT=`eval $RUN_SCRIPT`
      local RESULT_CODE=$?
      if [ $RESULT_CODE -eq 0 ]; then
        echo "$f"
        echo "$RESULT"
        echo ""
      else
        recurseCheck $f
      fi
      cd $PREV_DIR
    fi
  done;
}

START_DIR=`pwd`
recurseCheck $START_DIR

Usage example:

  recurse.sh "git status 2>&1"

Will output something like:

/home/zomis/gitstuff/a/Duga
On branch master
nothing to commit, working directory clean

/home/zomis/gitstuff/a/SudokuSharp
On branch master
nothing to commit, working directory clean

/home/zomis/gitstuff/c/Brainduck
On branch master
nothing to commit, working directory clean

/home/zomis/gitstuff/c/CodeReview-Shield
On branch master
nothing to commit, working directory clean

/home/zomis/gitstuff/c/SE-Scripts
On branch master
nothing to commit, working directory clean

The reason 2>&1 is there is because otherwise this, which goes to stderr would be printed:

fatal: Not a git repository (or any of the parent directories): .git

There is also an "example" directory on my GitHub repo which contains a script that I used a lot while testing, run ./recurse.sh /path/to/bash-recurse/example/filecheck.sh and it will output something like:

/home/zomis/bash-recurse/example/b/d
Running filecheck in /home/zomis/recursescript/example/b/d
Hello World
This is a recursive script

/home/zomis/bash-recurse/example/c
Running filecheck in /home/zomis/recursescript/example/c
OK in directory 'c'

Any comments welcome.

Check yourself before you wreck yourself...

shellcheck.net is an awesome website that checks your Bash syntax.
Among other things, it will tell you to:

Double-quote your path variables
Use $(...) instead of the legacy `...` for sub-shells

Avoid cd inside scripts

cd inside scripts is troublesome.
In case something goes wrong,
you might find yourself in unexpected places, and wreak havoc.

But if you must, like in your example,
there's a better way than saving the previous directory in PREV_DIR and then cd "$PREV_DIR" to go back, using a (...) sub-shell environment:

(
cd "$f" || break
RESULT=$(eval "$RUN_SCRIPT")
local RESULT_CODE=$?
if [ $RESULT_CODE -eq 0 ]; then
    echo "$f"
    echo "$RESULT"
    echo
else
    recurseCheck "$f"
fi
)

Notice that there is only one cd in, no cd out.
This is because the cwd changes only affect the environment inside the (...).

Also notice the cd "$f" || break: if the cd command fails for some reason,
you probably don't want to execute the rest of the block.
You might go as far as exit at that point (shellcheck recommends it too) and investigate the troublesome directory.

Minor things

You had a pointless ; in done;, and instead of echo "" you can write simply echo.

Input validation

What if you run this script without arguments?
eval "" exits with success, so it will simply print all directories.
If that's ok, then everything's fine.
If not, then you might want to add some input validation, for example:

if [ ! "$RUN_SCRIPT" ]; then
    echo usage: $0 'some script'
    exit 1
fi

Suggested implementation

Putting it together (coming to you in a Pull Request very soon):

#!/bin/bash

RUN_SCRIPT=$1

recurseCheck() {
    local f
    for f in $1/* ; do
        if [ -d "$f" ]; then
            (
            cd "$f" || break
            RESULT=$(eval "$RUN_SCRIPT")
            local RESULT_CODE=$?
            if [ $RESULT_CODE -eq 0 ]; then
                echo "$f"
                echo "$RESULT"
                echo
            else
                recurseCheck "$f"
            fi
            )
        fi
    done
}

START_DIR=$(pwd)
recurseCheck "$START_DIR"


Retrieving stock prices It takes around 5-8 seconds for me to retrieve a previously-closed stock price and a dividend rate from US Yahoo! Finance. If I wanted to retrieve 10+ stock prices, it would take me more than a minute with the below VBA code.

Is there any other way to speed up or improve the code?

I input multiple stock code on column B and retrieve data on column C & D:

Column B = stock code (e.g. HPQ, IBM, AAPL)
Column C = previous closed price
Column D = dividend rate

Sub Button1_Click()

Dim ie As Object
Dim r As Integer
Dim prevClose As String
Dim div As String
Dim Doc As HTMLDocument

With Application
 .DisplayAlerts = False
 .ScreenUpdating = False
 .Calculation = xlCalculationManual
 .EnableEvents = False
 .StatusBar = "Retrieving data from internet explorer"
End With

Dim time1 As Double, time2 As Double
time1 = Timer

Set ie = CreateObject("InternetExplorer.Application")

For r = 2 To Range("B65535").End(xlUp).Row

With ie
  .Visible = 0

  .navigate "http://finance.yahoo.com/q?s=" & Cells(r, "B").Value

   While .Busy Or .readyState <> 4 'add do or not
   DoEvents
   Wend

End With

 Set Doc = ie.document

 prevClose = Trim(Doc.getElementById("table1").getElementsByTagName("td")(0).innerText)
 Cells(r, "C").Value = prevClose

 div = Trim(Doc.getElementById("table2").getElementsByTagName("tr")(7).getElementsByTagName("td")(0).innerText)
 Cells(r, "D").Value = div

Next r

ie.Quit: Set ie = Nothing

With Application
 .DisplayAlerts = True
 .ScreenUpdating = True
 .Calculation = xlCalculationAutomatic
 .EnableEvents = True

End With

time2 = Timer
MsgBox Format(time2 - time1, "0.00 \s\ec")

End Sub

Now, I don't know much about webscraping, but I don't think there's much you can do to make this method of going about it any faster. You're restricted by how fast Internet Explorer can retrieve the website. There's also the fact that VBA is a single threaded language. So, multi-threading is out of the question.... or is it?

I would tackle this problem a little differently. I would create a swarm of VBScript scripts to retrieve the data you're after. This works by using VBA to writing many VBScript files to a drive location and using a shell to execute them. Each VBScript is responsible for writing the data it finds to the appropriate cell in Excel.

You can find a reasonable example of this (including a workbook with the code) here courtesy of Daniel Ferry.


Compute all valid configuration of eight queens on a 8x8 chess board Write an algorithm to print all valid ways of arranging eight queens
  on an 8x8 chess board so that none of them attack any other.

Following is my code for this. Do you see any performance or coding style issue with it? How should I have used grid_size? It should not be a parameter to a function that is already called "eight queen". But global also doesn't feel quite right. I would also like suggestions on naming variables and methods.

def is_valid (proposed_row, proposed_col, already_placed_queen_cols):
    for earlier_row in range(0,proposed_row):
        already_placed_queen_col = already_placed_queen_cols[earlier_row]

        if proposed_col == already_placed_queen_col:
            return False

        column_dist = abs(proposed_col - already_placed_queen_col)
        row_dist = proposed_row - earlier_row

        if column_dist == row_dist:
            return False

    return True

def get_arrangements (row, already_placed_queen_cols, grid_size):
    if row == grid_size:
        yield already_placed_queen_cols
        return

    for col in range(0,grid_size):
        if is_valid(row,col, already_placed_queen_cols):
            already_placed_queen_cols[row] = col
            yield from get_arrangements (row+1, already_placed_queen_cols, grid_size)

def eight_queen():
    global grid_size
    already_placed_queen_cols = [-1 for i in range(0,grid_size)]
    return get_arrangements (0, already_placed_queen_cols, grid_size)

grid_size = 8
arrangements = eight_queen()

for arrangement in arrangements:
    position_list = []
    for row in range(0,grid_size):
        position_list.append((row, arrangement[row]))
    print (position_list)

Coding style

Don't put a space between a function name and the opening bracket for its parameters. You should also consistently have a space after the comma between parameters in a function, iterable, etc. So, that should have been range(0, stop). Incidentally, if your starting value is 0, you can simply use range(stop).

When you need to do

for i in range(stop):
    value = seq[i]
    # do stuff

you should use enumerate().

for i, value in enumerate(seq):
    # do stuff

Obviously you can also enumerate only a part of your iterable and if the starting index isn't 0, you can specify that as well, i.e., enumerate(seq[start:stop], start).

When you want to initialise a list with the same immutable object, you can do

[value] * n

instead of

[value for i in range(stop)]

Care must be taken here, because if you tried to do [[]] * n, you would create n references of the same empty list and when it is modified, it changes for each index in your list. In such a case, you would have to fall back to initialising your iterable the way you did it. Also, while it's a matter of preference, I find it more natural to initialise the queen positions to None than some arbitrary value.

Your main function is simply a wrapper for get_arrangements(), where you simply initialise some objects. You can make get_arrangements() your main function and simply check whether you're currently at row 0 so you can initialise said objects.

def eight_queen(grid_size, row=0, already_placed_queen_cols=None):
    if not row:
        already_placed_queen_cols = [None] * grid_size

    if row == grid_size:
        yield already_placed_queen_cols
    # ...

This way you also solve the problem of the global variable, which is indeed ugly. I don't understand why it shouldn't be a parameter of your main function. The say I see it, I'm tasked with finding the solution to the n-queens puzzle, so the one thing I need to define is the board size. If I'm only interested in the solution for size 8, I can give it a default value in the parameters, so to call the function with no inputs, but that still allows me to generalise the solution should I feel like it later on.

The is_valid() function is only supposed to be called by your main function, and someone has no use for it unless he can replicate an intermediate state of its input parameters. As such, you can add a leading underscore to its name to indicate the user has no interest in calling it directly.

Naming style

I would call the main function solve_queens_puzzle() as that's what you're doing.

is_valid() correctly describes that it returns a boolean, but it's somewhat vague about the validity of what it tests. is_valid_placement() should suffice.

already_placed_queen_cols does feel a bit awkward indeed. queens_coords sounds equally descriptive and less verbose. Following on that, already_placed_queen_col is deceptive for having only a letter different in such a long name. I wouldn't complicate things here, from a glance at the code it's obvious the rows/columns iterated are up to proposed_row. I would simplify that to

for row, col in enumerate(queen_coords[:proposed_row]):
    if col == proposed_col:
        return False
    # ...

This one is entirely subjective, but you can either stick with a row/column convention, since mathematically speaking you're dealing with a 2D array, or you can use rank/file, which are the respective chess terms.

Performance

In the context of the backtracking algorithm you've implemented, there is one inefficiency. For each new row, you iterate over all 8 squares and check whether there are any conflicts. However, you repeat a lot of the same computations for each square, e.g., the proposed row against all the earlier rows, etc. Instead, you should go over the earlier rows once, while calculating any restrictions for the current row along the way. Specifically, for the current row you can't use the column of any previous row, or the diagonals that result from them. That way in one pass you can collect the available squares and return that from the function for iteration. This yields an exponential improvement for grid sizes over 10, when the solutions aren't computed almost instantly. Obviously, this changes the intent of the function, so its naming has to follow accordingly.

Overall, this is what I got.

def _get_possible_squares(size, current_rank, queen_coords):
    # no restrictions for the first row
    if not current_rank:
        return range(size)

    restrictions = set()
    for rank, square in enumerate(queen_coords[:current_rank]):
        restrictions.add(square)
        diff = current_rank - rank
        left_diagonal = square - diff
        right_diagonal = square + diff
        if left_diagonal >= 0:
            restrictions.add(left_diagonal)
        if right_diagonal < size:
            restrictions.add(right_diagonal)
    return (square for square in range(size) if square not in restrictions)

def solve_queens_puzzle(size, rank=0, queen_coords=None):
    if not rank:
        queen_coords = [None] * size

    if rank == size:
        yield tuple(queen_coords)
        return

    for square in _get_possible_squares(size, rank, queen_coords):
        queen_coords[rank] = square
        for solution in solve_queens_puzzle(size, rank+1, queen_coords):
            yield solution

for solution in solve_queens_puzzle(8):
    print(solution)


Coordinates of 2D points zoomed in/out This might be a simple issue that I am overcomplicating, but I've spent quite some time reading about polygon scaling and I've come to the conclusion that it is not precisely what I need.

Given a set of (x, y) coordinates for N points (shown in blue) I need the new set of coordinates that result after zooming in/out a given scale factor (shown in red).

I've come up with the simple method shown below, but I wonder if there might be another approach and/or more reasonable zooming methods.

import numpy as np
import matplotlib.pyplot as plt
import matplotlib.patches as patches

N = 5
xy = [np.random.uniform(0., 1000., 2) for _ in range(N)]
x, y = zip(*xy)

# Center of xy points, defined as the center of the minimal rectangle that
# contains all points.
xy_center = ((min(x) + max(x)) * .5, (min(y) + max(y)) * .5)

# Difference between the center coordinates and the xy points.
delta_x, delta_y = xy_center[0] - x, xy_center[1] - y

# Zoom scale (0. < scale)
scale = 1.5

# Scaled xy points.
x_scale = xy_center[0] - scale * delta_x
y_scale = xy_center[1] - scale * delta_y

ax = plt.subplot(111)
# Original xy points.
ax.scatter(x, y, c='b')
# Defined center.
ax.scatter(*xy_center, marker='x', c='g')
# Zoomed points.
ax.scatter(x_scale, y_scale, c='r')
# Square: bottom left corner, width, height
ax.add_patch(
    patches.Rectangle(
        (min(x), min(y)), (max(x) - min(x)), (max(y) - min(y)), fill=False))
plt.show()

When working with NumPy, it's best to keep all our data in the form of NumPy arrays, instead of converting back and forth between NumPy and Python data structures.

So instead of creating a NumPy array for each point and then zipping them to extract tuples of \$x\$-coordinates and \$y\$-coordinates, create one array for all the points:

p = np.random.uniform(0., 1000., (N, 2))

We can extract the \$x\$-coordinates and \$y\$-coordinates, if we need them, by transposing the array:

x, y = p.T

but we won't need to do so until the very end when we pass the data to Matplotlib.
The coordinates of the origin can be computed like this:

o = (p.min(axis=0) + p.max(axis=0)) * .5

To scale the points about the origin, use:

q = o * (1 - scale) + p * scale

instead of (as in the code in the post):

delta = p - o
q = o + delta * scale

The former is slightly quicker, as it has only two arithmetic operations on arrays as long as p whereas the latter has three.

Putting this together:

import numpy as np
import matplotlib.pyplot as plt
import matplotlib.patches as patches

N = 5
p = np.random.uniform(0., 1000., (N, 2))

# Center of points, defined as the center of the minimal rectangle
# that contains all points.
o = (p.min(axis=0) + p.max(axis=0)) * .5

# Scale factor (0. < scale)
scale = 1.5

# Points scaled about center.
q = o * (1 - scale) + p * scale

ax = plt.subplot(111)
ax.scatter(*p.T, c='b')           # Original points.
ax.scatter(*o, marker='x', c='g') # Center.
ax.scatter(*q.T, c='r')           # Scaled points.
ax.add_patch(patches.Rectangle(p.min(axis=0), *p.ptp(axis=0), fill=False))
plt.show()

Here numpy.ptp stands for peak-to-peak and computes the range of values (maximum − minimum) along an axis in an array.


Guice custom behavior for testing I have a Java/Swing/Guice application that uses multiple frames. When the application runs normally, I just want some of these frames to hide (and the "master" frame can unhide them).

When testing the view, I have various other main methods that can open just one frame, and perhaps populate them with sample data. However, then the test application has no way to close (because their normal operation is to be hidden). But, I don't want to mess around with the constructor because I don't want this testing functionality to affect appear anywhere else, I want the "normal operation" module to have no knowledge of this behavior, because it's for testing, only.

Note: this is View testing (like, does it look the way I want it to?), so it's not really appropriate for JUnit.

How is this solution? Is there a better one?

public class SubView {
    private JFrame frame;
    public SubView() {
        initialize();
    }
    private void initialize() {
        frame = new JFrame();
        // etc. note, I don't call frame.setDefaultCloseOperation
        // here, as hide is what I want
    }
    @Inject
    private void setCloser(Closer c) {
        frame.setDefaultCloseOperation(c.closeOperation);
    }
    private static class Closer {
        @Inject(optional=true) @Named("closeOperation")
        private int closeOperation = WindowConstants.HIDE_ON_CLOSE;
    }
}

And then in my test class:

public SubViewTester {
    public static void main(String... args) {
        Module testSpecific = new AbstractModule() {
            @Override
            protected void configure() {
                bindConstant().annotatedWith(Names.named("closeOperation"))
                        .to(WindowConstants.EXIT_ON_CLOSE);
            }
        };
        Injector inj = Guice.createInjector(/* maybeSomeOtherModule, */ testSpecific);
        SubView view = inj.getInstance(SubView.class);
        EventQueue.invokeLater(new Runnable() {
            view.setVisible(true);
        }
    }
}

The Closer class and its setCloser method does not close anything so I'd rather call it CloseOperationProvider. I guess you could get rid of it and annotate the setCloser() parameter instead:

@Inject(optional = true)
private void setCloseOperation(@Named("closeOperation") int closeOperation) {
    frame.setDefaultCloseOperation(closeOperation);
}


Naive php web hook endpoint for use in a production environment Below is the WebHook endpoint in its entirety.  I have five seconds to acknowledge if the payload is correctly formed.  After that, the payload is inserted into a queue for processing.
Importantly, it works, but it feels naive outside a Class and without any exception handling or error checking beyond the signature match. It does however, achieve exactly what I need.
Should for example, $rawPayload = file_get_contents("php://input"); and $authKey = $_SERVER['HTTP_SIGNATURE']; be inside a try catch block?
Alternatively, should it be threaded in case the endpoint is flooded?
<?php

namespace NJIS\Helpers;

use NJIS\Helpers\XeroWebHookHandler;

$key = $_ENV['XERO_WEBHOOK_KEY'];
$rawPayload = file_get_contents("php://input");
$signature = base64_encode(hash_hmac('sha256', $rawPayload, $key, true));
$authKey = $_SERVER['HTTP_SIGNATURE'];

if($signature === $authKey)
{
    http_response_code(200);
    XeroWebHookHandler::insertPayloadIntoDatabase($rawPayload);
}
else
{
    http_response_code(401);
}

Only several years late, but since someone bumped the question let's actually answer it. This code does look somewhat reasonable, but it should be programmed more defensively to avoid situations where people can insert anything in your queue.
Handle all possible outputs
hash_hmac can return a string or the boolean false. You pass this directly to base64_encode, which will happily turn false into an empty string.
var_dump(base64_encode(false)); // string(0) ""

Currently, hash_hmac returns false if the algorithm is not supported. If for some reason sha256 is dropped as an algorithm, you can put anything in the queue with an empty signature. Adding an !empty($signature) check to the if-statement will allow the code to fail safely in case something odd is going on.
file_get_contents can return false on failure, and hash_hmac will happily treat this as an empty string for key generation. You could add a sanity check that fails early if you have nothing useful to add to the database.
Exceptions, try and catch
None of your functions seem to throw, except maybe for XeroWebHookHandler::insertPayloadIntoDatabase. This also means that adding a try-catch block does not do anything useful for you. If you have a way of telling if this function fails, you can move http_response_code below this function and potentially return a different response code if the function fails.


Getting data with RxJava and retrofit I am using RxJava with retrofit to get data from an API then filtering the data with a for loop in the onNext Method.  Is there any way not to use the for loop and just use RxJava to filter or improve the code?

 _subscriptions.add(
                jamSpeakServiceApi.getAllWords()
                        .subscribeOn(Schedulers.io())
                        .observeOn(AndroidSchedulers.mainThread())
                        .subscribe(new Observer<List<WordApi>>() {
                            @Override
                            public void onCompleted() {
                                Log.d("retroffit Completed", "Completed");
                            }

                            @Override
                            public void onError(Throwable e) {
                                Log.d("retroffit error", e.toString());
                            }

                            @Override
                            public void onNext(List<WordApi> wordApis) {
                                /*Observable.from(wordApis)
                                        .filter(s -> Word.getWordFormDatabase(s.getWord()) != null)
                                        .filter(s -> s.getCountry().toLowerCase().equals("jamaica"))
                                        .filter(s -> Word.getWordLikeFormDatabase(s.getWord()) != null)
                                        .subscribe();*/

                                for (WordApi x: wordApis){
                                    if(x.getCountry().toLowerCase().equals("jamaica")== true){
                                        Word.saveWordFromApi(x);
                                        Log.d(x.getWord(),x.getWord());
                                    }
                                }

                            }
                        })
        );

You should use flat to emit each wordApi and then you will be able to filter your data and apply your function one by one.

Your stream should look like this:

jamSpeakServiceApi.getAllWords()
            .subscribeOn(Schedulers.io())
            .flatMap(wordApis -> Observable.from(wordApis))
            .filter(wordApi -> wordApi.getCountry().toLowerCase().equals("jamaica"))
            .observeOn(AndroidSchedulers.mainThread())
            .subscribe(new Observer<WordApi>() {
                @Override
                public void onCompleted() {
                    Log.d("retroffit Completed", "Completed");
                }

                @Override
                public void onError(Throwable e) {
                    Log.d("retroffit error", e.toString());
                }

                @Override
                public void onNext(WordApi wordApi) {
                    Word.saveWordFromApi(wordApi);
                    Log.d(wordApi.getWord(),wordApi.getWord());
                }
            })

I used some lambdas, and I recommend you to use them too. Take a look at the Retrolambda Gradle plugin. It works well with an Android environment.


Sleep with count down display I have a sleep function which displays the time remaining in sec, some feedback would be appreciated

use warnings;

$x=$ARGV[0];
$|++;

$n=0;
# arguments could be 1, 1s , 1m or 1h
if($x=~/([0-9]+)([a-zA-Z]?)/)
{
   $t = lc($2) ;
   if($t eq "h"){$n = $1*3600;}
   elsif( $t eq "m" ){$n = $1*60;}
   elsif( $t eq "s" || $t eq "") {$n = $1;}
   else {die;}
}
else {die;}

while($n)
{
   print $n;
   sleep 1;
   $len=length($n);
   print "\r", " "x$len, "\r"; $n--;
}

This code and updates are available at csleep

Naming

The names are very poor. Consider these renames:

n -> seconds
t -> unit
x -> input

Make the most out of regex

If you only allow "s", "m", or "h" as units, then instead of [a-zA-Z] you could use [smhSMH] in the regex.

Formatting

The formatting is really too compact. It's recommended to put spaces around operators.

use strict

It's recommended to use strict always.
It can help catching bugs.

Alternative implementation

Applying the above suggestions (and a bit more), consider this alternative implementation:

use warnings;
use strict;

my $seconds;

# input could be 1, 1s , 1m or 1h
my $input = $ARGV[0];
if ($input =~ /^([0-9]+)([smh]?)$/i) {
    $seconds = $1;
    my $unit = lc($2);
    my $multiplier = 1;
    if ($unit eq "h") {
        $multiplier = 3600;
    } elsif ($unit eq "m") {
        $multiplier = 60;
    }
    $seconds *= $multiplier;
} else {
    die;
}

$| = 1;  # force flush output on every write
while ($seconds) {
    print $seconds;
    sleep 1;
    my $len = length($seconds);
    print "\r", " " x $len, "\r"; $seconds--;
}


"filter" function in Scheme I know this is a bit elementary, but I am highly uncomfortable writing Scheme code, and I want to make sure that what I'm doing is good practice.  I know mutation is frowned upon in Scheme.
I have no other source of review so I have come to you all.
(define (filter-lst fn lst)
    (if (not (null? lst))
    (if (fn (car lst))
        (cons (car lst) (filter-lst fn (cdr lst)))
        (filter-lst fn (cdr lst))
        )
    '()
    )
)

Note: The function seems to be working according to a few test cases I ran.
As a follow-up PURELY STYLISTIC question, which of these is preferred?
#1
(define (merge l1 l2)
      (if (null? l1) l2
          (if (null? l2) l1
              (cons (car l1) (cons (car l2) (merge (cdr l1) (cdr l2)))))))

#2
(define (merge l1 l2)
    (if (null? l1) l2
        (if (null? l2) l1
            (cons (car l1) (cons (car l2) (interleave (cdr l1) (cdr l2))))
        )
    )
)

When you catch yourself using nested ifs, that's usually a sign you'd be better served by the cond form. (which is actually way more powerful than nested if due to a few quirks)
Here's how I'd clean that up
(define (filter-lst fn lst)
  (cond ((null? lst) '())
        ((fn (car lst))
         (cons (car lst)
               (filter-lst fn (cdr lst))))
        (else (filter-lst fn (cdr lst)))))

The above isn't entirely idiomatic (which would be to implement it as a one-liner fold with a lambda.) but does show off a few style notes. Of your choices I'd say #1 is closest to the preferred style.
First is where you may be tempted to tab, a double space will do as I do on the second line, its usually appropriate to distinct parts separate certain special forms particularly (define, let, cond). You really want to avoid creeping to the right any faster than necessarily.
(define (merge l1 l2)
  (if (null? l1) l2
      (if (null? l2) l1 (cons (car l1) (cons (car l2) (merge (cdr l1) (cdr l2)))))))

Second is that if you split a function, there should be one argument per line, aligned to the start of the first argument
(define (merge l1 l2)
  (if (null? l1)
      l2
      (if (null? l2)
          l1
          (cons (car l1)
                (cons (car l2)
                      (merge (cdr l1) (cdr l2)))))))

And you see why cond is preffered again
(define (merge l1 l2)
  (cond ((null? l1) l2)
        ((null? l2) l1)
        (else (cons (car l1)
                    (cons (car l2)
                          (merge (cdr l1) (cdr l2))))))

And if a spot seems unwieldy there are often algorithmic simplifications that are worth considering
(define (merge l1 l2)
  (if (null? L1)
      l2
      (cons (car l1)
            (merge l2 l1))))


Finding multiple documents I wrote a Mongoose Node/Express application. I have 4 documents. I am trying to write a route (end point) to get all seats for a table with their full data. I get a response like I expect.

I want to know if there is a better way to write the application like this.

seat.model.js

const mongoose = require('mongoose');
const SeatScheme = mongoose.Schema({
    table: {
        type: mongoose.Schema.Types.ObjectId,
        ref: 'Table',
        required: true
    },
    event: {
        type: mongoose.Schema.Types.ObjectId,
        ref: 'Event',
        require: true
    }
});

module.exports = mongoose.model('Seat', SeatScheme);

course.model.js

const mongoose = require('mongoose');
const CourseSchema = mongoose.Schema({
    seat: {
        type: mongoose.Schema.Types.ObjectId,
        ref: 'Seat'
    },
    meal: {
        type: mongoose.Schema.Types.ObjectId,
        ref: 'Meal'
    }
});

module.exports = mongoose.model('Course', CourseSchema);

meal.model.js

const mongoose = require('mongoose');

const MealSchema = mongoose.Schema({
    name: String,
    img: String,
    category: {
        type: mongoose.Schema.Types.ObjectId,
        ref: 'Category'
    }
});

module.exports = mongoose.model('Meal', MealSchema);

category.model.js

const mongoose = require('mongoose');

const CategorySchema = mongoose.Schema({
    displayname: String,
    type: String
});

module.exports = mongoose.model('Categoty', CategorySchema);

seat.route.js

const Seat = require('../../models/seat/seat.model');
const Course = require('../../models/course/course.model');
const Meal = require('../../models/meal/meal.model');
const Category = require('../../models/category/category.model');

module.exports = function (Router) {
    Router.get('/seats/:tableid', (req, res) => {

        Seat.find({ table: req.params.tableid }, (err, seats) => {
            const seatsPromise = seats.map(seat => {
                return Course.find({ seat: seat._id }).then((courses) => {
                    return Promise.all(courses.map(course => {
                        return Meal.findOne({ _id: course.meal }).then(meal => {
                            return Category.findOne({ _id: meal.category}).then(category => {
                                return {
                                    displayname: category.displayname,
                                    type: category.type,
                                    meal: meal
                                }
                            })
                        })
                    })).then( courses => {
                        return {
                            seat: seat._id,
                            courses: courses
                        };
                    });
                })
            });

            Promise.all(seatsPromise).then(seats => {
                res.json(seats);
            })
        })
    });
}

Why aren't using mongoose .populate() method instead?

Refer to the Mongoose documentation for Population here


Sudoku generator in Java I have written this Sudoku Generator. How good/bad is this? How can I improve this?

import java.util.Random;

public class SudokuUtility {

    static final int max = 8;
    static final int min = 0;

    static final int digitMax = 9;
    static final int digitMin = 1;

    static final int easyMin = 36;
    static final int easyMax = 49;

    static final int mediumMin = 32;
    static final int mediumMax = 35;

    static final int hardMin = 22;
    static final int hardMax = 27;

    public static void main(String[] args) {

        int[][] grid = new int[9][9];

        String option = "hard";

        Random random = new Random();

        int row = 0;
        int col = 0;

        int randomNumber = 0;
        int noOfCellsToBeGenerated = 0;

        if ("easy".equals(option)) {
            noOfCellsToBeGenerated = random.nextInt((easyMax - easyMin) + 1) + easyMin;
        } else if ("medium".equals(option)) {
            noOfCellsToBeGenerated = random.nextInt((mediumMax - mediumMin) + 1) + mediumMin;
        } else {
            noOfCellsToBeGenerated = random.nextInt((hardMax - hardMin) + 1) + hardMin;
        }

        for (int i = 1; i <= noOfCellsToBeGenerated; i++) {
            row = random.nextInt((max - min) + 1) + min;
            col = random.nextInt((max - min) + 1) + min;
            randomNumber = random.nextInt((digitMax - digitMin) + 1) + digitMin;

            if (grid[row][col] == 0 && noConflict(grid, row, col, randomNumber)) {
                grid[row][col] = randomNumber;
            } else {
                i--;
            }

        }

        for (int i = 0; i < 9; i++) {
            for (int j = 0; j < 9; j++) {
                System.out.print(grid[i][j] + "  ");
            }
            System.out.println();
        }

    }

    public static boolean noConflict(int[][] array, int row, int col, int num) {

        for (int i = 0; i < 9; i++) {
            if (array[row][i] == num) {
                return false;
            }
            if (array[i][col] == num) {
                return false;
            }
        }

        int gridRow = row - (row % 3);
        int gridColumn = col - (col % 3);
        for (int p = gridRow; p < gridRow + 3; p++) {
            for (int q = gridColumn; q < gridColumn + 3; q++) {
                if (array[p][q] == num) {
                    return false;
                }
            }
        }
        return true;
    }
}

Algorithm
Your algorithm is a bit flawed. One might think that it was that easy, but with your current approach one of three things can happen:

The result has exactly one solution (A traditional Sudoku)
The result has many solutions (Not seen as a traditional Sudoku, as there's no way to logically deduce one solution)
The result has no solutions (Yes, although unlikely, this is a possible outcome of your current approach)

A different approach to generating Sudokus is to fill the entire grid first with a valid and completed Sudoku puzzle, and then (pseudocode following):

Remove a number
Check if the current puzzle is solvable using a Sudoku solving algorithm

If it is solvable, continue
If it is not solvable, put the number you just removed back and either try removing another number, or stop.

By using this different approach your result Sudoku is guaranteed to be solvable and have only one solution.
Making it object-oriented
Your main method is doing many many things, I would recommend changing your main method to do only this:
private static final int DEFAULT_SIZE = 9;
public static void main(String[] args) {
    Sudoku sudoku = new Sudoku(DEFAULT_SIZE, DEFAULT_SIZE);
    sudoku.generate(Difficulty.HARD); // Using @Jeroen's lovely enum!
    sudoku.output();
}

With this structure of your main method, you can see that I'm expecting you to create a Sudoku class. You will hopefully see that you will have many benefits by having a specific Sudoku class. By having a Sudoku class, it will be much easier to expand your code, read your code, modify specific parts of your code, and create a world of new amazing Sudoku-ness.
Example of an unsolvable puzzle
For the sake of simplicity, consider a 4x4 puzzle divided into 2x2 boxes:

  12 34
  -----
a|00 03
b|02 04
 |
c|10 00
d|40 00

This puzzle does not break any Sudoku rules in it's current state, however, it is not possible to create a finished Sudoku puzzle from this state!
If you don't believe me, I ask: Which number should be in the top-left corner?

It cannot be a 1, as there is a 1 on position c1.
It can't be a 2 as there's a 2 in the same box on position b2.
It can't be a 3 as there's a 3 on a4.
It also can't be a 4 as there's a 4 on d1.

There is no valid number we can put in the top-left corner, and therefore it is unsolvable!


Initializing and populating a Python dictionary, key -> List I have a simple task that comes up often my work. I need to create a dictionary for an id key to a list of data associated with that key. I'm wondering if there is a better way to do this than the one I am using. The pattern is so common that I feel there must be better solution.

value_list_dict = {}

for line in f:
  line.strip()
  rec = line.split("\t")
  my_key = rec[0]
  important_value = rec[5] #or what ever it is

  # the repetitive pattern I find myself doing a lot of.
  if my_key not in value_list_dict:
    value_list_dict[my_key] = []
  value_list_dict[my_key].append(important_value)

While this solution is fairly concise and clear to me, I wonder if there is a better way.

I think that writing a custom function for this might be less readable although I'm open to suggestions.

line.strip() does not modify the variable, it returns a modified string. But you can chain it together with the split. I would also jchoose a better variable name than rec. As J.F.Sebastian suggested in the comments, you could go with items:

for line in f:
    items = line.strip().split('\t')

Your code is the perfect place for a collections.defaultdict. You can give it a type which it will use when the key is not defined. It makes implementing a counter a lot easier (just pass it int, whose default constructor returns an int with value 0) or, give it list and it will give you an empty list:

from collections import defaultdict

value_dict = defaultdict(list)

with open("file.txt") as f:
    for line in f:
        items = line.strip().split('\t')
        key, value = items[0], items[5]
        value_dict[key].append(value)

I also added the with..as construct in there, in case you are not yet using it.

In python 3.x I would use the extended iterable unpacking:

key, *other_vals, value = line.strip().split('\t')


HTML and CSS fullscreen landing page I would like to know if this fullscreen page with image and overlay is written clearly. I'm afraid that those two empty divs (topOverlay, and topImage) are not correct. Someone told me that I shouldn't use empty divs when there's another way to do it, but I couldn't find an alternative way in this case. Is there a different suitable tag for this instead of using div?

Everything works great otherwise. I just want it as clean as possible.

@import url(https://fonts.googleapis.com/css?family=Josefin+Slab);

body, html{
    margin: 0;
    height: 100%;
}

/* TOP - fullscreen image with overlay */
#top{
    width: 100%;
    height: 100%;
}
#topOverlay{
    position: absolute;
    width: 100%;
    height: 100%;
    background-color: rgba(26, 188, 156, 0.98);
    z-index: 2;
}
#topImage{
    width: 100%;
    height: 100%;
    background: url("../images/workspace.jpg");
    background-size: cover;
    background-position: center center;
    z-index: 1;
}
#topTextWrapper{
    position: absolute;
    width: 100%;
    height: 100%;
    z-index: 3;
}
#topText{
    position: relative;
    top: 40%;
    transform: translateY(-50%);
    font-family: 'Josefin Slab', serif;
    text-transform: uppercase;
    text-align: center;
    color: white;
}
#topText h1{
    font-size: 3.2em;
    margin:0;
    padding:0;
}
#topTextDescription h2{
   font-size: 1.4em;
    margin:0;
    padding:0;
}

Site header

Site description

I think your idea is good, just try to use less ID and more classes with atomic classes, for example:

Site header

Site description

CSS :

    body, html{
        margin: 0;
        height: 100%;
    }
    .m0{margin:0;}
    .p0{padding:0;}
    .f100{width: 100%;height: 100%;}
    .abs{position: absolute;}

    #topOverlay{
        background-color: rgba(26, 188, 156, 0.98);
        z-index: 2;
    }
    #topImage{
        background: url("../images/workspace.jpg") cover center center;
        z-index: 1;
    }
    #topTextWrapper{
        z-index: 3;
    }
    #topText{
        position: relative;
        top: 40%;
        transform: translateY(-50%);
        font-family: 'Josefin Slab', serif;
        text-transform: uppercase;
        text-align: center;
        color: white;
    }
    #topText h1{
        font-size: 3.2em;
    }
    #topText h2{
       font-size: 1.4em;
    }

See especially how these 4 classes are used in the above example:

.m0{margin:0;}
.p0{padding:0;}
.f100{width: 100%;height: 100%;}
.abs{position: absolute;}

I was able to reduce the CSS from 53 line to 35 ( 35% )
plus its more simple


Line rendering optimization I spent the past hour trying to optimize a block of code I wrote that renders a simple coloured line directly to a backbuffer.

I've compared it with one of the DrawLine method implementations of the WriteableBitmapEx project, and found that my algorithm isn't quite as fast (unless I use Parallel).

public static unsafe void DrawLineVector(this IntPtr target, Line2D line, int colour, int width, int height)
{
    var buf = (int*) target;
    var start = line.Start;
    var lineVector = line.Stop - start;
    var steps = (int)lineVector.Magnitude();
    var uX = lineVector.X / steps;
    var uY = lineVector.Y / steps;
    var col = colour;
    Parallel.For(0, steps, i =>
    //for (var i = 0; i != steps; ++i)
    {
        var x = start.X + uX * i;
        var y = start.Y + uY * i;
        if (x > -1 && y > -1 && x < width && y < height)
            buf[(int)y * width + (int)x] = col;
        //x += uX;
        //y += uY;
    }
    );
}

Note, the Line2D object is basically just a container for two VectorD objects (which are my own implementation of vectors).

Running this as it is currently, will get me, on my machine, roughly 300MPixels per second, while the "default" DrawLine method from WriteableBitmapEx gets me around 170MPixels per second. If I use the 'serial' version of this (i.e. not use Parallel), I get around 100MPixels per second.

Is there any way I can optimize this further?

Because you're comparing your performance with that of the WriteableBitmapEx project, you might like to compare your algorithm with theirs, which is here on github: WriteableBitmapShapeExtensions.cs

You say your performance isn't much slower than theirs (100 versus 170 MPixels/second).

A difference between yours and there's might be that yours is using float or double arithmetic, whereas theirs is using int arithmetic.

Their code is much longer (more lines of code) than yours, but their inner loop is something like this:

        for (int x = x1; x <= x2; ++x)
        {
           pixels[index] = color;
           ys += incy;
           y = ys >> PRECISION_SHIFT;
           if (y != previousY)
           {
              previousY = y;
              index += k;
           }
           else
           {
              ++index;
           }
        }

Note:

Only int (not double)
Few branches
Every write is to a significant pixels (in your algorithm if you write a line at 45 degrees then you do 1.414 writes per pixels).


Searching in array by key => values I need to write a function that searches the two-tier board according to certain parameters, which works, but I question whether there is a simpler, lighter way to perform this task.

<?php
function findValue(array $array, array $parameters, $multipleResoult = false){
    $result = array();//used when $multipleResoult == true
    $suspicious = false;
    foreach($array as $childArray){
        foreach($parameters as $k => $p){
            if(array_key_exists($k,$childArray)){
                if($childArray[$k] == $p){
                    $suspicious = $childArray;
                } else {
                    $suspicious = false;
                    continue 2;
                }
            } else {
                $suspicious = false;
                continue 2;
            }
        }
        if(is_array($suspicious)){
            $result[] = $suspicious;
            if($multipleResoult == true){
                $suspicious = false;
            } else {
                break;
            }
        }

    }
    return $result;
}
$arr =     Array
    (
        0 => Array
        (
            "id" => 1,
            "id_shop" => 1,
            "id_lang" => 1,
            "id_product" => 1,
            "id_field" => 3,
            "field_value" => "zxczxc"
        ),

    // find if single and multiple
    1 => Array
    (
            "id" => 2,
            "id_shop" => 1,
            "id_lang" => 2,
            "id_product" => 1,
            "id_field" => 3,
            "field_value" => "sdfsdfsdf"
        ),

    2 => Array
    (
            "id" => 3,
            "id_shop" => 1,
            "id_lang" => 2,
            "id_product" => 2,
            "id_field" => 3,
            "field_value" => "sdfsdfsdf"
        ),
    // find if multiple
    3 => Array
    (
            "id" => 3,
            "id_shop" => 1,
            "id_lang" => 2,
            "id_product" => 1,
            "id_field" => 3,
            "field_value" => "sdfsdfsdf"
        )

);
echo '-----------------SINGLE----------------------';echo "\n";echo '<pre>';
print_r(findValue($arr, array(
    'id_shop' => 1,
    'id_field' => 3,
    'id_lang'=> 2,
    'id_product' => 1)
                 ));
echo '-----------------MULTIPLE----------------------';echo "\n";echo '<pre>';
print_r(findValue($arr, array(
    'id_shop' => 1,
    'id_field' => 3,
    'id_lang'=> 2,
    'id_product' => 1)
                 ,true));
?>

Firstly, this could use a better description (in a comment) explaining what it does. I had to figure it out from the code. I think the description should probably be something like

// Searches $array for associative arrays which are supersets of $parameters.
// If $multipleResults it will return all matches in an array;
// otherwise just the first, in an array.

Secondly, names.

findValue isn't very descriptive. How about findSupersets?
$array would traditionally be called $haystack in PHP, since you're searching through it for matches to a predicate.
$multipleResoult is a misspelling. Correcting the spelling and adjective-noun agreement we get $multipleResults, which is ok, although I personally would prefer to communicate an action: $returnAll.
$suspicious has the wrong connotations for this native English speaker. If something is suspicious, it means that I think there's something wrong with it. My preferred variable name for something which may or may not pass an acceptance test is $candidate.

Thirdly, since PHP has a dynamic type system I think it would be more idiomatic to return just the element if !$multipleResults, rather than wrapping it in an array.

Now, you specifically ask whether there's a simpler way to do it. I think that using booleans it can be simplified quite a bit by inverting the assumption. Rather than prove it acceptable, prove it not acceptable. Taking into account various of my suggestions above and refactoring I get:

    foreach($haystack as $childArray){
        $isCandidate = true;
        foreach($parameters as $k => $p){
            if(!array_key_exists($k,$childArray) || $childArray[$k] != $p){
                $isCandidate = false;
                break;
            }
        }
        if($isCandidate){
            if(!$multipleResults){
                return $childArray;
            }
            $result[] = $childArray;
        }
    }

    return $result;

It's a couple of years now since I used PHP, and I'm not sure offhand whether there are built-in functions to test whether $childArray contains $parameters as a subset, or to filter an array by a predicate. It may be that the whole function can be reduced to two calls to built-ins. If not, you could consider whether you want to factor it out as

function findMatch(array $haystack, $predicate, $returnAll = false) {
    $results = []
    foreach ($haystack as $element) {
        if ($predicate($element)) {
            if ($returnAll) return $element;
            $results[] = $element;
        }
    }
    return $results;
}

function isSuperset(array $candidate, array $subset) {
    foreach ($subset as $k => $v) {
        if (!isset($candidate[$k]) || $candidate[$k] != $v) return false;
    }
    return true;
}

linking the two with a lambda.


Car price calculator Im writing this simple program for school and using NumberFormat on all the variables seems very repetitive (Note that we were asked to use the NumberFormat method). Is there any way to clean this code so it doesn't look so cluttered?
import java.util.Scanner;
import java.text.NumberFormat;

public class CarPrice {

    public static void main(String[] args) {

        double priceInitial;
        double HST;
        double PDI_CHARGE = 0.02;

        String makeModel;

        Scanner scan = new Scanner(System.in);

        System.out.print("makeModel?: ");
        makeModel = scan.nextLine().toUpperCase();

        do {
            System.out.print("Price?: ");
            priceInitial = scan.nextDouble();
            if (priceInitial < 0) {
                System.out.println("Enter A Value  Above 0");
            } else {
                System.out.println(priceInitial);
                break;
            }
        } while (true);

        do {
            System.out.print("HST?: ");
            HST = scan.nextDouble();
            if (HST < 0) {
                System.out.println("Enter A Value  Above 0");
            } else {
                System.out.println(HST);
                break;
            }
        } while (true);

        double tax = priceInitial * (HST / 100) + priceInitial;

        double PDI = (tax + priceInitial) * PDI_CHARGE;
        double AfterPDI = PDI + tax;

        String PDI_Print = NumberFormat.getCurrencyInstance().format(PDI);
        String tax_Print = NumberFormat.getCurrencyInstance().format(tax);
        String priceInitial_Print = NumberFormat.getCurrencyInstance().format(priceInitial);
        String tax_Amount = NumberFormat.getPercentInstance().format(HST / 100);
        String AfterPDI_Print = NumberFormat.getCurrencyInstance().format(AfterPDI);

        System.out.println("Congrats! Your " + makeModel + " is " + priceInitial_Print +
                "\n Your total after " + tax_Amount + " HST is " + tax_Print +
                "\n with a PDI fee of " + PDI_Print +
                "\n making your total " + AfterPDI_Print);

    }

}

The most important approach to reduce duplicated code is to put the repetitive code into a parameterized method. In case you use an integrated Development Environment (IDE) like eclipse, IntelliJ or alike they have an automated Refactoring to assist you with that. (lookup your IDEs online help how to invoke it).
Here is how I do this step by step
The initial step is to identify the duplicated  parts. In your case the two while/do loops are the first candidates. They are similar that means that there are variables and literals that differ. We need to make them looking the same so that we can replace all of them with the call to the new method to be created.

To be able to do that we need to provide a unique variable scope for both parts. This is done by enclosing each of the loop in braces {}
 {
     do {
         System.out.print("Price?: ");
         priceInitial = scan.nextDouble();
         if (priceInitial < 0) {
             System.out.println("Enter A Value  Above 0");
         } else {
             System.out.println(priceInitial);
             break;
         }
     } while (true);
 }
 {
     do {
         System.out.print("HST?: ");
         HST = scan.nextDouble();
         if (HST < 0) {
             System.out.println("Enter A Value  Above 0");
         } else {
             System.out.println(HST);
             break;
         }
     } while (true);
 }

Inside this braces we declare and use new variables, that have the same names in both places (it is important that the new variables are declared in each block individually and not reused in the lower block!):
 {
     String message = "Price?: ";
     double userInput = 0;
     do {
         System.out.print(message);
         userInput = scan.nextDouble();
         if (userInput < 0) {
             System.out.println("Enter A Value  Above 0");
         } else {
             System.out.println(userInput);
             break;
         }
     } while (true);
     priceInitial = userInput;
 }
 {
     String message = "HST?: ";
     double userInput = 0;
     do {
         System.out.print(message);
         userInput = scan.nextDouble();
         if (userInput < 0) {
             System.out.println("Enter A Value  Above 0");
         } else {
             System.out.println(userInput);
             break;
         }
     } while (true);
     HST = userInput;
 }

Now we select all the lines in the first part that look exactly the same as in the second part. Then you can apply the extract method refactoring of the IDE. The result should look like this:
 {
     String message = "Price?: ";
     // replaces by the IDE
     double userInput = extracted(scan, message);
     priceInitial = userInput;
 }

  // new method created by the IDE behind main()
 private static double extracted(Scanner scan, String message) {
     double userInput = 0;
     do {
         System.out.print(message);
         userInput = scan.nextDouble();
         if (userInput < 0) {
             System.out.println("Enter A Value  Above 0");
         } else {
             System.out.println(userInput);
             break;
         }
     } while (true);
     return userInput;
 }

Of cause you should give it a better name then what the IDE suggested.

In case your IDE did not replace the other occurrence of the selected code itself  you copy the method call from the first block the other one and delete the obsolete code:
  {
      String message = "Price?: ";
      double userInput = extracted(scan, message);
      priceInitial = userInput;
  }
  {
      String message = "HST?: ";
      double userInput = extracted(scan, message);
      HST = userInput;
  }

Then inline the new variables in both blocks
 {
     priceInitial = extracted(scan, "Price?: ");
 }
 {
     HST = extracted(scan, "HST?: ");
 }

And finally remove the blocks too.
 priceInitial = extracted(scan, "Price?: ");
 HST = extracted(scan, "HST?: ");

Why do I suggest this rather complicated approach?

Each single step is a very small change to your code.
You don't need to think about the signature of the new method beside giving it a good name.
After each step you can compile and run the code to check if it still does what it should. (having Unittest will be handy...)


Bi-directional atoi() I am implementing atoi() by traversing a string from the beginning as well as from the end. Is there anything I can improve?

using namespace std;

int atoiLefttoRight(char *s){
    int num = 0;
    int negative = 1;
    if(s)
        {
        if(*s == '-')
            {
            negative = -1;
            s++;
            }
            while(*s && (*s <= '9') && (*s >= '0'))
            {
            num = (num * 10) + (*s - '0');
            s++;
            }
        }
return (negative * num);
}

int atoiRighttoLeft(char *s){
    int i = 1;
    int num = 0;
    int negative = 1;

    if(s){
        if(*s == '-')
                      {
            negative = -1;
            s++;
           }
    char *temp = s;
    while(*temp)
    temp++;
    temp--;

    while(*temp && temp >= s)
    {
        if( *temp <= '9' && *temp >= '0'){
        num = num + ( (*temp - '0') * i);
        i = i*10;
        temp--;
        }
        else
        {
        temp--;
        num = 0;
        i = 1;
        }
    }
    }
return (negative * num);
}

int main()
{
cout<<atoiLefttoRight("-12ab3")<<"\n";
cout<<atoiRighttoLeft("1234h5")<<"\n";
}

For the moment I'll concentrate on the left to right version. Most of the same comments apply to the right to left version as well though.

int atoiLefttoRight(char *s){

Since you're not going to modify the original string, the parameter should be char const *s.

    int num = 0;
    int negative = 1;
    if(s)
        {
        if(*s == '-')

This is an...unusual indentation style. Personally, I'd normally prefer:

if (s) {
    if (*s == '-')

Others prefer:

if (s)
{
    if (*s == '-')

Given the degree to which these two styles dominate, I'd nearly always choose between them rather than trying to invent yet another.

            {
            negative = -1;
            s++;

Although it's unlikely (in the extreme) to make a difference in this case, in cases like this where you can use either prefix or postfix increment (or decrement) I'd choose the prefix form, making this ++s;

            }
            while(*s && (*s <= '9') && (*s >= '0'))

You have this indented under the if statement as if it were controlled by the if statement (but it's not).

I'd also prefer to use isdigit to determine whether a character is a digit.

Finally, this can fail to correctly convert at least one legitimate input. For example, given a 16-bit int, an input of -32768 is legitimate meaningful--but this won't convert it correctly, because it tries to represent the digits as a positive signed number (but the largest signed 16-bit number is 32767).

With a 32-bit or 64-bit int, the number gets larger but the same basic idea applies (and if you had an even larger int, such as 128-bit, the same would still be true--bigger number, but still a number where it would fail).


Initializing an Array of Alternating Values My goal in this project is to make something that can—as, quickly, effectively, and efficiently as Array(repeating: , count: )—initialize an Array of alternating values.

Note: I am using Swift 4.1 and Xcode 9.3

My Array Initialization:

extension Array {
    init (repeatingValues arr: Array, count: Int) {
        precondition(!arr.isEmpty, "Initialization values cannot be empty")
        precondition(count > 0, "Count cannot be less than 1")
        var newArr = Array<Element>()
        for i in 0..<count {
            newArr.append(arr[i % arr.count])
        }
        self = newArr
    }
}

Usage

Initialization - init(repeatingValues: , count: ):

let array = Array(repeatingValues: [true, false], count: 10)

print(array) //prints: [true, false, true, false, true, false, true, false, true, false]

Benchmark Comparison

Comparing: Array(repeating: , count: ) vs. Array(repeatingValues: , count: )

Benchmark Time Extension:

extension Date {
      func elapsedTime(to date: Date) -> String {

        let attoseconds100 = date.timeIntervalSince(self) * 10000000000000

        switch attoseconds100 {
        case 6048000000000000000...:
            let weeks : Int = Int(attoseconds100 / 6048000000000000000)
            return "\(weeks)w" + " " + "\(Int(attoseconds100 / 864000000000000000) - (weeks * 7))d"

        case 864000000000000000...:
            let days : Int = Int(attoseconds100 / 864000000000000000)
            return "\(days)d" + " " + "\(Int(attoseconds100 / 36000000000000000) - (days * 24))h"

        case 36000000000000000...:
            let hours : Int = Int(attoseconds100 / 36000000000000000)
            return "\(hours)h" + " " + "\(Int(attoseconds100 / 600000000000000) - (hours * 60))m"

        case 600000000000000...:
            let mins : Int = Int(attoseconds100 / 600000000000000)
            return "\(mins)m" + " " + "\(Int(attoseconds100 / 10000000000000) - (mins * 60))s"

        case 10000000000000...:
            let secs : Int = Int(attoseconds100 / 10000000000000)
            return "\(secs)s" + " " + "\(Int(attoseconds100 / 10000000000) - (secs * 1000))ms"

        case 10000000000...:
            let millisecs : Int = Int(attoseconds100 / 10000000000)
            return "\(millisecs)ms" + " " + "\(Int(attoseconds100 / 10000000) - (millisecs * 1000))μs"

        case 10000000...:
            let microsecs : Int = Int(attoseconds100 / 10000000)
            return "\(microsecs)μs" + " " + "\(Int(attoseconds100 / 10000) - (microsecs * 1000))ns"

        case 10000...:
            let nanosecs : Int = Int(attoseconds100 / 10000)
            return "\(nanosecs)ns" + " " + "\(Int(attoseconds100 / 10) - (nanosecs * 1000))ps"

        case 10...:
            let picosecs : Int = Int(attoseconds100 / 10)
            return "\(picosecs)ps" + " " + "\(Int(attoseconds100 / 0.01) - (picosecs * 1000))fs"

        case 0.01...:
            let femtosecs : Int = Int(attoseconds100 * 100)
            return "\(femtosecs)fs" + " " + "\((Int(attoseconds100 / 0.001) - (femtosecs * 10)) * 100)as"
        case 0.001...:
            return "\(Int(attoseconds100 * 100000))as"

        default:
            return "Less than 100 attoseconds"
        }
    }
}

Array(repeating: , count: ):

let start = Date()
let _ = Array(repeating: true, count: 1000000)
let end = Date()
print(start.elapsedTime(to: end)) //2ms 470μs

Execution Time: 2ms 470μs

Array(repeatingValues: , count: ):

let start = Date()
let _ = Array(repeatingValues: [true, false], count: 1000000)
let end = Date()
print(start.elapsedTime(to: end)) //472ms 555μs

Execution Time: 472ms 555μs

Results:

Array(repeatingValues: , count: ) 191.3x slower than Array(repeating: , count: )

How can I match Apple's efficiency and speed?

On my computer (a 1.2 GHz Intel Core m5 MacBook) I measured

Array(repeating:count:)        663μs 995ns
Array(repeatingValues:count:)  13ms 105μs

with the code compiled in the Release configuration, i.e. with
optimizations. This is approximately a factor of 20 between those methods.

It seems that one “culprit” is the remainder calculation in

        newArr.append(arr[i % arr.count])

If we replace that by an addition with wrap-around test

public init(repeatingValues arr: Array, count: Int) {
    precondition(!arr.isEmpty, "Initialization values cannot be empty")
    precondition(count > 0, "Count cannot be less than 1")

    var newArr = Array<Element>()
    var srcIndex = 0
    for _ in 0..<count {
        newArr.append(arr[srcIndex])
        srcIndex += 1
        if srcIndex == arr.count { srcIndex = 0 }
    }
    self = newArr
}

then the performance improves to

Array(repeatingValues:count:)   3ms 315μs

which is “only” by a factor of 5 slower than Array(repeating:count).

Another possible bottleneck is the array bounds check on each access.
This can be bypassed  by accessing the element storage directly:

init (repeatingValues2 arr: Array, count: Int) {
    precondition(!arr.isEmpty, "Initialization values cannot be empty")
    precondition(count > 0, "Count cannot be less than 1")
    var newArr = Array(repeating: arr.first!, count: count)
    newArr.withUnsafeMutableBufferPointer { src in
        arr.withUnsafeBufferPointer { dest in
            var j = 0
            for i in 0..<count {
                src[i] = dest[j]
                j += 1
                if j == arr.count { j = 0 }
            }
        }
    }
    self = newArr
}

This reduces the execution time to

Array(repeatingValues:count:)   1ms 889μs

which is now slower roughly by a factor of 3 than Array(repeating:count).

Other possibly useful technique is to avoid reallocations of the
array element storage by calling

    newArr.reserveCapacity(count)

However, this did not make a significant difference in my tests.

One further remark: Requiring the destination count to be strictly
positive seems unnecessary restrictive to me. I would change that to

init (repeatingValues arr: Array, count: Int) {
    precondition(!arr.isEmpty, "Initialization values cannot be empty")
    precondition(count >= 0, "Count cannot be negative")
    if count == 0 {
        self = []
        return
    }

    // ...
}


How to think smarter when you need diff args in functions I have this code, and the point is I want to dynamically change the mongo connecter to change between ssl, not ssl and with and without username in the same connecter function.

if auth_required:
    if config.getboolean(component, 'ssl', fallback=False):
        client = MongoClient(
            '{host}:{port}'.format(host=config[component]['host'], port=config[component]['port']),
            ssl=True,
            ssl_ca_certs=config.get(component, 'ssl_path'),
            serverSelectionTimeoutMS=timeout,
            username=username,
            password=password,
            authSource=auth_source,
            authMechanism=auth_mechanism
        )
    else:
        client = MongoClient(
            '{host}:{port}'.format(host=config[component]['host'], port=config[component]['port']),
            serverSelectionTimeoutMS=timeout,
            username=username,
            password=password,
            authSource=auth_source,
            authMechanism=auth_mechanism
        )
else:
    if config.getboolean(component, 'ssl', fallback=False):
        client = MongoClient(
            '{host}:{port}'.format(host=config[component]['host'], port=config[component]['port']),
            ssl=True,
            ssl_ca_certs=config.get(component, 'ssl_path'),
            serverSelectionTimeoutMS=timeout
        )
    else:
        client = MongoClient(
            '{host}:{port}'.format(host=config[component]['host'], port=config[component]['port']),
            serverSelectionTimeoutMS=timeout
        )

If you build a dictionary of keyword arguments (customarily called kwargs), you can address the two options separately and consolidate everything into one call to MongoClient. This has the advantage that if you update e.g. the SSL part, you only need to change it in one place instead of remembering to change it twice:

host = '{host}:{port}'.format(**config[component])
kwargs = {"serverSelectionTimeoutMS": timeout}

if config.getboolean(component, 'ssl', fallback=False):
    kwargs["ssl"] = True
    kwargs["ssl_ca_certs"] = config.get(component, 'ssl_path')

if auth_required:
    kwargs.update({"username": username, "password": password,
                   "authSource": auth_source, "authMechanism": auth_mechanism})

client = MongoClient(host, **kwargs)

I also used the fact that config[component] seems to be a dictionary, so you can unpack it in the format and the host and port keys will be used.

I used two different ways to change the values in the dictionary so you can choose, depending on which one is easier to read. Using both is also fine, as I did here. For the SSL part there is few values, so using dict.update would make it more cluttered, but for the auth part there are enough values that typing kwargs[...] = ... would get tedious.


Handling workflow rules for a job application process For some reason I always have a negative feeling towards having many if statements in my JavaScript functions. But I've got some code that is processing an object that defines rules of an event. I need to determine one or many things to do based on the "rules" outlined in each object.

(Particularly four if statements deep within a for loop just gives me the heebie-jeebies for some reason.)

Is there a faster way to handle this type of rules-like processing? Clearly I could break this into some smaller functions to clean up the syntax, but I am mostly looking for feedback/suggestions around speed of processing the request.

The idea here is to capture customer conversion events throughout a user's interaction with a job application. Each conversion has an 'initiate' and a 'complete'. The "rules" which determine when to initiate and when to complete each conversion are setup in a JSON file which I then parse into a JavaScript object: applicationConversionEvents.

The JSON looks something like so:

[
  {
    "eventId": "view",
    "eventName": "App View to Start",
    "initiateOnStepNumber": 100,
    "completeIfAuthenticated": true,
    "completeOnElementClick": "first_name"
  },
  {
    "eventId": "submit",
    "eventName": "App Start to Submit",
    "initiateOnEventComplete": "view",
    "completeOnStepNumber": 450,
    "completeOnEventInitiate": "decline"
  },
  {
    "eventId": "decline",
    "eventName": "App Decline",
    "initiateOnStepNumber": 350,
    "completeOnStepNumber": [
      350,
      375
    ]
  }
]

I am then using the following function to parse the rules for each conversion event, and when appropriate fire off a new event either as an 'initiate' or a 'complete'. I also track what events fire in sessionStorage to avoid duplicates per application.

var _processApplicationConversionEvents = function(pageInfoArrayIndex, options) {
    _log('Processing application conversion events.', LOG_LEVEL.INFO);

    if(!Array.isArray(applicationConversionEvents)) return;
    if(!_isValidPageInfoArrayIndex(pageInfoArrayIndex)) pageInfoArrayIndex = 0;

    _log('Conversions for pageInfoArrayIndex: ' + pageInfoArrayIndex, applicationConversionEvents, LOG_LEVEL.DEBUG);

    var pageInfo = _getPageInfo(pageInfoArrayIndex, options);
    _log('pageInfo details for conversion events:', pageInfo, LOG_LEVEL.DEBUG);

    for(var i = 0, len = applicationConversionEvents.length; i < len; i++) {
        var conversionEvent = applicationConversionEvents[i];
        if(typeof conversionEvent !== 'object') continue;

        if(conversionEvent.initiateOnStepNumber) {
            var appStepNumber = _asArray(conversionEvent.initiateOnStepNumber);

            if(appStepNumber.indexOf(+pageInfo.appStepNumber) > -1) {
                _newConversionEvent('initiate', conversionEvent, pageInfo);

                if(_toLowerCaseString(conversionEvent.completeIfAuthenticated) === 'true'
                   && pageInfo.authenticated) {
                    _newConversionEvent('complete', conversionEvent, pageInfo);

                    if(!conversionEvent.completeOnElementClick) continue;
                    _removeConversionOnClickListener(conversionEvent.completeOnElementClick);
                }
            }
        }

        if(conversionEvent.completeOnStepNumber) {
            var appStepNumber = _asArray(conversionEvent.completeOnStepNumber);

            if(appStepNumber.indexOf(+pageInfo.appStepNumber) > -1) {
                _newConversionEvent('complete', conversionEvent, pageInfo);
                continue;
            }
        }

        if(conversionEvent.completeOnElementClick) {
            _addConversionOnClickListener('complete', conversionEvent, pageInfo);
        }
    }
}

There is really not enough context here to talk to any performance improvements.  On the surface, there doesn't seem to be any way to optimize on this simple O(n) array iteration. I have no idea what is happening in any of the functions being called though.

You have some room for improvement on coding style IMO.

You understand the continue command but you are not using it effectively.  Consider inverting your conditionals in cases like this to get code structured more like this:

if (!someobj.hasOwnProperty(someProp)) continue;

// some logic

if (!someOthetRule) continue;

// some more logic

This eliminates all those nested ifs.

Your code is also extremely densely packed making it hard to read. You should really try to keep your lines under 80 characters, breaking longer lines across lines. And while I am normally a big proponent of meaningful variable and function names, you seem to almost take it to the extreme, using overly verbose variables when sometimes it does not seem necessary. Take for example how in your for loop you store the array length into conversionsLen when something short and simple like len would be just fine since it is clear what is happening here.


Is it proper to use nested loops a lot in python? I have a project and I am trying to do a complex cryptographic method. Is it normal to use nested loops a lot? Or Did I miss something?

I intend to create a method which tries all strings to find password. For example, It should create each of these one by one: ('A','A') ('B', 'A') ('A', 'B') ('B', 'B') ('A', 'AA' ) ('A', 'AB') ('B', 'AA') ('B', 'AB') ('A', 'BA') ('B', 'BA') ('A', 'BB') ('B', 'BB') , (triple permutations), (quadruple permutations).. and it goes...

def rulefinder():
    global externalrul1
    global externalrul2
    rul2 = uniquestring1[0]
    rul1 = uniquestring1[0]
    for n in range (0,3):
     for m in range (0, len(uniquestring1)):
      for z in range(0, n+1):
        for k in range (0,3):
            for i in range(0, len(uniquestring1)):
                for o in range(0, k+1):
                    for y in range (0,  len(uniquestring1)):
                        rul2 = rul2[:-1] + uniquestring1[y]
                        for x in range (0, len(uniquestring1)):
                            rul1= rul1[:-1] + uniquestring1[x]
                            code=""
                            for cha in Genx1:
                                if cha==uniquestring1[0]:
                                    code +=codereplacer(rul1)
                                elif cha==uniquestring1[1]:
                                    code +=codereplacer(rul2)
                            print(code)
                            print(uniquestring1[0],rul1)
                            print(uniquestring1[1],rul2)
                            print(LastString)
                            if code == LastString:
                                axiom1=uniquestring1[0]
                                axiom2=uniquestring1[1]
                                externalrul1=rul1
                                externalrul2=rul2
                                print('rules are found')
                                print("First RULE:", uniquestring1[0], rul1)
                                print("Second RULE:", uniquestring1[1], rul2)
                                findsubgeneration(code, axiom1, rul1, axiom2, rul2)
                                return
                    rul1 = rul1[:o] + uniquestring1[i] + rul1[(o + 1):]
            rul1 += codereplacer(uniquestring1[i])
        rul2 = rul2[:z] + uniquestring1[m] + rul2[(z + 1):]
        rul1 =""
     rul2 += codereplacer(uniquestring1[m])

From what I understand from your comment, using itertools did solve your problems.

Here are a few suggestions anyway (that you may want to take into account before submitting a new question with the updated code):

instead of using global variables, you should add parameters to your functions.
you could take this chance to add a proper documentation about what your function is supposed to do and what the parameters are.
the indentations seem to be of various sizes. PEP8, the Style Guide for Python code recommands using 4 spaces. It is probably worth reading the whole document.


Comparison with a range I'm developing a class to compare prices. For example, give me products which price are above 50 or under 50. Here's my API:

var indicator = new PriceIndicator(PriceComparison.Above, 50);
var actual = indicator.Apply(listOfProductPrices);

I managed to design my code for the operator Above and Under comparison:

public abstract class PriceComparison
{
    public static readonly UnderPriceComparison Under = new UnderPriceComparison();

    public static readonly AbovePriceComparison Above = new AbovePriceComparison();

    public abstract bool Evaluate(double price, double valueToCompare);
}

public class AbovePriceComparison : PriceComparison
{
    public override bool Evaluate(double price, double valueToCompare)
    {
        return price > valueToCompare;
    }
}

public class UnderPriceComparison : PriceComparison
{
    public override bool Evaluate(double price, double valueToCompare)
    {
        return price < valueToCompare;
    }
}

public class PriceIndicator : IBuyIndicator
{
    private readonly PriceComparison _comparison;
    private readonly int _value;

    public PriceIndicator(PriceComparison comparison, int value)
    {
        _comparison = comparison;
        _value = value;
    }

    public IList<BuyOrder> Apply(IList<int> productPrices)
    {
        return productPrices.Where(price => _comparison.Evaluate(price, _value))
                            .Select(b => new BuyOrder(b))
                            .ToList();
    }
}

Now, I want to manage Range price. I want my API to look as simple as the one above. Ideally, it would be

var indicator = new PriceIndicator(PriceComparison.Range, 50, 100); // Here's the change
var actual = indicator.Apply(listOfProductPrices);

I don't know how I can design this. What I tried to do so far is to add another Evaluate method in PriceComparison, which takes three parameters: double price, double left, double right.  But then, my previous two methods have to implement it and will have to throw IncorrectComparisonException because they can only compare with one value. So I'm not really fan of this design.

Here you go. I made Range inclusive only, but it's easy to modify. I also extracted a few interfaces for easy unit testing and use IEnumerable<T> instead of IList<T>.

public interface IPriceComparison
{
    bool Evaluate(decimal price, params decimal[] valuesToCompare);
}

public abstract class PriceComparison : IPriceComparison
{
    private static readonly IPriceComparison under = UnderPriceComparison.Create();

    private static readonly IPriceComparison above = AbovePriceComparison.Create();

    private static readonly IPriceComparison range = RangePriceComparison.Create();

    public static IPriceComparison Under
    {
        get
        {
            return under;
        }
    }

    public static IPriceComparison Above
    {
        get
        {
            return above;
        }
    }

    public static IPriceComparison Range
    {
        get
        {
            return range;
        }
    }

    public abstract bool Evaluate(decimal price, params decimal[] valuesToCompare);
}

public sealed class AbovePriceComparison : PriceComparison
{
    private AbovePriceComparison()
    {
    }

    public static IPriceComparison Create()
    {
        return new AbovePriceComparison();
    }

    public override bool Evaluate(decimal price, params decimal[] valuesToCompare)
    {
        return (valuesToCompare == null) || (valuesToCompare.Length != 1) || (price > valuesToCompare[0]);
    }
}

public sealed class UnderPriceComparison : PriceComparison
{
    private UnderPriceComparison()
    {
    }

    public static IPriceComparison Create()
    {
        return new UnderPriceComparison();
    }

    public override bool Evaluate(decimal price, params decimal[] valuesToCompare)
    {
        return (valuesToCompare == null) || (valuesToCompare.Length != 1) || (price < valuesToCompare[0]);
    }
}

public sealed class RangePriceComparison : PriceComparison
{
    private RangePriceComparison()
    {
    }

    public static IPriceComparison Create()
    {
        return new RangePriceComparison();
    }

    public override bool Evaluate(decimal price, params decimal[] valuesToCompare)
    {
        return (valuesToCompare == null)
            || (valuesToCompare.Length != 2)
            || ((price > valuesToCompare[0]) && (price < valuesToCompare[1]));
    }
}

public interface IBuyOrder
{
    int Price
    {
        get;
    }
}

public sealed class BuyOrder : IBuyOrder
{
    private readonly int price;

    private BuyOrder(int price)
    {
        this.price = price;
    }

    public static IBuyOrder Create(int price)
    {
        return new BuyOrder(price);
    }

    public int Price
    {
        get
        {
            return this.price;
        }
    }
}

public interface IBuyIndicator
{
    IEnumerable<IBuyOrder> Apply(IEnumerable<int> productPrices);
}

public sealed class PriceIndicator : IBuyIndicator
{
    private readonly IPriceComparison comparison;

    private readonly decimal[] values;

    private PriceIndicator(IPriceComparison comparison, params decimal[] values)
    {
        if (comparison == null)
        {
            throw new ArgumentNullException("comparison");
        }

        this.comparison = comparison;
        this.values = values;
    }

    public static IBuyIndicator Create(IPriceComparison comparison, params decimal[] values)
    {
        return new PriceIndicator(comparison, values);
    }

    public IEnumerable<IBuyOrder> Apply(IEnumerable<int> productPrices)
    {
        return productPrices.Where(price => this.comparison.Evaluate(price, this.values))
            .Select(BuyOrder.Create)
            .ToList();
    }
}


JavaScript to parse json that can be, string, object and array I have to parse a json object that can be a string, array, array of strings and array of object. I realised that it's not good from the beginning that one object can be many types, but I can't change the code from upstream so
I'll have to deal it in my code instead. I'm building a pixel library for modern browser so I'm not using jQuery or lodash. I'm supporting most modern browser and IE >= 9

Here's the example of the data that can be returned

"author": {
 "@type": "Person",
 "name": "Author"
}

Or

"author":[{"@type":"Person","name":"Author"}]

Or

"author": 'Author'

Or

"author": ['Author', 'Author1']

And this is my code.

  let obj = {};
  try {
    const json = document.querySelector('div.json');
    if (json) {
      let disc = JSON.parse(json.innerHTML);
      let authors = disc.author;

      if (typeof authors !== 'undefined' && Array.isArray(authors) && authors.length > 0) {
        authors = authors.map((author) => {
          if (typeof author === 'object' && author.name) {
            return author.name;
          } else {
            return author;
          }
        });
      }

      if (typeof authors !== 'undefined' && !Array.isArray(authors) && typeof authors === 'object') {
        authors = [authors.name];
      }

      if (typeof authors !== 'undefined' && typeof authors === 'string') {
        authors = authors.split(',');
      }

      obj.cAu: authors.join(',');
    }
  } catch (e) { }

  return obj;

My question is, is there a better way to do this in a more efficient way?

Personally I would have written the function as below.
One thing that stood out that I noticed is that you are not returning anything in the catch statement of your try. So there is a certain 'ambiguity' as to what the function will return.

Here is my reasoning for the code style below:

I expect the invoker to provide me with the JSON string that needs to data extracted from it, so that I can perform some immediate input validation. If the input is falsey I will return an empty string.
I check whether the parsed result is an instance of an array (undefined or null will not pass this check)
If it is an instance of an array I will check whether it is an array of objects, otherwise I will assume it is a string. I map these entries, and join them with a comma and return the result
if the type is of an object (and not array since we already checked for that), I will return the name property of that object.

Feel free to wrap this in a try'catch but make sure you return something on the catch. I assume the invoker is expecting a string back.

function parseAuthor(authorString){
  if(!authorString){
    return '';
  }

  const authorObj = JSON.parse(authorString);

  if (typeof authorObj === 'string') {
    return authorObj;
  }

  if (authorObj instanceof Array) {
    entries = authorObj.map((a) => {
      if(typeof a === 'object' && a.name){
        return a.name;
      } else {
        return a;
      }
    });

    return entries.join(',');
  }

  if (typeof authorObj === 'object' && authorObj.name) {
    return authorObj.name;
  }

  return '';
}