Knapsack algorithm in JavaScript - Integer weights and values My version of Knapsack works only when the weights or values of items are whole numbers.

Restrictions

You are given an array of objects each of which contains a weight and value.
You are also given a bag which has a maximum capacity.
Both the values and weights for every item are integers. This example will not cover cases where the weights/values contain decimals
The maximum capacity will also be a whole number, again no decimals.

Here is my code with commentary

var result = document.getElementById("result");

function knapsack(items, capacity) {
    var totalWeight = 0;
    var totalValue = 0;

Here I initialized the total weight and value of our bag, which is empty at first, so both are zero.

    var sorted = items.sort(function(a,b) {
        return (b.value / b.weight) - (a.value / a.weight);
    });

To get the most bang for the buck, I'm taking a greedy algorithm and first choosing the item with the highest value to weight ratio. I have to sort the array based on the item's value per cost. I will then set the index to zero, to start at the best value.

    var index = 0;
    while (totalWeight < capacity) {
        var ratio = sorted[index].value / sorted[index].weight;
        totalValue += ratio;
        totalWeight++;
        if (totalWeight === sorted[index].weight) {
            index++;
        }
    }

The loop is run until the weight is equal to the capacity. For every item, I will get the value per 1 unit of weight. This will be added to the value of the bag, whereas the weight of the bag will increase by one unit.

If the weight of the bag equals the weight of the item, I will move on to the next item and continue the while loop.

    return totalValue.toFixed(2);
}

var array = [
                {"value": 15, "weight": 10},
                {"value": 24, "weight": 15},
                {"value": 25, "weight": 18}
            ];

result.innerHTML = knapsack(array, 20);

This will not work if the item weights or values have decimals, but that's beyond the scope of this problem. This problem assumes that all are whole numbers.

What would the complexity be of this algorithm? Also, which type of knapsack does my algorithm solve?

I don't think your algorithm actually works.

First...

while (totalWeight < capacity) {

...is not the proper criteria for determining when loading into the knapsack should be completed.  What if the knapsack capacity is 10 and you got passed 4 objects each having weight of 3?   The most you could ever get in the knapsack is a weight of 9.  With your condition as is, you would get an infinite loop.

Second...

    totalValue += ratio;

...makes no sense.  Why are you adding the value/weight ratio to the total value summation?

Third...

    totalWeight++;

...also makes no sense. Why are you incrementing the total weight?

Probably something like the following is what you would need

function knapsack(items, capacity) {
    let totalValue = 0;
    let totalWeight = 0;
    let remainingItems = items.sort( (a, b) => {
        return (b.value / b.weight) - (a.value / a.weight);
    });
    while (remainingItems.length > 0) {
        const remainingCapacity = capacity - totalWeight;
        remainingItems = remainingItems.filter( (item) => {
            return (item.weight <= remainingCapacity);
        });
        if (remainingItems.length === 0) continue;
        const addedItem = remainingItems.shift();
        totalValue = totalValue + addedItem.value;
        totalWeight = totalWeight + addedItem.weight;
    }
    return totalValue.toFixed(2);
}

Note that this would work even with floats, however this does not validate input data against weight = 0 items which would cause error.  In production code, it might be needed to validate the item being passed against this (and possibly other things like negative values/weight, negative/zero capacity, etc.)


Array of objects to array What would be the "correct" way of transforming an array of objects into an object that contains an array?

Suppose I have this array that contains objects:

const paintings = [
  { painting: "Mona Lisa" },
  { painting: "The Starry Night" },
  { painting: "The Last Supper" },
  { painting: "Girl with a Pearl Earring" },
  { painting: "American Gothic" },
  { painting: "The Night Watch" }
];

And I would like to transform it into this:

const paintings = {
  artworks: [
    "Mona Lisa",
    "The Starry Night",
    "The Last Supper",
    "Girl with a Pearl Earring",
    "American Gothic",
    "The Night Watch"
  ]
};

I know that I can accomplish this with two nested loops like below, but is there a more "elegant" way of doing it? I think that these tasks are nowadays usually solved using map or reduce functions but I find them a bit confusing to use. I would like to learn though!

const paintingsNew = {
  artworks: []
};

for (const painting of paintings) {
  for (const val of Object.values(painting).values()) {
    paintingsNew.artworks.push(val);
  }
}

console.log(paintingsNew);

// { artworks:
//    [ 'Mona Lisa',
//      'The Starry Night',
//      'The Last Supper',
//      'Girl with a Pearl Earring',
//      'American Gothic',
//      'The Night Watch' ] }

Your code flattens the entire array of {} objects, indiscriminately flattening all properties. It does not mind, if there is only one single property, but it does not make sense semantically, especially if the array is named paintings.

Rather consider the resulting array a projection/mapping to a single property painting.

For a consistent array check and take only the painting property.

for (const p of paintings) {
  if ("painting" in p) {
    paintingsNew.artworks.push(p.painting);
  }
}

The extra if protects against undefined entering corrupted data.


Simple java code measuring I am trying to simply measure my code execution time.

public class PerfMeasureUtil {

    private static final Logger LOG = Logger.getLogger(PerfMeasureUtil.class.getName());
    private long startTime;
    private long duration;
    private static final EnvironmentProperties env = new EnvironmentProperties();

    /**
     * start counting the time
     *
     * @return self
     */
    public PerfMeasureUtil start() {
        if (env.isDev()) {// || env.isTest()) {
            LOG.log(Level.INFO, "====================================== measurement starts");
            this.startTime = System.currentTimeMillis();
        }
        return this;
    }

    /**
     * stops counting the time and returns the result
     *
     * @param methodName
     * @return
     */
    public long stop(String methodName) {
        if (env.isDev()) {// || env.isTest()) {
            this.duration = System.currentTimeMillis() - startTime;
            LOG.log(Level.INFO, "====================================== {0}", methodName);
            LOG.log(Level.INFO, "time spend: {0}ms", duration);
            LOG.log(Level.INFO, "======================================");
            return duration;
        }
        return -1;
    }
}

EnvironmentProperties has a boolean property based on the Maven build type. The measurement should be executed only on dev, or maybe testing environment, not in production. Now I call it like this everywhere, where it is needed:

    PerfMeasureUtil measureUtil = new PerfMeasureUtil().start();
    foo = readFooFromSomewhere();
    bar.applyFoo();
    printResult(bar);
    measureUtil.stop("FooBarApplication.aplyFooOnBar");

Is there something that could be made clearly better? I am not sure about the permanent if-check even on other built types when it is not needed.

And of course the pollution of code is pretty bad with it, but this way I have more control on the code amount measured. Maybe annotation could help?

Return Value

I don't think it's a good idea to return PerfMeasureUtil in start. What would it even be used for? The code would look like this, which doesn't make much sense:

PerfMeasureUtil meassure = new PerfMeasureUtil();
meassure = meassure.start();
int time = meassure.stop("someName");

I could imagine using it as sort of a factory method:

public static PerfMeasureUtil start() {
    PerfMeasureUtil meassure = new PerfMeasureUtil();
    if (env.isDev()) {// || env.isTest()) {
        LOG.log(Level.INFO, "====================================== measurement starts");
        meassure.setStartTime(System.currentTimeMillis());
    }
    return meassure;
}

And then use it like this:

PerfMeasureUtil meassure = PerfMeasureUtil.start();
int time = meassure.stop("someName");

But I would probably prefer to just not return anything.

Dev check

If you want to reduce the amount of environment checks, you could actually use the factory approach from above, and check on object creation if you are in development mode. If not, return a dummy object whose methods do nothing.

Misc

be consistent with your use of this. Currently, you use it sometimes, but not always.
duration doesn't really need to be a field.
I would add a comment on stop to mention what methodName does (that it's only used for logging).


Why is this program for extracting IDs from a file so slow? This is roughly what my data file looks like:

# Monid      U        B       V       R       I      u       g        r       i       J      Jerr     H      Herr      K      Kerr   IRAC1    I1err  IRAC2    I2err  IRAC3    I3err  IRAC4    I4err  MIPS24  M24err  SpT    HaEW       mem comp
Mon-000001  99.999  99.999  21.427  99.999  18.844  99.999  99.999  99.999  99.999  16.144  99.999  15.809   0.137  16.249  99.999  15.274   0.033  15.286   0.038  99.999  99.999  99.999  99.999  99.999  99.999  null   55.000        1  N
Mon-000002  99.999  99.999  20.905  19.410  17.517  99.999  99.999  99.999  99.999  15.601   0.080  15.312   0.100  14.810   0.110  14.467   0.013  14.328   0.019  14.276   0.103  99.999   0.048  99.999  99.999  null  -99.999        2  N

...and it's a total of 31mb in size. Here's my python script that pulls the Mon-###### IDs
(found at the beginning of each of the lines).

import re

def pullIDs(file_input):
    '''Pulls Mon-IDs from input file.'''

    arrayID = []
    with open(file_input,'rU') as user_file:
        for line in user_file:
            arrayID.append(re.findall('Mon\-\d{6}',line))
    return arrayID

print pullIDs(raw_input("Enter your first file: "))

The script works but for this particular file it ran for well into 5 minutes and I eventually just killed the process due to impatience. Is this just something I'll have to deal with in python? i.e. Should this be written with a compiled language considering the size of my data file?

Further info:
  This script is being run within Emacs. This, by the checked answer, explains why it was running so slow.

You said in comments that you don't know how to create a self-contained test case. But that's really easy! All that's needed is a function like this:

def test_case(filename, n):
    """Write n lines of test data to filename."""
    with open(filename, 'w') as f:
        for i in range(n):
            f.write('Mon-{0:06d} {1}\n'.format(i + 1, '  99.999' * 20))

You can use this to make a test case of about the right size:

>>> test_case('cr36275.data', 200000)
>>> import os
>>> os.stat('cr36275.data').st_size
34400000

That's about 34 MiB so close enough. Now we can see how fast your code really is, using the timeit module:

>>> from timeit import timeit
>>> timeit(lambda:pullIDs('cr36275.data'), number=1)
1.3354740142822266

Just over a second. There's nothing wrong with your code or the speed of Python.

So why does it take you many minutes? Well, you say that you're running it inside Emacs. That means that when you run

>>> pullIDs('cr36275.data')

Python prints out a list of 200,000 ids, and Emacs reads this line of output into the *Python* buffer and applies syntax highlighting rules to it as it goes. Emacs' syntax highlighting code is designed to work on lines of source code (at most a few hundred characters but mostly 80 characters or less), not on lines of output that are millions of characters long. This is what is taking all the time.

So don't do that. Read the list of ids into a variable and if you need to look at it, use slicing to look at bits of it:

>>> ids = pullIDs('cr36275.data')
>>> ids[:10]
[['Mon-000001'], ['Mon-000002'], ['Mon-000003'], ['Mon-000004'], ['Mon-000005'],
 ['Mon-000006'], ['Mon-000007'], ['Mon-000008'], ['Mon-000009'], ['Mon-000010']]


Calculating (\$a^2\$ + \$b(2c-d)^2)/(3e)\$ in assembly I made a simple assembly program to evaluate the function

$$f =\frac{a^2 + b(2c-d)^2}{3e}$$

Since this is basically my first ever, I would like to hear what I can improve and know how to use registers better.

.model small
.stack 100h
.data
a dw 4
b dw 15
c dw 86
d dw 155
e dw 8
res dw 0;
.code
mov ax,@data
mov ds,ax
mov dx,0
mov bx,0
mov cx,0
mov ax,c
shl ax,1
mov bx,d
sub ax,bx
mul ax
mul b
mov dx,ax
push dx
mov dx,0
mov ax,a
mul ax
pop cx
add ax,cx
mov dx,ax
push dx
mov dx,0
mov ax,e
mov bx,3
mul bx
mov bx,ax
pop cx
mov ax,cx
mov cx,bx
div cx
mov ax,4c00h
int 21h
end

Some registers have a basic purpose and they supposed to be used for those but it's not that you can't use cx for storing arithmetic operation result. There are few things you could improve in your code
Comment
I would really recommend you to start putting comments. Assembly is not a difficult language but it's very context specific. Each similar looking assembly line might be doing different thing depending on the context and comments are important, even in such small application codebase as this one. Do not try to write what the instruction does in english like
add ax,cx ;adds content of cx to ax

but rather explain the meaning.
add ax,cx ;after that ax  = a^2 + b(2*c - d)^2

Assembly
As I wrote above there's nothing wrong in your usage of assembly (unless you use some register that can't be used - like segment ones), but there are few things you could optimize.
mov ax,0

could be written as xor ax,ax and saved few bytes (only if you want to). And you don't have to zero ax & cx as you don't use it before you assign any value to it.
This is a strange construct too:
mov dx,ax
push dx
mov dx,0

You are moving the result from calculation to dx to push in to the stack and then clearing the register. Why not just
push ax

Instructions at the end also looks like they are doing too much of moving around so this could be simplified as:
mov ax,e
mov bx,3
mul bx
mov bx,ax
pop ax
div bx

You are writing that you want to use registers more so those push/pop could also be removed. Also res is unsued in the code - remove it.
Final program (probably - could be even more improved)
.model small
.stack 100h
.data
a dw 4
b dw 15
c dw 86
d dw 155
e dw 8
.code
_start:
  mov ax,@data
  mov ds,ax

  xor dx,dx
  mov ax,c
  shl ax,1  ; ax = 2*c

  mov bx,d
  sub ax,bx ; ax = 2*c - d
  mul ax    ; ax = (2*c - d)^2
  mul b     ; ax = b*(2*c - d)^2
  mov cx,ax

  xor dx,dx
  mov ax,e
  mov bx,3
  mul bx    ; ax = 3*e
  mov bx,ax

  mov ax,a
  mul ax    ; ax = a^2
  add ax,cx ; cx = a^2 + b*(2*c - d)^2

  div bx    ; ax = (a^2 + b*(2*c - d)^2)/3e

  mov ax,4c00h
  int 21h
end _start


PHP Prepare statement shortcut class I did some looking around and found some more topics on how to use the $stmt->bind_param function inside another function or class what means the reference variables have to be sort of passed on. With the information, I obtained I was able to create a class that allows a mysqli prepared statement with fewer lines in the code itself (faster coding) and allows for error checking in the prepared statement query. I'd like to know if this method is useful and "correct" as far as code can be correct. I've planned to add more of the prepared statement's functions to the class later on but for now, this is it.

class db_prepare {

    public function __construct($query = false) {
        global $DB;
        $this->stmt = $DB->prepare($query);
        $this->error = false;
        if($this->stmt == false) {
            $this->error = true;
            echo $mysqli->error;
        }
    }

    public function bind_param($type, &...$args) {
        if($this->error == false) {
            call_user_func_array(array($this->stmt, "bind_param"), array_merge([$type], $args));
        }

    }

    public function get_result() {
        if($this->error == false) {
            $this->stmt->execute();
            return $this->stmt->get_result();
        }
    }

    public function execute() {
        if($this->error == false) {
            return $this->stmt->execute();
        }else {
            return false;
        }
    }

    public function get_result_array() {
        $array = array();
        if($this->error == false) {
            $this->stmt->execute();
            $rows = $this->stmt->get_result();
            while($row = $rows->fetch_array()) {
                $array[] = $row;
            }
        }
        return $array;
    }

    public function close() {
        $this->stmt->close();
    }

}

This means i can call upon the class in a way like this:

$query = "SELECT * FROM `reactions` WHERE `date` = ? AND `author` = ?";
$stmt = new db_prepare($query);
$stmt->bind_param("ss", $date, $author);

$date = '2018-17-07';
$author = 'Admin';
$results = $stmt->get_result();

$date = '2018-17-07';
$author = 'Mike';
$results = $stmt->get_result();
$stmt->close();

Given the main added value in this class is a sort of error handler, I would suggest to make a little addition to your connection code that will make mysqli  report errors by itself, which will make your class pretty much obsoleted.

Just add the following line before new mysqli...

mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);

