Utilizing idle socket server to do meaningful thing (timeout after sleep on epoll_wait) I write an app server that uses TCP socket on Linux. When there is no traffic (no data is sent by client, no client connect() or close()), the process sleeps on epoll_wait() while waiting for events hit the socket file descriptors.
What is a good thing to do while the process is sleeping?
So my initiative is to make the sleep time short and force the process to read from memory again and again until the events come.
Reason for doing this is that to keep critical (for performance) data hot in the cache.

Is that a worthwhile thing to do?
Or better I let the process sleeping until events come?

My understanding
While the process is sleeping on epoll_wait() for too long, the kernel will schedule to run other processes.
If my app is scheduled away for too long, then its data on memory will be evicted from the cache, as cache is shared across multiple processes, then other processes' data will take place in the cache.
epoll_wait documentation

https://man7.org/linux/man-pages/man2/epoll_wait.2.html
Can also be read from man 2 epoll_wait.

Relevant Part of Code (event_loop and exec_epoll_wait)
struct srv_tcp_state {
    int                 epoll_fd;
    int                 tcp_fd;
    int                 tun_fd;

    bool                stop;
    struct_pad(0, 3);

    struct cl_slot_stk  client_stack;
    struct srv_cfg      *cfg;
    struct client_slot  *clients;
    uint16_t            *epoll_map;

    /*
     * We only support maximum of CIDR /16 number of clients.
     * So this will be `uint16_t [256][256]`
     */
    uint16_t            (*ip_map)[256];

    /* Counters */
    uint32_t            read_tun_c;
    uint32_t            write_tun_c;

    struct bc_arr       bc_arr_ct;
    utsrv_pkt_t         send_buf;
    struct iface_cfg    siff;
    bool                need_iface_down;
    bool                aff_ok;
    struct_pad(1, 4);
    cpu_set_t           aff;
};

static int exec_epoll_wait(int epoll_fd, struct epoll_event *events,
                           int maxevents, struct srv_tcp_state *state)
{
    int err;
    int retval;
    int timeout = 50; /* in milliseconds */

    retval = epoll_wait(epoll_fd, events, maxevents, timeout);
    if (unlikely(retval == 0)) {
        /*
         * epoll_wait() reaches timeout
         *
         * TODO: Do something meaningful here.
         */

        /*
         * Force the process to read critical data so
         * it is always hot (at least in L2 or L3?)
         */
        memcmp_explicit(state, state, sizeof(*state));
        return 0;
    }

    if (unlikely(retval < 0)) {
        err = errno;
        if (err == EINTR) {
            retval = 0;
            prl_notice(0, "Interrupted!");
            return 0;
        }

        pr_err("epoll_wait(): " PRERF, PREAR(err));
        return -err;
    }

    return retval;
}

static int event_loop(struct srv_tcp_state *state)
{
    int retval = 0;
    int maxevents = 64;
    int epoll_fd = state->epoll_fd;
    struct epoll_event events[64];

    /* Shut the valgrind up! */
    memset(events, 0, sizeof(events));

    while (likely(!state->stop)) {
        retval = exec_epoll_wait(epoll_fd, events, maxevents, state);

        if (unlikely(retval == 0))
            continue;

        if (unlikely(retval < 0))
            goto out;

        retval = handle_events(state, events, retval);
        if (unlikely(retval < 0))
            goto out;
    }

out:
    return retval;
}

memcmp_explicit
This code is located in different file, this prevents compiler to inline or optimize or remove the memcmp call (which happens when epoll_wait reaches its timeout).
int memcmp_explicit(const void *s1, const void *s2, size_t n)
{
    return memcmp(s1, s2, n);
}

likely and unlikely macros
#define likely(EXPR)   __builtin_expect(!!(EXPR), 1)
#define unlikely(EXPR) __builtin_expect(!!(EXPR), 0)

About __builtin_expect

https://stackoverflow.com/questions/7346929/what-is-the-advantage-of-gccs-builtin-expect-in-if-else-statements

Keeping the cache hot

So my initiative is to make the sleep time short and force the process to read from memory again and again until the events come.
Reason for doing this is that to keep critical (for performance) data hot in the cache.

If nothing else running on the same CPU, there is no reason for the cache to go cold. Things in the cache don't time out, they just stay there until they are evicted when necessary.
If there are other processes running, then your strategy might help your socket server, but there are several issues with this:

It is selfish; you might prevent other processes from keeping their data in the cache, thus lowering their performance.
It might not work at all, since once another process gets a time slice, their memory access patterns may evict your data from the cache again.
By not staying idle, your CPU might not get a chance to go into a low power state, thus keeping your cache literally hot.

My recommendation is to just use an infinite timeout for epoll_wait().
About memcmp_explicit()
Be aware that compilers are getting better and better at optimizing things. Even if you put memcmp_explicit() in a different translation unit than where it is called, the compiler might optimize it out if link time optimization is enabled.


Simpletron in C without initial storing I'm studying C with a Deitel's book. The book asks me to write a program that simulates a virtual PC (with virtual memory in an array called memory[100]).
Then the book asks me to write a program that sums 10 numbers. I did it but I had to initialize some variables (memory[20], memory[21], memory[22]) before running the program.
There is any way to make this program without initially storing the variables
and using the same number of instructions?

These are valid instructions:

#define READ 10;
#define WRITE 11;
#define LOAD 20;
#define STORE 21;
#define ADD 30;
#define SUBTRACT 31;
#define DIVIDE 32;
#define MULTIPLY 33;
#define BRANCH 40;
#define BRANCHANG 41;
#define BRANCHZERO 42;
#define HALT 43;

And the code:

int main(){
int memory[100] = {2022,1022,3022,2122,2021,3120,2121,4209,4000,1122,4300}; // my set of instruction for sum 10 numbers
memory[99] = -99999;
memory[20] = 1; // the variables initializations that i want to remove
memory[21] = 10;
memory[22] = 0;
int accumulator = 0000;
int instCounter = 00;
int instRegistrer = 0000;
int operationCode = 00;
int operand = 00;

while(memory[instCounter] >= 0){
    printf("\n**%d**  ", instCounter);
    instRegistrer = memory[instCounter];
    operationCode = instRegistrer / 100;
    operand = instRegistrer % 100;
    switch(operationCode){
        case READ:
            printf("READ ");
            scanf("%d", &memory[operand]);
            instCounter++;
            break;
        case WRITE:
            printf("WRITE ");
            printf("%d", memory[operand]);
            instCounter++;
            break;
        case LOAD:
            printf("LOAD ");
            accumulator = memory[operand];
            instCounter++;
            break;
        case STORE:
            printf("STORE ");
            memory[operand] = accumulator;
            instCounter++;
            break;
        case ADD:
            printf("ADD ");
            accumulator += memory[operand];
            instCounter++;
            break;
        case SUBTRACT:
            printf("SUBTRACT ");
            accumulator -= memory[operand];
            instCounter++;
            break;
        case DIVIDE:
            printf("DIVIDE ");
            accumulator /= memory[operand];
            instCounter++;
            break;
        case MULTIPLY:
            printf("MULTIPLY ");
            accumulator *= memory[operand];
            instCounter++;
            break;
        case BRANCH:
            printf("BRANCH ");
            instCounter = operand;
            break;
        case BRANCHANG:
            printf("BRANCHANG ");
            if(accumulator < 0)instCounter = operand;
            else instCounter++;
            break;
        case BRANCHZERO:
            printf("BRANCHZERO ");
            if(accumulator == 0)instCounter = operand;
            else instCounter++;
            break;
        case HALT:
            printf("HALT ");
            instCounter = 99;
            break;
    }
}
return 0;
}

Your instruction list:

#define READ 10;
#define WRITE 11;
#define LOAD 20;
#define STORE 21;
#define ADD 30;
#define SUBTRACT 31;
#define DIVIDE 32;
#define MULTIPLY 33;
#define BRANCH 40;
#define BRANCHANG 41;
#define BRANCHZERO 42;
#define HALT 43;

has a few issues. First of all, you shouldn't add semicolons after those defines. Also, this would be better represented as an enum, since op codes are mutually exclusive.

Your memory initialization:

int memory[100] = {2022,1022,3022,2122,2021,3120,2121,4209,4000,1122,4300}; // my set of instruction for sum 10 numbers

also needs a few adjustments. Since none of these values will exceed 9999, you should be storing them as int16_t, not int. Furthermore, you shouldn't be writing raw machine code in that array. You should be constructing those values like

{
    100*LOAD + 22,
    100*READ + 22,
    // ...
}

Also, it seems like you store -99999 as a magic value to indicate program termination, when this is technically not necessary. I don't have the specifications for your VM, but it would make more sense to

have all of the memory contents be of type int16_t
on HALT, simply terminate the program

rather than

on HALT, jump to the end of memory
at the end of memory, have a magic negative value.

There's a typo in instRegistrer. It's "register", not "registrer".

I propose that, instead of having to manually write a different printf string for each instruction,

