A* path finding getting the node neighbors

Just as I answered yesterday in BFS in a grid with wall breaking saldo in Java : define a direction and use it:

enum Direction {
    // define directions for each pair of offsets
    NORTH(-1, 0, "up"),
    NORTHEAST(-1, 1, "top-right"),
    SOUTH(1, 0, "...."),
    ...
    ;

    public final int offsetX;
    public final int offsetY;
    public final String drawableValue;

    private Direction(int offsetX, int offsetY, String drawableValue) {
        this.offsetX = offsetX;
        this.offsetY = offsetY;
        this.drawableValue = drawableValue;
    }
};

private boolean isDiagonal(int x, int y, Node node) {
    for(Direction dir : Direction.values())
        if(x == dir.offsetX && y == dir.offsetY) {
           node.arrow.setDrawable(dir.drawableValue);
           return true;
        }
    return false;
}

Apart from that, the method-name is a lie: it does NOT check for diagonals only, so you should find a name which describes the actual action.

(And just because this is code-review and someone will be tempted to say: add braces to the for loop: no. This is the style I prefer, I know that other people like it in other ways. ;-))


Obligatory link to the relevant Security question: XKCD #936: Short complex password, or long dictionary passphrase?

  can I be confident that the resulting password has 4log2354986≈74 bits of entropy and is safe to use as my master password?

That's two questions which are only partially related to each other. Let's start with the easy question first.

Is it safe to use as your master password?

A good password is:

Hard to guess.
Easy to remember.
Hard to brute-force.

You can enforce the first by making sure there's no personal data (birthday, name of your dog, name of the site, parts of your username, etc.) mentioned in the password. You got that covered.

The second issue is only tackled partially. Not all generated output makes sense or is easy to remember. However, it's likely that given enough tries a good candidate will pop-up. Keep in mind that words mentioned from categories mentioned in the previous paragraph should be avoided.

The third appears to be enforced for now, assuming you reach 74 bits of entropy as stated. I don't feel qualified to verify that, but entropy is about the selection process and not about the selected password itself. If the process has an entropy of 74 bits, it will always have 74 bits.

Computers change, brute-force methods change and what was a safe password 20 years ago may not be safe to use today. The generated passwords appear to be safe, since they take a long time to crack.


Option 1:

If your buttons and pages are actually numbered like that, you can simply extract the number from the ID:

var $container = $('#div1');

$('[id^=button]').click(function() {
    $container.load('page' + this.id.match(/\d+$/)[0] + '.php');
});

Option 2:

If that was just an example, but in reality your naming scheme is not so predictable, you can keep an object map with all the appropriate URLs:

var $container = $('#div1'),
    map = {
        button1: 'page1',
        button2: 'page2'
        // and so on...
    };

$('[id^=button]').click(function() {
    $container.load( map[ this.id ] );
});

Option 3:

You could store the page URL in the HTML:

and then use that in your JavaScript:

var $container = $('#div1');

$('[id^=button]').click(function() {
    $container.load( $(this).attr('data-page') );
});

I personally dislike this method, since it puts behavioral stuff in your HTML.


Observations.

Move disk 1 from 1 to 3
Move disk 2 from 1 to 2

I think these messages would be more descriptive (less ambiguous) if you included the word peg:

Move disk 1 from peg 1 to peg 3
Move disk 2 from peg 1 to peg 2

mov     r9d, r12d                   ; update r9d after hanoi call
mov     r8d, esi                    ; update r8d after hanoi cal

I found it confusing to read these comments. Registers R8D and R9D are being setup for the upcoming printf and nothing else. The mention of "update after hanoi call" would suggest they have a more permanent significance which they haven't.

Optimizations.

The .skip_recursive_move operation for when ECX is 1 on input can be much simpler. All registers are already conveniently setup for the tailcall (jmp) to printf.
The program uses the EBP register in a redundant fashion and hence you can shave off a number of instructions. You also no longer need to preserve EBP.

I suggest you take a look at my version below:

hanoi:
    cmp     ecx, 1                      ; if num_disks == 1
    je      .skip_recursive_move

    push    rbx
    push    rsi
    push    rdi
    push    r12

    mov     ebx, ecx                    ; ECX=2+
    mov     esi, edx                    ; esi = edx = src
    mov     edi, r9d                    ; edi = r9d = tmp
    mov     r12d, r8d                   ; r12d = r8d = dst

.move_disk:
    mov     r9d, r12d                   ; dst
    mov     r8d, edi                    ; tmp
    mov     edx, esi                    ; src
    lea     ecx, [rbx - 1]              ; num_disks - 1
    call    hanoi                       ; tmp & dst are swapped for this call

    mov     r9d, r12d                   ; the 'to' peg
    mov     r8d, esi                    ; the 'from' peg
    mov     edx, ebx                    ; num_disks
    lea     rcx, [prompt_move_disk]
    call    printf

    dec     ebx                         ; num_disks - 1
    cmp     ebx, 1                      ; if num_disks == 1
    je      .move_one_disk

    mov     eax, esi
    mov     esi, edi
    mov     edi, eax                    ; swaps tmp and src (esi and edi)
    jmp     .move_disk

.move_one_disk:
    mov     r8d, r12d                   ; the 'to' peg
    mov     edx, edi                    ; the 'from' peg

    pop     r12
    pop     rdi
    pop     rsi
    pop     rbx

.skip_recursive_move:
    lea     rcx, [prompt_move_one_disk]
    jmp     printf

Please notice that setting up the calling registers in a consistent order (RCX, RDX, R8, R9) helps to improve readability. Leaving a blank line further groups things into logical blocks.

One additional optimization comes from exchanging ESIand EDI regardless, thus being able to re-arrange the code to avoid having 2 jump instructions:

    ...
    mov     eax, esi
    mov     esi, edi
    mov     edi, eax                    ; swaps tmp and src (esi and edi)

    dec     ebx                         ; num_disks - 1
    cmp     ebx, 1                      ; if num_disks == 1
    jne     .move_disk

    mov     r8d, r12d                   ; the 'to' peg
    mov     edx, ESI                    ; the 'from' peg
    ...

See that this time ESI gives the 'from' peg!

EDIT (copy-paste version):

;; bits 64
default rel

extern printf, scanf

section .text
    global main
    global hanoi
main:
    sub     rsp, 56
    lea     rcx, [prompt]
    call    printf
    lea     rdx, [rsp+32]
    lea     rcx, [scan_fmt]
    call    scanf
    cmp     eax, 1
    jz      .call_hanoi
    lea     rcx, [scan_fail]
    call    printf
    mov     eax, 1                      ; return 1
.end:
    add     rsp, 56
    ret
.call_hanoi:
    mov     ecx, dword [rsp+32]         ; ecx = num_disks
    mov     r9d, 3                      ; r9d = tmp
    mov     r8d, 2                      ; r8d = dst
    mov     edx, 1                      ; edx = src
    call    hanoi                       ; hanoi(num_disks, src, dst, tmp)
    xor     eax, eax                    ; return 0
    jmp     .end
hanoi:
    cmp     ecx, 1                      ; if num_disks == 1
    je      .skip_recursive_move

    push    rbx
    push    rsi
    push    rdi
    push    r12

    mov     ebx, ecx                    ; ECX=2+
    mov     esi, edx                    ; esi = edx = src
    mov     edi, r9d                    ; edi = r9d = tmp
    mov     r12d, r8d                   ; r12d = r8d = dst

.move_disk:
    mov     r9d, r12d                   ; dst
    mov     r8d, edi                    ; tmp
    mov     edx, esi                    ; src
    lea     ecx, [rbx - 1]              ; num_disks - 1
    call    hanoi                       ; tmp & dst are swapped for this call

    mov     r9d, r12d                   ; the 'to' peg
    mov     r8d, esi                    ; the 'from' peg
    mov     edx, ebx                    ; num_disks
    lea     rcx, [prompt_move_disk]
    sub     rsp, 32                     ;shadow space
    call    printf
    add     rsp, 32

    mov     eax, esi
    mov     esi, edi
    mov     edi, eax                    ; swaps tmp and src (esi and edi)

    dec     ebx                         ; num_disks - 1
    cmp     ebx, 1                      ; if num_disks == 1
    jne     .move_disk

    mov     r8d, r12d                   ; the 'to' peg
    mov     edx, esi                    ; the 'from' peg

    pop     r12
    pop     rdi
    pop     rsi
    pop     rbx

.skip_recursive_move:
    lea     rcx, [prompt_move_one_disk]
    sub     rsp, 32                     ;shadow space
    call    printf
    add     rsp, 32
    ret

%macro str 2
    %2: db %1, 0
%endmacro

section .rdata
    str "How many disks do you want to play with? ", prompt
    str "%u", scan_fmt
    str {"Uh-oh, I couldn't understand that...  No towers of Hanoi for you!", 10}, scan_fail
    str {"Move disk 1 from peg %d to peg %d", 10}, prompt_move_one_disk
    str {"Move disk %d from peg %d to peg %d", 10}, prompt_move_disk


Here are some flaws in the current algorithm.

xdiff will have a roundoff error if x1-x2 is large in magnitude, likewise for the y variables.
slope is set to 1 for no reason, and then immediately reinitialized to something else.
slope is restricted to an integer, when most slopes will not be integral.
It's impossible to justify setting the slope of a vertical line to 2. There are slopes greater than 2 that are not vertical lines.
In the line slope = (int)xdiff/ydiff;, casting is higher precedence than division, so this will first cast xdiff to an int, overflowing if xdiff > MAXINT, throwing away the fractional part if there is one, then dividing this int by ydiff. The odds of this being the actual slope are very small.
startx and endx are initialized to 0 for no reason, then immediately reinitialized to somethiing else.
Roundoff errors will accumulate as you repeatedly add slope to y (or 1/slope to x).
What's the justification for the abs(y)? If y changes sign in the range, this will cause a kink in the line. Similarly for x.


Add a constructor. I'm pretty sure you can type hint your dependency too, so long as you know what you're working with, you can adjust to your needs.

use Symfony\Component\HttpFoundation\File\UploadedFile;

class Photo {

    protected $fileName;
    protected $extension;
    protected $hashedName;
    protected $path;

