Writing many cells to sheet causes Excel to not respond, replacing vlookups I have found that my code causes my Excel to 'not respond' when it gets to this for loop, For thisRow = 2 To lastWSrow(outWS). I'm guessing this is because I am writing cell by cell and the data is too much for excel to handle.

My code essentially does 55 of vlookups on sheet2. It would be similar to doing vlookup(c2&y2,Sheet1!I:J,2,false) but manually it takes long to do.

Could anyone look at this loop and help me optimize this area? I have left only the part needed adjustment. I have put comments on each line to provide description.

outws = sheet2

For thisRow = 2 To lastWSrow(outWS) '2 to last row of sheet2
    For thisCol = 1 To UBound(mappings, 1)

        'create unique key
        thisScen = outWS.Cells(thisRow, posIDcol).Value & "|" & mappings(thisCol, 1)

        'search
        thisDataRow = findInArrCol(thisScen, 1, scenData)

        'write to sheet2
        If thisDataRow = 0 Then
            If outWS.Cells(thisRow, posUnitsCol).Value <> 0 Then 'missing scenario
                outWS.Cells(thisRow, mappings(thisCol, 3)).Value = "No data"
                outWS.Cells(thisRow, mappings(thisCol, 3)).Value = 0

            End If
        Else
            If mappings(thisCol, 1) = "irpv01|PV01_Swap_1M" Then
                outWS.Cells(thisRow, PV01_1MCol).Value = outWS.Cells(thisRow, PV01_1MCol).Value + scenData(thisDataRow, 2) * scenData(thisDataRow, 3) 'since map PV01_0D to PV01_1m already, sum PV01_1M to that column in database.
            Else
                outWS.Cells(thisRow, mappings(thisCol, 3)).Value = scenData(thisDataRow, 2) * scenData(thisDataRow, 3) 'need to scale by position units to get correct risk
            End If
        End If

    Next thisCol
Next thisRow

findInArrCol

Public Function findInArrCol(matchVal As Variant, matchCol As Long, sortedArr() As Variant, Optional nearest As String) As Long

    Dim low As Long, mid As Long, high As Long

    findInArrCol = 0
    low = 1
    high = UBound(sortedArr, 1)
    Do While low <= high
        mid = (low + high) / 2
        If sortedArr(mid, matchCol) = matchVal Then
            findInArrCol = mid
            Exit Function
        ElseIf sortedArr(mid, matchCol) < matchVal Then
            low = mid + 1
        Else
            high = mid - 1
        End If
    Loop

    If findInArrCol = 0 Then
        If nearest = "lessThan" Then
            findInArrCol = WorksheetFunction.Max(high, 1)
        End If
        If nearest = "greaterThan" Then
            findInArrCol = WorksheetFunction.Min(low, UBound(sortedArr, 1))
        End If
    End If

End Function

As was explained in comments elsewhere, the Excel VBA thread is sending messages to the WM message queue to process information, but the same thread is running the for/next loop(s) and will not begin as you had the code until the loops are complete.  By inserting a DoEvents, it causes the VBA code to perform slower because it then allocates some time to the O/S to take care of other pending WM messages and returns back to the VBA code.  Too many DoEvents will greatly slow down your code processing and can even be harmful in some cases where another process can actually take over (hog) more processor time.  The solution is to use DoEvents sparingly to allow Excel to process the rows a few at a time.  What the ideal number is varies by situation, but you can experiment with various batch sizes determined by using modulo arithmetic.

Modify your loop in this way:

        Next thisCol
        If (thisRow Mod 100) = 0 Then
            DoEvents
        End If
    Next thisRow

You can tweak this number by replacing 100 with 10, 50, 250, 500, etc. to see what provides the most desirable result.


Job Scheduling Algorithm Changed Program based on suggestions. New Code: Job Scheduling Algorithm 2
I have created an algorithm for job scheduling. The algorithm goes through sub-lists in order with two nested for loops. Inside the nested for loops, the algorithm counts how many tasks for each job are completed. If this is equal to the number of tasks, then the profit for that job is added to the total profit.
Item start to Item end is an item using that machine from start to end. Machine start to Machine end is when the machines can process the items. A single task is a single machine doing items. The number required for a job is tasks to be completed while done tasks are tasks that will finish in the schedule. If those two counts are equal then the job is done, and profit is added to the profit var.
Here is the code
def output_profit(profit:int)->None:
    print("profit: " + str(profit), end = "\n")

def output_subset(subset:[str])->None:
    for item in subset:
        print(str(item), end = " ")

def main():
    items = ["a", "b"]
    items_starts = [0, 3]
    items_ends = [2, 4]

    #total number of tasks that are needed for job i
    tasks_to_complete = [1,1]

    #tasks that are done for job i
    done_tasks = [0, 0]

    machine_starts = [0, 0]
    machine_ends = [1, 7]

    profits_for_job = [10, 12]
    profit = 0

    for row in range(0, len(items)):
        for col in range(0, len(items) + 1):
            subset = items[row:col]
            for job_index in range(0, len(subset)):
                if items_starts[job_index] >= machine_starts[job_index]:
                    if items_ends[job_index] <= machine_ends[job_index]:
                        done_tasks[job_index] = done_tasks[job_index] + 1
            profit = 0
            for job_index in range(0, len(subset)):
                if tasks_to_complete[job_index] == done_tasks[job_index]:
                    profit = profit + profits_for_job[job_index]

            output_profit(profit)
            output_subset(subset)

if __name__ == "__main__":
    main()

I am looking for ways to improve the code readability and improve the algorithm's efficiency.

Functions
It's good that you're thinking about how to capture code in functions, but you haven't particularly chosen the right code to move into functions.
This is somewhat trivial:
print("profit: " + str(profit), end = "\n")

and does not deserve its own function; simply write
print(f'profit: {profit}')

at the outer level. The same applies for output_subset, which does not need a loop and can be
    print(' '.join(item for item in subset))

Instead, something that does deserve to be in a separate function is your set of loops starting at for row, which can be translated into a generator; also note that 0 is the default start for range:
ProfitPair = Tuple[
    int,
    List[str],
]

def get_profits( ... variables needed for iteration ...) -> Iterable[ProfitPair]:
    for row in range(len(items)):
        for col in range(len(items) + 1):
            subset = items[row:col]
            for job_index in range(len(subset)):
                if items_starts[job_index] >= machine_starts[job_index]:
                    if items_ends[job_index] <= machine_ends[job_index]:
                        done_tasks[job_index] = done_tasks[job_index] + 1
            profit = 0
            for job_index in range(len(subset)):
                if tasks_to_complete[job_index] == done_tasks[job_index]:
                    profit += profits_for_job[job_index]

            yield (profit, subset)

Type hints
It's good that you've tried this out. subset:[str] should be subset: List[str].
Indexing
for row in range(0, len(items)):
    for col in range(0, len(items) + 1):
        subset = items[row:col]

seems strange to me. Based on your initialization, items is not a two-dimensional (nested) list - unless you count string indexing as the second dimension. row and col are thus somewhat misnamed, and are basically start and end.
In-place addition
done_tasks[job_index] = done_tasks[job_index] + 1

should be
done_tasks[job_index] += 1

Summation with generators
        profit = 0
        for job_index in range(0, len(subset)):
            if tasks_to_complete[job_index] == done_tasks[job_index]:
                profit = profit + profits_for_job[job_index]

can be
profit = sum(
    profits_for_job[job_index]
    for job_index in range(len(subset))
    if tasks_to_complete[job_index] == done_tasks[job_index]
)

This raises another point, though. Consider "rotating" your data structure so that, instead of multiple sequences where the same index in each correspond to a description of the same thing, e.g.
profits_for_job[job_index]
tasks_to_complete[job_index]
done_tasks[job_index]

instead have a sequence of @dataclasses with attributes:
job[job_index].profits
job[job_index].tasks_to_complete
job[job_index].tasks_done

Predicate combination
            if items_starts[job_index] >= machine_starts[job_index]:
                if items_ends[job_index] <= machine_ends[job_index]:
                    done_tasks[job_index] = done_tasks[job_index] + 1

can just be
if (
    items_starts[job_index] >= machine_starts[job_index] and
    items_ends[job_index] <= machine_ends[job_index]
):
    done_tasks[job_index] += 1


Checking whether an array contains a value or another array I'm designing an isIn method that takes a value:any and checks whether it exists in an array. It's working, but I'm wondering whether I missed any edge cases:

/**
 * Checks if given value is in the target array of allowed values.
 *
 * @param value The value being checked.
 * @param target The target value to perform the check against.
 * @return True if the value is in the target array, false otherwise.
 */
export function isIn(value: any, target: any[]): boolean {
  if (!isArray(value)) {
    return !isArray(target) || target.indexOf(value)>-1;
  }
  else {
    return (JSON.stringify(target)).indexOf(JSON.stringify(value)) != -1;
  }
}

describe("isIn", () => {

  it(`should return true when the value is in the array`, () => {
    expect(isIn(2, [2])).to.be.true;
    expect(isIn('a', ['a', 'b'])).to.be.true;
    expect(isIn('a', ['a'])).to.be.true;
    expect(isIn([2,3], [[2,3]])).to.be.true;
  });
  it(`should return false when the value is not in the array`, () => {
    expect(isIn('a', ['b'])).to.be.false;
    expect(isIn([2,4], [[2,3]])).to.be.false;
  });
});

Objects

One edge case you didn't test is isIn({property: 'value'}, [{property: 'value'}]). I don't know what you want it to do here, but currently it will return false.

You also did not test what happens when no values are passed in, or just a value. While TypeScript may not agree with not passing values to functions that are asking for them, it seems to still be possible.

And another one: JSON.stringify has it's own quirks

If undefined, a Function, or a Symbol is encountered during conversion it is either omitted (when it is found in an object) or censored to null (when it is found in an array). JSON.stringify can also just return undefined when passing in "pure" values like JSON.stringify(function(){}) or JSON.stringify(undefined).
All Symbol-keyed properties will be completely ignored, even when using the replacer function.
The instances of Date implement the toJSON() function by returning a string (the same as date.toISOString()), thus they are treated as strings.
The numbers Infinity and NaN as well as the object null are all considered as null.
For all the other Object instances (including Map, Set, WeakMap and WeakSet), only their enumerable properties will be serialized.


Python: screen colors adjuster using Turtles I am a total beginner in Python and I am trying with the modules Turtle. I am currently trying to code for 3 sliders (R, G, B) for adjusting the colors of the turtle screen. However, I found parts of my codes filled with redundancies, could anyone please tell me how to trim the code to make it more compact and clear, especially in the draw_red, draw_green, draw_blue parts? Thanks in advance.
import turtle