declare an array of 44 character pointers
define a macro that indexes into the array based on the opcode value, and initializes that element to a stringized (#) version of the opcode
call the macro for every opcode that you have

You could take this strategy further and store a struct at each element of the array, containing a string name and an operation function pointer. This would nicely generalize your code and make execution of long programs faster.

You're also keen on avoiding separate assignment of memory contents for use by variables. This can be done by shifting your "data" memory to the address right after your "program" memory, and then doing the initialization in your array literal.

The following incorporates all of the above. It seems you're still learning basic C, so some of these concepts may require explanation - please let me know in the comments.

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

typedef enum
{
    READ = 10,
    WRIT = 11,
    LOAD = 20,
    STOR = 21,
    ADD = 30,
    SUB = 31,
    DIV = 32,
    MUL = 33,
    BRA = 40,
    BRNG = 41,
    BRZR = 42,
    HALT = 43
} OpCode;

typedef struct
{
    int16_t mem[100], acc;
    uint8_t counter;
} VM;

typedef struct
{
    const char *name;
    void (*execute)(VM *vm, uint8_t addr);
} Operation;

static uint16_t makeOp(OpCode code, uint8_t addr)
{
    return 100*code + addr;
}

#define DEFOP(op) ops[op] = (Operation){.name=#op, .execute=exc_##op}

static void exc_READ(VM *vm, uint8_t addr) {
    int32_t x;
    if (scanf("%d", &x) != 1)
    {
        perror("Failed to read integer");
        exit(1);
    }
    if (x >= 10000 || x <= -10000)
    {
        fprintf(stderr, "Invalid value %d", x);
        exit(1);
    }
    vm->mem[addr] = x;
    vm->counter++;
}
static void exc_WRIT(VM *vm, uint8_t addr) {
    printf("%d\n", vm->mem[addr]);
    vm->counter++;
}
static void exc_LOAD(VM *vm, uint8_t addr) {
    vm->acc = vm->mem[addr];
    vm->counter++;
}
static void exc_STOR(VM *vm, uint8_t addr) {
    vm->mem[addr] = vm->acc;
    vm->counter++;
}
static void exc_ADD(VM *vm, uint8_t addr) {
    vm->acc += vm->mem[addr];
    vm->counter++;
}
static void exc_SUB(VM *vm, uint8_t addr) {
    vm->acc -= vm->mem[addr];
    vm->counter++;
}
static void exc_DIV(VM *vm, uint8_t addr) {
    vm->acc /= vm->mem[addr];
    vm->counter++;
}
static void exc_MUL(VM *vm, uint8_t addr) {
    vm->acc *= vm->mem[addr];
    vm->counter++;
}
static void exc_BRA(VM *vm, uint8_t addr) {
    vm->counter = addr;
}
static void exc_BRNG(VM *vm, uint8_t addr) {
    if (vm->acc < 0)
        vm->counter = addr;
    else
        vm->counter++;
}
static void exc_BRZR(VM *vm, uint8_t addr) {
    if (!vm->acc)
        vm->counter = addr;
    else
        vm->counter++;
}
static void exc_HALT(VM *vm, uint8_t addr) {
    exit(0);
}

int main()
{
    const uint8_t X = 13, I = 12, DELTA = 11;

    Operation ops[HALT+1];
    DEFOP(READ);
    DEFOP(WRIT);
    DEFOP(LOAD);
    DEFOP(STOR);
    DEFOP(ADD);
    DEFOP(SUB);
    DEFOP(DIV);
    DEFOP(MUL);
    DEFOP(BRA);
    DEFOP(BRNG);
    DEFOP(BRZR);
    DEFOP(HALT);

    VM vm = {.counter = 0,
             .acc = 0,
             .mem = {
                makeOp(LOAD, X), // 0: beginning
                makeOp(READ, X),
                makeOp( ADD, X),
                makeOp(STOR, X),
                makeOp(LOAD, I),
                makeOp( SUB, DELTA),
                makeOp(STOR, I),
                makeOp(BRZR, 9),
                makeOp( BRA, 0),
                makeOp(WRIT, X), // 9: exit loop
                makeOp(HALT, 0),
                1,  // 11: DELTA
                10, // 12: I
                0   // 13: X
             }};

    for (;;) {
        int16_t current = vm.mem[vm.counter];
        uint8_t code = current/100,
                addr = current%100;
        const Operation *op = ops + code;
        printf("%02u: %4s %02u\n", vm.counter, op->name, addr);
        op->execute(&vm, addr);
    }
}


Python 3 simple external sort with heapq I've implemented a minimal external sort of text file using heapq python module.

On the few tests I did it seems to works well, but I would like to have some advice to have a cleaner and faster code. I do not know much of good practices and I want to learn (May wish to go from academics to industry one day). All remarks, advice and suggestions are warmly welcome.

There are 3 functions: one that splits the big file in smaller files,
one that does the merge, and one main function.

import os
import tempfile
import heapq
import sys
import shutil

# Algorithm based on
# https://github.com/melvilgit/external-Merge-Sort

def split_large_file(starting_file, my_temp_dir, max_line=1000000):
    """

    :param starting_file: input file to be splitted
    :param my_temp_dir: temporary directory
    :param max_line: number of line to put in each smaller file (ram usage)
    :return: a list with all TemporaryFile
    """
    liste_file = []
    line_holder = []
    cpt = 0
    with open(starting_file, 'rb') as f_in:
        for line in f_in:
            line_holder.append(line)
            cpt += 1

            if cpt % max_line == 0:
                cpt = 0
                line_holder.sort(key=lambda x: x.split(b"\t")[0])
                temp_file = tempfile.NamedTemporaryFile(dir=my_temp_dir, delete=False)
                temp_file.writelines(line_holder)
                temp_file.seek(0)
                line_holder = []
                liste_file.append(temp_file)

        if line_holder:

            line_holder.sort(key=lambda x: x.split(b"\t")[0])
            temp_file = tempfile.NamedTemporaryFile(dir=my_temp_dir, delete=False)
            temp_file.writelines(line_holder)
            temp_file.seek(0)
            liste_file.append(temp_file)

    return liste_file

def merged(liste_file, out_file, col):
    """

    :param liste_file: a list with all temporary file opened
    :param out_file: the output file
    :param col: the column where to perform the sort, being minimal the script will fail
     if one column is shorter than this value
    :return: path to output file
    """
    my_heap = []
    for elem in liste_file:
        line = elem.readline()
        spt = line.split(b"\t")
        heapq.heappush(my_heap, [int.from_bytes(spt[col], "big"), line, elem])

    with open(out_file, "wb") as out:
        while True:
            minimal = my_heap[0]
            if minimal[0] == sys.maxsize:
                break

            out.write(minimal[1])
            file_temp = minimal[2]
            line = file_temp.readline()
            if not line:
                my_heap[0] = [sys.maxsize, None, None]
                os.remove(file_temp.name)
            else:
                spt = line.split(b"\t")
                my_heap[0] = [int.from_bytes(spt[col], "big"), line, file_temp]

            heapq.heapify(my_heap)
    return out_file

def main(big_file, outfile, tmp_dir=None, max_line=1000000, column=0):

    if not tmp_dir:
        tmp_dir = os.getcwd()

    with tempfile.TemporaryDirectory(dir=tmp_dir) as my_temp_dir:
        temp_dir_file_list = split_large_file(big_file, my_temp_dir, max_line)
        print("splitted")
        merged(liste_file=temp_dir_file_list, out_file=outfile, col=column)
        print("file merged, sorting done")

Nit: running isort would organize your imports,
to reduce git merge conflicts.
The split_large_file() signature might default my_temp_dir.
Consider renaming it to simply temp_dir.
For compatibility with /usr/bin/sort, os.environ['TMPDIR'] would be a fair default.
Consider renaming starting_file to in_file (or input_file).
Docstrings start with an English sentence, e.g. "Splits a file into pieces."
If the intent is "...into sorted pieces," then say so,
and make the function name match.
Splitting a file is by itself a valuable service, even without sorting.
Please say "a list of" rather than "a list with all".
It appears liste_file has a typo, plus it should simply be files.
Simplify line_holder to lines.
algorithm

line budget vs RAM budget
chunking
loop style

You impose a million-line max on each piece.
But the true goal was to cap RAM consumption,
which is indirectly related through the average chars-per-line
for a given input file.
Consider adding up line lengths
rather than incrementing by one for each line.
Related to this, the line-by-line iteration is on the slow side,
especially for input files comprised of short lines.
Consider doing one or more binary .read()'s,
and then reading until newline so only complete lines go to the outfile.
(Or scan backwards through the chunk to find a newline,
then seek() to that position before issuing the next read.)
The if line_holder: line is a code smell,
it suggests that you didn't quite find a form of while loop
that matched your application.
Perhaps a boolean done flag would help to
remove the copy-n-pasted 2nd chunk of code.
If for some reason you feel it still needs to remain,
then definitely extract both copies into a small helper method.
DRY: https://en.wikipedia.org/wiki/Don%27t_repeat_yourself
This is correct, but weird:
            if cpt % max_line == 0:
                cpt = 0

If line number cpt (don't know what that stands for) hits limit,
we reset the line number and emit a chunk.
Computing modulo % is more expensive than a comparison >.
Usually modulo would be used if we're not going to reset the line number.
Three comments about the lambda:

This is the first we've heard about the input file being a .tsv. Improve the docstring.
The lambda is fine, but consider breaking it out as a named function.
The .split() copies out the entire line, but it only needs to copy as far as first TAB.

We evaluate merged() for side effects,
so we're looking for a verb name rather than an adjective.
Just call it merge(), or perhaps external_merge().
Consider defaulting col=0 in the signature.
Each docstring should start with an English sentence.
The comment "being minimal the script will fail if one column is shorter than this value" is accurate.
But, better to rewrite as a pre-condition on valid inputs:
"must exist in each input row."
    my_heap = []

Yes, it is your heap.
But that's not what is interesting about it.
Better to just call it heap.
    for elem in liste_file:

This would more naturally be: for file in files:
You used a hardcoded column zero in split_large_file(),
but now you are apparently processing unsorted input
since col might be positive.
Please assign sentinel = sys.maxsize,
if that is how you're trying to use it.
Or at least add a comment.
(It is 0x7FFFFFFFFFFFFFFF, is that what you want?)
The break apparently should be raising a fatal exception instead,
since the input file apparently is not allowed to contain that sentinel
(not allowed to contain arbitrary contents).
The references to minimal[0], [1], and [2] are obscure.
Please use tuple unpack to put them in val, line, file_temp
or similarly informative variables.
The early explicit .remove() is fine.
If you had specified delete=True (or let it default)
the temp files would be deleted when the interpreter does a normal exit.
Depending on platform, defaulting tmp_dir to something like /tmp
might be more appropriate than to CWD (which might be unwritable).
Overall, looks pretty good, pretty readable.
One can always improve.


Possible SQL injection vulnerability searching for a product by ID? I want to be sure that this isn't vulnerable to SQL injection. If yes, then how can it be improved?

$pid = $_REQUEST['product_id'];
$query1 = "SELECT * FROM products WHERE id = '"
  . $pid . "'";

$result = mysql_db_query($dbname, $query1) or die("Failed Query of " . $query1);
$thisrow=mysql_fetch_row($result);
if ($thisrow) {
  echo "The Product was found.";
}
else
{
  echo "The Product was not found.";
}

On demand -> issues mentioned in comments, reviewed & posted as an answer:

The answer to your question has already been given: Yes, you have a serious vulnerability, and the first step to solving this is using the extensions that aren't deprecated, like PDO  and mysqli.
Both of these extensions support the easiest, and pretty solid  (not perfect, but solid) prodection from injection attacks: prepared statements. Learn to use them, learn to love them.

The rest of your code is, I'm afraid to say pretty bad. A lot of the issues are the result of your using the mysql_*  extension, but there are other issues, too.
A line-by-line review pointing out issues as we go:

$pid = $_REQUEST['product_id'];

This statement can, and probably will at some point, produce notices. If the product_id parameter isn't set, you'll get an "undefined index" notice. When writing, and testing, new code, always set your error_reporting to E_STRICT | E_ALL (which, if you're running php 5.5+ will show E_DEPRECATED notices for your using the mysql_* extension). And set display_errors  to true, so you can See the problems. Anyway, always check if an index exists, before using it:

$pid = isset($_REQUEST['product_id']) ? $_REQUEST['product_id'] : null;

Next - Concatenating raw values into a query is never a good idea. Your query appears to have id = '<someValue>' as a WHERE clause, which suggests that id  is a string, but that wouldn't make sense. You're also assuming that the client will behave nicely, and not request a product_id  value that contains single quotes (which would mess up your query!). Nor do you seem to mind negative values, or any other non-valid input for that matter. Worst of all, its laughably easy for me to send a request that'll result in a valid query that performs a full table scan (a products table can be quite large, so this would be something that can be exploited in a DOS attack):

//the product_id value:
$pid = "1' OR '1";
//the resulting query would be
$query1 = "SELECT * FROM products WHERE id = '1' OR '1'";
//which, basically amounts to:
$query1 = 'SELECT * FROM products';

It's quite obvious that this is not what you want, you don't want your code to echo The product was found when you're actually querying to see if any product exists. Especially if that query basically selects all records in your table.
So this statement needs to be reworked:

$query1 = "SELECT * FROM products WHERE id = '"
. $pid . "'";

Given that you're using an extension that doesn't support prepared statements, the best thing you can do is actually casting the values, and using printf to insert the value of $pid in your query string.
Also: avoid SELECT * whenever possible. Select the fields you need, and name them. Be explicit.

I'd also add some basic validation, though like is_numeric, to make sure the request parameter is a valid id  value. If not, the client might attempting an injection attack. Anyway, the simple fix here would be:

$queryString = sprintf(
    'SELECT * FROM products WHERE id = %d',
    (int) $pid
);

I left out the validation, that's code you'll have to write yourself first...

Onwards - Making the resource parameter optional in mysql was, IMHO, a sure fire way to breed bad practices, and error-prone code. You've fallen into this trap... don't worry, we all have at some point, we've all written the infamous or die, and used mysql_query assuming that the last active connection would be the connection we needed. But most of us have found out the hard way that this isn't always the case... So please, fix this bit, too:

$result = mysql_db_query($dbname, $query1)
    or die("Failed Query of " . $query1);

The or die is just smelly, IMHO. I don't like code that just gives up like that, wrap this snippet in a function, and replace the or die  with throwing an exception, so that you can handle the error a bit cleaner than just dumping a query onto the screen (which is showing that the product_id parameter is used in this query, so it's basically sending an injection-attack-invitation).

You are also not passing a connection resource to mysql_db_query. I know it's an optional parameter, but you'd think it'd be nice to know what connection is actually being used to query this product stuff, if only to show that you actually know what DB this table is on, and to prevent bugs popping up, when you start adding multiple connections to the project.

$thisrow=mysql_fetch_row($result);

Again, in light of my aversion for or die constructs, I'd much rather see code actually bothering to check the return value of functions, like here: checking if $result is a resource, or if it's false or even true. If $result is true, your or die  statement won't stop the code from executing, BTW. I know it's not very likely to happen in this case, but you never know, if, down the road, the SELECT gets replaced with an INSERT or UPDATE, the or die is going to cause you grief.

$thisrow=mysql_fetch_row($result);
if ($thisrow)

Why are you fetching a row, if you're only checking to make sure the id is found in the database. Why not use mysql_num_rows, apart from the mysql extension being deprecated, of course.

TL;TR

The best thing for you to do now is to rewrite the code entirely. Preferably using PDO or mysqli  and prepared statements. It should look something along the lines of:

if (isset($_REQUEST['product_id']))
{
    $db = new PDO($dsn, $user, $pass, [ PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION]);
    $stmt = $db->prepare('SELECT id FROM products WHERE id = :id');
    $stmt->execute([':id' => $_REQUEST['product_id']]);
    if ($row = $stmt->fetch(PDO::FETCH_OBJ))
        echo 'Found product with id: ', $row->id;
    else
        echo 'Product not found';
}
else
{
    echo 'Please select a product or something...';
}


Move array element to the first position I wrote an update() function to make an entry the first element in an array.

var arr = [
    {v: 1},
    {v: 2},
    {v: 3},
    {v: 4},
];

function update(e) {
    var i = arr.findIndex(o => o.v === e.v);
    if (i === 0) return;
    if (i > 0) arr = [e].concat(arr.slice(0, i), arr.slice(i+1));
    if (i < 0) arr = [e].concat(arr.slice(0, arr.length - 1)); // keep the array has 4 elements
}

update({v: 1});
update({v: 2});
update({v: 6});

Is there any better way to do this? I think slice and concat will make the performance not good.

function update(e) {

    // find index
    var i = arr.findIndex(o => o.v === e.v);

    // if already at start, nothing to do
    if (i === 0) return;

    // remove old occurrency, if existing
    if (i > 0) {
        arr.splice( i, 1 );
    }

    // add e to the start
    arr.unshift( e );

    // keep array at the correct size
    arr.length = Math.min( arr.length, 4 );
}

Somewhat less code. I don't know about the performance.


Inti Sets: Hackerank Here is the original question.

  In order to motivate his Peruvian students, a teacher includes words
  in the Quechua language in his math class.

  Today, he defined a curious set for a given positive integer N. He
  called this set, an Inti set, and defined it as the set of all
  positive integer numbers that have the number 1 as their single common
  positive divisor with number N.

  The math class about Inti sets was amazing. After class, the students
  try to challenge to teacher. They each ask questions like this: "Could
  you tell me the sum of all numbers, between A and B (inclusive), that
  are in the Inti set of N?"

  Since the teacher is tired and he's sure that you are the best in
  class, he wants to know if you can help him.

  Input Format

  The first line of input contains an integer Q, 1 ≤ Q ≤ 20,
  representing the number of students. Each of the next Q lines contain
  three space-separated integers N, A and B, which represent a query.

  Constraints

  1 ≤ A ≤ B ≤ N ≤ 10^12

  Output Format

  The output is exactly Q lines, one per student query. For each query
  you need to find the sum of all numbers between A and B, that are in
  the Inti set of N, and print the sum modulo 1000000007.

  Sample Input

2
12 5 10
5 1 4

  Sample Output

12 10

  Explanation

  In the sample input, Q = 2, so you have to answer two questions:

  In the first question N = 12, A = 5 and B = 10. So you have to find
  the sum of all numbers between 5 and 10, that are in the Inti set of
  12.

  Inti set ( 12 ) = { 1, 5, 7, 11, 13, ... }

  2 and 4 are not in the Inti set (12) because 12 and these numbers are
  also divisible by 2.

  3 and 9 are not in the Inti set (12) because 12 and these numbers are
  also divisible by 3.

  The numbers in the Inti set, which are in the query's range, are 5 and
  7, so answer is ( 5 + 7 ) MOD 1000000007 = 12

  In the second question, the numbers in the Inti set of 5 between 1 and
  4 are: 1, 2, 3, 4; so the answer is ( 1 + 2 + 3 + 4 ) MOD 1000000007 =
  10

My code keep getting timeout. Is there any approach to tackle this question?

Here the A, B, N values can be very large. It can be upto 10^12

import java.io.*;
import java.util.*;

public class Solution {

    public static void main(String[] args) {
        /* Enter your code here. Read input from STDIN. Print output to STDOUT. Your class should be named Solution. */

        Scanner console = new Scanner(System.in);

        int queries = Integer.parseInt(console.nextLine());
        int[][] qArr = new int[queries][];

        for (int i=0; i<queries; i++){
            int N = console.nextInt();
            int A = console.nextInt();
            int B = console.nextInt();
            int[] singleLine = new int[3];
            singleLine[0] = N;
            singleLine[1] = A;
            singleLine[2] = B;
            qArr[i] = singleLine;
            console.nextLine();

        }

        for(int[] line : qArr){

            int N = line[0];
            int A = line[1];
            int B = line[2];

            //calculate sum
            long result = sumBetweenRange(A, B, N);
            System.out.println(result);

        }
    }

    private static long sumBetweenRange(int a, int b, int n){
        long sum = 0l;

        for(int i=a; i<=b; i++){

            if(!isCommonFactorAvailable(i, n)){
                sum += i;
            }
        }
        return sum;
    }

    private static boolean isCommonFactorAvailable(int no1, int no2){

        if(no1 != 1 && no2%no1 == 0){
            return true;
        }

        for(int i=2; i<no1/2; i++){

            if(no1%i == 0 && no2%i == 0){
                return true;
            }
        }
        return false;
    }

}

The input numbers \$ A, B, N \$ can be as large as \$ 10^{12} \$, which
means that a Java int (which is a 32-bit signed integer) is not large
enough to hold all possible values, you should use long instead.

You also do not reduce the sum modulo 1000000007, as required.

The main function can be simplified by calculating the output for each
input line as it is read, this makes the queries array obsolete.

The function sumBetweenRange does not describe what the function actually does.
Something like sumOfIntiNumbersInRange or – using the mathematical term –
sumOfCoprimeNumbersInRange might be a better choice.

The main function would then look like this:

public static void main(String[] args) {
    /* Enter your code here. Read input from STDIN. Print output to STDOUT. Your class should be named Solution. */

    Scanner console = new Scanner(System.in);

    int queries = Integer.parseInt(console.nextLine());

    for (int i = 0; i < queries; i++) {
        long N = console.nextLong();
        long A = console.nextLong();
        long B = console.nextLong();

        long result = sumOfCoprimeNumbersInRange(A, B, N);
        System.out.println(result);
    }
}

Your method to determine if two numbers have a common factor is
very slow, it does trial division for all numbers between 2 and
no1/2. That can be improved slightly by using sqrt(no1) as the upper bound,
but one can do much better.

An efficient method is the
Euclidean algorithm
which determines the
greatest common divisor
of two integers. Two integers \$ a, b \$ have no common divisor exactly if \$ \gcd(a, b) = 1 \$.

There are many known Java functions for calculation the
greatest common divisor, see for example https://rosettacode.org/wiki/Greatest_common_divisor#Java.
Here is a simple (tail-recursive) implementation:

public static long gcd(long a, long b) {
    if (b == 0) {
        return a;
    } else {
        return gcd(b, a % b);
    }
}

Then sumOfCoprimeNumbersInRange() can be implemented as

private static final int MOD = 1000000007;

// Compute the sum (modulo MOD) of all numbers in the range a...b which are coprime to N.
private static long sumOfCoprimeNumbersInRange(long a, long b, long n) {
    long sum = 0;

    for(long i = a; i <= b; i++){
        if (gcd(i, n) == 1) {
            sum = (sum + i) % MOD;
        }
    }
    return sum;
}

This is already much faster than your original implementation:
For \$ N=100000, A=10000, B=90000 \$ I measured on a
1.2 GHz Intel Core m5 MacBook:

Your code: 3 seconds.
GCD-based solution: 0.165 seconds.

But it may be not fast enough for the full range of numbers up to
\$ 10^{12} \$. A very elegant and efficient approach using the
Inclusion-Exclusion Principle is explained in

How to count co-prime to n in a range [1,x] where x can be different from n?

on Stack Overflow.
Let us consider \$ N = 300 = 2^2  \cdot 3 \cdot 5^2 \$ as an example. The distinct prime
factors are \$ 2, 3, 5 \$, and to compute the sum of all numbers in some range which
are coprime to \$ N \$ one proceeds as follows:

Compute the sum of all numbers in the range.
Subtract the sum of all multiples of \$ 2, 3 \$, and \$ 5 \$ in the range.
Add the sum of all multiples of \$ 2 \cdot 3, 2 \cdot 5 \$, and \$ 3 \cdot 5 \$ in the range.
Subtract the sum of all multiples of \$ 2 \cdot 3 \cdot 5  \$ in the range.

Note that apart from the factorization, no trial divisions are required anymore, only
multiplications.

First we need a function to get the distinct prime factors of a given number,
e.g. the function in Finding out the prime factors of a number,
modified slightly to return only distinct prime factors:

// List of all distinct prime factors of n.
static List<Long> primeFactors(long n) {
    List<Long> factors = new ArrayList<Long>();
    long md = 2;
    if (n % md == 0) {
        factors.add(md);
        do {
            n /= md;
        } while (n % md == 0);
    }
    md = 3;
    while (md <= java.lang.Math.sqrt(n) + 1) {
        if (n % md == 0) {
            factors.add(md);
            do {
                n /= md;
            } while (n % md == 0);
        }
        md += 2;
    }
    if (n > 1) {
        factors.add(n);
    }
    return factors;
}

Now the "challenge" is to translate the Inclusion-Exclusion approach to a
recursive function, and that could look as follows:

// Sum of all numbers 1...k, computed modulo MOD.
private static long sumUpTo(long k) {
    k = k % MOD; // Otherwise the following multiplication can overflow.
    let sum = k * (k+1) / 2;
    return sum % MOD;
}

// Sum of all numbers (modulo MOD) in a...b which are divisible by k, but not
// by any multiple of k with factors in the primes list:
private static long inclExcl(long a, long b, long k, List<Long> primes) {

    // Sum of all numbers in 1...b which are divisible by k:
    long s1 = sumUpTo(b / k) * k;
    // Sum of all numbers in 1...(a-1) which are divisible by k:
    long s2 = sumUpTo((a-1) / k) * k;

    long sum = (s1 - s2) % MOD;

    for (int idx = 0; idx < primes.size(); idx++) {
        // The magic recursion!
        sum = (sum - inclExcl(a, b, k * primes.get(idx), primes.subList(idx + 1, primes.size()))) % MOD;
    }

    return sum;
}

private static long sumOfCoprimeNumbersInRange(long a, long b, long n) {

    List<Long> primes = primeFactors(n);

    long sum = inclExcl(a, b, 1, primes) % MOD;
    if (sum < 0) {
        sum += MOD;
    }
    return sum;
}

For a (hopefully) better understanding of the implementation, here is how the
inclExcl function is called recursively if \$ N = 300 = 2^2  \cdot 3 \cdot 5^2 \$
as above (showing only the relevant arguments):

k=1, primes=[2, 3, 5]
    k=2, primes=[3, 5]
        k=6, primes=[5]
            k=30, primes=[]
        k=10, primes=[]
    k=3, primes=[5]
        k=15, primes=[]
    k=5, primes=[]

This is again much faster, because we have to iterate and recurse only over the distinct prime factors of \$ N \$
(and there can be no more than \$ 11 \$ prime factors if \$ N \le 10^{12} \$).
There is no iteration over the range  \$ A \ldots B \$ anymore.

Benchmark: For \$ N=96996900, A=100, B=96996000 \$:

GCD-based solution: 26 seconds.
Inclusion/exclusion-based solution: 0.181 seconds.


Webscraping Bing wallpapers I wanted to scrape all the wallpapers from the Bing wallpaper gallery.  This was for personal use and to learn about webscraping.  The gallery progressively gets images using javascript as the user scrolls down the page.  Then you can download the high-resolution image with a few clicks.  I actually succeeded in scraping all the wallpapers, but I sort of "cheated" and wanted to know if there's a better way.

Here's what I did:

First, I went to the site in the browser, started capturing network traffic with developer tools, and scrolled all the way down the page as far as possible.  Then I exported the log to a .csv file.  This produced a bunch of URLs like:

  http://az619519.vo.msecnd.net/files/PolandFarm_EN-US8019728095_320x180.jpg

Luckily, the full resolution version (for almost every image) is located in the same place, except with "1920x1200" substituted at the end:

  http://az619519.vo.msecnd.net/files/PolandFarm_EN-US8019728095_1920x1200.jpg

I ran the following script to retrieve the full resolution images:

import csv
import os
import re
import urllib.request

target_dir = 'C:/Users/[Username]/Desktop/Bing'

def main():
    os.chdir(target_dir)
    with open('NetworkData.csv', 'r', encoding='utf-8') as csvfile:
        for line in csv.reader(csvfile):
            url_request = repr(line[0])
            if r'vo.msecnd.net/files' in url_request:
                pic_url = re.sub('_\d+x\d+\.jpg', '_1920x1200.jpg', url_request)
                pic_url = pic_url.strip('\'"')
                try:
                    urllib.request.urlretrieve(pic_url, pic_url.split('/')[-1])
                except:
                    print("Image not found.")

if __name__ == '__main__':
    main()

I thought this solution was actually okay, but I was wondering if there was a way to automate or avoid the manual step of capturing the network traffic.  I was hoping to avoid selenium if possible, because it looks kind of slow and cumbersome.  Also, if you have any advice on improving the script, that would be good.  I had some issues with string encoding, and I feel like in the end the script isn't very readable.

Since you only use one function from both re and os, you could just import the specific function from each rather than the whole module.

from os import chdir
from re import sub

You shouldn't use a bare try except. You might be ignoring all sorts of errors without noticing. Instead, use the exception that you're expecting is likely to happen. According to this Stack Overflow answer urlretrieve will likely raise a URLError (which you also need to get with import URLError from urllib2). This way if you're having other problems you'll actually know about it.

try:
    urllib.request.urlretrieve(pic_url, pic_url.split('/')[-1])
except URLError:
    print("Image not found.")

Also it seems silly to have that message print out without indicating which images weren't found. You may be incorrectly parsing a lot of images without realising it. Just adding (pic_url.split('/')[-1] + " Image not found") would make it a much more useful error message. But it would be even more helpful if you logged the full path (pic_url).


Comparing version numbers with JavaScript I have written a function that compares two version number strings (e.g. 1.2.0 and 1.2.2) and return 1 if the first string is greater, -1 if the second string is greater and 0 if the strings are equal for a code challenge that I'm attempting.
Also, it's guaranteed that both strings contain an equal number of numeric fields and all decimals are non-negative.
So,

1.2.0 and 1.2.2 should return -1.
1.05.4 and 1.5.3 should return 1.
1.2.6.76 and 1.2.06.0076 should return 0.

My logic for the function was to simply split the string at each occurrence of the period ., compare each number and then add a character (e for equal, m for more and l for less) to a temporary variable z depending on the comparison result.
I then simply use some basic regex to return 1, -1 or 0 based on the content of the z variable as can be seen in the following Code Snippet:

function checkVersion(a,b) {
    let x=a.split('.').map(e=> parseInt(e));
    let y=b.split('.').map(e=> parseInt(e));
    let z = "";

    for(i=0;i<x.length;i++) {
        if(x[i] === y[i]) {
            z+="e";
        } else
        if(x[i] > y[i]) {
            z+="m";
        } else {
            z+="l";
        }
    }
    if (!z.match(/[l|m]/g)) {
      return 0;
    } else if (!z.match(/[l]/g)) {
      return 1;
    } else {
      return -1;
    }
}

console.log(checkVersion("1.2.2","1.2.0")); // returns 1 as expected
console.log(checkVersion("1.0.5","1.1.0")); // returns -1 as expected
console.log(checkVersion("1.0.5","1.00.05")); // returns 0 as expected
console.log(checkVersion("0.9.9.9.9.9.9","1.0.0.0.0.0.0")) // returns -1 as expected;

The above function seems to be working fine with any random two version numbers that I've tried so far but when I try to submit the above function for the challenge, there is always one hidden test with two unknown version numbers that keeps failing. What logic am I missing in the above code?

EDIT:
Thanks to @RomanPerekhrest's comment below, I have found out that my regex is the problem. Instead of using the 2nd regex, I just remove any occurence of e from the z variable using the split() method and then just check if the first character is m or l and now the function is working correctly as seen in the following Code Snippet:

function checkVersion(a,b) {
    let x=a.split('.').map(e=> parseInt(e));
    let y=b.split('.').map(e=> parseInt(e));
    let z = "";

    for(i=0;i<x.length;i++) {
        if(x[i] === y[i]) {
            z+="e";
        } else
        if(x[i] > y[i]) {
            z+="m";
        } else {
            z+="l";
        }
    }
    if (!z.match(/[l|m]/g)) {
      return 0;
    } else if (z.split('e').join('')[0] == "m") {
      return 1;
    } else {
      return -1;
    }
}

console.log(checkVersion("2.0.5","1.0.15")); // returns 1 as expected
console.log(checkVersion("1.2.2","1.2.0")); // returns 1 as expected
console.log(checkVersion("1.0.5","1.1.0")); // returns -1 as expected
console.log(checkVersion("1.0.5","1.00.05")); // returns 0 as expected
console.log(checkVersion("0.9.9.9.9.9.9","1.0.0.0.0.0.0")) // returns -1 as expected;

However, I still feel like there must be a shorter, more concise and cleaner way of doing this though. Any suggestions?

A small review;

Once you know that one version digit is larger than the other, you can exit immediately
You did not declare i with const or let
I would advise the use of a beautifier for your code, it's a bit compact in some places
The code does not handle well versions with different counts of digits
You should always pass the base, when you call parseInt

I wrote an alternative version with 2 extra tests;

function checkVersion(a, b) {
    const x = a.split('.').map(e => parseInt(e, 10));
    const y = b.split('.').map(e => parseInt(e, 10));

    for (const i in x) {
        y[i] = y[i] || 0;
        if (x[i] === y[i]) {
            continue;
        } else if (x[i] > y[i]) {
            return 1;
        } else {
            return -1;
        }
    }
    return y.length > x.length ? -1 : 0;
}

console.log(checkVersion("1.2.2", "1.2.0"), 1); // returns 1 as expected
console.log(checkVersion("1.0.5", "1.1.0"), -1); // returns -1 as expected
console.log(checkVersion("1.0.5", "1.00.05"), 0); // returns 0 as expected
console.log(checkVersion("0.9.9.9.9.9.9", "1.0.0.0.0.0.0"), -1) // returns -1 as expected;
console.log(checkVersion("1.0.5", "1.0"), 1); // returns 1 as expected
console.log(checkVersion("1.0", "1.0.5"), -1); // returns -1 as expected
console.log(checkVersion('2019.09', '2019.9'), 0) // returns 0


PHP class architecture and dependency injection Let me start off by saying I am not new to PHP or PDO, but very new to OO PHP, and have been struggling to get a firm idea of best practices in my head regarding dependency injection and structuring my classes and their methods.

The code I am providing will clearly be built on once I know I am doing things the correct way, so imagine more methods etc. using the same techniques I have used below, and what needs to be changed to make the whole package efficient and stable.

Whilst I am working on other PHP projects at work, the code below is not for or from any project, simply to help me learn before using OO in a real project. I also would normally do far more error checking, checking variables are set etc but have fiddled with this code so much, I haven't done this yet.

Here are the issues I see with it:

It seems that I am not taking advantage of the extra efficiency prepared statements bring when re-using the same statement but with different values. By doing this in a method like below, I would be preparing the statement every time, even if I just wanted to change the values.
I am not using a dependency injection container. Whilst it may not be necessary for small classes like these, I feel it would be better to use on in the long run.
I am worried I am on the track to creating tonnes of methods for every different type of query - see the findById and returnObjectById methods - they are structured very differently - one calling a query method in the Database class, the other running the query within the User class - I have no idea which is best. But also I feel I could end up creating a new page, realizing I need ANOTHER type of query, and then having to either write one in that file as a single instance (which would make me think why don't I do this for all), or create ANOTHER method in the User or Database class.
Are my methods within the correct classes - should the returnObjectById for example, be in the database class?

Note: Classes are auto loaded with spl_autoload_register in the config file, where DB constants are also set.

class_Database.php

    class Database
    {
    private $conn;

    public function __construct() {
        $this->openConnection();
    }

    public function getConnection()
    {
        $conn = $this->conn;
        return $conn;
    }

    public function openConnection() {
        try {
            $this->conn = new PDO('mysql:host=' . DB_SERVER . '; dbname=' . DB_NAME, DB_USER, DB_PASS);
            $this->conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
        } catch (PDOException $e) {
            echo 'There was an error connecting to the database, Error: ' . $e->getMessage();
            die();
        }
    }

    public function standardQuery($sql, array $params)
    {
        $stmt = $this->conn->prepare($sql);
        $stmt->execute($params);
        return $stmt->fetch(PDO::FETCH_ASSOC);
    }
}

class_User.php

 class User
    {
    public $id;
    public $first_name;
    public $last_name;
    public $username;
    public $password;
    public function __construct()
    {

    }
    public function findById(Database $db, $id)
    {
        $user = $db->standardQuery("SELECT * from users WHERE id = :id", array(':id' => $id));
        return $user;
    }
    public function returnObjectById(Database $db, $id)
    {
        $stmt = $db->getConnection()->prepare("SELECT * from users where id = :id");
        $stmt->bindParam(':id', $id);
        $stmt->execute();
        return $stmt->fetchAll(PDO::FETCH_CLASS, "User");
    }
}

Essentially, answers to my points above would be great, but I'd like to know if I am in any way moving in the right direction, or if I should stop and start over with a different technique.

Alright, let's start with your concerns, and then move on to the actual review:

A prepared statement costs about 50 times a normal statement. However, that cost is on the database server's side (i.e. on MySQL's side), preparing a statement A, then clearing the statement object on PHP, then preparing the same statement A (with different parameters) will not incur a penalty, because the query is cached by mysql.
A dependency injection container has advantages and disadvantages. It simplifies the dependency wiring process at the cost of magic. A dependency injection container is completely optional, don't sweat about it (especially in small projects).
What are you abstracting? That's a question that you should ask yourself every time you create a class or a layer (which is a bunch of classes that have the same purpose). You seem to trying to abstract the PDO object, hiding it from the user. But why? PDO is an abstraction on its own, you don't really need to abstract it. What you should abstract is either

Storage itself. Make a mapper that takes an object and stores/fetches it from storage. Note that I didn't say MySQL because mappers are storage agnostic, then you can work seamlessly across different storage types.
The SQL language. Make a criteria based API (also commonly known as DSL). Something like this: $db->select()->from('table')->where(Database::EQUALS("column", "value"))->limit(10); It's up to you.

Your current abstractions don't really add value to an application. Why would I use your class over plain 'ol PDO?
Yes, it should, and I'll explain. When you create an object, it should do one thing and one thing only. Your user object should be responsible for keeping track of data about the user. It's an object that represents a single user in your data model.

Your user should not be aware of its own means of storage. A user specifically can come from many places. It can come from a register form, it can come from a database, it can come from a migration CSV file containing hundreds of users to be inserted at bulk.

As a result, your user should be storage agnostic, it should not care where the data came from, it should just accept it in the constructor, and  work with what it's got.

A few more points:

Always keep your code properly and consistently indented.
Don't catch the database exception and kill the script every time when there's an error. Let the caller decide! What if I can live without a database and will rely on alternative storage methods until I can get the database back online?
Don't rely on constants for the database credentials! Pass the to the constructor!

public function openConnection(string $host, string $dbname, string $username, string $password) {

By default, refrain from having public fields. Everything should be as private as possible, and only what you want to expose to the outside world is public.
Assuming your user object is bound to the database, a database object would be part of the object's state, it's bound to that database instance. Why not pass it in the constructor and keep it as a property?
PHP7 added the ability to type-hint scalars (such as string), use that in your advantage.
Your instance methods seem to return a new user object, rather than changing the current one, that seems counter intuitive, make changes to the current object. If the consumer wants a different user, they should create a new User.


Back end validation as front end validation I am trying to use the back end validation for front end validation and this is what I got. Maybe there is a better way to do this? If someone knows please share the knowledge.

HTML:

<form id="post-store-form" action="posts/store" method="POST">

        <input id="post-title" type="text" name="title" class="form-control" placeholder="Title">

        <textarea id="post-content" name="content"></textarea>

        <input type="submit" value="Save" class="btn btn-primary btn-block">

</form>

JS:

$('#post-store-form').submit( function(e) {

    if (!isValid()) {
        e.preventDefault();
    }
});

function isValid() {

    var formIsValid = false;
    var fields = jQuery.param({
        title: $('#post-title').val(),
        content: $('#post-content').val()
    });

    $.ajax({
        async: false,
        type: "POST",
        url: "/posts/store",
        data: fields,
        dataType: 'json',
        success: function(){
            formIsValid = true;
        },
        error: function(result) {
            var messages = [];

            $.each(result.responseJSON.errors, function( index, value ) {
                messages.push('
' + value);
            });

            bootbox.alert("" + messages + "");
        }
    });

    return formIsValid;
}

PHP Controller:

public function postStore()
{
    $headers = apache_request_headers();
    $is_ajax = (isset($headers['X-Requested-With']) && $headers['X-Requested-With'] == 'XMLHttpRequest');

    $parameters = $this->formParams;
    $validationRules = [
        'title'     => 'required|alnum',
        'content'   => 'required',
    ];

    // if validation fails return back with error messages
    if ($this->validatorFails($parameters, $validationRules)) {
        if ($is_ajax) {
            $errors = $this->getValidatorErrors($validationRules);
            http_response_code(422);
            echo json_encode(['errors' => $errors]);
            die;
        }
    }

    // if no errors and ajax, return 200
    if ($is_ajax) {
        http_response_code(200);
        echo json_encode([]);
        die;
    }

    // create new user in DB
    $this->createPostFromParams($parameters);

    Session::put('success', 'Successfully created a post.');

    return redirect('posts');
}

Is there a better way to integrate the backend validation to be also the front end validation?

You should separate it in different functions.
Here is how you can have validation in different functions and use it in both places - for creating posts and for front-end validation. Besides you should extract this validation logic elsewhere, it's bad to put such logic in your controller.

public function postValidateStore()
{

    $requestValidation = $this->validateStore();

    if($requestValidation !== true){

       die($requestValidation);
    }

    die(json_encode(['success' => 1]));

}

/**
 * @return mixed
 */
public function postStore()
{

    $requestValidation = $this->validateStore();

    if($requestValidation === true){

        // create new user in DB
        $this->createPostFromParams($this->formParams);

        Session::put('success', 'Successfully created a post.');

    } else {

        Session::put('failure', 'Error creating post.');
    }

    return redirect('posts');
}

/**
 * @return bool|false|string
 */
private function validateStore()
{

    $parameters = $this->formParams;
    $validationRules = [
        'title'     => 'required|alnum',
        'content'   => 'required',
    ];

    // if validation fails return back with error messages
    if ($this->validatorFails($parameters, $validationRules)) {

        return json_encode(['errors' => $this->getValidatorErrors($validationRules)]);
    }

    return true;
}

function isValid() {

var formIsValid = false;
var fields = jQuery.param({
title: $('#post-title').val(),
content: $('#post-content').val()
});

$.ajax({
    async: false,
    type: "POST",
    url: "/posts/ValidateStore",
    data: fields,
    dataType: 'json',
    success: function(result){

        if(typeof result.success == "undefined"){

            var messages = [];

            $.each(result.errors, function( index, value ) {
                messages.push('
' + value);
            });

            bootbox.alert("" + messages + "");

        } else {

            formIsValid = true;
        }

    }

});

return formIsValid;
}


how to generate numbers for pagination? I made a function that returns the number displayed for pagination. I want to get something like this (parentheses is the only show where the active site):

if pages < 10

1 2 3 4 5 6 7 8 9 10

if pages > 10

1 2 3 ... 20 [30] 40 ... 78 79 80

where [30] is active page. If active page < 3

1 2 [3] 4 ... 20 30 40 ... 78 79 80

etc. My code:

    count_all - number of all items
    items_on_page - items displayed on one page
    page - current page

    countpages = int(float(count_all+(items_on_page-1))/float(items_on_page))
    linkitems = 10

    if countpages > (linkitems-1):
        countpagesstr = list()
        for i in range(1,int(float(linkitems+(2))/float(3))):
            countpagesstr += [str(i)]

        countpagesstr += ['...']
        sr = int(countpages/2)
        for i in range(sr-1, sr+1):
            countpagesstr += [str(i)]

        countpagesstr += ['...']
        for i in range(countpages-3, countpages):
            countpagesstr += [str(i)]
    else:
        cp = list()
        for c in range(1,countpages+1):
            cp.append(str(c))
        countpagesstr = cp

    return countpagesstr

How to do it better?

Your code doesn't run as written. It needs a function definition.
Your design needs work. Suppose the current page is 30 and the list of pages says "... 20 [30] 40 ..." as in your example. How do I get to page 29 or page 31? It doesn't look as if these page numbers will ever appear in the list.

The more usual design is to always show a small number of pages at the beginning and end, and a small number of pages adjacent to the current page. So that the list of pages would look more like "1 2 3 ... 28 29 [30] 31 32 ... 78 79 80".
The expression

int(float(count_all+(items_on_page-1))/float(items_on_page))

is unnecessarily complex. You can use Python's floor division operator (//) to achieve the same thing without converting to floats and back again:

(count_all + items_on_page - 1) // items_on_page

Your variables are poorly and unsystematically named. count_all, for example. All what? Or countpagesstr, which sounds like it might be a string, but is in fact a list. You use underscores in some cases (count_all) but not in others (countpages).
It is easiest conceptually to separate the two steps of deciding which page numbers to show and showing them in order with ellipses.

In the code below I build the set of the page numbers from the first three pages, the five pages around the current page, and the last three pages. I use Python's sets so that I can take this union without having to worry about duplicate page numbers.

def abbreviated_pages(n, page):
    """
    Return a string containing the list of numbers from 1 to `n`, with
    `page` indicated, and abbreviated with ellipses if too long.

    >>> abbreviated_pages(5, 3)
    '1 2 [3] 4 5'
    >>> abbreviated_pages(10, 10)
    '1 2 3 4 5 6 7 8 9 [10]'
    >>> abbreviated_pages(20, 1)
    '[1] 2 3 ... 18 19 20'
    >>> abbreviated_pages(80, 30)
    '1 2 3 ... 28 29 [30] 31 32 ... 78 79 80'
    """
    assert(0 < n)
    assert(0 < page <= n)

    # Build set of pages to display
    if n <= 10:
        pages = set(range(1, n + 1))
    else:
        pages = (set(range(1, 4))
                 | set(range(max(1, page - 2), min(page + 3, n + 1)))
                 | set(range(n - 2, n + 1)))

    # Display pages in order with ellipses
    def display():
        last_page = 0
        for p in sorted(pages):
            if p != last_page + 1: yield '...'
            yield ('[{0}]' if p == page else '{0}').format(p)
            last_page = p

    return ' '.join(display())


Find A Series of Numbers Who when Squared and Summed are equal to a given Square I am working on a CodeWars titled 'Square into Squares. Protect trees!' it can be found here: https://www.codewars.com/kata/54eb33e5bc1a25440d000891/train/javascript

I have a working solution, the issue is that the performance is so horrid that if I run it with an argument higher than around 15 my browser/tab freezes. I would like some suggestions as towards how I could create a more efficient algorithm for this kind of problem, or just improve my current algorithm to be more performant.

Codewars times out as a result of its bad performance on the full test kit for the Kata. I am entirely certain that the issue is caused by calculating the square of every number beneath my argument number, but since I'm not particularly talented at math that was my procedural way of finding the correct values. I thought if I could find a way of eliminating many of the values before calculating their square that would solve the problem, but I haven't thought of a way yet.

let sol = []

function decompose(n) {
    sol = []
    let expMap = {}
    for(let i = 1; i < n; i++){
      expMap[i] = Math.pow(i, 2)
    }

    for(let prop in expMap){
      solutionFinder(prop, expMap, Math.pow(n, 2), [])
    }
    if(sol.length > 0) return sol
    return null
}

const solutionFinder = (val, set, currentValue, currentSet) => {
  const dataArr = [...currentSet]
  const dataClone = {...set}

  currentValue -= dataClone[val]
  dataArr.push(val)
  delete dataClone[val]

  if(currentValue < 0) return;

  if(!(currentValue === 0)){
    for(let prop in dataClone){
      solutionFinder(prop, dataClone, currentValue, dataArr)
    }
  }

  if(currentValue === 0){
    if(sol.length === 0 || arrSum(sol) < arrSum(currentSet)){
      sol = dataArr.map(i => parseInt(i)).sort((a, b) => a - b)
      return
    }
  }

  return
}

const arrSum = arr => arr.reduce((a,b) => a + b, 0)

I am relatively new to programming (Started in February), and still struggle with developing algorithms with good time complexity, any advice about how to make this algorithm more efficient or an algorithm that would work better for this job would be welcome.

Optimization starts from a sound logical process

Let's think about the problem in particular.

  Given a positive integral number n, return a strictly increasing sequence (list/array/string depending on the language) of numbers, so that the sum of the squares is equal to n².

  If there are multiple solutions (and there will be), return the result with the largest possible values

And later, it specifies

  If no valid solution exists, return nil, null, Nothing, None (depending on the language) or "[]" (C) ,{} (C++), [] (Swift, Go).

You were on the right track with your solution, but the code does not exemplify that. Your solution iterates through a map of all  pairings and removes one at a time per iteration of solutionFinder. At worst, this will go through every permutation of 1, ..., . And ideally, you should never reach a case where you're even looking at numbers that have larger squares than your remaining value.

So, how can we improve this? Well, as I said before, optimization starts from a sound logical process. There are 2 conditions in the specifications that can help us build that solution:

The sequence must be strictly increasing
If there are multiple solutions, we must return the result with the largest possible values

Because I'm pretty sure you're already within this same line of problem solving for the problem itself, I'll just jump to what I'm thinking:

   Start with  and .

 Iterating from j=i down to 1, subtract  from . Skip  particularly.

 If that value equals 0, return a list with just i.

 Otherwise, recursively call the same method with that value and j. Return a list of that call with j appended to it if it's nonnull, and null otherwise.

 If i=0, return null.

This methodology guarantees the highest number solution, and doesn't iterate through all permutations. Done right, it'll output the right list or null in order.

So how does this constitute a valid code review? Well, it points out some extraneous points in your solution.

Couple miscellaneous points

There is no need to keep a map of integers to their squares. You can iterate down from the highest possible square to 1 and lower your time complexity while maintaining the uniqueness of your output.

You should build the list through return values, rather than modification within a method parameter. The beauty of recursion is the ability to use recursive method evaluations within another call of the same method.

You have 2 variables named sol in different scopes. It's hard to determine where the solution actually goes. I would recommend removing the global scoped sol, and using the above points to return the solution from solutionFinder.


Project Euler Problem 12 in Python I'm trying to do Project Euler Problem 12, which reads as:

  The sequence of triangle numbers is generated by adding the natural
  numbers. So the 7th triangle number would be 1 + 2 + 3 + 4 + 5 + 6 + 7
  = 28. The first ten terms would be:

  1, 3, 6, 10, 15, 21, 28, 36, 45, 55, ...

  Let us list the factors of the first seven triangle numbers:

  1: 1

  3: 1,3

  6: 1,2,3,6

  10: 1,2,5,10

  15: 1,3,5,15

  21: 1,3,7,21

  28: 1,2,4,7,14,28

  We can see that 28 is the first triangle number to have over five
  divisors.

  What is the value of the first triangle number to have over five
  hundred divisors?

My code is as follows:

import math
from time import time
t = time()
def divisors(n):
    number_of_factors = 0
    for i in range(1, int(math.ceil(math.sqrt(n)))):
        if n % i == 0:
            number_of_factors +=2
        else:
            continue
    return number_of_factors

x=1
for y in range(2,1000000):
    x += y
    if divisors(x) >= 500:
        print x
        break
tt = time()-t
print tt

My code takes 3.8 seconds to run, and I know this can be done in around 0.2 seconds in Java. Any suggestions on how to make this run faster?

Improving project Euler solutions is usually done by improving the algorithms itself rather than just optimising the code.

First things first, your divisors() function is wrong and does not work for perfect squares.  To fix this, you need to take care of the sqrt(n) case separately:

def divisors(n):
    number_of_factors = 0
    for i in xrange(1, int(math.ceil(math.sqrt(n)))+1):
        if n % i == 0:
            number_of_factors +=2
        if i*i==n:
            number_of_factors -=1
    return number_of_factors

Now we come to the main part of the code. The important observation to improve your code is that the nth triangular number is given by T(n) = n(n+1)/2. Now, n and n+1 are coprime. If a and b are coprime numbers, the number of divisors of a*b is just the product of number of divisors of a and b.

This suggests the following improvement for the main part of the code:

for n in xrange(1,1000000):
    Tn=(n*(n+1))/2
    if n%2==0:
        cnt=divisors(n/2)*divisors(n+1)
    else:
        cnt=divisors(n)*divisors((n+1)/2)
    if cnt >= 500:
        print Tn
        break

The improvement this provides is very significant. You can improve the performance further by modifying the divisor function to use the same technique:

def divisors(n,start):
    if n==1:
        return 1
    for i in xrange(st, int(math.ceil(math.sqrt(n)))+1):
        if n % i == 0:
            cnt=1
            while(n%i==0):
                n/=i
                cnt+=1
            return divisors(n,i+1)*cnt
    return 2

Essentially, we find p, the first prime factor of n. If p^k is the maximum power of p that divides n, (k+1)*divisors(n/p^k) is the number of divisors of n. start is just a starting point for checking prime divisors.

Overall, these modifications seem to reduce running time from ~5s to 0.15s on my laptop.


Using shims to turn my integration tests into unit tests I want to understand how to properly unit test a function such as this. The function doesn't return anything, but it does call other functions that themselves have already been unit tested. My understanding is that in order to properly unit test this method I will have to shim the dependant methods - otherwise this is just an integration test.

public void ResetColours(Colours colour)
{
    var colourInfo = colour.GetInfo();
    for (int i = 0; i < 9; ++i)
        this.SetColourInfo(i, colourInfo);
}

I'm having a hard time deciding on the right way to shim the methods. At the moment I am thinking of doing this - shim the GetInfo method so that it returns a hard coded value, then have the SetColourInfo shim remember the information that was passed in to make sure it matches.

[TestMethod]
public void ResetColours()
{
    Info[] colourInformation = new Info[9];

    using (ShimsContext.Create())
    {
        ShimColourExtensions.GetInfoColours = (Colours colour) => new Info("test");
        ShimFace.AllInstances.SetColourInfoIntInfo =
            (index, colourInfo) => colourInformation[index] = true;

        this.TestObject.ResetColours(Colours.Brown); //TestObject was initialized in my init method
        //ASSERT that hard coded GetInfo value was set into each array index
    }
}

Is this a valid unit test? Could I be doing anything better in situations like this? Should I even be writing a unit test for a method like this?

There was no need to delete the post on Stack Overflow, it belonged there just fine; perhaps even better than here.

You don't have to shim anything. Integration tests are tests that use an external dependency (system time, external API, filesystem, etc). In your case all your "dependencies" are simply method calls in your own project.

Here in your case this can simply be seen as a unit test: you're testing a single public method (~ unit) which encapsulates smaller unit tests. This is a perfectly valid unit.

Integration testing refers to a bigger set of components that and testing how these major interfaces work together.

Now, on to the bigger question: how do you test this [void method]? As is tradition: a link to an answer of mine.

void methods change the state of your program (adjusting values of variables) or they interact with an external dependency (printing to the console, logging to a file). So in order to assert against any expectations you'll have to either interpret the results or intercept them along the way.

Interpreting results

In the first case (adjusting of a variable) the answer is obvious: look at the variables that got changed and see if they now have the values you want.

In the latter case you get a "problem": you're now reading the result of an external dependency so you're entering the realm of integration testing. This is why we have the alternative solution..

Intercepting results

This is done with shims, mocks, stubs, seams, you name it. You're already aware how this works: a DI framework intercepts the call and you can provide your own values around the actual call, or you inject the unit under test with a specific implementation for testing purposes.


Thread Safe Events while utilizing EAP This is my code for a server using EAP (Sockets).

Is AsyncOperation.Post the right way to raise thread safe events?

AsyncOperation.Post states:

  Invokes a delegate on the thread or context appropriate for the application model.

Is there any other mistake in the code (i.e something that can be improved or is a bad thing to do)?

public sealed class Server
{
    private Socket _server;
    private SocketAsyncEventArgs _acceptArg;
    private AsyncOperation _asyncOperation;

    public EndPoint LocalEndPoing { get; private set; }
    public bool Listening { get; private set; }

    public delegate void ErrorOccuredEventHandler(Server sender, Exception e);

    public delegate void ClientConnectedEventHandler(Server sender, EndPoint remoteEndpoint);

    public event ErrorOccuredEventHandler ErrorOcurred;

    public event ClientConnectedEventHandler ClientConnected;

    private void OnErrorOccured(Exception e)
    {
        if(ErrorOcurred!=null)
            ErrorOcurred.Invoke(this,e);
    }

    private void OnClientConnected(EndPoint remoteEndPoint)
    {
        if(ClientConnected!=null)
            ClientConnected.Invoke(this,remoteEndPoint);
    }

    public Server(EndPoint localEndPoint)
    {
        LocalEndPoing = localEndPoint;
        Listening = false;
        _asyncOperation = AsyncOperationManager.CreateOperation(null);
    }

    public void Start()
    {
        try
        {
            if (Listening)
                return;

            _server = _server ?? new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp);
            _server.Bind(LocalEndPoing);
            _server.Listen(10);

            Listening = true;

            _acceptArg = _acceptArg ?? new SocketAsyncEventArgs();
            _acceptArg.Completed += _acceptArg_Completed;

            if (!_server.AcceptAsync(_acceptArg))
                _acceptArg_Completed(null, _acceptArg);

        }
        catch (Exception exception)
        {
            _asyncOperation.Post(ex=>OnErrorOccured(ex as Exception),exception);
        }
    }

    public void Stop()
    {
        try
        {
            if(!Listening)
                return;

            _server.Close();
            _acceptArg.Dispose();

            _server = null;
            _acceptArg = null;

            Listening = false;

        }
        catch (Exception exception)
        {
            _asyncOperation.Post(ex => OnErrorOccured(ex as Exception), exception);
        }
    }

    private void _acceptArg_Completed(object sender, SocketAsyncEventArgs e)
    {
        if (e.SocketError == SocketError.Success)
        {
            _asyncOperation.Post(remoteEndPoint=>OnClientConnected(remoteEndPoint as EndPoint),e.AcceptSocket.RemoteEndPoint);

            //Do nasty things with the accepted socket

            e.AcceptSocket = null;

            if(!_server.AcceptAsync(e))
                _acceptArg_Completed(null,e);
        }
        else
        {
            _asyncOperation.Post(ex => OnErrorOccured(ex as Exception), new SocketException((int)e.SocketError));
            Stop();
        }
    }
}

public EndPoint LocalEndPoing { get; private set; }

should be LocalEndPoint.

public void Start()
{
    try
    {
        if (Listening)
            return;

why is this if statement inside the try..catch ? It can be granted that this won't ever throw any exception. As a rule of thumb, only put code inside a try..catch which can throw.

You should always use braces {}, although they might be optional, to make your code less error prone.

_server.Listen(10);

why 10 and not 1000? Using magic numbers should be avoided to make the code easier to read and understand. You should extract magic numbers/strings into meaningful variables/constants so Sam the maintainer will see at first glance what this is about.

_asyncOperation.Post(ex=>OnErrorOccured(ex as Exception),exception);

Let your variables and operators have some space to breathe, its just making your code more readable like so

_asyncOperation.Post(ex => OnErrorOccured(ex as Exception), exception);

private void _acceptArg_Completed(object sender, SocketAsyncEventArgs e)

having an underscore prefixed methodname is a no go. Underscore prefixing is tolerated for class level variable names only.


Reinitialize cache periodically, catching and logging any Throwables My existing code has a scheduler that we init when a DataStore constructor is called.

private void initSchedulers() {
    int initialDelay = applicationStartupConfig.getSchedulerInitialDelay();
    int refreshPeriod = applicationStartupConfig.getSchedulerRefreshPeriod();
    // above are fetched using config.yml(dropwizard)

    Timer timer = new Timer(true);
    timer.scheduleAtFixedRate(new TimerTask() {
        @Override
        public void run() {
            try {
                initializeCache(); // some operation that we perform to fetch data on to the cache on a scheduled basis
            } catch (Throwable t) {
                logger.error("Scheduler could not complete : ", t);
            }

        }
    }, initialDelay, refreshPeriod);
}

Evaluating the code under Sonar Analysis reports this as Vulnerable --

  Throwable is the superclass of all errors and exceptions in Java.

  Error is the superclass of all errors, which are not meant to be
  caught by applications.

  Catching either Throwable or Error will also catch OutOfMemoryError
  and InternalError, from which an application should not attempt to
  recover.

Though I understand the above statements yet I am not able to draw a conclusion on

What if I catch Exception today, and a runtime error exception is thrown later? Would that not stop the scheduler?
Is there a way to specify handling both these cases and ensuring that this does not create impact on client calls to the DataStore?

That is exactly what Sonar says: "... from which an application should not attempt to recover".

Thus, if you catch Exception and NOT Error, yes, this will effectively terminate your scheduler and probably impact the DataStore. BUT: as Errors indicate a severity, where the VM probably won't be working reliably anyway (also see https://docs.oracle.com/javase/8/docs/api/java/lang/Error.html) this is exactly the thing to do: terminate the program, bail out, call the admin. When you continue working under error conditions, the result is generally somewhat undefined. Better avoid this.


Mutable 2d Vector classs I create a 2dVector class to use a position coordinates in a pygame application and maybe in the future as part of a physics engine. Does this implementation has any huge downsides? Is the type hinting ok?

import math
import random
from numbers import Real
from typing import Union, Sequence, cast

VectorType = Sequence[Real]

class Vector(VectorType):
    x: Real
    y: Real
    rotation: Real
    magnitude: Real

    def __init__(self, x: Union[VectorType, Real], y: Real = None) -> None:
        if y is None:
            x, y = cast(VectorType, x)
        self.x, self.y = cast(Real, x), y

    def __repr__(self):
        return f"{self.__class__.__name__}{self.x, self.y}"

    def __len__(self) -> int:
        return 2

    def __getitem__(self, i: int) -> Real:
        return (self.x, self.y)[i]

    def __add__(self, other: VectorType):
        return Vector(self.x + other[0], self.y + other[1])

    def __radd__(self, other):
        return Vector(self.x + other[0], self.y + other[1])

    def __iadd__(self, other: VectorType):
        self.x += other[0]
        self.y += other[1]
        return self

    def __sub__(self, other: VectorType):
        return Vector(self.x - other[0], self.y - other[1])

    def __rsub__(self, other: VectorType):
        return Vector(other[0] - self.x, other[1] - self.y)

    def __isub__(self, other: VectorType):
        self.x -= other[0]
        self.y -= other[1]
        return self

    def __mul__(self, other: Real) -> Vector:
        return Vector(self.x * other, self.y * other)

    def __rmul__(self, other: Real):
        return Vector(self.x * other, self.y * other)

    def __matmul__(self, other: VectorType):
        return Vector(self.x * other[0], self.y * other[1])

    def __rmatmul__(self, other):
        return Vector(other[0] * self.x, other[1] * self.y)

    def __truediv__(self, other: Union[VectorType, Real]) -> Vector:
        if isinstance(other, Sequence):
            return Vector(self.x / other[0], self.y / other[1])
        return Vector(self.x / other, self.y / other)

    def __rtruediv__(self, other: VectorType) -> Vector:
        self.x /= other
        self.y /= other
        return self

    def __round__(self, n=None) -> Vector:
        return Vector(round(self.x, n), round(self.x, n))

    def change_rotation(self, rotation: float) -> Vector:
        v = Vector(self.x, self.y)
        v.rotation += rotation
        return v

    @classmethod
    def random(cls, x_range=(0, 1), y_range=(0, 1)):
        return cls(random.uniform(*x_range), random.uniform(*y_range))

    @classmethod
    def from_polar(cls, rotation: Real, magnitude: Real):
        return Vector(math.cos(rotation) * magnitude, math.sin(rotation) * magnitude)

    def normalize(self):
        f = self.magnitude
        self.x *= f
        self.y *= f
        return self

    @property
    def magnitude(self) -> Real:
        return (self.x * self.x + self.y * self.y) ** 0.5

    @magnitude.setter
    def magnitude(self, value: Real):
        f = value / self.magnitude
        self.x *= f
        self.y *= f

    @property
    def rotation(self):
        return math.atan2(self.x, self.y)

    @rotation.setter
    def rotation(self, value):
        m = self.magnitude
        self.x = math.sin(value)
        self.y = math.cos(value)
        self.magnitude = m

    @property
    def rounded(self):
        return round(self.x), round(self.y)

Here is the code on github (with a few additions)

There are no docstrings.
I would suggest reconsidering the mutability of the vectors. The advantages of mutability are:

a. You get to reuse the original storage for the original vector when you make a change, avoiding a couple of memory management operations.

b. You save a couple of characters when writing some update operations, for example you can write v.normalize() instead of, say, v = v.normalized().

But the disadvantage of mutability is that it is harder to reason about operations. If you see a function that takes some vectors, let's say

def collide(p: VectorType, q: VectorType, r: Real) -> Bool:
    "Return True if p and q are separated by no more than r."

is it safe to pass p=sprite.position, or do you need to make a copy? With mutability, then for all you know, the function is implemented like this:

p -= q
return p.magnitude <= r

So in a world where vectors are mutable, you have to worry about this possibility, and perhaps document for each function which arguments get mutated and which don't. If vectors are immutable, then all this is unnecessary and reasoning about the behaviour of code is easier.

Immutability would avoid the need to implement the __iadd__ and __isub__ methods. Also, you could inherit from tuple and save some memory. See this vector class on github for some implementation ideas.
The __truediv__ method has different behaviour based on the type of the argument. This is inconsistent with the behaviour of __mul__ and __rmul__ and __rtruediv__, none of which have this behaviour. Also, dispatching based on the type makes it harder to read and reason about code. If you know that v is a vector and then you read the expression v / a, you can't immediately deduce that a must be a scalar as you could in the case of v * a.

I guess that you added the variant operation because of some use case. But nonetheless I would recommend giving the variant operation a different method name, for example scaled.
In __radd__ the operations should be reversed, with the other values on the left (just in case they have unusual implementations of __add__ method):

def __radd__(self, other):
    return Vector(other[0] + self.x, other[1] + self.y)

Similarly for __rmul__.
There's no checking that the other vector has the right length. There's nothing stopping me from writing:

Vector(1, 2) + [3, 4, 5]

which ought to be an error.
In __repr__ I suggest writing type(self) rather than self.__class__, just as you would write len(s) rather than s.__len__().
In from_polar you should call cls rather than Vector, to support subclasses.
normalize has the operations the wrong way round: you need to divide by the magnitude, not multiply. But it would be simpler to write self.magnitude = 1.
In magnitude it would be clearer to call the argument length or magnitude rather than value.
In magnitude, consider writing math.hypot(self.x, self.y).
In rotation it would be clearer to call the argument theta or angle rather than value.


Simple PHP image upload with error handling I wrote this method to upload a floor plan image:

function upload_image($path, $file) {
    $errors = [];
    $ext = pathinfo($file["name"])['extension'];

    //randomly generate filename to avoid conflicts
    $filename = uniqid().'.'.$ext;

    if ($file["size"] > 2000000) {
        $errors[] = "The image cannot be greater than 2MB";
    }

    if (!in_array($ext, ["jpg", "jpeg", "png"])) {
        $errors[] = "The image can only be of .jpg, .jpeg or .png format";
    }

    if (empty($errors)) {
        move_uploaded_file($file["tmp_name"], $path.$filename);
    }

    return array(
        "name" => $filename,
        "errors" => $errors
    );
}

Implementation:

if (isset($_POST['create'])) {
    $image = upload_image(IMG_PATH."/floorplans/", $_FILES['floorplan']);
    $floorplan = new FloorPlan($_POST['floorplan']);

    if (empty($image['errors'])) {
        $floorplan->image = $image['name'];
    } else {
        foreach ($image['errors'] as $error) {
            $floorplan->errors[] = $error;
        }
    }
}

I'm very happy with it and everything seems to work fine, but I am just wondering if there is a better/more efficient way to do this.

Rather than generate the full array of elements that pathinfo() generously offers only to access one value, use the PATHINFO_EXTENSION flag/option (the second parameter) to only ask for what you need.
For best efficiency, don't perform any  unnecessary processes for disquaified incoming data.
If you are going to declare errors at the start as an empty array, you won't need to call empty() which checks if the variable does not exist or if it is empty/falsey/null/zeroish.  You can more simply use !$errors.  Alternatively, don't declare the empty errors array and then use empty to perform the double check.
I assume that there is no point to generating a filename for an invalid submission, so I don't think that I'll support passing it with errors.
I try to avoid declaring single-use and temporary variable where possible.  In this case, I'd avoid temporary variable by always dealing with the array to be returned at the end.

How my recommendations boil down...

function upload_image($path, $file) {
    if ($file["size"] > 2000000) {
        $outcome['errors'][] = "The image cannot be greater than 2MB";
    }

    $ext = strtolower(pathinfo($file["name"], PATHINFO_EXTENSION)); // force lowercase for uniformity

    if (!in_array($ext, ["jpg", "jpeg", "png"])) {
        $outcome['errors'][] = "The image can only be in .jpg, .jpeg or .png format.";
    }

    if (empty($outcome['errors'])) {
         // for the record, randomness does not guarantee uniqueness
        $outcome['name'] = uniqid() . '.' . $ext;
        move_uploaded_file($file["tmp_name"], $path . $outcome['name']);
    }

    return $outcome;
}

If you like rabbit holes, here's a good read regarding the security aspects of your task: https://stackoverflow.com/q/6484307/2943403


Rails helper method refactor I have this messy helper method:

def gesture(klass, item, text, desc)
  element_class = klass.to_s + " gesture"
  content_tag :li do
    if klass == :sell
      link_to new_reply_path(item_id: item.id), title: desc, data: {qtip: desc}, class: element_class do
        content_tag(:div, nil, class: element_class) +
        content_tag(:span, text, class: 'gesture-text')
      end
    elsif klass == :alternative
      link_to new_reply_path(item_id: item.id, alternative: true), title: desc, data: {qtip: desc}, class: element_class do
        content_tag(:div, nil, class: element_class) +
        content_tag(:span, text, class: 'gesture-text')
      end
    elsif klass == :join
      link_to new_item_affiliation_path(item), title: desc, data: {qtip: desc}, class: element_class do
        content_tag(:div, nil, class: element_class) +
        content_tag(:span, text, class: 'gesture-text')
      end
    end
  end
end

As you can see, it has a lot of repetition, any idea on how I can refactor it?

I'd write:

def gesture(klass, item, text, desc)
  element_class = [klass.to_s, "gesture"]
  url = case klass
  when :sell
    new_reply_path(item_id: item.id)
  when :alternative
    new_reply_path(item_id: item.id, alternative: true)
  when :join
    new_item_affiliation_path(item)
  else
    fail("Unsupported klass: #{klass}")
  end

  content_tag(:li) do
    link_to(url, title: desc, data: {qtip: desc}, class: element_class) do
      content_tag(:div, nil, class: element_class) +
        content_tag(:span, text, class: 'gesture-text')
    end
  end
end


Making a simple leader board in memory Im trying to think of the most efficient way to structure an in memory leader board with the ability to read a specific rank quickly. Im talking about very micro optimisations as either way it will still be fast.

class Leaderboard
{
    constructor()
    {
        this.leaderboard = [];
    }

    insert(name, score)
    {
        const player = new Player(name, score, new Date());
        this.leaderboard.push(player);
        this.leaderboard.sort((a, b) => a.score - b.score);
    }

    getRank(rank)
    {
        if (isNaN(rank) || rank < 1 || rank > this.leaderboard.length) return null;
        return this.leaderboard[rank - 1];
    }
}

class Player
{
    constructor(name, score, date)
    {
        this.name = name;
        this.score = score;
        this.date = date;
    }
}

const leaderboard = new Leaderboard();

leaderboard.insert('john', 36);
leaderboard.insert('pete', 23);
leaderboard.insert('dave', 56);

console.log(leaderboard.getRank(2));

This is what I have come up with, any feedback is great and any small optimisations or completely different and better ways are welcome, thankyou

Your implementation depends on this.leaderboard.sort((a, b) => a.score - b.score);. Each and every time you insert, you sort again.

Depending on your access pattern, you could defer the sorting from adding to reading.

You could operate with a dictionary keeping the scores unorderd which makes inserting fast. And sort the entries only in case if you read them.

For the sake of the example I used plain numbers

let leaderboard = {
}

const addScore = (board, score) => {
  if(!board[score]) board[score] = []
  board[score].push(score)
  return board;
}

const getcurrentRanks = (board) => {
    const scoreranks = Object.keys(board).sort((a,b)=>b-a)
    return scoreranks.reduce((o,n )=>{
        return o.concat(board[n]);
    }, []);
}

But this has the downside of converging to your first implementation the more you read; e.g. for each time you insert a value you want to get all the ranks.

Otherwise I would encourage you to look into priority queues.


Merge and sorting arrays of String and int in more efficient methods Are there better methods for merge and sorting arrays of int and String in more efficient methods both time-complexity wise and code-pattern wise? I also wonder if I could have a generic method that could work for any kind of given type like <E>.

I am preparing for coding interviews, so I prefer to see solutions that involve known Java APIs and nothing fancy.

/**
 * Created by mona on 5/24/16.
 */

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
public class MergeArrays {

    public static String[] mergeAndSortStringArrays(String[] first, String[] second){
        List<String> merged = new ArrayList<>();
        Collections.addAll(merged,first);
        Collections.addAll(merged,second);
        Collections.sort(merged);
        return merged.toArray(new String[merged.size()]);
    }

    public static int[] mergeAndSortIntArrays(int[] firstInt, int[] secondInt){

        List<Integer> merged = new ArrayList<>();

        for (int i=0; i<firstInt.length; i++){
            merged.add(firstInt[i]);
        }

        for (int i=0; i<secondInt.length; i++){
            merged.add(secondInt[i]);
        }

        Collections.sort(merged);
        int[] result=new int[merged.size()];
        for (int i=0; i<merged.size(); i++){
            result[i]=merged.get(i);
        }
        return result;
    }

    public static void main(String[] args){
        String[] first={"hi", "mona", "how", "are", "you"};
        String[] second={"hello", "world", "good", "morning", "USA"};
        String[] merged=mergeAndSortStringArrays(first, second);
        for (int i=0; i<merged.length; i++){
            System.out.print(merged[i]+" ");
        }

        int[] firstInt={1, 4, 2, 6, 7};
        int[] secondInt={2, 8, 2, 9, 1, 12};
        int[] resultedMerge=mergeAndSortIntArrays(firstInt, secondInt);
        for (int i=0; i<resultedMerge.length; i++){
            System.out.print(resultedMerge[i]+" ");
        }
    }
}

mergeAndSortStringArrays looks good to me. What comes to mergeAndSortIntArrays, you can clean it a bit:

public static int[] mergeAndSortIntArrays(int[] firstInt, int[] secondInt){
    List<Integer> merged = new ArrayList<>();
    IntStream.of(firstInt) .forEach(merged::add);
    IntStream.of(secondInt).forEach(merged::add);
    Collections.sort(merged);
    return merged.stream().mapToInt((Integer i) -> (i)).toArray();
}

Generic implementation

Also, you could do

public static <E extends Comparable<? super E>> E[]
mergeArrays(final E[] firstArray, final E[] secondArray) {
    final E[] ret = Arrays.copyOf(firstArray,
                                  firstArray.length + secondArray.length);
    System.arraycopy(secondArray,
                     0,
                     ret,
                     firstArray.length,
                     secondArray.length);

    Arrays.sort(ret);
    return ret;
}

This, however, will not accept an int[] as the argument type, yet it will gladly work on Integer[] and other object arrays.

A minor tip

Instead of writing for loops for printing arrays, you can always say:

System.out.println(Arrays.toString(mergeArrays(first, second)));

Hope that helps.