    public function __construct(UploadedFile $input)
    {
        $this->fileName     = $input->getClientOriginalName();
        $this->extension    = $input->getClientOriginalExtension();
        $this->hashedName   = sha1(time() . $this->fileName);
        $this->path         = $this->hashedName . '.' .$this->extension;
    }

    // methods...

}

Please also note in your code you wrote protected $filename; then referenced back to that in your methods as $this->fileName - make sure you get the n or N consistent.

You can always make a getter for the path attribute instead of making it public keep it protected.

public function getPath()
{
    return $this->path;
}

Hope that helps!


Calculating the checksum digit with the Luhn-algorithm

Some comments:

Variable names could be improved a little bit. For example, I think nbr_string could be better as swedish_id
Please add a docstring (PEP257)
itertools.cycle is useful to iterate multiple times over the same sequence (in this case, the multiplication factors)
Use list comprehensions where possible instead of for loops

Things that might be different from the question's code:

I'm assuming the initial string has 9 digits, but if it has already 10, then go with your code to keep only the first 9 numbers, but note that you can use some assertions to be sure the data passed is correct.
I've calculated partial sums to apply the algorithm as in the explanation, but doing the sums as in the question should be fine.

Please find my solution based on your code below:

from itertools import cycle

def control_number(swedish_id):
    """Calculate control number in Swedish personal IDs

    :param swedish_id: A Swedish personal ID
    :type swedish_id: str

    """
    assert swedish_id.isdigit()
    assert len(swedish_id) == 9

    # Multiplication factors for each digit depending on its position
    mult_factors = cycle([2, 1])

    def partial_sum(number, mult_factor):
        """Calculate partial sum ofr a single digit."""
        quotient, remainder = divmod(number * mult_factor, 10)
        return quotient + remainder

    final_sum = sum(
        partial_sum(int(character), mult_factor)
        for character, mult_factor in zip(swedish_id, mult_factors))

    # Calculate control number based on partial sums
    control_number = -final_sum % 10

    return control_number

I hope this helps.


Using multiple popup windows with bPopup

He is right.  Think about maintenance.

If you wanted to add another link that popped up another element you would need to add yet another listener function.  Removing one would be the same thing.  Too many things to keep track of.

Change the code to add the event according to a class name and then you can bind to all at once.  Then include the popup element ID within the attributes of the opener link.

$('.popup-opener').bind('click', function(e) {
    e.preventDefault();
    $($(this).attr('popup-element')).bPopup();
});

Popup 1

// ...

Better yet, use the on() function to bind the event.  http://api.jquery.com/on/

Adding new openers now requires no changes to the javascript.


Micnic was almost there, but I think that you can lose the recursion completely by separating your if statement into several if statements

function choose(num){
    var looped = false;
    while (!looped) {
        if (num < 30 && apple > 0) {
            fruit.push("apple");
            apple--;
            return "added apple";
        }
        if (num < 70 && kiwi > 0) {
            fruit.push("kiwi");
            kiwi--;
            return "added kiwi";
        }
        if (num < 90 && banana > 0) {
            fruit.push("banana");
            banana--;
            return "added banana";
        }
        if (grapes > 0) {
            fruit.push("grapes");
            grapes--;
            return "added grapes";
        } else {
            if (looped) {
                return "no fruit left";
            }
            num = 1;
            looped = true;
        }
    }
};

This will remove the recursion (sounds like that is what you want to do)  and will still cascade through the fruit.  notice that if the number is 95 and there are no grapes it will still recursively call the choose function with a number of 1 to cover the rest of the fruit, The only difference here is that you don't have the recursive call in every if statement, and you have less than half of the if statement calls.  I also made sure that the this wouldn't loop more than once by implementing a while loop that will only ever loop once.

If the number is 40 it will start at checking for kiwi's if there are none then then it will check for bananas.

I think this is exactly what you were looking for.

I don't like a function returning undefined, so I added the return "added {fruit}";  but you could just have return; if that is your preference.


The equivalent to os.urandom is found in the crypto module: crypto.randomBytes. That module also contains most other functions you might want for password hashing etc.

To use the same password character set as your current code, you could do

exports.gen = (length = 10) ->
  throw new Error 'Length is not a number!' if typeof length isnt 'number'
  throw new Error 'Length must be [10..128]' unless 10 <= length <= 128

  range = 126 - 33
  buffer = require('crypto').randomBytes length # note: may throw an error

  (String.fromCharCode(33 + (range * c / 255) | 0) for c in buffer).join ''

I've changed a few small things

length instead of n
Slightly more descriptive error messages, i.e. what is not a number, and sterner wording for bad length values ("prefered" [sic] makes it sound as though it's just a suggestion, but it's actually a requirement)
CoffeeScript has chained comparisons like Python, so 10 <= length <= 128 can be used


Finds all valid words by arranging 3 cubes

It's just as easy to iterate over the characters in a string as over the elements in a list, so the initialization of the cubes can be simplified:

a, b, c = 'AJN BKO CLP'.split()

combinationsList can be computed using itertools.product:

combinationsList = list(itertools.product(a, b, c))

Instead of using a regular expression:

if re.match('.{3}$', word) != None:

just check the length:

if len(word) == 3:

(But once you have an efficient membership test, as discussed below, you can skip this check.)
The code checks for membership in a list by counting the number of occurrences:

if dictionList.count(word) > 0:

but it would be simpler to use the in operator:

if word in dictionList:

Testing for membership in a list is expensive — Python has to compare word with every member of dictionList in turn. It would be more efficient to use a set, which has a fast membership test.
The code in the post constructs a collection of dictionary words and checks each candidate word to see if it's in the dictionary. But because there are only 162 candidate words, and hundreds of thousands of entries in the dictionary, it would be more memory-efficient to do the operation the other way round — that is, to check each dictionary word to see if it's a candidate. That way, you wouldn't have to build a collection of dictionary words, you could just iterate over them one by one.

Revised code:

from itertools import permutations, product
cubes = 'AJN BKO CLP'.split()
candidate_words = {
    ''.join(permutation)
    for letters in product(*cubes)
    for permutation in permutations(letters)
}
valid_words = set()
with open('/usr/share/dict/words') as dictionary:
    for line in dictionary:
        word = line.strip().upper()
        if word in candidate_words:
            valid_words.add(word)
print(valid_words)


That is, more or less, a genetic algorithm, though it only implements mutation and not recombination:

@classmethod
def recombine(cls, parentA, parentB):
    traits = {}
    for t in ("r", "g", "b"):
        traits[t] = (getattr(parentA, t) + getattr(parentB, t))/2
    return cls(**traits)

There are, of course, many ways to implement recombination.  For example, instad of the simple averaging I do above, you could instead choose a random parent to take each trait from.  The best solution depends on your problem space.

To learn more, I'd suggest googling and finding tutorials or examples, and then trying to solve some more realistic problems with them.  Can you adapt them to solve any ProjectEuler problems?  Or find another application where they're useful?


Just a few things I would look into if this code will be used for any real purposes. The Client does not handle failing to connect to the Server. It assumes the Server will always be up and available. I don't see any kind of error handling if the Server/Client connection gets broken while sending messages. You should definitely add code to clean up your sockets properly after you are done with them.

Design wise I would separate the Socket connection from the actual data being sent. Maybe add connect/disconnect methods to the Client and Server to handle the Sockets and use an internal Thread that gets started after you have a valid connection. Should a data message know how to encode/decode itself? Should a data message know how to send/recv itself? In my opinion a separate Client/Server Service should be created to manage these functions. Data should be as dumb as possible. You also have a lot of Static state throughout your code I would remove that in favor of local class instance variables. Doing this will give you a lot more flexibility in the long run.

Another thing to consider is a Factory class which can create the Client and Servers. Use the Factory class to handle the configuration of the Client and Server rather than having them configure themselves. This will allow you to test the Client and Servers easier so you don't have to have a specific configuration file for testing.


I must admit that I didn't went through all of your code and have not even compiled it. Some ideas for improvement:

open_file:

I would change your function from

FILE *open_file(char const * restrict name, char const * restrict mode)

to something like this

int open_file(const char *name, const char *modes, FILE **file)

I prefer using a return value that indicates a successful function execution for functions that can fail. By using this you don't have to remember that the function is not successful if the return values is NULL. You will always get a FAILURE return for each function. Inside the function you could use something like this:

int open_file(const char *name, const char *modes, FILE **file) {
    if (name == NULL) { goto fail; }
    if (modes == NULL) { goto fail; }
    *file = fopen(name, modes);
    if (*file == NULL) { goto fail; }
    return SUCCESS;
fail:
    return FAILURE;
}

Even if someone tells you using goto would be bad. I absolutely love it for this situation. The main reason against goto is, that it can create code that is hard to understand. But in this case to me there is no clearer version due to the usage of goto.

If that is still not readable enough for you, you can create macros for error-checking. In these macros you could also integrate printing or logging error messages. For example for developement you could switch on printing error messages and for production code compile it without any error messages if the user shall not see them.

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

#define SUCCESS EXIT_SUCCESS
#define FAILURE EXIT_FAILURE

#define ERRTEST_ARG_NULL(arg, tag) \
    do { \
        if (!(arg)) { \
            printf("%s:%d: In function '%s': error: %s: ERROR_ARG_NULL\n", __FILE__, __LINE__, __func__, tag); \
            goto fail; \
        } \
    } while (0)

int open_file(const char *name, const char *modes, FILE **file) {
    ERRTEST_ARG_NULL(name, "name");
    ERRTEST_ARG_NULL(modes, "modes");
    *file = fopen(name, modes);
    ERRTEST_ARG_NULL(*file, "*file");
    return SUCCESS;
fail:
    return FAILURE;
}

int main(void) {
    FILE *file;
    printf("%d\n", open_file("C:/pathToFile/file.txt", "rb", &file));
    getchar();
    return 0;
}

assert

I would go with the above solution for error-checking because you are able to continue with you program even if an error occured. Think of an user that tries to open a file but uses a wrong file name. Using 'assert' your program would stop working from the first problem.

fread

'fread' will fail for loading bigger files than 1GB (at least on my machine). You can still use it for bigger files by reading them sequentially in chunks with sizes smaller than 1GB and then adding them at the right spot in your allocated array.

+1 for introducing restrict to me. That was new to me.