turtle.setworldcoordinates(0, 0, 4, 255)
turtle.colormode(255)
turtle.tracer(False)

colors = ['red', 'green', 'blue']
painters = []
red_x, green_x, blue_x = 1, 2, 3
turtle.bgcolor(0,0,0)
def spawningPainters():
    global painter
    for i in range(len(colors)):
        painter = turtle.Turtle()
        painters.append(painter)
        painter.up()
        painter.speed(0)
        painter.setheading(90)
        painter.color(colors[i])
        painter.pencolor('black')
        painter.shape('turtle')
        painter.shapesize(5, 5, 5)
        xPos = i + 1
        painter.goto(xPos, 0)

def assign_painters():
    global redPainter, greenPainter, bluePainter
    redPainter = painters[0]
    greenPainter = painters[1]
    bluePainter = painters[2]

def draw_red(x, y):
    redPainter.ondrag(None)
    x = red_x
    redPainter.goto(x, y)
    redPainter.ondrag(draw_red)
    update_screen_color()

def draw_green(x, y):
    greenPainter.ondrag(None)
    x = green_x
    greenPainter.goto(x, y)
    greenPainter.ondrag(draw_green)
    update_screen_color()

def draw_blue(x, y):
    bluePainter.ondrag(None)
    x = blue_x
    bluePainter.goto(x, y)
    bluePainter.ondrag(draw_blue)
    update_screen_color()

def update_screen_color():
    red = max(min(redPainter.ycor(), 255) , 0)
    green = max(min(greenPainter.ycor(), 255), 0)
    blue = max(min(bluePainter.ycor(), 255), 0)

    turtle.bgcolor(int(red), int(green), int(blue))

def back_to_origin():
    for painterIndex in range(len(painters)):
        painters[painterIndex].goto(painterIndex + 1, 0)

def listening_input():
    redPainter.ondrag(draw_red)
    greenPainter.ondrag(draw_green)
    bluePainter.ondrag(draw_blue)
    turtle.onkeypress(back_to_origin, 'c')

def main():
    spawningPainters()
    assign_painters()
    listening_input()

main()
turtle.listen()
turtle.tracer(True)
turtle.mainloop()

Review
I wasn't able to figure out a way to make the code more compact and in my opinion the code is pretty clear. However I do have some other feedback.
Use a proper main
Instead of:
turtle.setworldcoordinates(0, 0, 4, 255)
turtle.colormode(255)
turtle.tracer(False)
turtle.bgcolor(0,0,0)

...

def main():
    spawningPainters()
    assign_painters()
    listening_input()

main()
turtle.listen()
turtle.tracer(True)
turtle.mainloop()

Do:
...

if __name__ == "__main__":
    turtle.setworldcoordinates(0, 0, 4, 255)
    turtle.colormode(255)
    turtle.tracer(False)
    turtle.bgcolor(0,0,0)

    spawningPainters()
    assign_painters()
    listening_input()

    turtle.listen()
    turtle.tracer(True)
    turtle.mainloop()

For more information you can look at this post: https://stackoverflow.com/questions/419163/what-does-if-name-main-do
Prevent the turtle from going outside of the window
With the current code it is possible to drag the turtle outside of the window. If you then stop dragging you cannot get it back unless you reset the position. To prevent this from happening we can limit the boundaries of y inside of the draw functions.
def draw_blue(x, y):
    bluePainter.ondrag(None)
    x = blue_x
    y = min(y, 255)
    y = max(y, 0)
    bluePainter.goto(x, y)
    bluePainter.ondrag(draw_blue)
    update_screen_color()

By doing this the min and max calls can be removed from the update_screen_color function. However you may want to leave them as a form of defensive programming in case you add more features.
def update_screen_color():
    red = redPainter.ycor()
    green = greenPainter.ycor()
    blue = bluePainter.ycor()

    turtle.bgcolor(int(red), int(green), int(blue))

Update screen color when going back to the origin
When the position is put back to the origin the screen color is not redrawn. Unless this is intended it would be good to call the update_screen_color function at the end of the back_to_origin function.
Few tips
The global painter variable is not used anywhere outside of the spawningPainters function, so it can be removed.
Move the colors list to the spawningPainters function as it is not used anywhere else.
You might want to rename spawningPainters to spawn_painters.
Extra
Add typing to the painters list
When an empty list is declared the intellisense of your IDE may have trouble figuring out what type of elements the list will contain. By adding type annotations you can help the intellisense better understand your code.
Instead of:
painters = []

Do:
from typing import List

...

painters: List[turtle.Turtle] = []


A fast and (Ideally) thread safe object pool in C I have a helper struct called Scratch, that is a calculation 'Scratch Pad'.  Generally, I need only one of these objects, but if I have nested calculations I'll need one scratch per level of nesting.  And if I want to make the calculation thread safe then each thread will need one or more Scratch objects.

For simple cases a singleton Scratch object works fine and I'm able to achieve 38 steps per second.  If I change from using the singleton to just instantiating a Scratch object each time I drop to 10 steps per second.

Using a non thread safe version of the Pool below (commenting out the mutex locks and unlocks) got me just about back up to 35-38ish steps per second.  But the thread safe version as listed below knocks me down to 16 steps per second.  So, in order to make threading worth it, I'd have to overcome that initial performance blow.

I guess theoretically two cores should be able to almost breakeven and 3 or more should be able to make the lock cost worth it, but I haven't tried that yet.

Any suggestions, comments, criticisms greatly appreciated.

typedef struct Pool {
    Scratch** scratches;
    int n;
    int i;
    pthread_mutex_t mutex;
} Pool;

Pool* pool_;

Pool* AEPoolCreate() {
    Pool* pool = (Pool*)malloc(sizeof(Pool));
    pool->scratches = (Scratch**)malloc(sizeof(Scratch*));
    pool->scratches[0] = AEScratchCreate();
    pool->n = 1;
    pool->i = 0;
    pthread_mutex_init(&pool->mutex, NULL);
    return pool;
}
Scratch* AEPoolGet(Pool* pool) {
    pthread_mutex_lock(&pool->mutex);
    if (pool->i == pool->n) {
        pool->n *= 2;
        printf("[ Pool ===================== ]\n");
        printf("   increased to a size of %d\n", pool->n);
        printf("[ =========================== ]\n\n");
        pool->scratches = (Scratch**)realloc(pool->scratches, sizeof(Scratch*)*pool->n);
        for (int i=pool->i;i<pool->n;i++)
            pool->scratches[i] = AEScratchCreate();
    }
    Scratch* scratch = pool->scratches[pool->i++];
    pthread_mutex_unlock(&pool->mutex);
    return scratch;
}
void AEPoolPut(Pool* pool, Scratch* scratch) {
    pthread_mutex_lock(&pool->mutex);
    scratch->cp = 0;
    scratch->vp = 0;
    scratch->sp = 0;
    pool->i--;
    pool->scratches[pool->i] = scratch;
    pthread_mutex_unlock(&pool->mutex);
}

Here is the Scratch object as requested:

// Scratch =
typedef struct Scratch {
    Obj* stack;
    byte sp;
    byte cp;
    byte vp;
} Scratch;
Scratch* AEScratchCreate() {
    Scratch* scratch = (Scratch*)malloc(sizeof(Scratch));
    scratch->stack = (Obj*)malloc(sizeof(Obj)*10);
    scratch->cp = 0;
    scratch->vp = 0;
    scratch->sp = 0;
    return scratch;
}
void AEScratchRelease(Scratch* scratch) {
    if (scratch == 0) return;
    free(scratch->stack);
    free(scratch);
}

// Dim =====
typedef union {
    long n;
    double x;
    void* p;
} Dim;

// Obj =====
typedef struct {
    Dim a;
    Dim b;
    Dim c;
    //  byte type;
} Obj;

One small improvement would be to not acquire the lock in AEPoolPut until right before you access pool (i.e., clear out the fields in scratch before acquiring the lock).

There isn't really anything else I see that can change here that would help (all assuming the number of scratches required is more-or-less stable, so that AEPoolGet will quickly reach the point where it won't have to allocate any more scratches.  There are some tweaks that might be applicable if you continually grow the number of pools.

In code the threads that use this, you can implement some sort of thread local scratch caching mechanism, so that if a thread finishes using a scratch object it'll hang on to it locally and reuse it.  This will bypass all this code.  If there are too many of these "free" scratch objects, you can return the excess back to the pool.  Depending on your usage, you might only need to cache one scratch object.


addition of two list elementwise with abortion if condition satisfied python I have a function that sums two arrays element-wise and stops if the sum at a certain index is greater strictly than 1. This function is called lot of times (15817145 times for a small simulation and expects to grow a lot more as the simulation size grows). Profiling my code shows that it is among the five most consuming functions of my code  and  I believe can be optimized further.
def add_lis(lis1, lis2):
    res = []
    for i, j in zip(lis1, lis2):
        if i + j > 1:
            return
        res.append(i + j)
    return array('H', res)

lis1 and lis2 are both of type array.array('H',..). The two lists have the same length which can be up to 100 and they are both filled with zeroes and ones only. I tried with numpy arrays but there is too much overhead that it is not beneficial to use it.
My question is can this function be optimized? Maybe in a sort of list comprehension ?

Does it make sense (in the context of your program execution) to split your task into two stages?
Check for the presence of (1, 1) in zip(lis1, lis2), and if False, then pass the generator to the array: array('H', (i + j for i, j in zip(lis1, lis2))).


Script to create symlinks for dotfiles in a Git repository I have a GitHub repository containing all my dotfiles for bash, tmux and neovim. I'm using a Mac and whenever I factory reset it I like to just pull repository from GitHub and link all the dotfiles to the one in the repo.

Now I wrote a script to automatically create these symlinks and handle possible errors on the way.

This is what I came up with:

#!/usr/bin/env bash

RED=`tput setaf 1`
GREEN=`tput setaf 2`
RESET=`tput sgr0`