and you won't need to do things like echo $mysqli->error anymore simply because mysqli will do everything you wrote here by hand.

Which means that you will perfectly do with vanilla mysqli:

$query = "SELECT * FROM `posts` WHERE `ID` = ? AND `author` = ?";
$stmt = $DB->prepare($query);
$stmt->bind_param("ss", $ID, $author);

$ID = 1;
$author = 'Admin';
$results = $stmt->get_result();

$ID = 2;
$author = 'Admin';
$results = $stmt->get_result();
$stmt->close();

and there is even get_result_array() available in mysqli out of the box:

$query = "SELECT * FROM `posts` WHERE `author` = ?";
$stmt = $DB->prepare($query);
$stmt->bind_param("s", $author);
$results = $stmt->get_result()->fetch_all();
                                ^^^^^^^^^ notice this part

What I would do if I would decide to create a prepared statement shortcut class, is a method that accepts both query and parameters executes them together, like

$query = "SELECT * FROM `posts` WHERE `ID` = ? AND `author` = ?";
$results = DB::run($query, [$ID, $author])->get_result()->fetch_all();

such a function I would call a shortcut


C program to generate Seed programs from statically defined Befunge code My program takes static input, and then number crunches for a few hours. I am currently using Macros, but is there a better, more "user friendly" way to do this without losing on performance.

I have written a program in C to generate Seed programs from a Befunge program. It is a very long, brute force calculation. So that it can be heavily optimised, the input variables are hardcoded using Macros. However, I was wondering if there was a better way to do this that would lead to the same or better optimisation.

Here's the Mersenne Twister I used: math.sci.hiroshima-u.ac.jp/~m-mat/...

/*
Copyright wizzwizz4 (c) 2015
*/

#include <signal.h>
#include <stdio.h>
#include <stdlib.h>
#include "mt19937ar.h"

/*Settings*/
#define PROGRAM_STRING "\"ck,@!dlroW ,olleH"
#define PROGRAM_LENGTH 18
#define   INITIAL_SEED 0
/*End settings*/

/*Meta settings*/
#define PROGRAM_LENGTH_TYPE char
/*End meta settings*/

unsigned long seed = INITIAL_SEED;
char prog[PROGRAM_LENGTH] = PROGRAM_STRING;

/*Ctrl-C Handlers*/
void ignore(int sig) {
    signal(SIGINT, ignore);
}
void panic(int sig) {
    printf("Quick! Copy the output. Something's trying to close me!");
    signal(SIGINT, ignore);
}
void stopped(int sig) {
    printf("We got to %u\nPress any key to quit...", seed);
    signal(SIGINT, panic);
    getchar();
    exit(1);
}
void finished(int sig) {
    printf("Copied the output? Great.\nPress any key to quit...");
    signal(SIGINT, panic);
    getchar();
    exit(0);
}
/*End Ctrl-C Handlers*/

int main(void) {
    signal(SIGINT, stopped);

    PROGRAM_LENGTH_TYPE x; /*The program loop character*/
    _Bool y; /*Seed === Program?*/

    /*Take 32 from the program characters*/
    for (x = 0; x < PROGRAM_LENGTH; x++)
        prog[PROGRAM_LENGTH] = prog[PROGRAM_LENGTH] - 32;

    /*Do the checking*/
    while (1) {
        init_genrand(seed);
        y = 1;
        for (x = 0; x < PROGRAM_LENGTH; x++) {
            /*Compares generated character to expected one*/
            if (prog[PROGRAM_LENGTH] != (char)(genrand_res53() * 96) ) {
                y = 0;
                break; /*Break if wrong; no sense in checking the rest!*/
            }
        }
        if (y) break; /*If the for loop didn't break, seed was right!*/
        seed++;
    }

    /*Output, and make sure nobody gets hurt*/
    printf("COMPLETE Seed program:\n%u %u\nMake sure you copy this down\nPress ^C to quit...", PROGRAM_LENGTH, seed);
}

[Is] there was a better way to do this that would lead to the same or better optimisation.

Yes: don't hardcode stuff the compiler can't do any optimizations about and that makes your program a pain to use. (And an eye-sore because of all those ALL_CAPS_MACROS everywhere.)

Hardcoding the Befunge program gives the compiler next to no useful information. All you're doing with it is comparing it with random data that the compiler can't reason about. (And you're doing that after modifying the string at runtime anyway - the compiler might see through this, but you're adding a tough layer there.)

The hardcoded length could potentially be used to control the amount of loop unrolling in the main loop that the compiler will attempt/choose, but I won't believe that has a significant impact until I see an actual benchmark.

Using char for the loop variables rather than the plain old int won't buy you anything, except the obfuscation you got through that macro - not desirable. Smaller doesn't always mean faster - you might actually incur a performance loss here compared to using a plain int.

What might give the compiler an advantage is having the RNG functions bodies visible when it compiles your main loop - that's the only "computationally expensive" step in your code. But you've put them in a different translation unit, so you've prevented inlining unless you use some form of whole-program optimization/link-time optimization.

So, if I were you, I would first keep this program as it is, and measure its performance on known input(s).

Then start a new one without any of those macros:

read the parameters from command line arguments
move the actual processing code to a function that takes those arguments and returns a seed if found

At this step, again, measure the performance. If there's a measurable difference either way, try to figure out why. Once you have the why, you can look further at what needs to be done to take advantage (or reject) that difference.

Then you can try again after move the relevant RNG functions into the main file.

Then you can check out your compiler's profile-guided optimization mode if it has it, and see if that changes things.

Measure, rinse, repeat.


Generate list of shape in Lisp I quickly created this function in Lisp that resembles

numpy.random.randint(low, high=None, size=None, dtype='I')

The goal is simply to have a function that creates a list of random integers between low and high and that the list has a certain shape. I did not add the ability to choose the dtype as well, but this would be a possibility.

My function for doing this is the following:

(defun randint (&key (start 0) end (shape '(1)))
  (if (> (length shape) 3)
    (error "Shape can be maximally 3 long")
  (loop for i from 1 upto (first shape)
        if (not (null (rest shape)))
        collect (loop for j from 1 upto (first (rest shape))
                      if (not (null (rest (rest shape))))
                      collect (loop for k from 1 upto (first (rest (rest shape)))
                                    collect (+ start (random (+ 1 (- end start)))))
                      else
                      collect (+ start (random (+ 1 (- end start)))))
        else
        collect (+ start (random (+ 1 (- end start)))))))

Is there a more concise or more elegant way of writing this in Common Lisp?

Actually, I find the structure of the program quite complicated and difficult to read and understand. I think that the reason for this is that the program structure is patterned on the shape parameter, whose limitation to three elements is, in my opinion, rather arbitrary.

Why do not eliminate such limitation, and make the function more similar to the original numpy one, in which shape can be any list, even an empty list?

The elimination of this restriction could seem to complicate the problem, but instead it simplifies the function if we use a recursive approach on the structure of shape:

if shape is empty, then generate a single random number
if shape is a list, get the first element, which is a number n, apply n times the function to the rest of the shape and collect the result in a list.

In this way the function is greatly simplified:

(defun randint (&key (start 0) (end (1+ start)) (shape '()))
  (if (null shape)
      (+ start (random end))
      (loop as i below (car shape) collect (randint :start start :end end :shape (cdr shape)))))

Finally, I don't like the fact that a function has a key parameter which, if omitted, causes an error (end). So I would prefer to give it an optional value (for instance start + 1). Finally, to complete the similarity with the original function, I think we can default the shape parameter to the empty list, so that the call (randint :start n :end m) produce a single integer in the range specified.


Improve this image file browser (remove redundancy)? Let me start by saying that the code works as is but I think it has a lot of redundancy. It is written this way as I kept getting nullpointerexceptions if I try to put the fileCounter > gameDiff if/else inside one for loop.

The program allows a user to create directories and add images to them. It will also display a certain number of images based on the users selection. If the user's chosen directory doesn't have enough images, it drops back to the default directory to add more.

String userDir = cboImages.getSelectedItem().toString();
File filePath = new File(baseDir + "/" + userDir + "/");

comboFile = filePath.listFiles();
int fileCounter = 0;
for (int ctr = 0; ctr < comboFile.length; ctr++) {
    if (comboFile[ctr].isFile()) {
        fileCounter++;
    }
}

filePath = new File(baseDir + "/default/");
defaultFile = filePath.listFiles();

List<File> listShuffle = Arrays.asList(comboFile);
List<File> defaultShuffle = Arrays.asList(defaultFile);
Collections.shuffle(listShuffle);
Collections.shuffle(defaultShuffle);

if(fileCounter > gameDiff){
    for (int ctr = 0; ctr < gameDiff; ctr++) {

        Card card = new Card();
        card.setbackImage(new ImageIcon(cardBackImage));

        Image img = null;

        if (listShuffle.get(ctr).isFile()) {
            img = new ImageIcon(listShuffle.get(ctr).getPath()).getImage();
        }
        Image dimg = img.getScaledInstance(144, 216, Image.SCALE_SMOOTH);
        card.setfrontImage(new ImageIcon(dimg));

        // Populate card with db results
        cardsList.add(card);
    }
}
else{
    for (int ctr = 0; ctr < gameDiff; ctr++) {

        Card card = new Card();
        card.setbackImage(new ImageIcon(cardBackImage));

        Image img = null;

        if (ctr < listShuffle.size()) {
            if (listShuffle.get(ctr).isFile()) {
                if (fileCounter <= gameDiff) {
                    img = new ImageIcon(listShuffle.get(ctr).getPath()).getImage();
                }
            }
        } else if (ctr < gameDiff) {
            if (defaultShuffle.get(ctr).isFile()) {
                img = new ImageIcon(defaultShuffle.get(ctr).getPath()).getImage();
            }
        }

        Image dimg = img.getScaledInstance(144, 216, Image.SCALE_SMOOTH);
        card.setfrontImage(new ImageIcon(dimg));

        // Populate card with db results
        cardsList.add(card);
    }
}

Using two separate groups insures that the chosen directory images get added.

First of all, Java code doesn't really make sense outside of the context of a class and a method. Sometimes you can get away without the class, but the method signature is definitely important. In this case, you've included references to variables without a type: are these arguments to a method? Members of a class?

Second, your description of what this code does has too many ands in it. Code should be organized in terms of abstractions. For instance, if you describe something this way, it's a bit confusing as to what it actually does: "It lets you write arrays of bytes to a magnetic medium and causes the read heads to seek to a particular place on the disk and also it lets you refer to a group of bytes by a name, etc, etc." Instead you might start from the bottom: "Files are sequences of bytes intended for long-term storage; they support the following operations: append, truncate, etc. Files are organized into a directory structure by a filesystem, which is built on top of a storage device, etc." This description divides the details into various levels of abstraction.

I'll give a new listing and afterward describe why I made the changes I did.

class Card {
    private final ImageIcon frontImage;
    private final ImageIcon backImage;

    public Card(ImageIcon frontImage, ImageIcon backImage) {
        this.frontImage = frontImage;
        this.backImage = backImage;
    }
    // Probably more goes on here
}

class Deck {
    private static final String DEFAULT_SUBDIRECTORY = "default";
    private final String baseDir;
    private final ImageIcon cardBackImage;

    private List<File> loadSubdirectory(String subdir) {
        List<File> files = new ArrayList<File>();
        File[] listing = new File(baseDir, subdir).listFiles();
        for (int i = 0; i < listing.length; i++) {
            if (listing[i].isFile()) {
                files.add(listing[i]);
            }
        }
        return files;
    }

    public List<Card> selectCardsFromDirectory(int numCards, ComboBox cboImages) {
        return selectCardsFromDirectory(numCards, cboImages.getSelectedItem().toString());
    }

    /** Selects numCards at random from the user's directory,
        filling in from the default directory if the user has
        too few */
    public List<Card> selectCardsFromDirectory(int numCards, String userDirectory) {
        List<File> userFiles = loadSubdirectory(userDirectory);
        List<File> defaultFiles = loadSubdirectory(DEFAULT_SUBDIRECTORY);

        Collections.shuffle(userFiles);
        Collections.shuffle(defaultFiles);

        List<Card> cards = new ArrayList<Card>();
        for (int i = 0; i < numCards; i++) {
            ImageIcon frontImage;
            if (i < userFiles.size()) {
                frontImage = loadScaledImage(userFiles.get(i));
            } else {
                frontImage = loadScaledImage(defaultFiles.get(i - userFiles.size()));
            }
            cards.add(new Card(frontImage, cardBackImage));
        }
        return cards;
    }

    private ImageIcon loadScaledImage(File file) {
        Image image = new ImageIcon(file.getPath()).getImage();
        return new ImageIcon(
            image.getScaledInstance(IMAGE_WIDTH, IMAGE_HEIGHT, Image.SCALE_SMOOTH));
    }
}

This is a quick and dirty refactoring and I haven't compiled it to check that it all builds, but it should give you the idea. In particular, the code is not presented in any reasonable order.

It appears that you had a bug in the code; you use fileCounter to determine how many normal files are in a directory, and only take fileCounter files from the user directory, but you don't skip past directories, etc. I've modified the logic.

This Deck class knows about too many things. Why does it know about the disk layout? Why does it know about elements of the GUI (ie cboImages)? Try reading up on MVP or MVC design for ideas of how to restructure your code to have better encapsulation. Some thoughts:

Separate the shuffling logic out of the class that deals with the file system; that should be as simple as moving loadSubdirectory into a new class.
Remove the overload of selectFilesFromDirectory that takes a ComboBox; the GUI should extract the String before calling into this class.

I've pulled out some helper functions. These do two things: reduce code duplication, and improve abstraction. As a good rule of thumb, most methods shouldn't exceed 25 lines. By pulling out the file-loading logic, I've made it so you don't need to think about how exactly the files are loaded when you're reading selectFilesFromDirectory.

Card has some problems as a class. It is odd that you are default-constructing it, and then constructing it more afterward. I've redesigned part of it.

ctr is not a very good name for a generic loop counter. Most programmers use i, j, and other one-letter names for short loops, or actually descriptive names. For this reason, when reading the code, my brain tries to figure out what ctr means. I'm lucky; English is my first language -- for some people who don't have English as their first language, unusual abbreviations of English words are difficult to decipher.


MapReduce program for finding low value hashes For class we were to make a MapReduce program in Python to find low value hashes of our name. I have completed the assignment but want to try and speed it up. The program currently takes about 45s to complete. I would like to see if there are any suggestions on speeding it up some.

The requirements are to find hashes of your name with 5 leading 0's when printed in hex. We are to try 40 million nonces. I did a few naive implementations before I finally settled on what is below. What I do is send a dict of 40 consecutive numbers to use as multipliers in the Map function. The multiplier represents the range of millions to go through. So when mult = 0 I will use the nonces 0-1mil, when mult = 23 use the nonces 23mil-24mil.

#!/usr/bin/env python
import mincemeat

def mapfn(k, v):
    #Hash the string with the given nonce, if its good save it
    import md5
    #Create a md5 hash and fill it with out initial value
    #The "blockheader" in Bitcoin terms
    m = md5.new()
    m.update("Kevin")
    #Now, step through 1 million nonces with v as a multiplier
    for i in range(v*1000000, ((v+1)*1000000), 1):
        mPrime = m.copy()
        mPrime.update(str(i))
        hashOut = mPrime.hexdigest()
        if(hashOut[:5] == '0' * 5):
           yield hashOut, i
        else:
           pass #Hash trash!

def reducefn(k, vs):
    return (k, vs)

if __name__ == "__main__":
    #Import some useful code
    import sys
    import collections

    #Build the data source, just a list 0-39
    nonces = [i for i in range(0, 40)]
    datasource = dict(enumerate(nonces))

    #Setup the MapReduce server
    s = mincemeat.Server()
    s.mapfn = mapfn
    s.reducefn = reducefn
    s.datasource = datasource

    #Get the results of the MapReduce
    results = s.run_server(password="changeme")

    #List the good hashes
    print "\nHashed on the string: Kevin\nResults..."
    for i in range(0, len(results)):
        key, value = results.popitem()
        hashStr, nonce = value
        print "Nonce: " + str(nonce[0]) + ", hash: " + str(hashStr)

From PEP 8 :

  Imports are always put at the top of the file, just after any module
  comments and docstrings, and before module globals and constants.

Edit : Ok, from your comment, it seems like this is a requirement for mincemeat.

The default value for the third argument of range is 1 so you don't need to provide it.

You shouldn't use magic numbers. Especially when they have so many digits and are so tedious to read/compare.

It makes the following snippet a bit awkward:

for i in range(v*1000000, ((v+1)*1000000), 1):

Just trying to understand how many iterations there will be is a bit of a pain.
I'd much rather read :

nb_nounces = 1000000
for i in range(v*nb_nounces, (v+1)*nb_nounces):

There is no point in having :

    else:
       pass

nonces = [i for i in range(0, 40)]
datasource = dict(enumerate(nonces))
s.datasource = datasource

can be much more concisely written :

s.datasource = {i:i for i in xrange(40)}

using dict comprehension and xrange.

for i in range(0, len(container)): is usually an antipattern in Python. This usually corresponds to something that can be written with straight-forward iteration. In your case, I guess (but I haven't tested), you could just do : for key,value in results.iteritems().


constexpr Precalculate size of serialized data with templates and C++20 concepts Im writing a serializer that will do serialization/deserialization as fast as possible, and that uses templates so that i dont have to create a ton of functions for every data structure,
This is the part of it that pre-calculates the size of the data to be serialized
I have only tested this on MSVC and GCC
So this code has get_size_needed() which should get the total size (in bytes) needed to store the data for the arguments.
You should be able to put any container/arithmetic type (and any combination of those types) into it as a argument and it should calculate its size at compile time.
It should also support calculating the size of nested containers.
sorry if its messy, this is my first time dealing with templates and concepts (with some help from some people)
Serialization.h
#include <vector>
#include <cstdint>
#include <string>
#include <ranges>
#include <numeric>
#include <type_traits>

#ifdef _MSC_VER // vvv MSVC implementation vvv (MSVC does not support <ranges> nor templates with auto arguments)

// checks the requires size of a variable
template <typename T>
constexpr std::size_t get_size_needed_for_type(const T& value)
{
    // If the value is a bool/int/float then just get the size of it
    if constexpr (std::is_arithmetic_v<T>)
    {
        return sizeof(value);
    }

    // If the value is a container, then get the size of the container, and the size of its elements
    else if constexpr (std::is_class_v<T> || std::is_union_v<T>)
    {
        // Get the size of every element
        return std::accumulate(value.cbegin(), value.cend(), std::size_t{ 0 }, [](std::size_t acc, const auto& v)
        {
            return acc + get_size_needed_for_type(v);
        });
    }

    // I dont know what whould end up here, but its probably not something thats serializable
    else
    {
        static_assert(false, "Unsupported type for serialization!");
    }
}

#else // vvv GCC implementation vvv - ^^^ MSVC implementation ^^^

// Only accepts bool/int/float
template <typename T>
    requires std::integral<T> || std::floating_point<T>
constexpr std::size_t get_size_needed_for_type(const T& value)
{
    return sizeof(value); // return the size of it
}

// If the is a range, then its probably a container, then get the size of the container, and the size of its elements
constexpr std::size_t get_size_needed_for_type(const std::ranges::range auto& value)
{
    // Get the size of every element
    return std::accumulate(value.cbegin(), value.cend(), std::size_t{ 0 }, [](std::size_t acc, const auto& v)
    {
        return acc + get_size_needed_for_type(v);
    });
}

#endif // ^^^ GCC implementation ^^^

// Iterate through arguments, and accumulate the size of all of them
template <typename... Args>
constexpr std::size_t get_size_needed(const Args&... args) {
  return (... + get_size_needed_for_type(args));
}

Whould be nice to get thoughts / criticism / optimization tips on this

Stick with supporting the lowest common denominator
I don't see a difference in functionality in the GCC and MSVC version of your code, so I wouldn't bother writing two implementations. That is just repeating yourself in a way.
Also note that it's not really GCC vs MSVC, it is some version of GCC and some version of MSVC that you checked, that doesn't mean all versions of those compilers have the same difference. Older versions of GCC might also not support some C++20 features that you used, and newer versions of MSVC will likely support those at some point.
Also, what about Clang, ICC and all the other compilers out there?
Use feature testing macros
If possible, use feature testing macros instead of testing for compiler brand to check whether you can use a given feature.
You can't serialize all containers by just looking at their contents
Containers can be more than just their contents. The ordered STL containers for example allow you to provide them with a functor that tells them how the elements are to be ordered. And if you go outside the STL, consider what happens if you have for example a class that implements a binary tree, and that provides begin() and end() functions to traverse the tree in-order. If you serialize just the elements, you miss the way the nodes in the tree are linked together, which might be significant information.


php most efficient way to check if a variable contains only certain chars I have a small function which I regularly use to check if a variable contains only [a-z][0-9] and the special chars '-' and '_'. Currently I'm using the following:

function is_clean($string){
    $pattern = "/([a-z]|[A-Z]|[0-9]|-|_)*/";
    preg_match($pattern, $string, $return);
    $pass = (($string == $return[0]) ? TRUE : FALSE);
    return $pass;
}

1 - Can anyone tell me if this is the best/most efficient way to do this and if not try to explain why?

2 - When I view this function through my IDE I get a warning that $return is uninitialized... should I be initializing variables in advance with php, if so how and why?

No need for all that. Just use one bracket group, negate it (be prepending a ^), and use the return value directly:

function is_clean ($string) {
    return ! preg_match("/[^a-z\d_-]/i", $string);
}

Here's a quote from the PHP docs:

  Return Values
  preg_match() returns 1 if the pattern matches given subject, 0 if it does not, or FALSE if an error occurred.

In the regex above, we're looking for any characters in the string that are not in the bracket group. If none are found, preg_match will return 0 (which when negated will result in true). If any of those characters are found, 1 will be returned and negated to false.


PHP create JSON file from Twitter API I've been reading more on PHP JSON and APIs I wanted to play around and pull a user's tweets so I wrote a function that requires the TwitterAPIExchange wrapper.

The code:

ini_set('display_errors', 1);
require_once('TwitterAPIExchange.php');
get_twitter_json();

function get_twitter_json() {
    $username       = 'foobar';
    $count          =   5;
    $json_folder    = 'json';
    $pull_time      =   1;

    $settings = array(
        'oauth_access_token'        => "",
        'oauth_access_token_secret' => "",
        'consumer_key'              => "",
        'consumer_secret'           => ""
    );

    if (is_dir($json_folder)) :
        echo "Directory $json_folder exists
";
    else :
        echo "Did NOT find directory $json_folder
";
        exit();
    endif;

    $file_loc   = "json/twitter.json";
    $cachetime  = @filemtime($file_loc);
    $diff       = time() - $cachetime;

    if ($diff <= 60 * $pull_time) :
        $results    = json_decode(file_get_contents($file_loc));
    else :
        $url = 'https://api.twitter.com/1.1/statuses/user_timeline.json';
        $getfield = '?screen_name=' . $username . '&count=' . $count;
        $requestMethod = 'GET';
        $twitter = new TwitterAPIExchange($settings);
        $results = json_decode(
            $twitter->setGetfield($getfield)
                    ->buildOauth($url, $requestMethod)
                    ->performRequest(), true
        );

        $i = 0;
        if (isset( $results['error'])) :
            echo 'Issue found in request';
            exit();
        else :
            foreach($results as $tweet) :
                $tweets[$i]['created']      = $tweet['created_at'];
                $tweets[$i]['text']         = $tweet['text'];
                $tweets[$i]['username']     = $tweet['user']['name'];
                $tweets[$i]['screenname']   = $tweet['user']['screen_name'];
                $tweets[$i]['url']          = $tweet['user']['url'];
                $i++;
            endforeach;
        endif;

        $jsonified = json_encode($tweets);
        $jsoncache = $file_loc;
        $fp = fopen($jsoncache, "w");
        fwrite($fp, $jsonified);
        fclose($fp);
    endif;

}

Are there any improvements I can make to my code?  Eventually I will implement this into a WordPress Plugin.  I am curious about any better approaches than fwrite and fclose so if there is a better way to write to a JSON file that would be helpful.

This is a partial review.

$file_loc and $json_folder

Use $json_folder in the construction of $file_loc. If you decide to use a different folder, you need change two variables.

$file_loc = "{$json_folder}/twitter.json";

Naming of function

You named your function get_twitter_json()... which... does essentially nothing. It does not return json like the function name suggests. It doesn't even return the twitter result. Instead this seems a partial implementation of a processing function. If it is supposed to return tweets, then name at least something like get_tweets().

Whitespace

Use consistent whitespace. Trying to align all assignments to the same indentation will create problems sooner than later. Changing one assignment will mess up the rest. Multi-line assignments will look even weirder. I would recommend 1 space before the assignment operator (=) and one space after it.

$results and $tweets

The if-else-statement should have some code behind it. The if-statement loads the decoded json-file in $results, while the else-statement loads it in $tweets. Use one of them.

While you are at it, while php allows you to use $tweets undefined like that, it doesn't make for clear code.

foreach with a counter

You are using a foreach loop with a custom counter. That... defeats the purpose of a foreach loop. Either use a for loop and use the exposed counter from that, or use the append syntax to don't need a counter.

foreach($results as $tweet) :
    $new_tweet = array();
    $new_tweet['created']      = $tweet['created_at'];
    $new_tweet['text']         = $tweet['text'];
    $new_tweet['username']     = $tweet['user']['name'];
    $new_tweet['screenname']   = $tweet['user']['screen_name'];
    $new_tweet['url']          = $tweet['user']['url'];
    $tweets[] = $new_tweet;
endforeach;


Basic iterator supporting vector implementation On the fly, I just implemented this iterator supporting vector class to understand how range-based for loop works.
It's really basic but let me know what can be improved.

Vector.h

#ifndef VECTOR_H
#define VECTOR_H

#include <stdexcept>

template <typename T>
class Vector
{
public:
    /* The iterator */
    class Iterator
    {
    public:
        Iterator(const Vector<T> *vector, int nIndex);
        const T &operator*() const;
        Iterator &operator++();
        bool operator!=(const Iterator &other) const;

    private:
        const Vector<T> *m_pVector;
        int m_nIndex = -1;
    };

public:
    // constructors
    Vector() = default;
    explicit Vector(int nSize);

    ~Vector();

    void insert(const T &value);

    int size() const;
    const T &operator[](int nIndex) const;

    Iterator begin() const;
    Iterator end() const;

private:
    T *m_pData = nullptr;
    int m_nSize = 0;
    int m_nCapacity = 0;
};

/*
 * Vector methods
 **/
template <typename T>
Vector<T>::Vector(int nCapacity)
{
    m_nCapacity = nCapacity;
    m_pData = new T[m_nCapacity];
}

template <typename T>
Vector<T>::~Vector()
{
    delete m_pData;
    m_nSize = 0;
    m_nCapacity = 0;
}

template <typename T>
void Vector<T>::insert(const T &value)
{
    if (m_nSize == m_nCapacity)
    {
        if (m_nCapacity == 0)
            m_nCapacity = 1;

        m_nCapacity *= 2;

        // allocate 2x larger memory
        auto pNewMemory = new T[m_nCapacity];

        // copy data to there
        for (auto idx = 0; idx < m_nSize; ++idx)
            pNewMemory[idx] = m_pData[idx];

        delete m_pData;
        m_pData = pNewMemory;
    }

    // insert the new element
    m_pData[m_nSize] = value;
    ++m_nSize;
}

template <typename T>
int Vector<T>::size() const
{
    return m_nSize;
}

template <typename T>
const T &Vector<T>::operator[](int nIndex) const
{
    if (nIndex >= m_nSize)
        throw std::exception("Index out of range");

    return m_pData[nIndex];
}

template <typename T>
typename Vector<T>::Iterator Vector<T>::begin() const
{
    return Vector<T>::Iterator{ this, 0 };
}

template <typename T>
typename Vector<T>::Iterator Vector<T>::end() const
{
    return Vector<T>::Iterator{ this, m_nSize };
}

/*
 * Iterator methods
 **/
template <typename T>
Vector<T>::Iterator::Iterator(const Vector<T> *pVector, int nIndex)
    : m_pVector(pVector)
    , m_nIndex(nIndex)
{
}

template <typename T>
const T &Vector<T>::Iterator::operator*() const
{
    return m_pVector->operator[](m_nIndex);
}

template <typename T>
typename Vector<T>::Iterator &Vector<T>::Iterator::operator++()
{
    ++m_nIndex;
    return *this;
}

template <typename T>
bool Vector<T>::Iterator::operator!=(const Vector<T>::Iterator &other) const
{
    return m_nIndex != other.m_nIndex;
}

#endif // !VECTOR_H

main.cpp

#include <iostream>

#include "Vector.h"

int main()
{
    try
    {
        Vector<int> vector;
        vector.insert(8);
        vector.insert(3);
        vector.insert(1);
        vector.insert(7);
        vector.insert(2);

        for (auto i : vector)
            std::cout << i << std::endl;
    }
    catch (const std::exception &e)
    {
        std::cerr << e.what() << std::endl;
    }

    std::cin.get();
    return 0;
}

The range-based for loop
This works via the begin() and end() functions.
So for any class X if there are stand alone functions begin(X&) and end(X&) the range based for will work. Normally this will default to the std::begin() and std::end() but if you have written explicit versions of these functions in the same namespace as X these will be used.
Example:
#include <iostream>

namespace Y
{
    class X
    {
        public:
            char data[14] = "This is text";
    };

    char* begin(X& obj) {
        return obj.data;
    }

    char* end(X& obj) {
        return obj.data + 10;
    }
}

int main()
{
    Y::X   x;
    for(auto const& a: x) {
        std::cout << a;
    }
}

OK. So what happens if you don't write your own begin() and end() functions?
In this case it will use the standard versions of these functions. std::begin() which will call the begin() method on the object and std::end() which will call the end() method on the object.
Example 2:
#include <iostream>

namespace Y
{
    class X
    {
            char data[14] = "This is text";
        public:
            char* begin() {
                return data;
            }

            char* end() {
                return data + 10;
            }
    };
}

int main()
{
    Y::X   x;
    for(auto const& a: x) {
        std::cout << a;
    }
}

How to think of range based for
You can think of ranged based for as a compiler shortcut.
for(<Type> <Var> : <Obj>) {

}

Is just short hand for:
{
    using std::begin;
    using std::end;

    auto __end = end(<Obj>);

    for(auto __loop = begin(<Obj>); __loop != __end; ++__loop) {
        <Type> <Var> = *__loop;

    }
}

Implementation of Iterators
The implementation of an iterator is based on the "Iterator Concept". There are actually five types of iterator in C++. But since you are using vector as your example implementation we should consider the "Random Access Iterator Concept".
To qualify as a random access iterator you have to uphold a specific contract. Here is some documentation on it.
https://en.cppreference.com/w/cpp/named_req/RandomAccessIterator
Not only does the iterator need to define certain methods (with specific requirements on how they work together). But your type also needs to define certain types via std::iterator_traits.
Types
You need to define the following types:

value_type:
When the iterator is de-referenced this is the type you get.
difference_type:
When you subtract two iterators this is the type of the result
pointer:
A type that points at type value_type
reference:
A type that can be used as a reference to a value of type value_type
iterator_category:
A type that represents the category of the iterator.

So you could have written your own specialization of the iterator_traits class
namespace std
{
    template<typename T>
    class iterator_traits<Vector<T>::Iterator>
    {
        using value_type = T;
        using difference_type = std::ptrdiff_t;
        using pointer = T*;
        using reference  = T&;
        using iteratory_category = std::random_access_iterator_tag;
    };
}

But that is a little bit like actual hard work. The designers of the STL actually got smart and the default implementation of this type refer back to the iterator type to get these types. So you can define these types in your iterator (and not define a specialization of the iterator_traits class).
Rather do this:
template<typename T>
class Vector
{
    class Iterator
    {
        using value_type = T;
        using difference_type = std::ptrdiff_t;
        using pointer = T*;
        using reference  = T&;
        using iterator_category = std::random_access_iterator_tag;
    };
    ....
}

Note: If you follow the advice above and use a pointer as your iterator. The iterator_traits for pointer types are well defined in the standard and no work is required.
Properties of an Iterator
The basic properties of an iterator are:

Default Constructible    *** Currently not supported.
Copy Constructible
Copy Assignable
Destructible
Swappable      *** Currently not supported.
Equality Comparable  *** Currently not supported (you need ==)
objects returned via de-referencing need to be mutable (not supported)

Supported actions for a random access iterator:
*i                  // returns the current value
*i  = v             // Assign value `v` to the value dererenced.
++i                 // increments the iterator (returns a reference)
--i                 // See above
i++                 // increments the iterator (but returns a reference to the original)
i--                 // See above
*i++                // returns the current value and increments the iterator
*i--                // See above

i += n              // Moves the iterator forward
i -= n              // Same as last one
i + n               // Creates a new iterator moved forward from i
n + i               // Same as last one
i - n               // Same as last one

i1 - i2             // returns the distance between the iterators.

i[n]                // returns the item n steps forward from this iterator.
i->m                // Access the member `m` referenced by i

i1 <  i2            // Compare iterators
i1 >  i2
i1 <= i2
i1 >= i2
i1 == i2
i1 != i2

OK So I have dumped an all lot of requirements on iterators here. But it is not as bad as it seems. Have a look at an iterator I built for a stack overflow question.
https://stackoverflow.com/a/1120224/14065
This iterator is only a forward iterator (so it has fewer requirements). But it shows you how to implement the main basic requirements very quickly. The rest should not be too hard to implement for random accesses iterator.
Got bored so here:
class Iterator
{
public:
    using value_type = T;
    using pointer    = T*;
    using reference  = T&;
    using difference_type = std::ptrdiff_t;
    using iterator_category = std::random_access_iterator_tag;

    Iterator():                    v(nullptr), i(0) {}
    Iterator(Vector<T>* v, int i): v(v),       i(i) {}
    // Default Copy/Move Are Fine.
    // Default Destructor fine.

    reference       operator*()             {return (*v)[i];}
    const reference operator*()       const {return (*v)[i];}
    pointer         operator->()            {return &((*v)[i]);}
    const pointer   operator->()      const {return &((*v)[i]);}
    reference       operator[](int m)       {return (*v)[i + m];}
    const reference operator[](int m) const {return (*v)[i + m];}

    Iterator& operator++()       {++i;return *this;}
    Iterator& operator--()       {--i;return *this;}
    Iterator  operator++(int)    {Iterator r(*this);++i;return r;}
    Iterator  operator--(int)    {Iterator r(*this);--i;return r;}

    Iterator& operator+=(int n)  {i += n;return *this;}
    Iterator& operator-=(int n)  {i -= n;return *this;}

    Iterator operator+(int n)   const {Iterator r(*this);return r += n;}
    Iterator operator-(int n)   const {Iterator r(*this);return r -= n;}

    difference_type operator-(Iterator const& r) const {return i - r.i;}

    // Note: comparing iterator from different containers
    //       is undefined behavior so we don't need to check
    //       if they are the same container.
    bool operator<(Iterator const& r)  const {return i <  r.i;}
    bool operator<=(Iterator const& r) const {return i <= r.i;}
    bool operator>(Iterator const& r)  const {return i >  r.i;}
    bool operator>=(Iterator const& r) const {return i >= r.i;}
    bool operator!=(const Iterator &r) const {return i != r.i;}
    bool operator==(const Iterator &r) const {return i == r.i;}

private:
    Vector<T>* v;
    int        i;
};

The Container.
Since you were asking about Iterators I will not give input on the Vector just yet (but lets say it needs work).
I have written a couple of articles on implementing the vector that you can have a look at.
http://lokiastari.com/series/


One class unit-tests Its my first attempt to unit-test in Js. Not yet TDD.
What I can change?

EventsPersistancyService.js:

var Q = require('q');

var EventsPersistancyService = {
  accept: function acceptService(msg) {

    var worker_id = WorkerCacheService.get('some login');
    var app_category = AppCategoryService.get('some');

    var p = Q.all([worker_id, app_category]).then(function () {
      var content = msg.content.toString();
      content = JSON.parse(content);
      return Event.create(content);
    });

    return p;
  }
}

module.exports = EventsPersistancyService;

Unit tests:

"use strict";

var assert = require('assert');
var chai = require('chai');
var expect = chai.expect;
var chaiAsPromised = require('chai-as-promised');
var sinonChai = require("sinon-chai");
chai.use(chaiAsPromised);
chai.use(sinonChai);
var sinon = require('sinon');
var Q = require('q');

var TPL = {
};

describe('EventsPersistancyService', function () {

  describe('#accept', function () {

    var s1, s2, s3, s4;

    var message = {
      fields: null,
      properties: null,
      content: new Buffer(JSON.stringify(TPL))
    };

    beforeEach(function () {
      s1 = sinon.stub(WorkerCacheService, 'get').returns(Q.resolve(""));
      s2 = sinon.stub(AppCategoryService, 'get').returns(Q.resolve(""));
      s3 = sinon.stub(Event, 'create').returns(Q.resolve(1));
    });

    afterEach(function () {
      sinon.sandbox.restore();
      s1.restore();
      s2.restore();
      s3.restore();
    });

    it('should return error when accept param is null', function () {
      var res = EventsPersistancyService.accept(null);
      return expect(res).to.be.rejected;
    });

    it('should return resolve promise on non error situation', function () {
      var res = EventsPersistancyService.accept(message);
      return expect(res).to.be.not.rejected;
    });

    it('should call WorkerCacheService to get worker_id', function () {
      EventsPersistancyService.accept(message);
      return expect(s1).to.have.been.calledOnce;
    });

    it('should call AppCategoryService to get app_category', function () {
      EventsPersistancyService.accept(message).then();
      return expect(s2).to.have.been.calledOnce;
    });

    it('should call Event.create when all if ok', function () {
      EventsPersistancyService.accept(message).then(function () {
        expect(s4).to.have.been.calledOnce;
      }, function () {
        expect(s4).to.have.been.calledOnce;
      });
    });

    it('should reject when Event.create reject', function () {
      s3.restore();
      s3 = sinon.stub(Event, 'create').returns(Q.reject(1));
      var p = EventsPersistancyService.accept(message);
      return expect(p).to.be.rejected;
    });

    it('should reject when Event.create throw', function () {
      var p = EventsPersistancyService.accept({});
      return expect(p).to.be.rejected;
    });

    it('should reject when WorkerCacheService.get fails', function () {
      s1.restore();
      s1 = sinon.stub(WorkerCacheService, 'get').returns(Q.reject(''));
      var p = EventsPersistancyService.accept(message);
      return expect(p).to.be.rejected;
    });

    it('should reject when AppCategoryService.get fails', function () {
      s2.restore();
      s2 = sinon.stub(AppCategoryService, 'get').returns(Q.reject(''));
      var p = EventsPersistancyService.accept(message);
      expect(p).to.be.rejected;
    });

  });
});

Use sinon-promises to simplify this:

'use strict';
const chai = require('chai');
const sinon = require('sinon');
const sinonChai = require('sinon-chai');
const chaiAsPromised = require('chai-as-promised');
const sinonStubPromise = require('./..');

sinonStubPromise(sinon);

chai.use(chaiAsPromised);
chai.use(sinonChai);

const expect = chai.expect;
let stub;

describe('stubbing promises', () => {
  beforeEach(() => {
    stub = sinon.stub();
  });

  it('should not throw an error when invoked', () => {
    stub.returnsPromise();
  });

  it('should allow defining a resolve value', () => {
    stub.returnsPromise();
    stub.resolves(1);

    return expect(stub()).to.eventually.equal(1);
  });

  it('should allow defining a reject value', () => {
    stub.returnsPromise();
    const err = new Error();
    stub.rejects(err);

    return expect(stub()).to.eventually.be.rejectedWith(err);
  });

  it('should allow overwriting a resolves with a reject', () => {
    stub.returnsPromise();
    const err = new Error();
    stub.resolves(1);
    stub.rejects(err);

    return expect(stub()).to.eventually.be.rejectedWith(err);
  });

  it('should allow overwriting a reject with a resolves', () => {
    stub.returnsPromise();
    const err = new Error();
    stub.rejects(err);
    stub.resolves(1);

    return expect(stub()).to.eventually.equal(1);
  });

  it('should be last-write-wins', () => {
    stub.returnsPromise();
    stub.resolves(1);
    stub.resolves(2);
    stub.resolves(3);

    return expect(stub()).to.eventually.equal(3);
  });
});

References

sinon-promises


jQuery substitute for multiple hovers I would like to find a more efficienct way to use jQuery for my question and answer page.

Here is the code which I want to change. If a .q_container is clicked, I want its corresponding answer div will slide down.

$(document).ready(function() {
    $('#qc1').hover(
        function () {
            $('#a1').slideDown('fast');
        },
        function () {
            $('#a1').slideUp('fast');
        }
    );
    $("#qc2").hover(
        function () {
            $('#a2').slideDown('fast');
        },
        function () {
            $('#a2').slideUp('fast');
        }
    );
    $("#qc3").hover(
        function () {
            $('#a3').slideDown('fast');
        },
        function () {
            $('#a3').slideUp('fast');
        }
    );
});

All of my Code:

            What is a question?

            It is a way to discover something.

            What is a question2?

            It is a way to discover something2.

            What is a question3?

            It is a way to discover something3.

Working Demo http://jsfiddle.net/jEjYp/2/

Rest feel free to play around with the code or demo.

Hope it fits your cause. :)

code

$(document).ready(function() {
    $('#qc1,#qc2,#qc3').hover(
        function () {
            $(this).find('.answer').slideDown('fast');
        },
        function () {
            $(this).find('.answer').slideUp('fast');
        }
    );

});


Beginner Hangman Game Hey guys I am trying to improve my Python Programming skills, I tried to build a hangman code.
I know it is not perfect but I works. I am looking for some hints or advice to improve this script.

    txt_file = open('dictionnary.txt','r') # Open the file with read option

    words = txt_file.readlines() # Read all the lines

    words =[word.replace('\n','') for word in words] # Remove '\n' and words is now a list

    import random

    random_number = random.randint(11,len(words)-1) # Choose a random number between 11 (to avoid words like 'aa', 'aaargh',etc) and the lenght of words

    word_to_guess = words[random_number]

    user_guesses=['#' for i in word_to_guess]

# Function to check if the input letter is in the world to guess if yes replace '#' with the input letter

def check_index_and_replace(letter):
    letter_index=[i[0] for i in enumerate(word_to_guess) if i[1]==letter]
    for i in letter_index:
        user_guesses[i] = user_guesses[i].replace('#',letter)
    return(user_guesses)

# Function to tell how many letter to guess left

def letterleft(user_guesses):
    return(user_guesses.count('#'))

# My core code (input and prints)

tries = int(input('How many tries you want ? '))
test = 0
while test < tries+1:
    letter = input('Try a letter ')
    print(check_index_and_replace(letter))
    print(letterleft(user_guesses),'letter left to guess !')
    test=test+1
print(word_to_guess)

Getting lines from a file

You can use simple list comprehension to get all the lines from a file.

words = [word for word in open("dictionary.txt", "r")]

However, this does not ensure the file will be closed. To be safe, I would do this:

with open("dictionary.txt", "r") as file:
    words = [word for word in file]

The with ensures the file will be closed once you're done working with the file.

Globals

With a small program like this, globals can be quite helpful since you don't have to pass word_to_guess through functions. However, as you begin to develop more complicated programs, you should be mindful and careful about your globals "leaking" into other parts of your programs, should you be using multiple files.

Random choices

Instead of generating a random number between the min and max of the list, use random.choice(...) to chose a random word from the dictionary. And if you're worried about the beginning of the alphabet, you can create a buffer variable and splice the list so the beginning of the alphabet is removed.

# The first 11 words will be removed from the list #
buffer = 11
word_to_guess = random.choice(words[buffer:])

check_index_and_replace

Instead of casting replace on each index, you can use an if statement to make sure the letter is equal to the letter in word_to_guess, and in the same position. If it is, then assign that letter to the position in the list.

from typing import List

def check_and_replace(letter: str) -> List[str]:
    """
    For each character in the word, if that character
    is equal to the passed letter, then the position in
    user_guesses is changed to that letter.
    """
    for idx, char in enumerate(word_to_guess):
        if char == letter:
            user_guesses[idx] = letter
    return user_guesses

Type Hints

These help yourself and other people looking at your code understand what types of variables are being passed and returned to/from functions. As above, the function accepts a str for letter, and returns a list of strings.

lettersleft

If you have a function that only has one line, most of the time you can delete the function and put that line where the function is called. And since this function utilizes a built-in function to count the occurrences of # in the list, this function doesn't need to be written.

The main body

Instead of keeping track for each try, use a for loop and only run as many times as the user inputed. If the user enters 4, the loop only runs four times.

When I first played this game, it was impossible to win. I could guess the word, but the game wouldn't end. A quick fix is to check if the number of letters left is 0. If it is, display a game won message and exit the program. If it isn't, print how many are left and go through the loop again.

All in all, your program would look something like this:

import random
from typing import List

with open("dictionary.txt", "r") as file:
    words = [word for word in file]
buffer = 11
word_to_guess = random.choice(words[buffer:])
user_guesses = ['#' for _ in word_to_guess]

def check_and_replace(letter: str) -> List[str]:
    """
    For each character in the word, if that character
    is equal to the passed letter, then the position in
    user_guesses is changed to that letter.
    """
    for idx, char in enumerate(word_to_guess):
        if char == letter:
            user_guesses[idx] = letter
    return user_guesses

def main():
    tries = int(input('How many tries you want? '))
    for _ in range(tries):
        letter = input('Try a letter ')
        print(check_and_replace(letter))
        letters_left = user_guesses.count("#")
        if letters_left == 0:
            print("You guessed the word!")
            quit()
        else:
            print(letters_left, "letters remaining!")
    print("The word was", word_to_guess)

if __name__ == '__main__':
    main()


Collecting all relevant data from xlsm files Is there a way to speed this guy up? The code is designed to go into a directory, open up all of the .xlsm files in the folder, and copy specific data into a target file. This code works as is, but it is extremely slow. Is there a way to speed it up?

Const FOLDER_PATH = "C:\Users\maxd\OneDrive - Nortek, Inc\Coil Test Data\coils_35_and_36\36\WET\Testing\"  'REMEMBER END BACKSLASH

Sub ImportWorksheets()
   '=============================================
   'Process all Excel files in specified folder
   '=============================================
   Dim sFile As String           'file to process
   Dim wsTarget As Worksheet
   Dim wbSource As Workbook
   Dim wsSource As Worksheet
   Dim rowTarget As Long         'output row

   rowTarget = 11

   'check the folder exists
   If Not FileFolderExists(FOLDER_PATH) Then
      MsgBox "Specified folder does not exist, exiting!"
      Exit Sub
   End If

   'reset application settings in event of error
   On Error GoTo errHandler
   Application.ScreenUpdating = False

   'set up the target worksheet
   Set wsTarget = Sheets("Sheet1")

   'loop through the Excel files in the folder
   sFile = Dir(FOLDER_PATH & "*.xlsm*")
   Do Until sFile = ""

      'open the source file and set the source worksheet - ASSUMED WORKSHEET(1)
      Set wbSource = Workbooks.Open(FOLDER_PATH & sFile)
      Set wsSource = wbSource.Worksheets("Report")

      'import the data
      With wsTarget
         .Range("A" & rowTarget).Value = wsSource.Range("E9").Value 'Year
         .Range("B" & rowTarget).Value = wsSource.Range("D30").Value 'CFM
         '.Range("D" & rowTarget).Value = wsSource.Range("D30/(30*30/144)").Value 'Face Velocity
         .Range("E" & rowTarget).Value = wsSource.Range("D36").Value 'AVG Capacity
         .Range("F" & rowTarget).Value = wsSource.Range("D29").Value 'APD
         .Range("G" & rowTarget).Value = wsSource.Range("D34").Value 'WPD
         .Range("H" & rowTarget).Value = wsSource.Range("D22").Value 'Inlet db
         .Range("I" & rowTarget).Value = wsSource.Range("D23").Value 'Inlet  wb
         '.Range("J" & rowTarget).Value = wsSource.Range("").Value 'Inlet dp
         .Range("K" & rowTarget).Value = wsSource.Range("L16").Value 'Inlet WT
         .Range("L" & rowTarget).Value = wsSource.Range("L17").Value 'Outlet WT
         .Range("M" & rowTarget).Value = wsSource.Range("L22").Value 'Heat Balance

         'optional source filename in the last column
         .Range("N" & rowTarget).Value = sFile
      End With

      'close the source workbook, increment the output row and get the next file
      wbSource.Close SaveChanges:=False
      rowTarget = rowTarget + 1
      sFile = Dir()
   Loop

'Loop for face velocity
  Dim r As Integer
  Dim i As Integer

i = Cells(Rows.Count, 1).End(xlUp).Row
    For r = 11 To i
        Cells(r, 4) = "=RC[-2]/(30*30/144)"
    Next r

errHandler:
   On Error Resume Next
   Application.ScreenUpdating = True

   'tidy up
   Set wsSource = Nothing
   Set wbSource = Nothing
   Set wsTarget = Nothing

End Sub

Private Function FileFolderExists(strPath As String) As Boolean
    If Not Dir(strPath, vbDirectory) = vbNullString Then FileFolderExists = True
End Function

It's been a while since I wrote VBA, but I seem to remember Application.Calculation = xlCalculationManual often sped up my programs considerably. This prevents formulae from being evaluated while you're updating cell values, and can be reversed via Application.Calculation = xlCalculationAutomatic.


Get JSON script code with pushing to heatmap I have a script that makes a database call and then filters the results. After filtering it pushes it to an array and shows it on a map.

Here is my function:

function getDriving() {
    pointArray.length = 0;
    markers.length = 0;
    var todayval = new Date();

    var url = $('#map').data('request-url2');
    $.getJSON(url,
        function (data) {
            $.each(data,
                function (i, item) {

                    markers.push({
                        'location': new google.maps.LatLng(item.Latitude, item.Longitude),
                        'map': map,
                        'weight': item.Speed,
                        'radius': 10,
                        'date': item.CurrentDateTime,
                        'imei': item.Imei
                    });
                });
            imei = parseInt($('#selectVehicles').val());

            var startValue = $('#startDate').val().split("/");
            var endValue = $('#endDate').val().split("/");
            var valDateStart = new Date(startValue[2], startValue[1] - 1, startValue[0]);
            var valDateEnd = new Date(endValue[2], endValue[1] - 1, endValue[0]);

            filtered = markers.filter(function (marker) {
                var getDate = marker.date.match(/\d/g).join('');
                var markerDate = new Date(parseFloat(getDate));
                return ((markerDate >= valDateStart && markerDate <= valDateEnd) && marker.imei === imei);
            });
            var pointArray = new google.maps.MVCArray(filtered);

            heatmap = new google.maps.visualization.HeatmapLayer({
                data: pointArray
            });
            heatmap.setMap(map);
        });
};

It works well, but I want to know if everything is okay with the code or how I can improve it.

Scope of Variables

Unless imei is used outside of the code in your post (which would be grounds for making it off-topic), then you should declare it as local to the function using var (or let if you want it to have block scope. The same is true for filtered and heatmap. That way those variables won't be global, which could cause confusion if you did use var to declare a variable inside a function but not before that.

For more information on that topic, see the section Keep your scopes close and your scope even closer on this page.

Accessing DOM elements multiple times via $()

DOM lookups are slow. It is wise to cache them once in a variable and then refer to the variable when needed. This is also mentioned in the aforementioned article under the section Cache DOM Lookups

Those lookups could be added above the function (possibly assigned in a DOM ready function like jQuery's $(function(){}), though it is argued that isn't necessary with modern browsers):

var map = $('#map');
var selectedVehicles = $('#selectVehicles');
var startDate = $('#startDate');
var endDate = $('#endDate');

Then those variables can be used later on when the function is run- for example:

var url = $('#map').data('request-url2');

Can be updated to use map:

var url = map.data('request-url2');

And map could also be used when setting up the Google Map instance, since presumably that is the same element used for the map.

Using parseInt() without a radix

If you are going to use parseInt(), it is wise to specify the radix using the second parameter - unless you are using a unique number system like hexidecimal, octal, etc. then specify 10 for decimal numbers.

  Always specify this parameter to eliminate reader confusion and to guarantee predictable behavior. Different implementations produce different results when a radix is not specified, usually defaulting the value to 10.1

So I would update the line:

imei = parseInt($('#selectVehicles').val());

To pass the radix 10:

imei = parseInt($('#selectVehicles').val(), 10);

That way, if a value like 022 is entered, it isn't interpreted as the octal value (I.e. Decimal number 18).

1https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/parseInt#Parameters


Print the sum of corresponding elements in two arrays In this exercise, I'm supposed to sum up elements from 2 arrays, then display the answer on one line with spaces in between. I did it and my answer got accepted as correct, but I'm wondering if there is a better way to do this since I feel my way was more complicated than it needed to be.

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

public class Main {

public static void main(String[] args) {
    int n = 11;

    int[] firstArray = {811594, 993574, 299729,559604,161945,969851,588210,
            692459, 28350,43017,797855};
    int[] secondArray = {725888, 233750,191700,944750,380402,319860,766872,764921,
            330218,906679, 65309};

    List<Object> total = new ArrayList<>();

    for(int i = 0; i < n; i++){
        int sum = firstArray[i] + secondArray[i];
        total.add(sum);
    }

    Integer[] sumArray = total.toArray(new Integer[total.size()]);
    for(int i = 0; i < n; i++) {
        System.out.print(sumArray[i] + " ");
    }

}
}

You don't need to store the intermediate result.
You don't need to give the length of the arrays.
This assumes that the arrays are allways of equal length.

for(int i = 0; i < firstArray.length; i++){
    System.out.print((firstArray[i] + secondArray[i]) + " ");
}


Validate the set accessors in a simple Triangle class I have the following sample Triangle class and I want to validate the set accessors' values, obviously I cannot have a zero side or height Triangle. But while I am writing the validation rules for each property I found myself repeating the same code over and over again. Is there a more concise way to write these properties or to group them together? You can assume how newbie to C# I am.

    //  The Shape class
    abstract class Shape
    {
        // Shape's Methods
        abstract public double GetArea();
        abstract public double GetPerimeter();
    }

    //  The Triangle class
    class Triangle : Shape
    {
        private double sideA;
        private double sideB;
        private double baseT;
        private double height;

        public double SideA
        {
            get { return sideA; }
            set
            {
                if (value > 0)
                {
                    sideA = value;
                }
            }
        }

        public double SideB
        {
            get { return sideB; }
            set
            {
                if (value > 0)
                {
                    sideB = value;
                }
            }
        }

        public double BaseT
        {
            get { return baseT; }
            set
            {
                if (value > 0)
                {
                    baseT = value;
                }
            }
        }

        public double Height
        {
            get { return height; }
            set
            {
                if (value > 0)
                {
                    height = value;
                }
            }
        }

        // Triangle's constructor.
        public Triangle(double sideA, double sideB, double baseT, double height)
        {
            this.sideA = sideA;
            this.sideB = sideB;
            this.baseT = baseT;
            this.height = height;
        }

        // Get Triangle's area.
        public override double GetArea()
        {
            return (this.baseT * this.height) / 2;
        }

        // Get Triangle's perimeter.
        public override double GetPerimeter()
        {
            return sideA + sideB + baseT;
        }
    }

Assuming you'll have other shapes that would have the same requirement, one option you have is to have a class Side inside the class Shape, and put the validation in there.  Now every shape can declare as many sides as needed and each will be validated.  Some thing like this should work:

private class Side
{
    double length;
    public double Length
   {
        get{return length;}
        set
        {
            if(value > 0)
            {
                 length = value;
            }
            else
            {
                 throw new ArgumentException("Value must be greater than 0");
            }
         }
    }
    Side(double _length)
    {
        Length = _length;
    }
}

In regards to the height, instead of having it passed to the constructor, calculate the height from the length of the sides, Heron's formula should work.


Assembly: Sum up the single bytes of a 32bit-register to a checksum Exercise description:

"Write a program that takes a double word (4 bytes) as an argument, and then adds all the 4 bytes. It returns the sum as output.
Note that all the bytes are considered to be of unsigned value.

Example: For the number 03ff0103 the program will calculate 0x03 + 0xff + 0x01 + 0x3 = 0x106, and the output will be 0x106

HINT: Use division to get to the values of the highest two bytes."

Full description on GitHub: xorpd

The code I've  written:

format PE console
entry start

include 'win32a.inc'

; ===============================================
section '.text' code readable executable

start:
    mov     eax,    0x01020304

    xor     ebp,    ebp
process_eax:
    movzx   ebx,    al
    add     ecx,    ebx
    movzx   ebx,    ah
    add     ecx,    ebx
    cmp     ebp,    0x1
    je      print_result
    xor     edx,    edx
    mov     ebx,    0xffff
    div     ebx
    mov     ebp,    0x1
    jmp     process_eax
print_result:
    mov     eax,    ecx
    call    print_eax   ; Provided by the teacher. Prints eax to the console.

exitProgram:
    ; Exit the process:
    push    0
    call    [ExitProcess]

include 'training.inc'

I think it works. I've tried it with different values and the sums were correct.

Screenshot with the output of the code above (with 0x01020304 as the hardcoded value).

But it's surely not the most efficient way to solve the exercise.

Since you're still learning, I won't cheat you out of the opportunity to discover for yourself, but I will offer some words of advice on how you can improve your program.

Minimize register use

The current code uses eax, ebx, ecx, edx and ebp.  One of the most important things for an assembly language programmer is to use registers efficiently and effectively.  This particular task can easily be done with just two registers.

Prefer shift to division

As alluded to in a comment, shift instructions are typically much faster to execute than divide instructions.  For that reason, in tasks like this, it's much more common to see a shift than a divide.

Avoid loops

Branching tends to be computationally disruptive for processors.  While modern desktop machines tend to compensate for this via speculative execution and large cache sizes, code often runs faster if loops and branches are avoided entirely.  This can confer other benefits such as more predictable running time which can be important for the scheduling of Real Time Operating Systems (RTOS) and in some kinds of cryptographic code to provide some resistance to side channel attacks.


Invoicing - calling multiple pages of Api and concatinating the results I have the following code: It works completely as expected, but I feel there is a better way to refactor it out.

The flow is as follows:

Call the end point with 1 page
Get the total number of results.
If total count of items more than itemsPerPage.
Call the API more times till results from all pages are returned, join them together as single invoice list
Append with initial request and send back.

Do I need GetAllInvoices function? Reason it is kept there is because I believe the responsibility of calling each pages of API does not belong to main function.

public async Task<IEnumerable<Invoice>> GetAllInvoicesAsync()
{
    const string endPoint = @"foo/{0}/invoices?pageNum={1}&itemsPerPage={2}";
    const int itemsPerPage = xxx;
    InvoiceCollection response = await _apiClient
        .GetAsync<InvoiceCollection>(string.Format(endPoint, _apiClient.OrgId, 1, itemsPerPage));

    if (response?.TotalCount > itemsPerPage)
    {
        var allInvoices = (await GetAllInvoices(endPoint,response.TotalCount, itemsPerPage))
            .SelectMany(i => i.Invoices?? Enumerable.Empty<Invoice>());
        response.Invoices = (response.Invoices ?? Enumerable.Empty<Invoice>()).Concat(allInvoices);
    }

    return response?.Invoices;
}

private async Task<List<InvoiceCollection>> GetAllInvoices(string endPoint,int totalCount, int itemsPerPage)
{
    var totalPages = (int)Math.Ceiling(totalCount / (double)itemsPerPage);
    var tasks = new List<Task<InvoiceCollection>>();

    for (int currentPage = 2; currentPage<=totalPages;currentPage++)
    {
        tasks.Add(_apiClient
            .GetAsync<InvoiceCollection>(string.Format(endPoint, _apiClient.OrgId, currentPage, itemsPerPage)));
    }
    var response = new List<InvoiceCollection>(await Task.WhenAll(tasks));
    return response;
}

public class InvoiceCollection
 {
     [JsonProperty("foo")]
     public IEnumerable<Invoice> Invoices { get; set; }
     public int TotalCount { get; set; }
 }

When you have an assumption of getting a mix of return one or more values. You need to favor collection over single value return; because treating it as a collection would make things easier to deal with.

So, you can get rid of GetAllInvoices and adjust GetAllInvoicesAsync to something like this :

public async Task<IEnumerable<Invoice>> GetAllInvoicesAsync()
{
    const string endPoint = @"foo/{0}/invoices?pageNum={1}&itemsPerPage={2}";
    const int itemsPerPage = xxx;
    int pageNumber = 1;
    int iterationCount = 0;

    var tasks = new List<Task<InvoiceCollection>>();

    var isFirstRound = true;

    do
    {
        var response = await _apiClient.GetAsync<InvoiceCollection>(string.Format(endPoint, _apiClient.OrgId, pageNumber, itemsPerPage));

        if(isFirstRound)
        {
            iterationCount = response?.TotalCount <= itemsPerPage ? response?.TotalCount : (int) Math.Ceiling(totalPageItems / (double)itemsPerPage);
            isFirstRound = false;
        }

        tasks.Add(response);

        pageNumber++;
    }
    while(pageNumber <= iterationCount)

    var invoiceResponseList = new List<InvoiceCollection>(await Task.WhenAll(tasks));

    var invoiceCollection = invoiceResponseList.SelectMany(i => i.Invoices ?? Enumerable.Empty<Invoice>());

    return invoiceCollection?.Invoices;
}


Determine when a task will be completed A friend of mine told me he had the following exercises at his interview :

  You need to determine when a task will be completed if you are given :

  A date when the task is started
  The duration of the task in hours (how much hours it should take in total)
  The starting time and end time of a working day

  Weekends are not work days and you can assume that there are no holidays.
  Taking this in consideration what's the date when the task will be completed
  (hours and minutes are taken in consideration) ?

(The original exercise was meant to work with database but I simplified it a bit.)

Here's my solution to the problem :

public class Program
{
    private static void Main()
    {
        DateTime startOfTask = new DateTime(2016, 2, 27, 10, 30, 0);

        TimeSpan durationOftask = new TimeSpan(20, 30, 0);

        TimeSpan startOfDay = new TimeSpan(8, 30, 0);
        TimeSpan endOfDay = new TimeSpan(16, 0, 0);

        DateTime endOfTaskTime = GetEndOfTaskTime(durationOftask, startOfTask, startOfDay, endOfDay);
        Console.WriteLine(endOfTaskTime.ToString("yyyy.MM.dd HH:mm"));
        Console.ReadKey();
    }

    private static DateTime GetEndOfTaskTime(TimeSpan durationOftask, DateTime startOfTask, TimeSpan startOfDay,
        TimeSpan endOfDay)
    {
        DateTime endOfTask = startOfTask;
        TimeSpan timeWorkingOnTask = new TimeSpan(0, 0, 0);
        while (timeWorkingOnTask < durationOftask)
        {
            if (endOfTask.DayOfWeek != DayOfWeek.Saturday && endOfTask.DayOfWeek != DayOfWeek.Sunday)
            {
                TimeSpan timeStartOfToday = new TimeSpan(endOfTask.Hour, endOfTask.Minute, 0);
                TimeSpan workTime = timeStartOfToday == startOfDay
                    ? endOfDay - startOfDay
                    : endOfDay - timeStartOfToday;
                if (timeWorkingOnTask + workTime < durationOftask)
                {
                    endOfTask = endOfTask.Add(workTime);
                    timeWorkingOnTask += workTime;
                }
                else
                {
                    endOfTask = endOfTask.Add(durationOftask - timeWorkingOnTask);
                    break;
                }
            }
            endOfTask = endOfTask.AddDays(1);
            endOfTask = new DateTime(endOfTask.Year, endOfTask.Month, endOfTask.Day, startOfDay.Hours,
                startOfDay.Minutes, 0);
        }
        return endOfTask;
    }
}

I would add some checks to make sure the date is correct. For instance your start date is on a Saturday.

-- Updated from comment that task doesn't need to start during working times.

if (startTime.TotalDays > 1)
{
    throw new ArgumentOutOfRangeException("startTime", "Start time greater than 24 hours");
}
if (endTime.TotalDays > 1)
{
    throw new ArgumentOutOfRangeException("endTime", "End time greater than 24 hours");
}
if (endTime < startTime)
{
    throw new ArgumentOutOfRangeException("endTime", "End time before start time");
}

For the logic you don't need to do a loop.

var numberOfDaystoStart = 0;
if (startOfTask.DayOfWeek == DayOfWeek.Saturday)
{
    numberOfDaystoStart = 2;
}
else if (startOfTask.DayOfWeek == DayOfWeek.Sunday)
{
    numberOfDaystoStart = 1;
}
else if (startOfTask.TimeOfDay > endTime)
{
    // Start time after the end of the work day need to move to the next day
    numberOfDaystoStart = 1;
}

var startOffset = TimeSpan.Zero;
if (numberOfDaystoStart > 0 || startOfTask.TimeOfDay < startTime)
{
    // IF started on weekend or after work hours adjust for first work day and start of working hours
    var startOfTaskDate = startOfTask.AddDays(numberOfDaystoStart);
    startOffset =
        new DateTime(startOfTaskDate.Year, startOfTaskDate.Month, startOfTaskDate.Day,
            startTime.Hours, startTime.Minutes, startTime.Seconds, startTime.Milliseconds) - startOfTask;
}

var workingtime = endTime.Subtract(startTime);
var nonWorkingTime = TimeSpan.FromDays(1) - workingtime;

// This will give long from C#
var numbersOfDays = durationOftask.Ticks/workingtime.Ticks;

// add in the duration then add in the extra time for non work hours
// add in the weekends
var endOfTask = startOfTask
    .Add(durationOftask)
    .Add(startOffset)
    .Add(new TimeSpan(numbersOfDays*nonWorkingTime.Ticks))
    // Add in the extra non-working time for each day
    .AddDays((numbersOfDays/5)*2); // Add in 2 extra days for every 5 days of work

// Need to account for starting later in the week and rolling over weekend
var extraDays = numbersOfDays%5;

var endOffset = TimeSpan.Zero;
if ((int) startOfTask.Add(startOffset).DayOfWeek + extraDays > 5)
{
    endOffset = TimeSpan.FromDays(2);
}
// If we ended after the end of the day need to account for non-worktime
if (endOfTask.TimeOfDay > endTime)
{
    endOffset = endOffset.Add(nonWorkingTime);
}

return endOfTask.Add(endOffset);

If starting before the start time or starting on the weekend if valid then there would need to be special code just for those cases but it would be at the top to just to add to the duration the extra days to move on to the start date.


Variable length string validation against whitelisted characters I need to validate a variable length string against some white-listed characters. The algorithm that I have works but I have a sneaking suspicion that there is a more elegant solution. The code below hasn't been identified as a bottleneck so ease of maintenance is preferred over a clever but confusing implementation.

private static readonly Regex ReferralCodeValidator = new Regex(
   @"[ABCDEFGHIJKLMNOPQRSTUVWXYZ123456789]",
   RegexOptions.Compiled | RegexOptions.CultureInvariant | RegexOptions.IgnoreCase);

private const int MinLength = 3;

private const int MaxLength = 8;

public static bool IsValidReferralCode(this string code)
{
   if (string.IsNullOrWhiteSpace(code))
   {
       return false;
   }

   if (code.Length < MinLength || code.Length > MaxLength)
   {
       return false;
   }

   // Variable for clarity and easier regex troubleshooting
   var strippedCode = ReferralCodeValidator.Replace(code, string.Empty);
   return strippedCode == string.Empty;
}

I also have the following NUnit tests to validate/exercise the solution:

[Test]
[TestCase("AARPeVER", true)]
[TestCase("1234EVE", true)]
[TestCase(null, false)]
[TestCase("AARP", true)]
[TestCase("AA", false)]
[TestCase("AAr", true)]
[TestCase("123456789", false)]
[TestCase("AARP0", false, Description = "Zero isn't allowed.")]
[TestCase("AARPO", true, Description = "The letter O is allowed.")]
[TestCase("", false)]
[TestCase("  ", false)]
[TestCase("AAR ", false)]
[TestCase("AAR\n", false)]
[TestCase("Ever||12", false)]
public void IsValidReferralCode(string input, bool success)
{
    Assert.AreEqual(success, input.IsValidReferralCode());
}

Instead of removing valid characters, you may look for invalid ones:

    private static readonly Regex ReferralCodeValidator = new Regex(
       @"[^ABCDEFGHIJKLMNOPQRSTUVWXYZ123456789]",
       RegexOptions.Compiled | RegexOptions.CultureInvariant | RegexOptions.IgnoreCase);

    public static bool IsValidReferralCode(this string code)
    {
        // preliminary tests omitted for brevity
        return !ReferralCodeValidator.Match(code).Success;
    }

I can't say if this solution is more elegant (beauty is in the eye of beholder), but it surely looks more straightforward.


Processing an image to extract green-screen mask I am using OpenCV to process an image, and in my code, I have to check / edit each pixel separately:

import cv2, numpy

# we just use an empty image for the purpose of this MCVE
img = cv2.imread("source.jpg")
width = len(img[0])
height = len(img)

empty_img = numpy.zeros((height, width, 3), numpy.uint8)

i = 0
r = 0
c = 0

for line in img:
    c = 0

    for pixel in line:
        blue = pixel[0]
        green = pixel[1]
        red = pixel[2]

        if green != max(red, green, blue) or green < 35:
            # this has a greenishy hue
            empty_img.itemset((r, c, 0), 255)

        c += 1
    r += 1

This code works, but is quite slow (it takes about 23 seconds to process the image). How could/should I speed this up?

source.jpg:

result should be:

Use conditional indexing:

RED, GREEN, BLUE = (2, 1, 0) # Your numbers

empty_img = numpy.zeros((height, width, 3), numpy.uint8)

reds = img[:, :, RED]
greens = img[:, :, GREEN]
blues = img[:, :, BLUE]

empty_img[(greens < 35) | (reds <= greens >= blues)][BLUE] = 255

Edit:

empty_img[(greens < 35) | ((reds <= greens) & (blues <= greens))][BLUE] = 255

I was wondering if the compound relation would work. Sadly, not. This should. If it doesn't you can try composing it step by step - look at just the greens < 35 case, then the reds <=greens, etc.