Is there a better way to return values with merge sort?

The expression merge(x,y) will always return a value. val r = merge(x,y) stores the value so that you can use it again.

You could write your code

var leftTemp = mergeSort(left);
var rightTemp = mergeSort(right);

var result = merge(leftTemp, rightTemp)

result

as

merge(mergeSort(left),mergeSort(right))

It is a matter of preference if you wish to use leftTemp and rightTemp like this, but the use of result, and then returning it on the next line is not idiomatic Scala.

You must assign values to an identifier when you want to use them more than once (as you do with temp in your final example). But as I said above you may want to do this for other reasons such as readability and keeping down line lengths.


Your solution is certainly neater than the other one. Programming challenge websites often contain horrible code.

There's a few things to improve though:

Use the container index type for indexing into a container. This ensures the index values cover the necessary range.

In this case, we should use std::size_t instead of int for indexing the std::string (or std::string::size_type if we're being paranoid). This would mean rethinking the algorithm slightly (perhaps we need a std::optional<std::size_t>).

Note that for storing the difference between indices (i.e. word length) we should use std::ptrdiff_t (or std::string::difference_type).

In C++ it's more idiomatic to use iterators, instead of indices. This makes it easier to genericize functions (e.g. finding the longest run satisfying an arbitrary precondition on an arbitrary sequence).

I'd be inclined to implement this with an outer loop to iterate each word, and an inner loop to find the start and end points.

It's more verbose, but perhaps a bit easier to follow:

#include <algorithm>
#include <cctype>
#include <string>

std::string get_longest_word(std::string const& input)
{
    auto const is_in_word = [] (unsigned char c) { return std::isalpha(c); };

    auto max_word_length = std::string::difference_type{ 0 };
    auto max_word_start = input.end();
    auto max_word_end = input.end();

    for (auto i = input.begin(); i != input.end(); )
    {
        auto word_start = std::find_if(i, input.end(), is_in_word);

        if (word_start == input.end())
            break;

        auto word_end = std::find_if_not(word_start, input.end(), is_in_word);

        auto const word_length = std::distance(word_start, word_end);

        if (word_length > max_word_length)
        {
            max_word_length = word_length;
            max_word_start = word_start;
            max_word_end = word_end;
        }

        i = word_end;
    }

    return std::string(max_word_start, max_word_end);
}

Which is on the way to becoming:

template<InputItT, PredicateT>
std::pair<InputItT, InputItT> find_longest_run(InputItT begin, InputItT end, PredicateT predicate);


A matrix with square diagonals and transpose being increments of other

Throwing a different spin on the process, I think you are missing an alternate algorithm that produces the same result....

Here is a 'simple' loop that 'works'. It works because it uses a different 'rule' that is based on the pattern that your matrix creates.

The Pattern is obvious when you study the matrix from the bottom-right corner, and work backwards. The initial value is the square of the matrix size. Then you subtract 1 to the left, then another 1 above, and another 1 to the next left, and another 1 to the next above, etc.

private static final int[][] buildMagicMatrix(int size) {
    int value = size * size + 1;
    int[][] matrix = new int[size][size];
    for (int i = size - 1; i >= 0; i--) {
        matrix[i][i] = --value;
        for (int d = i - 1; d >= 0; d--) {
            matrix[i][d] = --value;
            matrix[d][i] = --value;
        }
    }
    return matrix;
}

Using this algorithm you produce results like:

Size 9
    1    2    5   10   17   26   37   50   65
    3    4    7   12   19   28   39   52   67
    6    8    9   14   21   30   41   54   69
   11   13   15   16   23   32   43   56   71
   18   20   22   24   25   34   45   58   73
   27   29   31   33   35   36   47   60   75
   38   40   42   44   46   48   49   62   77
   51   53   55   57   59   61   63   64   79
   66   68   70   72   74   76   78   80   81


Try using pathlib over os when you're dealing with file/glob operations. It has a lot cleaner interface, and is part of standard library in python 3.
Name constants in your code with the CAPITAL_SNAKE_CASE. It is recommended guideline from the PEP-8.
Split your code into individual functions doing a single task each.
Put the execution flow for your code inside an if __name__ == "__main__" block.


A class that represents a key combination

I would start by fixing the warnings, one of which is an actual bug:

[1]
Operator = not returning a value

main.cpp: In member function 'Key& Key::operator=(Key&)':
main.cpp:50:5: warning: no return statement in function returning non-void [-Wreturn-type]
     }
     ^
main.cpp: In member function 'bool Key::operator==(const Key&) const':

Also: your operator= gets Key& key instead of const Key& key

Wait! don't rush to fix it. You can just eliminate it altogether and rely on the default assignment operator that does the same job ("rule of zero").

[2] using comma in the long list of equality check -- an actual bug

main.cpp:54:100: warning: left operand of comma operator has no effect [-Wunused-value]
         return ( ... && _win == key._win && _key == key._key, _c == key._c);
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~

Wait! don't rush to fix it. We will use std::bitset so the entire operator== would become simpler.

After fixing the warnings above we can proceed for improvements.

[3] Use std::bitset for the bool flags

Instead of:

bool _shift;
bool _ctrl;
bool _alt;
bool _win;

Use:

enum SpecialKeys {SHIFT, CTRL, ALT, WIN, _SIZE_};
std::bitset<SpecialKeys::_SIZE_> specialKeys;

[4] No need for assignment operator and copy ctor (rule of zero).

Just use the defaults.

Proposed fixed code:

#include <bitset>

class Key {
    enum SpecialKeys {SHIFT, CTRL, ALT, WIN, _SIZE_};
    std::bitset<SpecialKeys::_SIZE_> specialKeys;
    int _key = 0;
    char _c = 0;
public:
    constexpr Key() {}
    constexpr Key(bool shift, bool ctrl, bool alt, bool win, int key, char c)
        : _key(key), _c(c)
    {
        specialKeys[SpecialKeys::SHIFT] = shift;
        specialKeys[SpecialKeys::CTRL]  = ctrl;
        specialKeys[SpecialKeys::ALT]   = alt;
        specialKeys[SpecialKeys::WIN]   = win;
    }

    bool operator== (const Key& key) const
    {
        return (specialKeys == key.specialKeys && _key == key._key && _c == key._c);
    }

    bool operator!= (const Key& key) const
    {
        return !(*this == key);
    }
};


React multi-select-like component with filter

Overall this is good, but there are few things we can do improve this and make it more "modular".

Lets say someone wants to use your component and instead of an array of player objects, they have just an array of player names, this introduces a couple of problems.

The prop to pass this in the list is called objects which doesn't reflect the type of data that they have and their list is long so they can't manually change it.
Your component is expecting the data passed in objects to be an array of objects.

How can we resolve this?

Lets first rename the prop from objects -> data, this is better because it generalizes what this prop receives and the person using this can possibly assume you are able to handle multiple data types.

Next, we should update places where we are assuming the data to be an object and instead check to see if it is an object before handling it. One example is in <MultiSelect />, where you are iterating over each item. We can change that to be:

class MultiSelect extends React.Component {
  render() {
    return (

        {this.props.data.map((item, index) => (

 this.props.onClickFunction(index)}>
            {typeof item === "object" ? item.name : item}

        ))}

    );
  }
}

Now we can display data from an array of strings!

But what if the person using your component was using an array of objects? Unfortunately there is no documentation so when he created the objects he used the property name firstName instead of name. This now breaks the code we re-wrote above, but we can fix it...

Lets set a new prop on <DoubleMultiselect /> called something like nameField, which would be a string that is equal to the object property to display in the list.

So now wherever we were accessing the name property of the player we can replace it with this.props.nameField, re-writing the code above can now be:

class MultiSelect extends React.Component {
  render() {
    return (

        {this.props.data.map((item, index) => (

 this.props.onClickFunction(index)}>
            {typeof item === "object" ? item[this.props.nameField]: item}

        ))}

    );
  }
}

We've now added support for dynamic data, allowing anyone with any type of data to use this!


Why not use a set? For a set in is O(1). You can extend the set by adding another acc with db['users']['1']['accs'].add('misty') (add instead of append). After this change your lookup is only O(n), where n is the number of users (at worst) and in reality will depend on where the user is in the internal representation of the dict.

Alternatively you could just use a tuple (which is hashable). This just makes it different to add another account to a user:

db['users']['1']['accs'] += ("misty",)

This way you could build the reverse dictionary, speeding the lookup up to O(1). However, you would have to keep the reversed and actual db in sync.

In addition, to iterate over the keys of a dict, it is sufficient to do:

for key in db['users']:

But here I  would actually do:

for key, user in db['users'].items():
    if name in user['accs']:
        return key

You don't need the explicit return None because None is already the default return value of a function in python.

Your function get(name, key) is confusing, because name is actually id (so "1", "2", ...).

Python has an official style guide, PEP8. One of the things it recommend is using lower_case for variable and function names.


Moving or copying images from a flat directory to subdirectories

This looks pretty decent for a first Python program, so good job on that.

I only have a few small nitpicks

Some PEP8 violations, but nothing major

There should be a <space> after the ,
Line too long (max 79)

Consider making the directory paths global constants

PEP8 reference

PICTURES_PATH = "C:\\temp\\picturepy\\pictures\\"
GALLERIES_PATH = "C:\\temp\\picturepy\\galleries\\"

I think whatif could have a better name

What if what? It's not really clear what this variable is supposed to do

Maybe rename it to keep_original, because the copy will leave the original intact
Possible to do the if else in one line with unpacking

do = shutil.copyfile if whatif else shutil.move
verb = "copied" if whatif else "moved"

Would become

do, verb = (shutil.copyfile, "copied") if keep_original else (shutil.move, "moved")


Any recommendations to improve this logging framework? Or is it overkill?

You don't need to re-implement MessageFormat. It is much more powerful than what your implementation provides.

You don't show us the class of LOGGER, but based on the fact that you have a Log class, I suspect you are not using a logging library. I highly recommend you look into using one instead of implementing your on custom implementation.

If you are using a logging library, you are abstracting to much away from the library. All of the frameworks I've used support message formating and detailed exception logging (with stack traces).

The problem you are really trying to solve here is to allow yourself to reuse a number of common log message formats. This can be done with a simple set of Strings set to public static final.


Finding a word with the most occurrences of a character