custom_link() {
  file_loc="$1"
  symlink_loc="$2"

  # Check whether original file is valid
  if [[ ! -e "$file_loc" ]]; then
    echo "${RED}WARNING${RESET}: Failed to link $symlink_loc to $file_loc"
    echo "         $file_loc does not exist"
    return 1
  fi

  # Check whether symlink already exists
  if [[ -e "$symlink_loc" ]]; then
    if [[ -L "$symlink_loc" ]]; then
      if [[ "$(readlink $symlink_loc)" == "$file_loc" ]]; then echo "--> $symlink_loc -> $file_loc ${GREEN}exists${RESET}."; return 0; fi

      current_dest=$(readlink "$symlink_loc")
      echo "It seems like $symlink_loc already is symlink to $current_dest."
      read -p "Do you want to replace it? [Y/N] " -n 1 -r
      if [[ $REPLY =~ ^[Yy]$ ]]
      then
        echo
        echo "Removing current symlink..."
        unlink  $symlink_loc
        ln -s "$file_loc" "$symlink_loc"
        echo "Created a symlink $symlink_loc -> $file_loc"
        return 0
      fi
      echo
      return 2
    else
      echo "It seems like $symlink_loc already exists."
      read -p "Do you want to [d]elete, [m]ove (-> $HOME/.other/) or [k]eep $symlink_loc? " -n 1 -r
      if [[ $REPLY =~ ^[Dd]$ ]]
      then
        printf "\nDeleting $symlink_loc..."
        rm -rf "$symlink_loc"
        ln -s "$file_loc" "$symlink_loc"
        echo "Created a symlink $symlink_loc -> $file_loc"
        return 0
      elif [[ $REPLY =~ ^[Mm]$ ]]; then
        printf "\nMoving $symlink_loc to ~$HOME/.other..."
        mkdir "$HOME/.other"
        mv "$symlink_loc" "$HOME/.other"
        ln -s "$file_loc" "$symlink_loc"
        echo "Created a symlink $symlink_loc -> $file_loc"
        return 0
      else
        printf "\nKeeping $symlink_loc...\n"
        return 2
      fi
    fi
  fi

  symlink_dir=$(dirname "$symlink_loc")
  if ! mkdir -p "$symlink_dir" ; then
   echo "${RED}WARNING${RESET}: Failed to link $symlink_loc to $file_loc"
   echo "         Could not create folder $symlink_dir/"
   return 1
  fi

  ln -s "$file_loc" "$symlink_loc"
  echo "Created a symlink $symlink_loc -> $file_loc"
  return 0
}

custom_link "$HOME/.dotfiles/bash/.bashrc" "$HOME/.bash_profile"
custom_link "$HOME/.dotfiles/bash/.bashrc" "$HOME/.bashrc"
custom_link "$HOME/.dotfiles/nvim/init.vim" "$HOME/.config/nvim/init.vim"
custom_link "$HOME/.dotfiles/tmux/.tmux.conf" "$HOME/.tmux.conf"

It was my very first time writing a bash script and I just googled, whenever I had a specific question.

I choose the return codes as follows:
- 0 for successful creation
- 1 for failure
- 2 for cases, when the link was not created, because the user decided so.

My questions are:

How is my general bash programming style? Any important idioms I missed?
The script contains a lot of duplicate code (especially the three lines: ln -s "$file_loc" "$symlink_loc", echo "Created a symlink $symlink_loc -> $file_loc" and return 0. How can I reduce the number of duplicates?
How could I implement more successful return values?
I handles a lot of cases (the file already exists, etc.). Did I miss any important cases?
Currently the function custom_link displays a lot of text. How could I outsource this echoing to the outside of the function? The function should do the bare minimum (maybe connected to the question about useful return codes).

I noticed that some of your questions have similar aspects, so I clustered them to provide a single answer:

"How is my general bash programming style?"
"How can I reduce the number of duplicates?"
"How could I outsource this echoing to the outside of the function?"

Shell script is ugly compared to other scripting languages, so we need to make an extra effort to keep it readable.

So here are some suggestions to improve the readability of your script:

Check your script with ShellCheck for insights on best practices and idioms.
Extract those file checks to functions with meaningful names, so that the reader won't need to remember (or lookup) what each flag means:

if [[ -e "$symlink_loc" ]]; then
    if [[ -L "$symlink_loc" ]]; then

could become:

if file_exists "$symlink_loc"; then
    if is_symlink "$symlink_loc"; then

Hide low level details that are polluting the custom_link function:

custom_link() {
    # 1. checking if original file is valid
        # 1.1 if not valid, print error message and return 1

    # 2. handling existing file where symlink should be placed
        # 2.1 if is already existing symlink
            # 2.1.1 if symlink already points to file, print success and return 0
            # 2.1.2 otherwise, ask user if symlink should be replaced
                # 2.1.2.1 if yes, replace it

        # 2.2 if is a regular file
            # 2.2.1 prompt user to delete, move or keep file
                # 2.2.1.1 if delete, delete file and place symlink
                # 2.2.1.2 if move, create dir $HOME/.other, move file there and place symlink
                # 2.2.1.3 if keep, print "keeping the file"

    # 3. creating symlink
        # 3.1 extract dirname of symlink
        # 3.2 try to create extracted dir
            # 3.2.1 if fails, print error and return 1
        # 3.3 create symlink, print success and return 0
}

It should be easy for the reader of custom_link to understand what is going on, on a high level:

custom_link() {
    file_loc="$1"
    symlink_loc="$2"

    if ! file_exists "$file_loc"
    then
        return invalid_file_error "$file_loc" "$symlink_loc"
    fi

    if file_exists "$symlink_loc"
    then
        return handle_existing_file "$file_loc" "$symlink_loc"
    fi

    create_new_symlink "$file_loc" "$symlink_loc"
}

And then you can hide lower level details of each step on their own dedicated functions. (You can apply this recursively, until the functions are very small and obvious)
Here are some examples:

file_exists() { test -e "$1"; }

invalid_file_error() {
    file_loc="$1"
    symlink_loc="$2"

    echo "${RED}WARNING${RESET}: Failed to link $symlink_loc to $file_loc"
    echo "         $file_loc does not exist"
    return 1
}

handle_existing_file() {
    file_loc="$1"
    symlink_loc="$2"

    if is_symlink "$symlink_loc"
    then
        return handle_existing_symlink "$file_loc" "$symlink_loc"
    else
        return handle_existing_regular_file "$file_loc" "$symlink_loc"
    fi
}

"Any important idioms I missed?"
"How could I implement more successful return values?"

The convention is to return 0 for success and anything else for errors.

So you got it right when you are returning 0 (symlink already there pointing to expected file) and 1 (file doesn't exist).

But when you prompt the user on which actions should be taken, you are successfully executing what was requested, but returning a 2.

If you think from the user's perspective, the exit code should be 0, since the script did exactly what the user wanted.

"Did I miss any important cases?"

The user can choose to move a file to $HOME/.other, but what should happen if there is already a file there (on $HOME/.other) with the same name, maybe from a previous run?


Selective Array Reversing I have the following code that given an array, returns the reversed version of the array, reverse portions of the array.

For example

selReverse([1,2,3,4,5,6], 2)
//=> [2,1, 4,3, 6,5]

function selReverse(array, length) {
  console.log(array)
  var resultArray = [];
    for (var i = 0, j=array.length ; i < j; i += length) {
      resultArray.push.apply(resultArray, array.slice(i, i+length).reverse());
    }
  return resultArray;
}

This code passes the tests, but apparently, it's too slow. Is there a way to optimize it?

Don't put console.log calls in you functions unless there's a really good reason to. In this case, you're just printing the input, which is pretty pointless. It can also contribute greatly to the function's running time, since printing a large array isn't exactly free (a lot depends on how the runtime handles output)
You can simplify the code. You can do without slicing, reversing and applying chunks of the array

I'd just use two for-loops:

function selReverse(array, chunkSize) {
  var output = [],
      l = array.length;

  for(var i = 0 ; i < l ; i += chunkSize) {
    var start = Math.min(i + chunkSize, l) - 1;
    for(var j = start ; j >= i ; j--) {
      output.push(array[j]);
    }
  }
  return output;
}

Edit: The code above doesn't work, but I thought it was due to very large inputs or something, but that's not the case.

The simple solution is basically, that the for loop - both in my code and the original code - goes into an infinite loop if the chunkSize is zero. This works, though:

function selReverse(array, chunkSize) {
  var output = [],
      l = array.length;

  if(chunkSize === 0) return array.slice(); // could also do <= 0 to be safer

  for(var i = 0 ; i < l ; i += chunkSize) {
    var start = Math.min(i + chunkSize, l) - 1;
    for(var j = start ; j >= i ; j--) {
      output.push(array[j]);
    }
  }
  return output;
}

Since it's got nothing to do with large inputs, I imagine the original code works just as well, as long as it too checks for the chunkSize.


Populate inorder successor for all nodes Given a binary tree where each node has following structure, write a
  function to populate next pointer for all nodes. The next pointer for
  every node should be set to point to inorder successor.

struct node
{
  int data;
  struct node* left;
  struct node* right;
  struct node* next;
}

  Initially, all next pointers have NULL values. Your function should
  fill these next pointers so that they point to inorder successor.

This code is attributed to GeeksForGeeks. Looking for code review, optimizations and best practices.

public class PopulateInorderSuccessor<T> {

    private TreeNode<T> root;

    public PopulateInorderSuccessor(List<T> items) {
        create (items);
    }

    private void create(List<T> items) {
        if (items.size() == 0) {
            throw new IllegalArgumentException("There should atlease be single item in the tree");
        }

        root = new TreeNode<>(items.get(0));

        final Queue<TreeNode<T>> queue = new LinkedList<TreeNode<T>>();
        queue.add(root);

        final int half = items.size() / 2;

        for (int i = 0; i < half; i++) {
            if (items.get(i) != null) {
                final TreeNode<T> current = queue.poll();
                final int left = 2 * i + 1;
                final int right = 2 * i + 2;

                if (items.get(left) != null) {
                    current.left = new TreeNode<T>(items.get(left));
                    queue.add(current.left);
                }
                if (right < items.size() && items.get(right) != null) {
                    current.right = new TreeNode<T>(items.get(right));
                    queue.add(current.right);
                }
            }
        }
    }

    private static class TreeNode<T> {
        private TreeNode<T> left;
        private T item;
        private TreeNode<T> right;
        private TreeNode<T> next;

        TreeNode(T item) {
            this.item = item;
        }
    }

    private static class TreeNodeHolder<T> {
        private TreeNode<T> treeNode;
    }

    public void populateInorderSuccessor() {
        populate(root, new TreeNodeHolder<T>());
    }

    private void populate(TreeNode<T> node, TreeNodeHolder<T> treeNodeHolder) {
        if (node != null) {
            populate(node.right, treeNodeHolder);
            node.next = treeNodeHolder.treeNode;
            treeNodeHolder.treeNode = node;
            populate(node.left, treeNodeHolder);
        }
    }

    public List<T> toInOrderList() {
        TreeNode<T> node = root;

        while (node.left != null) {
            node = node.left;
        }

        final List<T> list = new ArrayList<>();
        while (node != null) {
            list.add(node.item);
            node = node.next;
        }
        return list;
    }
}

public class PopulateInorderSuccessorTest {

    @Test
    public void testCompleteTree() {
        PopulateInorderSuccessor<Integer> pis1 = new PopulateInorderSuccessor<Integer>(Arrays.asList(1, 2, 3, 4, 5, 6, 7));
        pis1.populateInorderSuccessor();
        assertEquals(Arrays.asList(4, 2, 5, 1, 6, 3, 7), pis1.toInOrderList());
    }

    @Test
    public void testLeftSkewedTree() {
        // left skewed
        PopulateInorderSuccessor<Integer> pis2 = new PopulateInorderSuccessor<Integer>(Arrays.asList(1, 2, null, 4));
        pis2.populateInorderSuccessor();
        assertEquals(Arrays.asList(4, 2, 1), pis2.toInOrderList());
    }

    @Test
    public void testRightSkewedTree() {
        // right skewed
        PopulateInorderSuccessor<Integer> pis3 = new PopulateInorderSuccessor<Integer>(Arrays.asList(1, null, 3, null, null, null, 7));
        pis3.populateInorderSuccessor();
        assertEquals(Arrays.asList(1, 3, 7), pis3.toInOrderList());
    }
}

I think this equivalent, but simpler. It removes the need for the TreeNodeHolder class by returning a TreeNode<T>.

public void populateInorderSuccessor() {
  populate(root, null);
}

private TreeNode<T> populate(TreeNode<T> root, TreeNode<T> successor) {
  if (root == null) {
    return successor;
  }

  root.next = populate(root.right, successor);
  return populate(root.left, root);
}

PopulateInorderSuccessor is not a good name for a class. Let's change it to BinaryTree.

Tree construction can be simplified:

public BinaryTree(T[] items) {
  this.root = create(items, 0);
}

private TreeNode<T> create(T[] items, int i) {
  if (i >= items.length || items[i] == null) {
    return null;
  }

  TreeNode<T> root = new TreeNode<T>(items[i]);
  root.left = create(items, 2 * i + 1);
  root.right = create(items, 2 * i + 2);
  return root;
}

I find that helper methods setLeft and setRight on TreeNode can make tree code a bit cleaner, in general. It's not of much advantage here, but just for reference:

public TreeNode<T> setLeft(TreeNode<T> left) {
  this.left = left;
  return this;
}

public TreeNode<T> setRight(TreeNode<T> right) {
  this.right = right;
  return this;
}

This way we can write

private TreeNode<T> create(T[] items, int i) {
  if (i >= items.length || items[i] == null) {
    return null;
  }

  return new TreeNode<T>(items[i])
    .setLeft(create(items, 2 * i + 1))
    .setRight(create(items, 2 * i + 2));
}

Putting it all together, we get something like this

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

public class BinaryTree<T> {
  private TreeNode<T> root;

  public BinaryTree(T[] items) {
    this.root = create(items, 0);
  }

  private TreeNode<T> create(T[] items, int i) {
    if (i >= items.length || items[i] == null) {
      return null;
    }

    return new TreeNode<T>(items[i])
      .setLeft(create(items, 2 * i + 1))
      .setRight(create(items, 2 * i + 2));
  }

  public void populateInorderSuccessor() {
    populate(root, null);
  }

  private TreeNode<T> populate(TreeNode<T> root, TreeNode<T> successor) {
    if (root == null) {
      return successor;
    }

    root.next = populate(root.right, successor);
    return populate(root.left, root);
  }

  public List<T> toInOrderList() {
    TreeNode<T> node = root;
    while (node.left != null) {
      node = node.left;
    }

    final List<T> list = new ArrayList<>();
    while (node != null) {
      list.add(node.item);
      node = node.next;
    }
    return list;
  }

  private static class TreeNode<T> {
    private TreeNode<T> left;
    private T item;
    private TreeNode<T> right;
    private TreeNode<T> next;

    TreeNode(T item) {
      this.item = item;
    }

    public TreeNode<T> setLeft(TreeNode<T> left) {
      this.left = left;
      return this;
    }

    public TreeNode<T> setRight(TreeNode<T> right) {
      this.right = right;
      return this;
    }
  }
}


Bookstore classes design This is not homework. I am trying to learn OOD principles and design patterns myself.

Suppose I have a problem like this:

  A book-shop buys and sells two types of books:

  Non-technical {Title, Author, Price}
  Technical {Title, Author, Price, CD}

  Also, customer gets a CD when he buys a Technical book. A CD object is
  defined as, CD {Title, Price}.

  A Non-technical book's price will be only the price of the book. A
  Technical book's price will be the sum of the price of the book and
  the CD.

  Create a C# program to show the following info:

  Total number of book Bought & Price: XXX & XXX.XX
  Total number of book Sold & Price: XXX & XXX.XX
  Total Technical Book Sold & Price: XXX & XXX.XX
  Total Non-technical Book sold & Price: XXX & XXX.XX

I have designed the program like this:

abstract class Publication
{
    public virtual string Title { get; set; }
    public virtual double Price { get; set; }
}

class CD : Publication
{
}

abstract class Book : Publication
{
    public virtual string Author { get; set; }
}

class TechnicalBook : Book
{
    public CD Cd { get; set; }
    public override double Price
    {
        get
        {
            return (base.Price + Cd.Price);
        }
    }
}

class NonTechnicalbook : Book
{
}

abstract class Shop
{
    private IDictionary<string, Book> boughtDictionary;
    private IDictionary<string, Book> soldDictionary;

    public Shop()
    {
        boughtDictionary = new Dictionary<string, Book>();
        soldDictionary = new Dictionary<string, Book>();
    }

    public virtual void Buy(Book item)
    {
        boughtDictionary.Add(item.Title, item);
    }

    public virtual void Sell(string title)
    {
        Book book = boughtDictionary[title];
        boughtDictionary.Remove(book.Title);
        soldDictionary.Add(book.Title, book);
    }

    public virtual int GetBoughtBookCount()
    {
        return boughtDictionary.Count;
    }

    public virtual double GetBoughtBookPrice()
    {
        double price = 0.0;

        foreach (string title in boughtDictionary.Keys)
        {
            price = price + boughtDictionary[title].Price;
        }
    }

    public virtual int GetSoldBookCount()
    {
        return boughtDictionary.Count;
    }

    public virtual double GetSoldBookPrice()
    {
        double price = 0.0;

        foreach (string title in soldDictionary.Keys)
        {
            price = price + soldDictionary[title].Price;
        }
    }

    public virtual double GetTotalBookCount()
    {
        return this.GetBoughtBookCount() + this.GetSoldBookCount();
    }

    public virtual double GetTotalBookPrice()
    {
        return this.GetBoughtBookPrice() + this.GetSoldBookPrice();
    }

    public virtual void Show()
    {
        Console.WriteLine("Total number of books Bought & Price: ", this.GetTotalBookCount() + " & " + this.GetTotalBookPrice());
        Console.WriteLine("Total number of books Sold & Price: ", this.GetSoldBookCount() + " & " + this.GetSoldBookPrice());
    }
}

Does this design conform to the Open-Closed principle? Now I am unable to understand how to separate Technical and Non-technical books at this point.

Overriding the Price property in TechnicalBook modifies the behavior of Publication. Which means you've broken the OCP. Besides, what do you expect TechnicalBook.Price to be if you have just set it to 10.0? (Does it even compile when you only override get?)

What you could do is employ the template method pattern:

abstract class Publication
{
    // ...
    public decimal IndividualPrice { get; set; }
    public decimal Price {
        get { return CalculatePrice(); }
    }

    protected decimal CalculatePrice() {
        decimal price = IndividualPrice;
        price += GetAdditionalPrices();
        return price;
    }

    protected virtual decimal GetAdditionalPrices() {
        return 0;
    }
}

public class TechnicalBook : Publication
{
    // ...
    protected override decimal GetAdditionalPrices() {
        return Cd.Price;
    }
}

This makes Publication closed for modification (cannot change price / individual price), but open for extension (can add to price).

(Naming and structure in my example could probably be improved, but it serves the purpose)

The behavior is still possible to modify, though, so an even better solution would be to have some collection or set of rules you can add to, so you don't accidentaly override GetAdditionalPrices without calling base.GetAdditionalPrices().


Call Action every X cycles I want to call an Action every X cycles (in my case, to a transaction commit every 100 records), so I do:

public class CyclicActionCaller
{
    private int _counter;
    private int _cycleValue;
    private Action _methodToCall;

    public CyclicActionCaller()
    {
        _counter = 0;
    }

    public CyclicActionCaller Every(int cycleValue)
    {
        _cycleValue = cycleValue;
        return this;
    }

    public CyclicActionCaller Call(Action methodToCall)
    {
        _methodToCall = methodToCall;
        return this;
    }

    public void PerformCall()
    {
        Interlocked.Increment(ref _counter);
        // IS THIS THREAD SAFE - SHOULD BE CALLED EVERY _counter % _cycleValue - EVEN IN MULTITHREADING ENVIRONMENT
        if (_counter % _cycleValue == 0)
        {
            _methodToCall();
        }
    }
}

CyclicActionCaller cyclicActionCaller = new CyclicActionCaller();
using (ModelContainer container = new ModelContainer())
{
   cyclicActionCaller.Every(100).Call(() => { container.SaveChanges(); });

   while(...) { // Import Loop - assume multiple workers inside
       cyclicActionCaller.PerformCall(); // every worker calls this after inserting a row
   }
}

Is this function thread safe? I want that - even if multiple threads call PerformCall - that it is just called once every 100 calls.

No, it's not thread-safe. Between the call to Interlocked.Increment() and the if check, another thread could call Interlocked.Increment(). That means both of them could see that _counter is 101, which means a call to _methodToCall would be skipped.

But this is exactly why Interlocked.Increment() returns the incremented value. So the thread-safe version would be:

public void PerformCall()
{
    if (Interlocked.Increment(ref _counter) % _cycleValue == 0)
        _methodToCall();
}

This is exactly why writing code like this is hard (although this is a simple situation, so it's not that hard here). It's much easier to write correct multi-threaded code, if you lock all shared values.

Also, if you do something like this, you need to be sure that inserting a row in your code is also thread-safe. And the same applies to SaveChanges(). For example, if you used something like ConcurrentQueue in a straightforward way, it would be possible for SaveChanges() to save more than 100 items at a time, because new items could have been inserted between the call to Interlocked.Increment() and the final dequeue from the queue. This may not be a problem for you, but you should think about that.


Render the area of circle using HTML and JavaScript 1. Problem Statement

  Write a JavaScript function calcCircleArea(r) that takes as a parameter the radius of a circle and calculates and returns its area. Put the function in a file named circle-area.js. Write a HTML page circle-area.html that includes the script circle-area.js and calculates and prints in the page body the area of circles of size r=7, r=1.5 and r=20.

  2. Expected Output

r = 7; area = 153.93804002589985
r = 1.5; area = 7.0685834705770345
r = 20; area = 1256.6370614359173

Solution:

function calcCircleArea(r){
    if(r >= 0){
        return window['Math']['PI'] * r * r;
    }else{
        return "wrong input";
    }
}

document.write('r = 7; area = ' + window['calcCircleArea'](7) + "
");
document.write('r = 1.5; area = ' + window['calcCircleArea'](1.5) + "
");
document.write('r = 20; area = ' + window['calcCircleArea'](20) + "
");
<!doctype html>

        <meta charset="UTF-8">
        <title>Circle area</title>

Can I invoke calcCircleArea from the HTML code to render the given output?
Can the else condition in calcCircleArea have better code?

To answer your questions directly:

Yes.
Maybe. Depends on what you define as "better".

First, though: Don't use bracketed (aka subscripting) access to properties when regular dot notation works, e.g.:

window.Math.PI

is exactly the same as:

window['Math']['PI']

but a lot simpler.

Secondly, don't bother with window when there's no need. window is the global object in a browser's JS runtime, so you can just say

Math.PI

The exception to this is if you have variable shadowing, but that's outside the scope of this review.

Now, to tackle question 2 first, you could do this:

function calcCircleArea(r) {
    if(r >= 0) {
        return Math.PI * r * r;
    }
    return "invalid input";
}

It's quite subtle, but instead of having an else branch, you just.. don't have an else branch. If the first condition is true, the function returns the area - the rest of the function never runs. But if it's false, we skip straight past the if block. The point of this is to ensure that the function always returns something. It already did that - there was nothing wrong with your code - but here it's even more explicit.

However, you could argue that handling invalid input is the less common option, hence that should be the thing to short-circuit the function, and return early:

function calcCircleArea(r) {
    if(r < 0) {
        return "invalid input";
    }
    return Math.PI * r * r;
}

In other words, you could assume good intentions (that the function will typically be called with proper arguments), and treat invalid input as the exception to the rule. However, you might be better served by having your function raise hell if its input is invalid. Right now you're returning something regardless of input, but maybe you want to completely bail out, if things aren't as expected (after all, a negative radius just doesn't make sense, so there's literally no good answer). Hence why I mentioned "exception to the rule", since exceptions, as concept, are made for that purpose. Now, exceptions are also a topic on their own, but in this case you could do:

function calcCircleArea(r) {
    if(r < 0) {
        throw "Invalid input";
    }
    return Math.PI * r * r;
}

throw works the same as return in the sense that the function just stops. But the caller has to handle it differently, or - if it's not handled - it'll bubble up as an error. But I digress.

Now, for question 1, yes, you can call the function from the HTML. In fact, you really should do that here, since you're currently using document.write in a script that's placed in the head element. Since document.write happens immediately, your HTML actually ends up looking like this:

<!doctype html>

        <meta charset="UTF-8">
        <title>Circle area</title>
r = 7; area = 153.93804002589985
r = 1.5; area = 7.0685834705770345
r = 20; area = 1256.6370614359173

because the browser reads the scripts, and runs it. Obviously, this isn't really valid HTML. Thankfully, a browser is very, very, very tolerant, and renders things just fine anyway, even if it makes no sense.

Now (still before actually answering your question), let me quickly point out that

it's <!DOCTYPE html> with all-capital DOCTYPE
since that doctype means HTML5, you don't need XHTML-style self-closed tags like
. Just write
.

Finally, to answer your question, remove the three document.write lines from your script, and use this HTML instead:

<!DOCTYPE html>

        <meta charset="UTF-8">
        <title>Circle area</title>

            r = 7; area =

            r = 1.5; area =

            r = 20; area =


Using Java's instanceof operator I am developing a BlackJack game using Java, and it came to a point that I am using instanceof operator to determine if it is a type of some subclass.

Here's an example:

public void checkForBlackJack(Player player) {
        Hand fHand = player.getHands().get(0);
        if (fHand.getCardScore() == 21) {
            fHand.setBlackjack(true);
        }

        if(player instanceof BlackJackPlayer){
            BlackJackPlayer bjplayer = (BlackJackPlayer) player;

            if(bjplayer.isSplit()){
                //Check the users other hand if it is already blackjacked
            }
        }
    }

Is using instanceof considered to be bad for such example?  Just as an overview, here is my class diagram:

I've decided to use instanceof to check for blackjack because The Dealer can also get blackjack.

In this case you could push the isSplit method to the Player class and implement it with return false;. Then the need to instanceof disappears.

I wouldn't say that it is bad to use instanceof but often there are other options that are better.


LINQ for extracting N bottom items (async streams) source.Bottom(n, x => x) should be the same as well known LINQ source.OrderBy(x => x).Take(n) but is more memory/run-time efficient as it does not keep all items in memory as OrderBy does:

public static class BottomN
{
    public static async IAsyncEnumerable<T> Bottom<T, TValue>(
        this IAsyncEnumerable<T> source, int number, Func<T, TValue> selector)
        where TValue : IComparable<TValue>
    {
        var list = new List<T>();
        await foreach(var item in source)
        {
            list.Insert(list.BestIndex(item, selector), item);
            if (list.Count > number)
                list.RemoveAt(number);
        }

        foreach (var item in list)
            yield return item;
    }

    static int BestIndex<T, TValue>(this IList<T> list, T value, Func<T, TValue> selector)
        where TValue : IComparable<TValue>
    {
        return bestIndex(0, list.Count);
        int bestIndex(int s, int e) =>
            s == e ? s :
                selector(list[(s + e) / 2]).CompareTo(selector(value)) > 0
                ? bestIndex(s, (s + e) / 2)
                : bestIndex((s + e) / 2 + 1, e);
    }
}

To test:

    [TestMethod]
    public async Task Keep_Three()
    {
        var values = new[] { 10, 22, 3, 55, 66, 100, 200, 2 };
        var bottom = await values.ToAsyncEnumerable().Bottom(3, v => v).ToArrayAsync();
        CollectionAssert.AreEqual(new int[] { 2, 3, 10 }, bottom);
    }

where TValue : IComparable<TValue>

Don't do this. OrderBy doesn't require its type parameters to implement this interface like this either. Instead it uses the IComparer<T> interface. What's the difference? With IComparable<T>, the implementation of the comparison logic is sitting on the class T itself. This means that there is one and only one way of ordering elements of T. If I wanted to sort them using some customized logic, I would be out of luck.

Instead, IComparer<T> is a separate class that compares Ts. Providing an instance of that interface to the method allows me to use whatever logic I want to order T.

But what if I don't want to implement an entire class, but instead want to use IComparable<T>? This is where Comparer<T>.Default comes to play. This static property provides a default implementation for IComparer<T> for T, which, if T implements IComparable<T>, will default to call that logic.

So how would your interface look? We have an overload with the IComparer<TValue> argument, and an overload without:

public static async IAsyncEnumerable<T> Bottom<T, TValue>(
        this IAsyncEnumerable<T> source, int number, Func<T, TValue> selector, IComparer<TValue> comparer)
{
    return Bottom(source, number, selector, Comparer<TValue>.Default);
}

public static async IAsyncEnumerable<T> Bottom<T, TValue>(
        this IAsyncEnumerable<T> source, int number, Func<T, TValue> selector, IComparer<TValue> comparer)
{
    // Actual implementation.
}

Binary tree

I think this problem would call for a binary tree, instead of a list that's sorted/ranked continuously. This would you can quickly check whether the item you are iterating would even be in the top-number of items in the collection, without having to add and subsequently having to remove it again. The downside is that C# doesn't have a built-in Binary Tree that isn't hidden within the SortedX collection set. These classes sadly require unique values in their collections, which isn't guaranteed here.

Alternatively, if you can handle adding another few lines to your solution, you can check if the index returned by BestIndex is equal to number, and skip adding and removing it from the list if this is the case.

Code style

This needs to be said. Your code is really compact. It took me multiple reads to figure out what on earth BestIndex was actually doing. Docstrings, comments and/or intermediate variables with clear names please. Something as simple as "Returns the rank of value in the list by performing a merge-sort style algorithm." is enough to understand its role in Bottom.


Single method for different parameterized mysql command I currently have these two methods. One method returns all records, while the other one returns only a specific record.

function GetLeaveRequestDetailsAll($leaveCurrentStatus){

    global $connPDO;

    $sql = "select *, emp.Employee_Name, emp.Designation, emp.Emp_Dept_Category, lt.Leave_Name
            from tbl_emp_leave_details eld
            Inner join tbl_leave_types lt
            ON lt.leave_id = eld.leave_type
            Inner Join employee emp
            on emp.PK = eld.emp_id
            Inner JOIN dept_category dc
            ON emp.Emp_Dept_Category = dc.PK
            where eld.leave_current_status_id = :leave_current_status_id";

    $binds = array(
        ":leave_current_status_id"   =>  $leaveCurrentStatus
    );

    $statementEntryCount = $connPDO->prepare($sql);

    $statementEntryCount->execute($binds);
    $queryData = $statementEntryCount->fetchAll(PDO::FETCH_ASSOC);
    echo json_encode($queryData);
}

function GetLeaveRequestDetailsById($leaveCurrentStatus, $leaveDetailId){

    global $connPDO;

    $sql = "select *, emp.Employee_Name, emp.Designation, emp.Emp_Dept_Category, lt.Leave_Name
            from tbl_emp_leave_details eld
            Inner join tbl_leave_types lt
            ON lt.leave_id = eld.leave_type
            Inner Join employee emp
            on emp.PK = eld.emp_id
            Inner JOIN dept_category dc
            ON emp.Emp_Dept_Category = dc.PK
            where eld.leave_current_status_id = :leave_current_status_id
            And eld.leave_detail_id = :leave_detail_id";

    $binds = array(
        ":leave_current_status_id"   =>  $leaveCurrentStatus,
        ":leave_detail_id"   =>  $leaveDetailId
    );

    $statementEntryCount = $connPDO->prepare($sql);

    $statementEntryCount->execute($binds);
    $queryData = $statementEntryCount->fetchAll(PDO::FETCH_ASSOC);
    echo json_encode($queryData);
}

As you can see, both are doing the same work with a small difference, which is leave_detail_id. How can I convert these two functions into a single parameterized method to handle both queries?

If I assume that leave_detail_id contains an auto-increment value, starting at 1, you can do:

function queryDatabase($query, $bindings = [])
{
    global $connPDO;
    // prepare statement
    $statement = $connPDO->prepare($query);
    // execute statement
    $statement->execute($bindings);
    // return result in associative array
    return $statement->fetchAll(PDO::FETCH_ASSOC);
}

function getLeaveRequestDetails($leaveCurrentStatus, $leaveDetailId = 0)
{
    // build query
    $query = "SELECT *,
                     emp.Employee_Name,
                     emp.Designation,
                     emp.Emp_Dept_Category,
                     lt.Leave_Name
              FROM tbl_emp_leave_details eld
              INNER JOIN tbl_leave_types lt ON lt.leave_id = eld.leave_type
              INNER JOIN employee emp ON emp.PK = eld.emp_id
              INNER JOIN dept_category dc ON emp.Emp_Dept_Category = dc.PK
              WHERE eld.leave_current_status_id = :leave_current_status_id";
    // define bindings
    $bindings = [":leave_current_status_id" => $leaveCurrentStatus];
    // should details id be use?
    if ($leaveDetailId > 0) {
          $query .= " AND eld.leave_detail_id = :leave_detail_id";
          $bindings[":leave_detail_id"] = $leaveDetailId;
    }
    // query database and return result
    return queryDatabase($query, $bindings);
}

I used a default function argument value to set $leaveDetailId to zero, if it is absent, when the function is called. If it does get a value bigger than zero the condition is added to the query and the value is bound.

I also removed the actual database querying from this function and put it in a separate function. The reason is that this code can probably be used more often in your scripts. Instead of repeating it every time you can now call queryDatabase().

Finally I removed the echo json_encode(); from the function. The function name starts with "get" and that almost always implies something will be returned. Also, echoing JSON would restrict this function to that task alone. Now you can still use it to echo JSON, but you can also use it to do something else with the result. A call, returning JSON, would look something like this:

echo json_encode(getLeaveRequestDetails("fallen",15));

As you can see I've now split your code into more parts than you had before. That's not a bad thing. Now each part performs a single function: Querying the database, getting details of a leave, and echoing JSON. This makes your code easier to read.

In summary:

Do not repeat yourself (DRY). This was your reason for asking this question.
functions should be created with reusability in mind, even the specialized ones. That's the whole reason for their existance.
functions should only perform one task (Single responsibility principle).


Roman Numerals in Java I wrote a class that can convert to and fro from Roman Numerals and Decimals. I would appreciate critical points on everything, especially:

Shortening my code.
Choosing a better approach.
Using better/new features of Java 8.

And so on...

I didn't add JavaDoc because I am expecting suggestions for a better approach. I will document it then. Here is the class:

RomanNumeral.java

package library.util;

import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.Map;

/**
 * A "recreational" class that provides that functionality of converting
 * values to and fro from Roman numerals.
 *
 * @author Subhomoy Haldar
 */
public class RomanNumeral {

    private final String representation;
    private final int value;

    private RomanNumeral(String representation, int value) {
        this.representation = representation;
        this.value = value;
    }

    @Override
    public String toString() {
        return representation;
    }

    public int getValue() {
        return value;
    }

    public static RomanNumeral of(String representation) throws
            NumberFormatException,
            ArithmeticException {
        // Zero wasn't supported then...
        if (representation.isEmpty()) {
            throw new NumberFormatException("Empty String.");
        }
        int value = 0, previous = 0;
        StringBuilder builder = new StringBuilder(representation);
        builder.reverse();  // go in the opposite manner
        for (int i = 0; i < builder.length(); i++) {
            try {
                int current = map.get(String.valueOf(builder.charAt(i)));
                // If the value suddenly drops then subtract, don't add.
                value += current < previous ? -current : current;
                previous = current;
            } catch (NullPointerException e) {
                // A NullPointerException will be thrown only if the
                // character is not present in the map.
                throw new NumberFormatException
                        ("Unrecognised character : " + builder.charAt(i));
            }
        }
        // calling representationOf() makes sure that "IIIII" becomes "V".
        return new RomanNumeral(representationOf(value), value);
    }

    public static RomanNumeral valued(int value) {
        return new RomanNumeral(representationOf(value), value);
    }

    public static String representationOf(int value)
            throws ArithmeticException {
        // The valid range is (0, 5000)
        if (value <= 0 || value >= 5_000) {
            throw new ArithmeticException("Unsupported value : " + value);
        }
        int copy = value;
        StringBuilder builder = new StringBuilder(10);  // 4999 gives 10 chars
        // The descending order is maintained
        for (Map.Entry<String, Integer> entry : map.entrySet()) {
            int v = entry.getValue();
            String k = entry.getKey();
            while (copy >= v) {
                copy -= v;
                builder.append(k);
            }
        }
        return builder.toString();
    }

    private static final Map<String, Integer> map;

    static {
        // Using LikedHashMap ensures retention of order.
        Map<String, Integer> temporary = new LinkedHashMap<>(13);
        // Initialize in descending order.
        temporary.put("M", 1_000);
        temporary.put("CM", 900);
        temporary.put("D", 500);
        temporary.put("CD", 400);
        temporary.put("C", 100);
        temporary.put("XC", 90);
        temporary.put("L", 50);
        temporary.put("XL", 40);
        temporary.put("X", 10);
        temporary.put("IX", 9);
        temporary.put("V", 5);
        temporary.put("IV", 4);
        temporary.put("I", 1);
        // Make an unmodifiable map out of it .
        map = Collections.unmodifiableMap(temporary);
    }

}

The algorithm for converting from Roman to Decimal is adapted from this post.

I like that you use an immutable map.  However I don't like the name map: you should give it a more descriptive name and it should be capitalized since it is a constant.  You should also declare the type of map as LinkedHashMap: it is important here since you do use the ordering property.  You can define map as private static final Map<String, Integer> map = new LinkedHashMap<String, Integer>() {{ this.put(x, y); ... }}.

Your api should probably return an error on invalid roman numeral instead of: // calling representationOf() makes sure that "IIIII" becomes "V".

In method of (string to int), I think you don't have to use a StringBuilder and to invert it.  You can easily modify your algorithm to work forward and just use toCharArray.  You could use the Java 8 "functional" (Stream) features here instead of a for-loop.  I like functional programming since it makes for shorter and more readable code, but they really messed things up in Java 8 and I couldn't find an easy solution.  I could do it in Scala, but in Java 8 it would end up being longer than your code and completely unreadable.


Find the letter 'z' within three characters after an 'a' It works, just feel that my solution is too complicated.

# Write a method that takes a string in and returns true if the letter
# "z" appears within three letters **after** an "a". You may assume
# that the string contains only lowercase letters.
#
# Difficulty: medium.

def nearby_az(string)

  i = 0
  while i < string.length

    if string[i] == "a"
      a_index = i
    end

    if string[i] == "z"
      z_index = i
    end

    if (a_index) && (z_index) && (a_index < z_index) && ((z_index - a_index) <= 3)
      return true
    end
    i += 1
  end
  return false
end

# These are tests to check that your code is working. After writing
# your solution, they should all print true.

puts('nearby_az("baz") == true: ' + (nearby_az('baz') == true).to_s)
puts('nearby_az("abz") == true: ' + (nearby_az('abz') == true).to_s)
puts('nearby_az("abcz") == true: ' + (nearby_az('abcz') == true).to_s)
puts('nearby_az("a") == false: ' + (nearby_az('a') == false).to_s)
puts('nearby_az("z") == false: ' + (nearby_az('z') == false).to_s)
puts('nearby_az("za") == false: ' + (nearby_az('za') == false).to_s)

Riffing on @Shepmaster's excellent answer:

def nearby_az(string)
  !!string.match(/a.{,2}z/)
end

['baz', 'abz', 'abcz'].each do |s|
  puts "nearby_az('#{s}') expect true: #{nearby_az(s)}"
end
['a', 'z', 'za'].each do |s|
  puts "nearby_az('#{s}') expect false: #{nearby_az(s)}"
end


Tic Tac Toe Game using Python I'm brand-new (just joined today) to code review and I am super excited to share my tic tac toe game code with all of you! Please let me know what I could fix up in my program to make it more efficient. Thanks in advance!
P.S. I'm also not sure how to add an actual board to this game (an actual board rather than the way I have done here)
Code:
import time

# Initialize board
board = {1: ' ', 2: ' ', 3: ' ',
         4: ' ', 5: ' ', 6: ' ',
         7: ' ', 8: ' ', 9: ' '}

# Initialize variables
count = 0  # counter to track number of filled slots
winner = False  # boolean to check if anyone won
play = True  # boolen to check if the game should continue
tie = False  # boolean to check if there is a tie
curr_player = ''  # variable to store current player identifier
player_details = []  # list to store player identifier and marker

# Helper functions
def get_player_details(curr_player):
    """Function to get player identifier and marker"""
    if curr_player == 'A':
        return ['B', 'O']
    else:
        return ['A', 'X']

def print_board():
    """Function to print the board"""
    for i in board:
        print(i, ':', board[i], ' ', end='')
        if i % 3 == 0:
            print()

def win_game(marker, player_id):
    """Function to check for winning combination"""
    if board[1] == marker and board[2] == marker and board[3] == marker or \
            board[4] == marker and board[5] == marker and board[6] == marker or \
            board[7] == marker and board[8] == marker and board[9] == marker or \
            board[1] == marker and board[4] == marker and board[7] == marker or \
            board[2] == marker and board[5] == marker and board[8] == marker or \
            board[3] == marker and board[6] == marker and board[9] == marker or \
            board[1] == marker and board[5] == marker and board[9] == marker or \
            board[3] == marker and board[5] == marker and board[7] == marker:

        print_board()
        time.sleep(1)
        print("Player", player_id, "wins!")
        return True

    else:
        return False

def insert_input(slot_num, marker):
    """Function for capturing user inputs"""
    while board[slot_num] != ' ':
        print("spot taken, pick another no.")
        slot_num = int(input())
    board[slot_num] = marker

def play_again():
    """Function to check if player wants to play again"""
    print("Do you want to play again?")
    play_again = input()

    if play_again.upper() == 'Y':
        for z in board:
            board[z] = ' '
        return True
    else:
        print("Thanks for playing. See you next time!")
        return False

# Main program
while play:

    print_board()

    player_details = get_player_details(curr_player)
    curr_player = player_details[0]
    print("Player {}: Enter a number between 1 and 9".format(curr_player))
    input_slot = int(input())

    # Inserting 'X' or 'O' in desired spot
    insert_input(input_slot, player_details[1])
    count += 1

    # Check if anybody won
    winner = win_game(player_details[1], curr_player)
    if count == 9 and not winner:
        print("It's a tie!!")
        tie = True
        print_board()

    # Check if players want to play again
    if winner or tie:
        play = play_again()
        if play:
            curr_player = ''
            count = 0

Your board dictionary can be simplified from
board = {1: ' ', 2: ' ', 3: ' ',
         4: ' ', 5: ' ', 6: ' ',
         7: ' ', 8: ' ', 9: ' '}

to a dict comprehension:
board = {n: ' ' for n in range(1, 10)}

Your win_game function can be greatly simplified, from
def win_game(marker, player_id):
    """Function to check for winning combination"""
    if board[1] == marker and board[2] == marker and board[3] == marker or \
            board[4] == marker and board[5] == marker and board[6] == marker or \
            board[7] == marker and board[8] == marker and board[9] == marker or \
            board[1] == marker and board[4] == marker and board[7] == marker or \
            board[2] == marker and board[5] == marker and board[8] == marker or \
            board[3] == marker and board[6] == marker and board[9] == marker or \
            board[1] == marker and board[5] == marker and board[9] == marker or \
            board[3] == marker and board[5] == marker and board[7] == marker:

        print_board()
        time.sleep(1)
        print("Player", player_id, "wins!")
        return True

    else:
        return False

to
def win_game(marker, player_id):
    """Function to check for winning combination"""
    if board[1] == board[2] == board[3] == marker or \
       board[4] == board[5] == board[6] == marker or \
       board[7] == board[8] == board[9] == marker or \
       board[1] == board[4] == board[7] == marker or \
       board[2] == board[5] == board[8] == marker or \
       board[3] == board[6] == board[9] == marker or \
       board[1] == board[5] == board[9] == marker or \
       board[3] == board[5] == board[7] == marker:
       print_board()
       time.sleep(1)
       print("Player", player_id, "wins!")
       return True
    return False

You see, the else statement is not necessary, as the return statement will make the program jump out of the function. Also, as you can probably tell,
a == 1 and b == 1 and c == 1

is the same as
a == b == c === 1

The unnecessary else statement applies to your other functions as well
For get_player_details:
def get_player_details(curr_player):
    """Function to get player identifier and marker"""
    if curr_player == 'A':
        return ['B', 'O']
    else:
        return ['A', 'X']

to
def get_player_details(curr_player):
    """Function to get player identifier and marker"""
    if curr_player == 'A':
        return ['B', 'O']
    return ['A', 'X']

For play_again:
def play_again():
    """Function to check if player wants to play again"""
    print("Do you want to play again?")
    play_again = input()

    if play_again.upper() == 'Y':
        for z in board:
            board[z] = ' '
        return True
    else:
        print("Thanks for playing. See you next time!")
        return False

to
def play_again():
    """Function to check if player wants to play again"""
    print("Do you want to play again?")
    play_again = input()
    if play_again.upper() == 'Y':
        for z in board:
            board[z] = ' '
        return True
    print("Thanks for playing. See you next time!")
    return False

In your print_board function, you can increase the readability of your print statement with a formatted string. Using not instead of == 0 is considered more pythonic:
def print_board():
    """Function to print the board"""
    for i in board:
        print(i, ':', board[i], ' ', end='')
        if i % 3 == 0:
            print()

to
def print_board():
    """Function to print the board"""
    for i in board:
        print(f'{i} : {board[i]}  ', end='')
        if not i % 3:
            print()


Optimizing a jquery user interface application I'm working on a javascript intensive user-interface application. (At least it's intensive for me, it's my first serious javascript project).

I have a few jquery functions going whenever a div is rolled over or mouseout. For example there are some divs which are draggable objects, and so when they are mouseovered, the cursor must look ready to move. But my interface so slow that my boss thinks my code isn't working, when in fact it is, but it's just slow and so the user has to sometimes wait upto 2-3 seconds for the cursor to look as expected, or for a div to look as expected. Just an example. My code is something like below, I can't reproduce it entirely or properly since my company has copyrights on this, so I've written some lines as comments just to give a clear idea of the way I'm doing things. There's more to it, but this is basically what it's like:

$.fn.extend({
    mouseoverBox: function() {
    return this.each(function() {
        var $this = $(this);
                if(!$this.hasClass('ready'))
                {   if($this.hasClass('activated'))
                    {
                        $this.removeClass('activated');
                    }
                    $this.addClass('ready');
                }
                var img_id = $this.children('.theimg').attr('id');
                //someitem.children('somechildren').remove();
                //someitem.append(somemenu div)
                //$this.draggable();
                //$this.resizable();
                if($this.hasClass('unlocked'))
                {
                    $this.draggable( "option", "disabled", false );
                    $this.resizable( "option", "disabled", false );
                }
                $this.bindUnlock();
            });
     }
// end mouseoverBox
});

Now this is just the mouseoverBox function, which is triggered like myBox.mouseoverBox(). on the mouse over event. Then of course this plugin is calling bindUnlock() which has simple operations like changing classes and adding classes to a menu. Also, before any mouseover, there's usually been a mouseout out of another div... so as you can see there's a lot of stuff happening. There aren't usually more than 10 interactive divs on at any one moment though. How can I optimize this kind of code? I haven't given every specific but trust me most of the relevant is pretty much just like this.

I've already gzipped my javascript, css, images and fonts. I also tried minifying using the Yahoo Compressor but it actually bloated one of my files instead of compressing it.

Without actual code, we cannot review it if it actually contains significant issues. Thus, I've set forth guidelines instead.
Compression
The real help that compression does is actually speeding up loading times. Unless you are actually loading scripts on demand on mouseover, then you might need to compress them.
However, a better solution is to actually preload these resources, or load them in parallel, and execute once available. That's pretty much how RequireJS does things. That way, you are not actually waiting for resources. The same thing is also true for other resources like fonts, images, cursors, videos and all the rest.
Other scripts
Anyways, there are a lot of things that could cause your script to go wrong, and that includes other scripts on the page. Due to the fact that JS is naturally single-threaded (unless made to thread), any blocking operation, be it in other scripts or within your script, could potentially be slowing down your entire operation. An example is a very long loop, or SJAX (Synchronouse XHR).
To make sure that it's not your script that's breaking apart, try isolating your development enviroment to only your script and a minimal version of the UI and without other scripts. Also, test other functionalities on the page without your script to verify that it's another script that is causing the problem.
>700ms === lag
Yes, that's true. According to a YUI video on their YouTube channel, 200ms is considered "instant" in the perspective of the user. That also explains that game latencies of <200 are best. However, in your case, 2-3 seconds? The user has already left your page by then.
Aim for responsiveness that are less than or are in the range of 200-300ms. And yes, your boss is for real and knows what he's doing.
Hey! Cache!
Your script is potentially DOM heavy by its looks and description. But DOM manipulation is heavy work. Cache as much as possible whenever possible, especially on elements that don't change over the lifetime of the page.
Also, minimize fetching stuff on the DOM.


Web-App for to determine gender, age, emotions of a person (shown on an image) I have made an app using Microsoft's Face API: Face API JavaScript Quick Start

It shall analyze an image, showing a person's portrait, concerning the gender of the person, her/his age and emotions.

One pastes the URL of an image into the text-box. Then clicks the button.

The results are then listed below the image.

Caution! I'm using a free, temporary key in there. The app might stop working some time after my post.

(function(window) {
  var headersObject = new Headers();
  var photo = document.getElementById('analyzedPhoto');
  var attributesList = document.getElementById('attributes');

  headersObject.append('Ocp-Apim-Subscription-Key', '32fee7696d9545bdab41fd09ffb00034');
  headersObject.append('Content-Type', 'application/json');

  document
    .getElementById('analyse')
    .addEventListener('click', () => {
      var imgUrl = document.getElementById('imageUrl').value;

      var initObject = {
        headers: headersObject,
        method: 'post',
        body: '{"url": "' + imgUrl + '"}'
      }

      photo.src = imgUrl;

      fetch('https://westcentralus.api.cognitive.microsoft.com/face/v1.0/detect?returnFaceId=true&returnFaceAttributes=emotion,age,gender', initObject)
        .then((response) => {
          !response.ok &&
            Promise.reject(new Error('The request of analysis data has failed.'));

          return response.json();
        })
        .then((json) => {
          let facesAttributes =
            Object.assign({},
              json[0].faceAttributes,
              json[0].faceAttributes.emotion);
          let listItems = [];
          // Otherwise emotion-properties AND emotion-object self are contained in attributes-object.
          delete facesAttributes.emotion;

          for (let key in facesAttributes) {
            listItems.push('
' +
              (key.slice(0, 1).toUpperCase()) +
              (key.slice(1)) + ': ' + facesAttributes[key] +
              '
');
          }

          attributesList.innerHTML = listItems.join('');
        })
        .catch((error) => {
          console.dir(error);
          attributesList.innerHTML = 'Have you entered a valid image URL?';
        });
    });
})(window);
html {
  overflow-y: scroll;
  background-color: rgba(245, 245, 245, 1.0);
  font-family: "Trebuchet MS", "Lucida Grande", "Lucida Sans Unicode", "Lucida Sans", Tahoma, sans-serif;
}

.main-wrap {
  margin: 0 auto 30px;
  max-width: 800px;
}

#imageUrl {
  display: inline-block;
  width: 100%;
  max-width: 780px;
}

#analyzedPhoto {
  margin: 15px;
  border-radius: 3px;
  max-width: 100%;
  height: auto;
}

#attributes {
  list-style: none;
  padding-left: 0;
}

#attributes .list-item {
  margin-bottom: 10px;
}

.list-item-key {
  font-weight: 900;
  color: rgba(20, 20, 20, 1.0);
}

Enter Image URL

      <input type="text" value="https://upload.wikimedia.org/wikipedia/commons/thumb/5/56/Tesla3.jpg/220px-Tesla3.jpg" id="imageUrl" />

    <button id="analyse">Analyse</button>

Attributes

Is my usage of the fetch-API and Promises correct and done in a good way?

These concepts are fairly new to me.

Moreover: I had a bit of hassle merging the two objects into one. Please see my comment within the code.

Is there a better solution than deleting the unnecessary sub-object?

Addressing Your Questions

Is my usage of fetch-API and Promises correct and done in a good way?

Yes it seems fine. The Fetch API is supported by all major browsers but not IE1 but some argue that it doesn't really have a simple way to cancel requests. If there arises a need to abort requests, then other libraries like SuperAgent or reqwest could be used.

Is there are better solution then deleting the unnecessary sub-object?

Obviously there are other ways to handle setting the values within the emotion property on faceAttributes, but those might likely require more lines. One could also conditionally display list items (e.g. add if (typeof facesAttributes[key] !== "object") { before the line that pushes strings into listItems). If there was a way to filter with a callback when calling Object.extend() that would be an advantage here but unfortunately it doesn't appear that is the case.
Other review points
Feedback
The code looks quite succint. There are two DOM lookups at the start but then one DOM lookup in the click handler so that could be cached in a variable.
Suggestions
var vs let/const
Perhaps you have already learned more about let and const since posting this question but many es-6 developers would prefer using const for values that don't change (e.g. imgUrl, initObject, etc.)
Template
One could also utilize a script template to display the list items, though that may not be desirable here.
Invalid HTML when Error occurs
This line in the promise catch:

attributesList.innerHTML = 'Have you entered a valid image URL?';

Sets the inner HTML of the unordered list tag to a text string. That is technically unpermitted content for an unordered list. Perhaps a better solution would be to have a separate element for the error message.
P.S.
Here is the output from that AJAX call using that image of Tesla, so you could fake it now that the key is invalid:

const output = [{"faceId":"950c26ae-70f0-4365-8279-fcd987f56e70","faceRectangle":{"top":75,"left":32,"width":117,"height":117},"faceAttributes":{"gender":"male","age":37.0,"emotion":{"anger":0.0,"contempt":0.0,"disgust":0.0,"fear":0.0,"happiness":0.007,"neutral":0.992,"sadness":0.0,"surprise":0.0}}}];
const p1 = new Promise(function(resolve, reject) {
  setTimeout(resolve, 100, output);
  });
p1.then(console.log);


Using a singleton class to get and set program wide settings The following code works and does what I want, but I'd like to confirm I'm using this OOP concept correctly. I'm using the following class to get and set some configuration parameters for my program. It used $GLOBALS for this before, but I'm trying to use a singleton pattern for this instead.

Below is the Config class. Does anything jump out as aggressively stupid? Seeing as how I'm going to be including this in most files any way I figured it would make sense to throw the autoload function in there too- is this bad design?

<?php

  //This class will be included for every file
  //needing access to global settings and or
  //the class library

  spl_autoload_register(function($class){
      require_once 'classes/' . $class . '.php';
  });

  final class Config {

      private static $inst = null;

      //Array to hold global settings
      private static $config = array(
          'mysql' => array(
              'host' => '127.0.0.1',
              'username' => 'root',
              'password' => '123456',
              'db' => NULL
          ),
          'shell' => array(
              'exe' => 'powershell.exe',
              'args' => array(
                  '-NonInteractive',
                  '-NoProfile',
                  '-NoLogo',
                  '-Command'
              )
          )
      );

      public static function getInstance() {
              if (static::$inst === null) {
                  static::$inst = new Config();
              }
              return static::$inst;
      }

      public function get($path=NULL) {

          if($path) {
              //parse path to return config
              $path = explode('/', $path);

              foreach($path as $element) {
                  if(isset(static::$config[$element])) {
                      static::$config = static::$config[$element];
                  } else {
                      //If specified path not exist
                      static::$config = false;
                  }
              }
          }

          //return all configs if path NULL
          return static::$config;
      }

      public function set($path=NULL,$value=NULL) {

          if($path) {
              //parse path to return config
              $path = explode('/', $path);
              //Modify global settings
              $setting =& static::$config;

              foreach($path as $element) {
                      $setting =& $setting[$element];
              }

              $setting = $value;
          }
      }

          //Override to prevent duplicate instance
          private function __construct() {}
          private function __clone() {}
          private function __wakeup() {}
  }

I access it like so:

$aaa = Config::getInstance();
var_dump($aaa->get());
$aaa->set('mysql/host','zzzzzzzzzzzz');
var_dump($aaa->get());

I read a little bit about how singletons are less useful in php. I will research that further at some point, but for the time being I just want to make sure I'm doing this right.

There should be something about a Singleton class that requires that it allow and present a single, globally available, instance.  Otherwise, the pattern is being misused.

The way your class is built, there's not really such a reason.  You could have any number of instances, and they'd all work (though they'd all look like the same instance anyway).

As for recommendations to make things less weird...?

Use instance variables.

Everything is essentially global.  (Make no mistake; hiding it in a class doesn't do much at all to change that.  static is just how a lot of OO languages spell "global".  :P)  Your "instance" is just an interface to get and set that global stuff.  Every "instance" has the same ability, since it gets and sets the same variables -- and in fact, any update through one "instance" will be reflected in every other.  So new Config and Config::getInstance() really differ only in whether another instance is created.  The caller could say one or the other, and get the exact same results.

If you insist on a singleton, moving the static variables into the instance would legitimize the existence of an instance at all, and provide a less dodgy reason to require exactly one authoritative instance.  (Though it'd also prompt the question, "Why can't i create a separate configuration for testing?".  Among others.)
Lose the public setter, or provide a way to disable it.

There's next to no encapsulation or data hiding going on here; the only thing that even remotely counts is your path stuff.  Any caller can change global settings at will.  This kind of "action at a distance" can cause all kinds of weirdness.  It is the very reason mutable global state is discouraged, and set makes it even easier to abuse.

You should consider either getting rid of the setter, making it private (and thus disabling outside modification altogteher), or adding some way for your setup code to disable it -- effectively "freezing" the instance -- once the configuration is established.
Use self::, not static::.

static (when used in place of a class name) uses PHP's "late static binding" to look up variables through the name of the called class rather than the one where the method lives.  It's there to allow subclasses to override static members.  But you've explicitly disallowed inheritance...so values will only ever be looked up in this class.  static is thus wasteful, and a bit of a mixed message.
As for the autoloader, it's not exactly a mortal sin or anything to put that in the same file as the config class.  Unless the classes are going to be moving around, though, their current location isn't really part of the config settings, is it?

I'd suggest making the autoloader a part of the startup code, unless the class files are going to move around (thus making the current location a part of the configuration).


Directory operations Do you have any suggestions about how to make this code faster/better? Maybe suggest new features or better comments/docstrings?

from time import time, ctime, sleep
from random import choice, uniform
from glob import glob
import os

# Gets current working directory
DIRECTORY_NAME = os.getcwd()

def load():
    """Prints loading messages"""
    os.system("clear")
    MESSAGES = ["Deleting hard drive...", "Reticulating Spines...", "Fetching Your Credit Card Number...", "Hacking your computer..."]
    print(choice(MESSAGES))
    sleep(uniform(1, 5))
    os.system("clear")

def print_dir(dirname):
    """Prints the current directory"""
    print("Directory: %s" % dirname)
    print("-"*80)

def number_of_files(dirname):
    """Finds the number of files in the directory using glob"""
    num_of_files = len(glob("*"))
    print(num_of_files, "files")
    print("-"*80)

def last_access(dirname):
    """Prints a ctime representation of the last access to a file"""
    print("Last Access: ")
    print(ctime(os.path.getatime(dirname)))
    print("-"*80)

def last_change(dirname):
    """Prints a ctime representation of the last change to a file"""
    print("Last Change: ")
    print(ctime(os.path.getmtime(dirname)))
    print("-"*80)

def size_of_dir(dirname):
    """Walks through the directory, getting the cumulative size of the directory"""
    sum = 0
    for file in os.listdir(dirname):
        sum += os.path.getsize(file)
    print("Size of directory: ")
    print(sum, "bytes")
    print(sum/1000, "kilobytes")
    print(sum/1000000, "megabytes")
    print(sum/1000000000, "gigabytes")
    print("-"*80)
    input("Press ENTER to view all files and sizes")

def files_in_dir(dirname):
    """Walks through the directory, printing the name of the file as well as its size"""
    print("Files in directory: %s" % dirname)
    for file in os.listdir(dirname):
        print("{0} => {1} bytes".format(file, os.path.getsize(file)))

load()
print_dir(DIRECTORY_NAME)
number_of_files(DIRECTORY_NAME)
last_access(DIRECTORY_NAME)
last_change(DIRECTORY_NAME)
size_of_dir(DIRECTORY_NAME)
files_in_dir(DIRECTORY_NAME)

from time import time, ctime, sleep
from random import choice, uniform
from glob import glob
import os

# Gets current working directory
DIRECTORY_NAME = os.getcwd()

Given that its global and all caps, I'd expect it to be a constant. But it's not really a constant.

def load():
    """Prints loading messages"""
    os.system("clear")

This isn't portable, you may want to check out: https://stackoverflow.com/questions/2084508/clear-terminal-in-python

    MESSAGES = ["Deleting hard drive...", "Reticulating Spines...", "Fetching Your Credit Card Number...", "Hacking your computer..."]

I'd put this as a global outside of the function.

    print(choice(MESSAGES))
    sleep(uniform(1, 5))
    os.system("clear")

def print_dir(dirname):
    """Prints the current directory"""

It prints the directory passed in, not the current directory

    print("Directory: %s" % dirname)
    print("-"*80)

You do this several times, perhaps write a function for it

def number_of_files(dirname):
    """Finds the number of files in the directory using glob"""

I wouldn't put details like how it counts the files in the docstring

    num_of_files = len(glob("*"))
    print(num_of_files, "files")
    print("-"*80)

def last_access(dirname):
    """Prints a ctime representation of the last access to a file"""
    print("Last Access: ")
    print(ctime(os.path.getatime(dirname)))
    print("-"*80)

def last_change(dirname):
    """Prints a ctime representation of the last change to a file"""
    print("Last Change: ")
    print(ctime(os.path.getmtime(dirname)))
    print("-"*80)

These two functions are quite similar. Considering combining them, passing the os.path.xtime function as a parameter

def size_of_dir(dirname):
    """Walks through the directory, getting the cumulative size of the directory"""
    sum = 0

sum isn't a great choice because there is a builtin python function by that name

    for file in os.listdir(dirname):
        sum += os.path.getsize(file)

I'd use directory_size = sum(map(os.path.getsize, os.listdir(dirname))

    print("Size of directory: ")
    print(sum, "bytes")
    print(sum/1000, "kilobytes")
    print(sum/1000000, "megabytes")
    print(sum/1000000000, "gigabytes")
    print("-"*80)
    input("Press ENTER to view all files and sizes")

As palacsint mentioned, this should really be in the next function, or perhaps in your main function.

def files_in_dir(dirname):
    """Walks through the directory, printing the name of the file as well as its size"""
    print("Files in directory: %s" % dirname)
    for file in os.listdir(dirname):

file is builtin class in python, I'd avoid using it as a local variable name

        print("{0} => {1} bytes".format(file, os.path.getsize(file)))

load()
print_dir(DIRECTORY_NAME)
number_of_files(DIRECTORY_NAME)
last_access(DIRECTORY_NAME)
last_change(DIRECTORY_NAME)
size_of_dir(DIRECTORY_NAME)
files_in_dir(DIRECTORY_NAME)

I'd suggesting putting all of these in a main() function.