arr and array

These arrays have too similar names. It's better to avoid using too similar names, to avoid confusion and mistakes.

These names are also poor, as they tell nothing about what's in the arrays. $words would have been a more natural name.

There's no need for both of these arrays at the same time. When you loop over the words to create another array of chars-of-words, you could go one step further, skip the array creation and do the character counting, and save the extra storage of another array.

Code organization

It's good to break your program to multiple small functions, each with a single responsibility. It will make it possible to test smaller and bigger logical units alike independently. For example, you could have functions like:

read a file and return an array of words
find the count of the most frequent character in a letter
find the word with the most frequent character in an array of words

Naming

$x is a bit unusual the counter variable in loops. $i is more common. Some of the improvements I suggested above will hopefully help you replace the counting for loop with a foreach loop. That way instead of a counter variable, you will be able to use something more natural like $word.


Algorithm

I think you've gotten distracted by focusing on the wrong aspect of the problem.

The challenge asks for the largest prime factor of n = 600851475143.  There are three approaches you might consider in tackling this challenge:

Construct the list of all relevant prime numbers, then take the largest one that is a factor of n.

How high do we need consider?  The largest candidate would be \$\frac{n}{3}\$.

What algorithm could we use?  A good method to build a list of many prime numbers is the Sieve of Eratosthenes.  That would require \$\frac{n}{3}\$ bits of memory, or 23 GiB. Even if you had a machine with that much memory, just initializing that much memory to zero would be too slow to be reasonable as a Project Euler solution.

Any other algorithm for building a list of primes would only be worse.

You didn't choose Door #1.  Good for you!
Test the largest candidate factors first.

Where would you start?  The largest candidate would be \$\left\lceil\sqrt{n}\right\rceil\$.  You can test odd candidates in descending order, stopping as soon as you find an answer that is prime.

How do you guarantee that the answer is prime?  You have to check for primality.  We've already ruled out the Sieve of Eratosthenes above, so you would have to use trial division or a fancier primality-testing algorithm.

How long would we expect this to take?  \$\left\lceil\sqrt{n}\right\rceil\$ is 775147.  We would have 387573 possible odd factors to test.  There's no telling where we'll find numbers that are actually factors of \$n\$.  Once we find a factor, we still have to verify whether it's prime.

Food for thought: what would you do if \$n\$ were even?

You didn't choose Door #2, even though some other users have advocated it.  Good for you!
Test the smallest candidate factors first.

Suppose that I asked you to find the largest prime factor of 10241.  How would you go about it?  Would you start at \$\left\lceil\sqrt{10241}\ \right\rceil\$, trying 103, 101, 99, 97, 95, …?

Chances are, you would prefer to do it the following way instead, and you could do it with just pencil and paper.  (It's also easy to implement as one function.)

10241 / 2 … no.
10241 / 3 … no.
10241 / 5 … no.
10241 / 7 = 1463
1463 / 7 = 209
209 / 7 … no.
209 / 9 … no.
209 / 11 = 19
19 / 11 … no.
19 / 13 … no.
19 / 15 … no.
19 / 17 = no.
19 / 19 = 1
done!

As you can see, the big win comes when you find that 10241 is divisible by 7 — you immediately reduce the search space by a factor of 7.  In just a few iterations, you've reduced the problem size from 10241 down to 209 — a much more manageable number.

Bonus question: in the example above, do we still need to test 19 for primality?  Why or why not?

As it turns out, strategy 3 also works well for Project Euler Problem 3.  In general, strategy 3 is a better bet than strategy 2, since prime numbers tend to be more densely clustered closer to 0.

You've chosen an algorithm that is similar to strategy 3, which is good.  However, it isn't the same solution.  Your computeFactors() function very closely mimics the algorithm illustrated for strategy 3, but with a subtle and crucial difference.  Can you spot it?


Memory

You allocate memory twice but you never deallocate it later. A good practice is to have all your new and new[] followed by a delete and delete[] respectively when you're done with the memory.

Constructor

It is a good habit to use the initialization list for your constructor. In your example, the constructor will first initialize number_of_vertices with a default value, then it'll dereference the this pointer and finally assign the value given in parameter. Using the initialization list will avoid the first two steps.
It is also better the avoid the use of this for initialization as it is not necessary and will add the cost of dereferencing the pointer.

Arrays

In general you should avoid the use of array + new as it can lead to errors (for example, you forgot to release the memory at the end).

std::vector are arrays under the hood with the same performance, you should not fear to use them even if the size of your array won't change later. In your case, you can even initialize it with the correct size and default value, avoiding the for loop:

std::vector<bool> visited(number_of_vertices, false);


Here are some comments on your code:

raw_input is a very inflexible way to get data into a program: it is only suitable for interactive use. For more general use, you should get your data into the program in some other way, for example via command-line arguments.
urllib.urlopen was removed in Python 3, so your program will be more forward-compatible if you use urllib2.urlopen instead. (You'll need to change the way your error handling works, because urllib2.urlopen deals with an error by raising an exception instead of returning a response object with a getcode method.)
The comment # vars does not seem to contain any information. Improve it or remove it.
The range function starts at 0 by default, so range(0,100) can be written more simply as range(100).
100 seems rather arbitrary. Why not take this as a command-line parameter too?
The variable pic seems poorly named. Is it short for picture? The urlopen function returns a file-like object from which the resource at the URL can be read. The examples in the Python documentation accordingly use f or r  as variable names.
I assume that this code is only going to be used for testing your own site. If it were going to be used to time the fetching of resources from public sites, it would be polite (a) to respect the robots exclusion standard and (b) to not fetch resources as fast as possible, but to sleep for a while between each request.
If an attempt to fetch the resource fails, you probably want to exit the loop rather than fail again many times.
You say in your post that the code is supposed to time "the entire transaction", but it does not do this. It only times how long it takes to call urlopen. This generally completes quickly, because it just reads the headers, not the entire resource. You do read the resource once (at the very end), but outside the timing code.
You multiply the time taken by 100 and then print it as an integer. This seems unnecessarily misleading: if you want two decimal places, then why not use "%.2f"?
Finally, there's a built-in Python library timeit for measuring the execution time of a piece of code, which compensates for things like the time taken to call time.

So I might write something more like this:

from sys import argv
from timeit import Timer
from urllib2 import urlopen

if __name__ == '__main__':
    url = argv[1]
    n = int(argv[2])
    length = 0

    def download():
        global length, url
        f = urlopen(url)
        length += len(f.read())
        f.close()

    t = Timer(download).timeit(number = n)
    print('{0:.2f} seconds/download ({1} downloads, average length = {2} bytes)'
          .format(t / n, n, length / n))


Follow up Implementation on Monty Hall Simulation

Positive Feedback

I liked how you follow the rules of Clean Code, that you write small methods and small classes and give all readable names!

You've introduced your own data types that make the code more readable. The MontyHall-Algorithm can now be read like a poem:

montyHall.chooseDoor();
montyHall.openDoor();
if (player.preferresChange()){
   montyHall.changeDoor();
}
if (Price.CAR == montyHall.getPrice()){
   player.increaseWinCounter();
}

And all your methods have only one intention level what makes them so easy to understand.

Some Criticism

Class Names Refer to Unexpected Behavior

I read on Wikipedia:

  You [the player] pick a door, say No. 1, and the host, who knows what's behind the doors, opens another door, say No. 3

After I read your code I was a little bit confused at some places. But not because of the quality of your code, which is for me very high, but because of the quote above.

In your game the Player can't pick a Door. Instead the Player gets picked a door by MontyHall: (montyHall.chooseDoor()).

Additional to that the Player has a winCounter, but actually the Simulation should track how often a Player wins the MontyHall.

No Outcome

What would you expect your main to do without any knowledge about our code base?

public static void main(String[] args) {
    Simulator simulator = new Simulator();
    simulator.simulate(10000, 3);
}

It could run something like a game loop or start a GUI to interact with but instead it writes data to the console.

Much cleaner would be:

public static void main(String[] args) {
    Simulator simulator = new Simulator();
    simulator.printSimulations(10000, 3);
}

But now imagine you want to compare a "Changer" and a "Stayer". You can't do it because you have no outcome! You through your outcome away by printing it to the console instead of storing and returning it.

public static void main(String[] args) {
    Simulator simulator = new Simulator();
    SimulationResult result = simulator.simulate(10000, 3);
    int average = result.calculateAverageOfStayer();
    System.out.println(average);
}

Feature Envy

  A method accesses the data of another object more than its own data.

The method playOneRound in playOneRound is in my eyes a feature envy. All the tasks should be done by MontyHall. So the method playOneRound should look like:

private void playOneRound(Player player, MontyHall montyHall) {
    montyHall.playGameWith(player);
}

Use Methods instead of Getters and Operations

In your code I can find code structure like x.getValue == something, but much better to read would be x.isSomething()

Some inspiration for you:

Price.GOAT == d.getPrice() means door.isHidingGoat()

Price.CAR == montyHall.getPrice() means montyHall.isWon()

!d.equals(selected) means door.notEquals(selected)

In your older post @Simon Forsberg mentioned:

  I have a feeling that using a class for StraightPlayer, ChangePlayer and ShowMaster will be a bit overkill.

I know you still have the flag argument in Player. But that's fine. In general, refactoring can be an overkill at all. And you can see that for this little problem you can divide the logic into so many different classes and methods. When you write code, you have to find a balance between all OOP/clean code and your task.

Thanks for this cool problem and that you let me review it. I learned a lot while read your code :]


Fast strings in Javascript
It is important to note that something as simple as assigning a string to a variable will cost the length of the string in complexity a = "12345" will cost 5 characters. Assign to another variable and the cost is the same b = a will cost 5.
Every time you assign a string you need to count the length of that string. If you copy an array of strings then you move all the characters, not just the number of strings in the array.
You do a lot of string assignments in your code.
Avoid string assignment
The trick to handling strings is to avoid assigning strings if you can. Arrays and string array character referencing (eg a = "abcdef"; b = a[1])  lets you treat the JS string like a C (char *) string. You dont copy the string, you are just indexing into the string.
The only time any sequence of characters are copied is on the return. The performance increase is very significant.
const longestCommonPrefix = words => {
    var i, pos = 0, len = words.length;
    const char = words[0][pos];
    var min = words[0].length;
    for (i = 1; i < len; i++) { // finds the min word length and check first char
        min = words[i].length < min ? words[i].length : min;
        if (char !== words[i][pos] || pos === min) { return "" }
    }
    pos ++;
    while (pos < min) {  // check up to min word length.
        const char = words[0][pos];
        for (i = 1; i <len; i++) {
            if (char !== words[i][pos]) { return words[0].substring(0,pos) }
        }
        pos ++;
    }
    return words[0].substring(0,pos);
}


A text-templating system similar to PHP...but with Clojure

Cool idea! I could definitely see this coming in handy in a variety of domains.

I noticed that you have some def and defn statements within the function definition of process-match. It's generally considered un-idiomatic to use def/defn anywhere except for at the top level of the namespace you're in. Usually you would use let instead if what you're trying to define isn't used outside of the function, but it doesn't look like that's the case here -- I think you are wanting buffer and echo
to be used at the top level, so I would pull them out of the process-match function and do something like this:

(ns closeddoor.core
  (:require clojure.string)
  (:gen-class :main true))

(def tag-start "<\\?clj")  ; The long/normal start tag.
(def tag-end "(\\?>|\\Z)") ; The long/normal end tag.

(def echo-start "<%") ; The short/echo start tag.
(def echo-end "%>")   ; The short/echo end tag.

; the compiled regex patterns for each of the above
(def tag-pattern (re-pattern (str tag-start "((?s:.+?))" tag-end)))
(def echo-pattern (re-pattern (str echo-start "((?s:.+?))" echo-end)))

(def buffer (StringBuilder.))

(defn- echo [string]
  "A small helper function for echoing things."
  (.append buffer string))

(defn- process-match [[match group]]
  "Processes the given match, and returns the output of the given match."
  (load-string (str "(use 'closeddoor.core)" group))
  (clojure.string/replace (.toString buffer) "$" "\\$"))

(defn- process-match-echo-wrapped [[match group]]
  "Process the given match, but wraps it first in the echo function."
  (process-match [match (str "(echo " group ")")]))

(defn parse [input]
  "Parses the given input, processes the matches and returns the result.
   The long/normal tags are processed first, after that the short/echo tags.
   Order of appearance does not matter, all normal tags are processed first."
  (clojure.string/replace
    (clojure.string/replace input tag-pattern process-match)
    echo-pattern
    process-match-echo-wrapped))

(defn process [source out]
  "Processes the given source, which means that it reads everything from
   the source with slurp, runs it through parse and spits it out into out.
   So (process input *out*) is basically shorthand for (spit *out* (parse input))."
  (spit out (parse (slurp source))))

(defn -main [& args]
  "The main function which does everything."
  (if (empty? args)
    (process *in* *out*)
    (doseq [arg args]
      (if (= arg "-")
        (process *in* *out*)
        (process arg *out*)))))

BTW, I also changed your spacing from 4 spaces to 2 which is the Clojure standard. I guess there's nothing wrong with using 4 spaces, but 2 is predominantly what you see.

I've generally rearranged the spacing and line breaks to suit my own aesthetic preference, but it isn't necessarily "the right way," just my opinion on what formatting I find more readable. Just my 2 cents!

I also moved the (require 'clojure.string) into the namespace definition, which is idiomatic and more concise.

I'm a bit confused about your parse function... did you mean for the last argument of each call to clojure.string/replace function to be a function (process-match and process-match-echo-wrapped, respectively)? I believe you need to have strings there, indicating what to replace each match with. Maybe you meant (process-match match group), where match and group are replaced with the actual match and group? Maybe I'm missing something.

By the way, if you wanted to simplify the nested calls to clojure.string/replace in that function, you could do something like this:

(defn parse [input]
  "Parses the given input, processes the matches and returns the result.
   The long/normal tags are processed first, after that the short/echo tags.
   Order of appearance does not matter, all normal tags are processed first."
  (reduce (fn [s [pat rep]]
            (clojure.string/replace s pat rep))
          input
          [[tag-pattern process-match]
           [echo-pattern pattern-match-echo-wrapped]]))

If I can offer one last criticism: your methods that deal with modifying buffer (which is initially defined as (StringBuilder.) are destructive actions, so you might consider adding ! onto the end of your function names (echo!, process-match!, etc.)

Hope this helps!


I don't know the LRU Algorithm. So I can't criticize the implementation. The most glaring problems are:

Undescriptive variable names, like "tmr", "n" and "m".
Undescriptive procedures, like "max" (max of what? The return value is not the maximum value of the array, but the position).
The use of "magic" numbers. Like the number 20 for example. Store that number in a descriptive constant.
The logic of the main procedure.

For example

 else
 {
     if(...)

could be written as such

else if(...)


There is little point in using code markup here. Technically, it might not be completely wrong “semantically”, since the YYYY-MM-DD notation is really meant for computers rather than human beings, but what would it achieve? Monospace font by default?

Using kbd is equally, if not more, pointless.

Using var might be “semantically” appropriate, since you are using placeholders of a kind. But, here, too, the question is: Why? Do you want some browsers to render the text in italics? What else would you achieve in practice?

Using abbr is “semantically” wrong, since e.g. YYYY is not an abbreviation. It is a conventional notation. In practice, abbr creates by default a dotted bottom border on some borders. Why use some markup when the only real implication is that you get some default rendering that you don’t want?

Using dfn would be equally wrong. You are not giving any definition for e.g. YYYY. Instead, you are implying a definition, i.e. assuming that the user knows what YYYY means.

The title element is poor usability, but if you wish to use it, you can use it for a span element, which has no defined semantics (so it cannot be semantically wrong) and has no impact on default rendering.

If you don’t think that the explanation ”(YYYY-MM-DD)” is not sufficient, the best approach is to explain it in normal content. E.g.,

Date (YYYY-MM-DD, i.e. 4-digit year, hyphen, 2-digit month, hyphen, 2-digit day):

Note that a label element that has neither a for attribute nor a form field as content is pointless. The label markup is useful only when used to associate a form field with its label text.


Is my code utilizing proper Object Oriented features?

I don't suggest encapsulating such standard functionality in classes unless you are just playing around or learning.
I suggest renaming element to ElementFactory or ElementSpawner or something alike as it doesn't encapsulate a DOM element but creates and appends them. You then might want to return the created elements, too.
Also, what happens when you call createNewWithAttribute without previous call to setElementAttribute? Shouldn't you rather keep an internal map of attributes and make their addition optional via createNew(withAttributes = true) parameter?
Also, elem.innerHTML = this.text should probably be written as elem.textContent = this.text unless text means html in which case you should rename the class.
Don't mix CamelCase (elementWithText) with unser_scores (attr_value) and preferably capitalize class names.


Project Euler #12 (Highly divisible triangular numbers) in Python 3.x

Your function facCount, which should be called something like count_factors, according to Python's official style-guide, PEP8, can be greatly improved by noticing that if num % i == 0, then automatically num % num / i == 0 (in other words, factors always come in pairs of two, except for if the number is a square, in which case you double count one). This means you only need to check factors up to \$\sqrt{n}\$:

from math import sqrt

def count_factors(num):
    """Return the number of factors of `num`"""
    sum_ = 2 * sum(num % i == 0 for i in range(1, int(sqrt(num)) + 1))
    if int(sqrt(num))**2 == num:
        sum_ -= 1
    return sum_

I also added a docstring describing what the function does in an accessible way.

The other improvement concerns the way you get the triangular numbers. While it is good to know Gauss formula, IMO it is here easier to manually calculate them. Your function needs to do one increment, one multiplication and one division, when all you really need is one addition per loop iteration:

from itertools import count

def triangular_nums():
    """Yield the triangular numbers"""
    t = 0
    for i in count():
        t += i
        yield t

If, for some reason, you dislike itertools, you can also replace it with a while True loop.

In Python 3+, you can use the new function itertools.accumulate (3.2+) and the new keyword combination yield from (3.3+), as mentioned in the comments by @MaartenFabré:

from itertools import accumulate, count

def triangular_nums():
    yield from accumulate(count())

With this your main function (which you should either make a real function or at least put under an if __name__ == "__main__": guard) becomes:

def main():
    for t in triangular_nums():
        if count_factors(t) > 500:
            return t

if __name__ == "__main__":
    ans = main()
    if ans is not None:
        print(ans)

The timing you should factor out and make into a decorator:

import time

def timeit(func):
    def wrapper(*args, **kwargs):
        start = time.time()
        ret = func(*args, **kwargs)
        print("--- %s seconds ---" % (time.time() - start))
        return ret
    return wrapper

@timeit
def main():
    ...


Graceful way to iterate through two arrays and build a master array?

If you need to do the combination in Ruby (and I don't think you do; read on), you could do something like this, using Array#product

# just extracting these 2 params for clarity
controller_ids = params[:controller_id]
zone_numbers = params[:zone_number]

# product will give you every id/number combo,
# which is then mapped to a Zone record's ID
zone_ids = controller_ids.product(zone_numbers).map do |controller_id, number|
  Zone.find_by_controller_id_and_number(controller_id, number).id
end

However! I'd suggest less Ruby and more SQL for this.

# line-breaks are for clarity
zone_ids = Zone.where(
  'controller_id IN (?) AND number IN (?)', controller_ids, zone_numbers
).pluck(:id)

That should pluck just the id values from the Zones table without loading entire records.

While you can generate an array that contains every combination of controller ID and zone number, you'd still (with your current code) do separate DB queries for each of those combinations. And doing SQL queries in a loop is a sure sign you should look for another approach altogether (namely doing more in the DB layer, if possible).


Using volatile instead of synchronized for a simulation

volatile should perform better and is appropriate in this situation. A volatile read/write is almost as fast as a non volatile read/write on modern architectures. On the other hand, locking with synchronized will have an overhead, especially in a highly contented scenario (many threads fighting for the lock).

Note however that unless you call the pause/run methods thousands or millions of times per second, you will probably not notice the difference.

Additional comments

This line:

this.simulations = Collections.unmodifiableList(simulations);

probably does not do what you think it does: this.simulations is unmodifiable, but the calling code can still modify the original collection, and the changes will be reflected to your local version (or they won't depending on memory consistency). It would probably be better to do a defensive copy:

this.simulations = new ArrayList<> (simulations);

Another note: because your simulations is effectively immutable,

this.simulations = new ArrayList<> ();

could be replaced by:

this.simulations = Collections.emptyList();

And by the way, you could make simulations final.


You should define one click handler that removes all siblings of the clicked element.  That is, remove all children of the parent, except the target of the event itself.

document.getElementById('game1').addEventListener('click', function(event) {
    // Remove all siblings of the clicked element
    for (var c = this.childNodes.length - 1; c >= 0; c--) {
        if (this.childNodes[c] != event.target) {
            this.removeChild(this.childNodes[c]);
        }
    }
});
<div id="game1">
  <button>Alpha</button>
  <button>Bravo</button>
  <button>Charlie</button>
  <button>Delta</button>
</div>

You could also iterate this way, but there's an awkward c--:

for (var c = 0; c < this.childNodes.length; c++) {
    if (this.childNodes[c] != event.target) {
        this.removeChild(this.childNodes[c--]);
    }
}


Aside from the other optimizations, if you are using the sieve to check for primality of a number, you can optimize further, every prime after 2 is odd, therefore any even number that isn't 2 is not prime. This can easily be checked without the sieve.  I would suggest that marking all the even numbers false is superfluous.  So mark 2 as true and start the outer loop at 3 and increment by 2. As was mentioned, the inner loop can start at i*i, and since this will be an odd number you only need to mark every other multiple, therefore you can increment by i*2.  Something like this:

std::vector<bool> sieve_of_eratosthenes(int n)
{
    std::vector<bool> primes(n+1, true);
    primes[0] = false;
    primes[1] = false;
    int limit = (int)sqrt(n) + 1;
    for (int i = 3; i < limit; i += 2)
    {
        if (primes[i])
        {
            int step = i * 2;
            for (int j = i * i; j <= n; j += step)
            {
                primes[j] = false;
            }
        }
    }
    return primes;
}


Finding the first pair of prime numbers that have some specified difference

First thing first, get rid of these ugly a and c. They do not add any value, but only obfuscate the code.
def gap(p, q, m):
    """To generate gap in between two prime numbers"""
"""p is the difference,q is the lower limit where the list of numbers in between which prime is filtered,m is the upper limit"""
    b = []
    q = (q // 2) * 2 + 1
    for i in range(q, m + 1, 2):
        if prime(i):
            b.append(i)
            if len(b) > 1:
                if b[-1] - b[-2] == p:
                    return [b[-2], b[-1]]

Notice that I also removed a redundant == True.
Second, you don't need to keep the entire list of primes. You are only interested in the last two of them. Consider
def gap(p, q, m):
    b = find_first_prime_after(q)
    for i in range(b + 2, m + 1, 2):
        if prime(i):
            if i - b == p:
                return b, i
            b = i

Finally, your primality test is very suboptimal. Implementing the sieve would give you a boost.


Calculating the minimum distance of a linear code

The n and k parameters seem pointless: you can derive them from the dimensions of the received generator matrix:

int n = generator.length;
int k = generator[0].length;

Instead of this:

for (int j = 0; j < n; j++) {

You could replace using for-each, which is just more natural:

for (int[] row : generator) {

I don't really see a reason to store the parameters as fields.
The class doesn't need to hold data.
You need just a pure function,
so this can be a public static method.

Putting it together

Applying the above suggestions (+ a few minor things):

public static int getMinimumDistance(int[][] generator) {
    int n = generator.length;
    int k = generator[0].length;
    int minDistance = n;
    int maxKString = 1 << k;

    // generate all k-strings
    for (int i = 1; i < maxKString; i++) {
        int distance = 0;

        // matrix multiplication
        for (int[] row : generator) {
            int p = 0;
            for (int z = 0; z < k; z++) {
                int a = row[k - z - 1];
                int b = (i >>> z) & 1;
                p = (p + a * b) & 1;
            }
            // distance as number if 1 bits
            distance += p;
        }

        minDistance = Math.min(minDistance, distance);
    }

    return minDistance;
}

Unit testing

I recommend adding unit tests to verify your solution, for example:

@Test
public void testSampleExercise1() {
    assertEquals(3, LinearCode.getMinimumDistance(new int[][]{
            {1, 0, 0, 0},
            {0, 1, 0, 0},
            {0, 0, 1, 0},
            {0, 0, 0, 1},
            {0, 1, 1, 1},
            {1, 0, 1, 1},
            {1, 1, 0, 1},
    }));
}

@Test
public void testSampleExercise2() {
    assertEquals(0, LinearCode.getMinimumDistance(new int[][]{
            {1, 1},
            {0, 0},
            {1, 1},
    }));
}


The use of regular expressions is an implementation detail.

I think you should continue to structure your API as functions that take a higher level input, such as a game text, and destructure it into the end product that your API's clients want.

That said, you can define a private helper that takes a regular expression and does the work of matching the text and extracting group 0.  Then each of your public functions would delegate most of the work to that just providing it with the appropriate regular expression and maybe the index of the group to return.

private function gameTextExtractor(gameText:String, regex:EReg):String
{
    regex.match(gameText);
    return regex.matched(0);
}


An alternative way could be using a map, the average lookup time should be O(1) so this algorithm would be O(n) on average.

public static boolean isBalanced(int[] a)
{
    Map<Integer, Integer> map = new HashMap<>();
    for(int number : a)
    {
        int key = Math.abs(number);
        Integer value = map.get(key);

        if(value == null)
            map.put(key, number);
        else if (value != number)
            map.put(key, 0);
    }
    for (int v : map.values())
    {
        if(v != 0) return false;
    }

    return true;
}

A more readable version in java 8 ( Autoboxing and unboxing may hurt the performance a lot thought.)

 public static boolean isBalanced(int[] a)
 {

       Map<Integer, Integer> map = Arrays.stream(a)
                                         .boxed()
                                         .collect(Collectors.toMap(
                                                 Math::abs,
                                                 Function.identity(),
                                                 (x, y) -> x.equals(y)? x: 0 )
                                         );
        return map.values()
                  .stream()
                  .allMatch(p -> p == 0);
 }


I would create two extension method , ToApplicationData and ToAddress and will define mapping over there. it will sorten you code and much better readablity. you can break this methods too if you want.

 public static class Mapping
    {
        public static Data[] ToApplicationData (this DecisionRequest request)
        {
            return new[]
            {
                new Data {category = "ID", attribute = "PubID", Value = request.PubID},
                new Data {category = "ID", attribute = "AID", Value = request.AID},
                new Data {category = "APP", attribute = "NID", Value = request.NID},
                new Data {category = "BOOK", attribute = "DupeApps90", Value = request.DupeApps90},
                new Data {category = "BOOK", attribute = "DupeApps30", Value = request.DupeApps30}
            };
        }

        public static Address[] ToAddresses(this DecisionRequest request)
        {
            return new Address[]
            {
                new Address
                {
                    City = request.City,
                    Country = request.Country,
                    HouseNumber = request.HouseNumber,
                    HouseNumberExtension = request.HouseNumberExtension,
                    Street = request.Street,
                    ZIP = request.ZIP,
                    kind = "MAIN"
                }
            };
        }
    }

This is how I will use this code

    public static class Mapper
    {
        public static Request FromDecisionRequestToZRequest(DecisionRequest request)
        {
            var applicationDetails = request.ToData();

            var address = request.ToAddresses();

            var mappedRequest = new Request
            {
                UserName = request.UserName,
                RequestDateTime = Convert.ToDateTime(request.RequestDateTime),
                CompanyRegistrationID = request.CompanyRegistrationID,
                Addresses = new Addresses {Address = address},
                Phone1 = new Phone1 {type = "1", Value = request.Phone1},
                Phone2 = new Phone2 {type = "2", Value = request.Phone2},
                Phone3 = new Phone3 {type = "3", Value = request.Phone3},
                Email = request.Email,
                Amount = new Amount {Value = request.Amount},
                ApplicationDetails = new ApplicationDetails {Data = applicationDetails}
            };

            return mappedRequest;
        }
    }


How effective is it to default to user input in C++ function parameters?

I haven't seen this pattern in use, and I wouldn't use it personally, for two reasons:

1) Unintuitive - Changing the function's behavior in this significant of a way is not something you'd expect from the default value of a parameter.

2) Mixing Concerns - It's almost always better to separate your I/O from your algorithm.

Considering that this is a compile-time difference anyway, it would be more clear to have another method to read input.  For example:

foo(1);
foo(foo_input());


Adding a replica of some DOM elements on click

First suggestion

One first refactoring idea is to create a function that creates an element, sets its attributes and adds it to a parent element.

function createElement(elementType, classes, parent) {
    var e = document.createElement(e);
    e.setAttribute('class', classes);
    parent.appendChild(e);
    return e;
}

You could also separate your structure from your DOM manipulation.
I suggest you create your structure as a JSON, and then create the nodes recursively.

For example, you could define your structure as:

{
  element: 'div',
  class: 'col-wd-12 col-md-6 col-sm-4',
  children: [{
    element: 'div',
    class: 'col loby',
    children: [
      //...
    ]
  },
  // ...
}

and then:

function createDOMElements(data) {
  // ...

  // create the element
  var elem = document.createElement(data.element);

  // add its attributes
  for (var attribute in data) {
    if (data.hasOwnProperty(attribute) && attribute !== 'element' && attribute !== 'children') {
      var attributeValue = data[attribute];
      elem.setAttribute(attribute, attributeValue);
    }
  }
  // ...

  // add the children (forEach is from ES5 on)
  data.children.forEach(function(child) {
    // recursive call to create child elements
    var childElement = createDOMElements(child);
    elem.appendChild(childElement);
  });

  return elem;
}

Here is a jsfiddle with the whole working example.

Second suggestion (less JS)

You could create a hidden HTML 'template' and display it when needed.

You can set:

visibility to hidden if you still want to affect the layout (take the space needed)
or display to none

in css (more info here) directly in the style attribute or through a class. On the click event you make this element visible.

The HTML:

  <div id="template" class="col-wd-12 col-md-6 col-sm-4" style="display:none;">
      <!-- all the children -->
  </div>

And then in the JS:

document.getElementById("addLobby").addEventListener("click", function(e) {
  document.getElementById("template").style.display = "initial";
});

Here is the second fiddle.

If you need to add the template multiple times, or if you have multiple templates on your page, you should define them at the end on the page.
When needed, clone the template and append it to the desired parent.

Conclusion

If you don't have a particular need to create the elements in the JS, I think the second solution it's more elegant (and less code ;) )


We don't normally do this in real (non-toy) programs:

stTree *node = malloc(sizeof *node);
if (!node) abort();

It's convenient and probably okay for a simple exercise like this, though.

Code of the form if (c) then return true; else return false; simplifies to just return c;, so we can reduce
    if (sum == root->value)
        return true;
    else
        return false;

to just
    return sum == root->value;

I think we can do better than that, if we're happy to consider an empty tree as having a sum of zero:
bool hasPathSum(const stTree *node, int sum)
{
    if (!node) {
        return sum == 0;
    }

    sum -= node->value;
    return hasPathSum(node->left, sum) || hasPathSum(node->right, sum);
}

Notice that I've used || rather than | in my version - there's no need to continue looking once we've found a successful result.  Also, because this function is recursive, the name root is misleading for the parameter - it could be any node, and it's only the root in the top-level call.

void freeTree(stTree *node)
{
    if (!node) return true;

That's a simple error - can't return a value from a function declared void(...).  That suggests you're not enabling enough compiler warnings.

Good to see unit tests.  I'm not sure I'd pile all the test data together like that - I'd probably create a small function for each test, so that each test is independent.  Consider using one of the available test frameworks to help relieve the repetitive work of testing.
When writing tests, it's a good idea to write the smallest test you can think of (e.g. the null tree) as the first test, and start with it failing (so you know the test works).  Then make it pass, and write the next simplest failing tests (tree with one node - make sure you test calls that should return different values).
Here's how I would begin (using Google Test, which is C++, but it's easy to test C code by using extern "C"):
#include <gtest/gtest.h>

extern "C" {
    struct stTree;

    stTree* createNode(int value, stTree *leftNode, stTree *rightNode);
    bool hasPathSum(const stTree *node, int sum);
    void freeTree(stTree *node);
}

TEST(PathSum, empty)
{
    stTree *tree = NULL;
    EXPECT_FALSE(hasPathSum(tree, 1));
    EXPECT_TRUE(hasPathSum(tree, 0));
}

TEST(PathSum, one_node)
{
    stTree *tree = createNode(2, NULL, NULL);
    EXPECT_FALSE(hasPathSum(tree, 0));
    EXPECT_TRUE(hasPathSum(tree, 2));
}

TEST(PathSum, two_nodes)
{
    stTree *tree = createNode(2, NULL,
                              createNode(3, NULL, NULL));
    EXPECT_FALSE(hasPathSum(tree, 0));
    EXPECT_TRUE(hasPathSum(tree, 2));
    EXPECT_TRUE(hasPathSum(tree, 5));
}


Database abstraction class in C# (version 2)

Some organization nitpicks first :

Your class file should start with the const
Even your private functions should be PascalCased meaning getAlreadyDownloadedCommand should be GetAlreadyDownloadedCommand
Your public methods should appear before your private methods IMO

You don't implement the Dispose pattern correctly. You should check this article but basically, to properly dispose your object, the pattern is the following :

public void Dispose()
{
    Dispose(true);
    GC.SuppressFinalize(this);
}

protected virtual void Dispose(bool disposing)
{
    if (disposing){
        if (connection != null) connection.Dispose();
        //etc.
    }
}

I must say I'm not 100% why we need to do it this way, but this is how it is supposed to be implemented according to Microsoft's documentation and developer tools like Resharper.

When you have a public method/constructor/etc, you should always check your input for null. Don't trust the user of your code, even if it's yourself! If you check your inputs for null, you'll know very fast if your input is null, instead of having a weird exception that tells you little to nothing.

public DatabaseAccess(string dbconnectionstring)
{
    if(String.IsNullOrWhitespace(dbconnectionstring) throw new ArgumentNullException(nameof(dbconnectionstring));

    connection = new SqlConnection(dbconnectionstring);

    connection.Open();
    alreadyDownloadedCommand = getAlreadyDownloadedCommand(connection);
    markedDownloadedCommand = getMarkedDownloadedCommand(connection);
}


The code can be made simpler by using some built-in Laravel validation rules:

Validation of MIME types and extension can already be done in Laravel.

With the rule image as you have for profile_picture, you're already validating if the uploaded file is of the mimetype jpeg, png, bmp, gif, or svg (docs here). If you'd like to specify the mimetype however, you can use the mimes rule.
To validate the max size of the image, you can use the max rule.
You can also validate the dimensions of the image if needed (docs here).

On the matter of security, I suppose you mean with the uploading of the images. The snippets of code you provided do seem enough (have not perused it), but I would advise using the built-in validation provided by Laravel to provide the best of convenience and security.


Pass Async function to new Promise constructor

There's simply no reason to wrap your existing promise with another promise and no reason to use await or async here either:

const getData = function(){
  return axios.get(some_data);
}

Wrapping an existing promise with a manually created promise is a promise anti-pattern and should not be done.  Instead, you should just return the promise you already have.

If you're going to use more than one async operation and thus find you want to use await rather than chain your promises with .then(), then you can do that like this:

const getData = async function(){
  let val = await axios.get(some_data);
  let otherVal = await someOtherFunction();
  return something;
}

Note also that if you want a rejected promise to just get returned as a rejection, you don't have to catch it locally in an async function.  If you one of the promises you are using await on rejects, that will just reject the promise that your async function returns with that error.

If you wanted to catch the error locally and do something about it, then you would use try/catch:

const getData = async function(){
  try {
    let val = await axios.get(some_data);
    let otherVal = await someOtherFunction();
    return something;
  } catch(e) {
    // some something with the error and decide what to return
  }
}


I'd make a few modest changes.

Don't use the helpers: use even? and count instead.
Reorder the arguments to insert-at to comply with those of assoc.
Make the even-ness condition in n-> an assertion: don't try to deal
with invalid cases.

Thus

(defn insert-at [xs n x]
  (let [[f-half s-half] (split-at n xs)]
    (concat f-half (cons x s-half))))

(Note the use of cons)

For example,

(insert-at [1 2 3 4 5] 3 :a)
;(1 2 3 :a 4 5)

And

(defmacro n-> [val & forms]
  (assert (even? (count forms)))
  (if (seq forms)
    (let [[n form & rest-forms] forms
          threaded  (insert-at form (inc n) val)]
      `(n-> ~threaded ~@rest-forms))
    val))

For example,

(macroexpand '(n-> 1, 1 (+ 2), 0 (+ 3)))
;(+ (+ 2 1) 3)

Note that the macro now works (as an identity) with one argument:

(macroexpand '(n-> (whatever you like)))
;(whatever you like)

Another approach is to do the work of the macro inside a function:

(defn n->fn [val & forms]
  (assert (even? (count forms)))
  (let [pairs (partition 2 forms)]
    (reduce
     (fn [acc [n form]]
       (insert-at form (inc n) acc))
     val
     pairs)))

For example,

(apply n->fn '(1, 1 (+ 2), 0 (+ 3)))
;(+ (+ 2 1) 3)

Once this function gets hold of the forms as data, it can do what the macro did without implicit recursion.

So we can define the macro in terms of the function:

(defmacro n-> [& stuff]
  (apply n->fn stuff))

... with identical results.


Finding terminal server user is connected on

A couple things here... I would use the Win32_LoggedOnUser class to query, that'll get you the user name and "domain" for anybody with a connection to the computer. I have quotes on domain, because it considers the computer itself to be the domain for any local accounts. You just have to do a little string parsing to get the info you want into a nice, readable format.

Get-WmiObject -Class Win32_LoggedOnUser -ComputerName 'Server01' |
    Select PSComputerName,@{l='Domain';e={$_.Antecedent -replace '^.+?Domain="(.+?)".*$','$1'}},@{l='Name';e={$_.Antecedent -replace '^.+?Name="(.+?)".*$','$1'}} -Unique

Then, you can pass multiple computer names to the Get-WmiObject cmdlet at once, and it'll get the info from all of them. Plus, if you run it as a background job you can set a throttle limit, and it will try to process as many connections as you set the limit to.

$servers = @("server01","server02","server03","server04","server05","server06","server07","server08")

$Job = Get-WmiObject Win32_LoggedOnUser -ComputerName $servers -AsJob -ThrottleLimit 10
Wait-Job $Job
$Results = Receive-Job $Job
$Results | Select PSComputerName,@{l='Domain';e={$_.Antecedent -replace '^.+?Domain="(.+?)".*$','$1'}},@{l='Name';e={$_.Antecedent -replace '^.+?Name="(.+?)".*$','$1'}} -Unique | Sort PSComputerName,Domain,Name


Start the Python interpreter with an import statement

Use an alias: alias 3=python3.
Use the PYTHONSTARTUP environment variable: import your most frequently used modules whether or not they'd actually get used in this session.
If you insist on doing it your way, I've made an "improvement" over your code:

function import
{
    python3 -ic "import $*"
}

function from
{
    args=$*
    python3 -ic "from ${args%import*} import ${args#*import}"
}

This doesn't guard against anything, nor should you need to.

Hardened (?) version (use at your own risk):

function import
{
    args=$(awk -v q="'" -v ORS=',' '{for(i=1; i<=NF; i++) print q $i q}' <<< "${@/,/ }")
    python3 -ic "for module in eval("\""[$args]"\""): exec(f'import {module}')"
}

Feel free to use any combination of the 3 methods.


Scraping and using JSON data from thousands of files in a directory

records seems to just count each enumeration of json_files. Just use len(json_files) instead as that returns the length of the json_files list, which is basically what records is.

While we're at it, index is never used, so why enumerate? Just use for js in files.

json_text is a misleading name, because that's actually a dictionary of json data! Name it data or json_data for clarity.

You call on multiple keys any one of which could raise a KeyError if the key doesn't exist or is spelled wrong. You might be very confident in the structure being consistent, but I think it's better to be safe, after all who knows what'd mess with your data or break the structure. There's two ways to ensure that you wont get errors, and I think you'd do well to use both of them.

The first way is to use try except KeyError. This is Python's error catching and it works so well that it's considered more Pythonic than testing if the key is there. However it would stop you from reading other keys if you caught that 'firstName' didn't exist, so I suggest just using this to access json_data['details']. This would also give you a shortcut, rather than needing to type that out fully for each key. Here's how it'd look:

    json_text = json.load(json_file)
    try:
        details = json_text['details']
    except KeyError:
        records -= 1
        print("Bad file {}".format(json_file))
        continue

If a KeyError arises, I'm doing a few things though what you want to do may vary. First, I subtracted one from the records count since we don't have a valid record. You might want to instead track an invalid records number, or not touch that number at all. I also print a message warning the user that one of the files had a problem in it, so they can inspect why it didn't work. You could adjust that message, adding detail about what key was missing would be good but I definitely recommend including the actual name so that they can find the specific file. Lastly I call continue, which will basically go onto the next iteration of your loop. Meaning it immediately skips to the next file, allowing your script to continue.

Now the second method you can use is called get. It's a dictionary method that allows you to specify a default value if the key doesn't exist. I think it would be better here because even if a key is missing you still want to be able to build your string at the end to print. ie. Even if 'surname' isn't included, you could just set it as a blank string and print the rest of the information. So here's how it'd look:

    first_name = details.get('firstName', '')
    last_name = details.get('surname', '')

The first parameter to get is the key name, and the second is your default value. Empty strings seem appropriate, unless you want to highlight that there's missing data by putting in values like "SURNAME" or "VOID". When you need to get a nested value like ['questions']['newsletterOptIn'] you could call get, pass an empty dictionary as default and then call get on that. Basically just chaining get calls will do what you need:

    newsletter_opt_in = details.get('questions', {}).get('newsletterOptIn', '')
    email = details.get('email', '')

Now, when I see your if command I realise that we might not even need most of these keys! So you should reorder the commands so that you just first get newsletter_opt_in and then test that to determine if you even need the other keys:

    newsletter_opt_in = details.get('questions', {}).get('newsletterOptIn', '')
    if(newsletter_opt_in == 'true'):
        first_name = details.get('firstName', '')
        last_name = details.get('surname', '')
        email = details.get('email', '')
        print('Name: {}\nE-mail: {}\n\n'.format(first_name + ' ' + last_name, email))
        opted_records += 1
    elif(newsletter_opt_in != 'false'):
        unopted_records += 1

Also you don't need brackets around the expressions for if statements. if newsletter_opt_in == 'true' is perfectly fine. Also it would be good to add a comment clarifying newsletter_opt_in != 'false'. It took me a minute to realise that it was !=, meaning that you're testing for any value that doesn't match the two boolean possibilities. It's a little confusing but a comment would clear it up easily. Though I recommend switching to actual boolean values in the data if possible. If your source can store newsletter_opt_in as a boolean then Python can read it as such.

Style notes

Instead of putting all your imports on one line you should space them out. Python likes to give each import it's own line and to group imports together. Also since you only need sys for version_info you could use from sys import version_info. That will only import the version_info tuple and makes it clearer why you need that import.

import os
import json

import pandas as pd

from sys import version_info

Using % to format strings is the old way, Python has a better way known as the str.format method. Since you're passing three values out of a tuple, you will need the unpacking operator to make it work. The unpacking operator basically takes values out of a collection (ie. a list or a tuple) and passes them as arguments. So instead of passing a tuple to format we can pass the string elements it contains. To use it, just add the * character before a collection variable.

print("Scraping E-mail Subscribers")
print("Using Python version {}.{}.{}".format(*version_info[:3])

You're using snake_case to name things, which is great! That's exactly what the Python style guide recommends. However since json_path seems to be a constant, you should use UPPER_SNAKE_CASE for it, ie. JSON_PATH.

Speaking of names, pos_json is not great. I assume it's meant to be 'possible json' but that doesn't read well. It's better to name it after what it is, a file. You can't use file as that will shadow a builtin function, but you could use f or filename. Your endswith('.json') makes it pretty clear that it's about detecting if it's a json file so you can use a more direct name that describes what these filenames actually are. Also in your loop, I don't like js for the same reason, and I'd likewise call that filename.

You can also assign multiple values at once in Python, so all your records can be set as 0 simultaneously.

records = opted_records = unopted_records = 0


I would improve the following things:

move the parsing argument part into a separate function (extract method)
define descriptions and helps in multiline string constants
use os.path.join() to join the path components - would also help to avoid worrying about a trailing slash
order imports alphabetically (PEP8 reference)

The improved code:

#!/usr/bin/python
import argparse
import getpass
import glob
import os
import random
import subprocess as sp

DESCRIPTION = "Open a random MP3 in the player of choice, or the default player"
DIRECTORY_ARGUMENT_HELP = """
The path to the directory where your music is stored.
If the path has spaces, wrap the entire path in single-quotes ('/home/ubuntu/My Music/' for example).
If not specified, the current user's Music directory in their /home/ folder is used.
"""
PLAYER_ARGUMENT_HELP = """
The executable name of the media player to open the MP3 with.
If none specified, uses the system default player.
"""

def parse_args():
    """Parses command-line arguments and returns argparse arguments."""
    user_home_directory = os.path.join('/home', getpass.getuser())
    default_music_directory = os.path.join(user_home_directory, 'Music')

    parser = argparse.ArgumentParser(description=DESCRIPTION, add_help=True)
    parser.add_argument('--dir', '--directory', '--music-dir',
                        dest='dirpath', type=str, default=default_music_directory, required=False,
                        help=DIRECTORY_ARGUMENT_HELP)
    parser.add_argument('player', type=str, help=PLAYER_ARGUMENT_HELP)
    return parser.parse_args()

if __name__ == "__main__":
    args = parse_args()

    # Go to the directory your music is in.
    os.chdir(args.dirpath)
    mp3s = glob.glob('*.mp3')

    # Actually open the MP3 file, and /dev/null to suppress output messages from the process.
    DEV_NULL = open(os.devnull, 'w')
    execpath = [args.player, os.path.join(args.dirpath, random.choice(mp3s))]
    sp.Popen(execpath, stdout=DEV_NULL, stderr=DEV_NULL)

Note that if you would have been on Python 3.x, the DEVNULL could have been imported from the subprocess module:

from subprocess import DEVNULL


Sorting a list in ascending/descending order with a button toggle

Instead of resorting the list in state just store the preferred order in state. Then in render simply sort or reverse the order as needed to present the data. Be sure to use keys that don't change when the order is changed and it'll be quite performant.


Is it too awful code?

Not too awful and not even bad, but the huge wall of code looks ugly.

There are a few minor issues like missing blank after "for" in

for(ProcessStep processStep : process.getSteps()){

and before "{" in

if (activity.getId().equals(vps.getActivityId())){

but that's something your IDE can do for you (Ctrl-Shift-F in Eclipse).

Otherwise, all you need is to extract a few methods like

private VisualProcessStep toVisualProcessStep(Process process, ProcessStep processStep) {
    VisualProcessStep result = new VisualProcessStep();
    result.setActivityId(processStep.getActivityId());
    result.setDefinition(processStep.getDefinition());
    result.setId(processStep.getId());
    result.setParameters(processStep.getParameters());
    result.setStatus(processStep.getStatus());
    for (Activity activity : process.getActivities()) {
        if (activity.getId().equals(result.getActivityId())) {
            result.setActivityName(activity.getName());
            result.setActivityRealization(activity.getRealization());
            break;
        }
    }
    return result;
}

This can be repeated many times and you surely shouldn't extract everything. When the methods don't get used elsewhere, this is very subjective as you're trading a too long method for too many methods.

The sweet spot are methods somewhere about 15 lines long. As all the methods do is just copying everything and there isn't much to think about, they may be a bit longer. I'd personally split it into probably 3 methods.

Obviously, there may be a smarter way avoiding the manual copying. It surely can be done via reflection, but reflection is verbose and slow and ugly itself, so I wouldn't consider it for this task.

Maybe you could create a constructor (or a factory method) for VisualProcessStep?


pattern for validating an object against some condition

You have two things to consider here:

the entity shouldn't know anything about any database or repository. I would suggest you rename your method to IsValid().
the entity will be manipulated or operated upon by other classes, i.e. view models or controllers, etc. It can be an anti-pattern (or just smelly) to include business rules in the entity when those business rules rightfully belong elsewhere

So, I would suggest that the validation within your object simply evaluates the property values in an immediate way, avoiding any business rules.


Calculate the remainder once. Right now you calculate it to the comparison, and again if you need to output it.
Use .zero? for readable, idiomatic code
Fix your linebreaks and indentation (don't put code on the same line as else
Consider what to do if the divisor is zero.
Optionally, extract the bit of logic that gets a number into a method

This is a (intentionally overwrought) example:

def get_int(message)
  puts message
  gets.chomp.to_i
end

numerator = get_int("Enter numerator")
divisor = get_int("Enter divisor")

if divisor.zero?
  puts "Can't divide by zero"
  exit
end

remainder = numerator % divisor

if remainder.zero?
  puts "#{numerator} is divisble by #{divisor}"
else
  puts "#{numerator} is not divisble by #{divisor}. Remainder is #{remainder}"
end


Select list items in ViewModel fetched using a Repository

Yes I think its bad practise because you have introduced both complexity and dependencies when it simply isn't necessary. Its going to make your unit testing more difficult than it needs to be and it isn't the intended purpose of the your view models.

Just create a private method in your controller that loads the data and populates the ListItems and call it where necessary. Agreed that it's not pretty but it will cause you less issues in the long run than what you are doing now.

public class AdminController : Controller
{
    ...ctor etc...

    public ActionResult New()
    {
        var vm = new NewUserViewModel();
        PopulateListItems(vm);
        return View(vm);
    }

    [HttpPost]
    public ActionResult Create(NewUserViewModel vm)
    {
        if (ModelState.IsValid)
        {
            userRepository.Add(vm.User);
            return RedirectToAction("Index");
        }
        else
        {
            PopulateListItems(vm);
            return View(vm);
        }
    }

    private static void PopulateListItems(NewUserViewModel vm)
    {
        vm.Cities = _addressRepository.GetAllCities().Select(c => new SelectListItem() { Text = c.Name, Value = c.Id });
    }
}