Recursively parsing JSON response using GSON I have a URL which I am executing it and it is returning the below string as a JSON response. I am using GSON to parse JSON.

In the below JSON, "resourceId" contains an IP address.

{
   "status":{
      "code":"200",
      "msg":"ok",
   },
   "hasmore":true,
   "count":1000,
   "result":[
      {
         "_type":"Calculate",
         "resourceId":"1.2.3.4",
         "_oid":"522b216"
      },
      {
         "_type":"Calculate",
         "resourceId":"5.6.7.8",
         "_oid":"53cea4ad1"
      },
      {
         "_type":"Calculate",
         "resourceId":"9.10.11.12",
         "_oid":"51f98cce1c12c6"
      }
   ],
   "pagination":{
      "cursor":[
         "",
         "545b3bba43a2",
         ""
      ],
      "limit":[
         10000,
         1000,
         1000
      ],
      "skipEmptyPage":false
   },
   "next":{
      "method":"GET",
      "url":"/urlB"
   }
}

Now from the above JSON response, I need to extract all the resourceId values and populate it into a List<String>.

In my above JSON string, there is a property called "hasmore" and in all the JSON response this field will be there always so if "hasmore" is true, then it means there are more IP Address which will be there in another URL and that URL is present in "next" JSON object in the same JSON response at the bottom so I need to execute that URL and get the JSON response back and that response will be similar to what I have above so populate the IP address into the same list and keep on doing this until you see "hasmore" is false.

In general, I will have original URLA which I will execute it and get the JSON response back which will look like as shown above, parse that JSON populate all the resourceId into a List and then check if "hasmore" is true, then it means there will be another URL at the bottom which is URLB so now execute this URL again, get the JSON response back which will be similar to what I have above and then keep on populating resourceId into the same list
and again check if "hasmore" is true or not. Keep on doing this until you see "hasmore" is false.

In short, ips list should have all the IP Address in it since JSON response is coming back as paginated.

I have got the below code working fine:

public static List<String> getListOfIps(String alias) {
    List<String> ips = new ArrayList<String>();
    String url = null;
    boolean hasMore = true;
    try {
        // original url
        url = getURL(alias);
        while (hasMore) {
            String response = getJsonResponse(url);

            // parse response
            if (response != null) {
                JsonElement jelement = new JsonParser().parse(response);
                JsonObject jobject = jelement.getAsJsonObject();

                hasMore = jobject.get("hasmore").getAsBoolean();

                JsonArray jarray = jobject.getAsJsonArray("result");
                if (hasMore) {
                    url = "http://machine.vip.domain.com/ppt" + jobject.getAsJsonObject("next").get("url").getAsString();
                }

                for (JsonElement type : jarray) {
                    ips.add(type.getAsJsonObject().get("resourceId").getAsString());
                }
            }
        }
    } catch (Exception e) {
        // log an exception using logger
    }
    return ips;
}

I am opting for code review for this method to see whether there is any better way of doing this.

Have the following POJOs

class Result {
    private String resourceId;
}

class Next{
    private String url;
}

class Response{
    private boolean hasMore;
    private List<Result> result;
    private Next next;
}
// If other fields are required add them.
// Have their getters/setters.
// Have the hashCode,equals,toString

Your getListOfIps() could look like this:

List<String> ips = new ArrayList<String>();
Response response = null;
String url = getURL(alias);
do{
    String responseJson = getJsonResponse(url);
    try{
        response = new Gson().fromJson(responseJson , Response.Class);
    } catch( // Json Exceptions thrown by this function){
        // Log the exception and handle it .
    }
    // Check if response and response.result is non null
    // Loop through and extract the non-null resourceId
    // update url
    if(response.hasMore && (response.next!=null) && (response.next.url.length()>0)){
        url= response.next.url;
    }else{
        url = null;
    }
  } while(url);

The code is more of a "steps to follow". Hope it helps.
Try the code and you can post the updated code if its better.
For Gson help follow this link.


Prime sieve in Python I was trying to write Sieve for the first time and I came up with this code:

def sieve(num):
    numbers = set(range(3, num, 2))
    for i in range(3, int(num**(1/2)) + 1 , 2):
        num_set = set(i*c for c in range(3, num//2, 2))
        numbers = numbers - num_set
    return list(sorted((2, *numbers)))

The problem is that for num > 10**6 the time to create prime numbers increases.

Also, when I tried num = 10**8 my computer stopped working, started to make awkward noises and I had to restart it.

I think the problem is that I am dealing with sets. For large numbers (for instance, in num = 10**8 case) the set cannot be produced since my computer cannot process that much information, hence it stops.

Is there a way to solve this memory or time problem using my code or should use a different algorithm?

Using a set() is your bottleneck, memory-wise.

>>> numbers = set(range(3, 10**8, 2))
>>> sys.getsizeof(numbers)
2147483872
>>> sys.getsizeof(numbers) + sum(map(sys.getsizeof, numbers))
3547483844

A set of odd numbers up to 100 million is consuming 2GB 3.5GB (thank-you @ShadowRanger) of memory.  When you do an operation like numbers = numbers - num_set, you'll need to have 3 sets in memory at once.  One for the original set, one for the set of numbers you are removing, and one for the resulting set.  This will be greater than 4GB 7GB of memory, since some of the numbers you are removing aren't in the original set.

You don't need to realize the entire set of numbers you are removing in memory.  You could simply remove the numbers from the set one at a time:

for c in range(3, num // 2, 2):
    numbers.remove(i * c)

This is modifying the set in place, so the memory requirement will not exceed the initial 2GB of memory for the set.

Why are you looping c over range(3, num // 2, 2)?  This is doing way too much work.  The maximum value c should obtain should satisfy i*c < num, since no product i*c larger than num will be in the original set.

You should instead loop over range(3, num // i + 1, 2).  This will decrease the size of the set of numbers you are removing as the prime numbers you find increase.

Why start removing primes at 3*i?  When i is 97, you've already removed multiples of 3, 5, 7, 11, 13, 17, ... up to 89. The first multiple you need to remove is 97*97.  You would then continue with 99*97, 101*97, and so on, up to num.  So the range should begin with i, not 3.

for c in range(i, num // i + 1, 2):
    numbers.remove(i * c)

Actually, this is still too complicated.  Let's get rid of the multiplication.  This also greatly simplifies the upper limit of the range.

for multiple in range(i*i, num, 2*i):
    numbers.remove(multiple)

Or equivalently, passing a generator to difference_update to remove items in bulk, but without realizing the set of numbers to be removed in memory simultaneously.

numbers.difference_update(multiple for multiple in range(i*i, num, 2*i))

Even with all of the above changes, you still require 2GB of memory to compute the primes up to 100 million.  And since a set is unordered, you still have to sort the surviving numbers afterwards to get your ordered list of primes.

A better way is to maintain an array of flags, one per candidate number.  With 100 million candidate numbers, if each flag used only a single byte, you'd only require 100 MB of memory, a savings of a factor of 20.  And since the array of flags is ordered, no sorting of the array would be required.

The bytearray is one such structure.  It is an array of bytes.  You can store your candidates in the array as a 1, and any non-primes (multiples of other primes) as 0.

def sieve(num):
    flags = bytearray(num)      # Initially, all bytes are zero

    flags[2] = 1                # Two is prime
    for i in range(3, num, 2):
        flags[i] = 1            # Odd numbers are prime candidates

    # Find primes and eliminate multiples of those primes
    for i in range(3, int(num ** 0.5) + 1, 2):
        if flags[i]:
            for multiple in range(i * i, num, 2 * i):
                flags[multiple] = 0

    return [ i for i, flag in enumerate(flags) if flag ]

Conserving a little bit more memory, you can store your list of primes in an array

import array

def sieve(num):
    flags = bytearray(num)      # Initially, all bytes are zero

    flags[2] = 1                # Two is prime
    for i in range(3, num, 2):
        flags[i] = 1            # Odd numbers are prime candidates

    # Find primes and eliminate multiples of those primes
    for i in range(3, int(num ** 0.5) + 1, 2):
        if flags[i]:
            for multiple in range(i * i, num, 2 * i):
                flags[multiple] = 0

    return array.array('I', (i for i, flag in enumerate(flags) if flag))

For primes up to \$10^8\$, the array.array('I', ...) stores the 5.7 million primes in a mere 23MB of memory.  The list version takes a whopping 212MB.

Note: If you are using a 32-bit version of Python, you may need the type-code 'L' instead of 'I' to get storage for 4-byte integers in the array.

For the truly memory conscious, install the bitarray module.

pip3 install bitarray

In addition to using only a single bit per flag, for 1/8th the memory usage in the sieve, it allows some truly fantastic slice assignments from a single boolean scalar, which makes clearing all multiples of a prime number into a simple single statement.

import array
from bitarray import bitarray

def sieve(num):

    flags = bitarray(num)
    flags.setall(False)
    flags[2] = True                      # Two is prime
    flags[3::2] = True                   # Odd numbers are prime candidates

    for i in range(3, int(num ** 0.5) + 1, 2):
        if flags[i]:
            flags[i*i:num:2*i] = False   # Eliminate multiples of this prime

    primes = array.array('I', (i for i, flag in enumerate(flags) if flag))

    return primes

Timings:

10^3:  0.000
10^4:  0.000
10^5:  0.004
10^6:  0.051
10^7:  0.428
10^8:  4.506

Note: Updated timing info.  I just noticed I had for i in range(3, num + 1, 2) in the last implementation instead of for i in range(3, int(num ** 0.5) + 1, 2), resulting in a lot of wasted time doing nothing.

Python 3.8 Update: Using math.isqrt(num) is better than int(num ** 0.5):

    for i in range(3, math.isqrt(num) + 1, 2):


Python code to run a C program that calculates whether points are in the mandelbrot set This is part of a project to explore the Mandelbrot set. The code to graphically navigate the complex plane and allow the user to adjust the colormap and number of iterations is all written in Python, because that sort of user interface work is easiest in a scripting language. However, Python is not very good at inner loops that run hundreds of times for each pixel, so I wrote a C program that does the actual computation. The python function runs the C executable in a separate process that passes the result back from C to python as a human-readable string. Parsing the string back to integers is slow. What is a more pythonic way of creating the interface between C and python? Also, any general tips for writing faster, better C are appreciated, I'm new to the language.

Python code:

import numpy as np
from subprocess import Popen
import subprocess

def getcounts(xmin, xmax, ymin, ymax, dim, depth):
    """render a view of the mandelbrot set on the given rectangle of the
    complex plane, by calling a C script and interpreting stdout as a
    series of integers
    output is a numpy array reshaped into a dim*dim square"""

    p = Popen(["./a.out", repr(xmin), repr(xmax), repr(ymin),
    repr(ymax), str(dim), str(depth)], stdout=subprocess.PIPE)

    counts = np.fromstring(p.stdout.readline()[:-1], dtype = np.int, sep = ' ')

    return counts.reshape([dim, dim]).transpose()

C code:

#include <stdio.h>
#include <stdlib.h>
int main(int argc, char *argv[]){
  int dim = 100;        /*defaults produce a nice overview of the mandelbrot set*/
  int depth = 300;
  double minx = -2;
  double maxx = 2;
  double miny = -1.5;
  double maxy = 1.5;

  if(argc > 6){
      minx = atof(argv[1]);
      maxx = atof(argv[2]);
      miny = atof(argv[3]);
      maxy = atof(argv[4]);
      dim = atoi(argv[5]);
      depth = atoi(argv[6]);
  }

  double xstep = (maxx - minx)/dim;
  double ystep = (maxy - miny)/dim;

  int i, j;
  for(i = 0; i < dim; i++){
    double c_real = minx + i * xstep;
    for(j = 0; j < dim; j++){
      double c_imag = miny + j * ystep;
      int count = 0;
      double z_real = 0;
      double z_imag = 0;
      double zreal_temp;
      while((z_real * z_real + z_imag * z_imag < 4) && count < depth){
          count ++;
          zreal_temp = z_real * z_real - z_imag * z_imag + c_real;
          z_imag = z_real * z_imag * 2 + c_imag;
          z_real = zreal_temp;
      }
      printf("%d ", count);
    }
  }
}

Finally, here's a python script to test the above function: I've cut out all the bits for moving around and adjusting settings since this question is about the C - Python interface

import numpy as np
import matplotlib.pyplot as plt
xmin = -2
xmax = 2
ymin = -2
ymax = 2
depth = 60
dim = 600
counts = getcounts(xmin, xmax, ymin, ymax, dim, depth)
plt.imshow(counts, extent = [xmin, xmax, ymax, ymin], interpolation = 'nearest')
plt.show()

In my opinion, Popen is simply the wrong approach to this problem. Everything else follows from that. Python provides an API to directly expose C functions to Python. Boost.Python is a fantastic library to make it easy to expose C++ classes without having to deal with the messy direct Python API. Boost.Numpy makes it easy to return numpy arrays directly.

Regardless of which one you choose, you should choose one and use it to create something to the effect of mandelbrot.so which will have one function that returns a list of lists. You can then just use that function directly. Rather than write getcounts() to do some work doing string processing, just call mandelbrot.getcounts() with all the same args. Something like:

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

xmin = -2
xmax = 2
ymin = -2
ymax = 2
depth = 60
dim = 600
counts = mandelbrot.getcounts(xmin, xmax, ymin, ymax, dim, depth)
plt.imshow(counts, extent = [xmin, xmax, ymax, ymin], interpolation = 'nearest')
plt.show()

There are several huge advantages to doing it this way:

The ability to actually do any sort of error checking.
Not having to do any output processing converting between one program's output and another program's desired input.
Not having to start a new process and do input handling on that one will make the code perform dramatically better.
It'll read more Pythonic on one end (just call a function that returns a value) and more C/C++-ic on the other end (have a function that returns a value there too, instead of printing output). This has the added benefit of being able to actually write tests for your C/C++ code.


Simple tree addition implementation I am trying to learn Rust and I feel like a dummy every step of the way.

I went through the tutorial on making lists in Rust. I am trying to apply the knowledge to implement a simple BST.

The code looks pretty horrible though. Any suggestions on how it can be improved?

pub struct Tree<T: Ord> {
    root: Link<T>,
}

type Link<T> = Option<Box<Node<T>>>;

pub struct Node<T:Ord> {
    elem: T,
    left: Link<T>,
    right: Link<T>
}

impl<T: Ord> Tree<T> {
    pub fn new() -> Self {
        Tree {root: None}
    }

    pub fn add(&mut self, elem: T) {
        let mut current;
        match self.root {
            None => {
                self.root = Some(Box::new(Node{elem: elem, left: None, right: None}));
                return;
            },
            Some(_) => {
                current = self.root.as_mut();
            }
        }
        loop {
            if elem < current.as_ref().unwrap().elem {
                if current.as_ref().unwrap().left.is_none() {
                    current.unwrap().left = Some(Box::new(Node{elem: elem, left: None, right: None}));
                    break;
                } else {
                    current = current.unwrap().left.as_mut();
                }
            } else {
                if current.as_ref().unwrap().right.is_none() {
                    current.unwrap().right = Some(Box::new(Node{elem: elem, left: None, right: None}));
                    break;
                } else {
                    current = current.unwrap().right.as_mut();
                }
            }
        }
    }
}

#[cfg(test)]
mod test {
    use super:: Tree;

    #[test]
    fn basics() {
        let mut tree = Tree::new();
        tree.add(5);
        tree.add(3);
        tree.add(2);
        tree.add(4);

        assert_eq!(tree.root.as_ref().unwrap().elem, 5);
        assert_eq!(tree.root.as_ref().unwrap().left.as_ref().unwrap().elem, 3);
        assert_eq!(tree.root.as_ref().unwrap().left.as_ref().unwrap().left.as_ref().unwrap().elem, 2);
        assert_eq!(tree.root.as_ref().unwrap().left.as_ref().unwrap().right.as_ref().unwrap().elem, 4);
    }
}

There are a few things you can do to reduce repetition in your code. The first would be encapsulating your new Node<T> into a method.

impl<T> Node<T> {
    fn new(elem: T) -> Self {
        Self {
            elem,
            left: None,
            right: None,
        }
    }
}

Then all the constructions of Node in add can be replaced with Node::new(elem).

Another thing that can reduce verbosity is having current be an actual (boxed) node rather than an option. Right now you maintain the invariant that current is the Some variant of an Option<&mut Box<Node<T>>> (Link<T>). By instead keeping the &mut Box<Node<T>>, it'll allow you to reduce the number of unwraps. Conceptually, this means that we're expressing the invariant using the type system.

For example, to initialize current,

let mut current;
match self.root {
    None => {
        self.root = Some(Box::new(Node::new(elem)));
        return;
    }
    Some(ref mut node) => {
        current = node;
    }
}

(matching on Some(ref mut node) makes node a mutable reference - there are some other ways to get the same effect, but this is the clearest).

Having this allows you to change the later blocks to

if elem < current.elem {
    if current.left.is_none() {
        current.left = Some(Box::new(Node::new(elem)));
        break;
    } else {
        current = current.left.as_mut().unwrap();
    }
}

My last major suggestion is to use matching to get rid of all the unwrapping. Blocks like

if current.left.is_none() {
    current.left = Some(Box::new(Node::new(elem)));
    break;
} else {
    current = current.left.as_mut().unwrap();
}

are just the poor man's match statement. They can be replaced by

match current.left {
    None => {
        current.left = Some(Box::new(Node::new(elem)));
        break;
    }
    Some(ref mut node) => {
        current = node;
    }
}

(again, ref mut node means we're capturing a mutable reference).

This could more succinctly be expressed using if let.

if let Some(ref mut node) = current.left {
    current = node;
} else {
    current.left = Some(Box::new(Node::new(elem)));
    break;
}

Now for some extra things. First, as I mentioned in the comments, the code as written handles duplicates by placing them to the right. This isn't necessarily what you want, but it's easy to change. The trait Ord provides a cmp method that has three possible outputs: Less, Greater or Equal. By doing a match on the output of cmp, we can handle all three.

match elem.cmp(&current.elem) {
    std::cmp::Ordering::Less => {
        //...
    }
    std::cmp::Ordering::Equal => break,
    std::cmp::Ordering::Greater => {
        //...
    }
}

If you don't want to type out the whole names, you could do use std::cmp::Ordering::*; inside the fn add block (or wherever else, but it's best to keep the scope small).

This suggestion would be a pretty major rework of your code, but it leads to a much more elegant solution. As I said before, you maintain the invariant that current is a Some variant. I suggested that you could keep a &mut Box<Node<T>> instead so that the invariant is expressed in the type of current. What you can do instead is not try to keep that invariant at all, but instead check whether current is Some or None at the start of the loop. Then you wouldn't need the initial test on self.root.

fn add(&mut self, elem: T) {
    let mut current = &mut self.root;
    loop {
        match current {
            Some(node) => {
                // check how elem compares to node.elem
                // assign node.left/right to current as appropriate (or break)
            }
            None => {
                // set the data behind `current` to a new node.
                // i.e. `*current = ...`
            }
        }
    }

Finally, just a fairly minor thing. Don't forget to run cargo fmt and cargo clippy. cargo fmt will make your code more readable to other Rust programmers and keep things in a consistent style. cargo clippy will give you a few helpful tips for making your code more idiomatic and sometimes help avoid logic errors.


Output the average values of binned columns in Excel VBA This code is meant to run through a column of values, bin the values based on specified ranges, then output the average value of each bin.  The problem is the code is running quite slowly (approximately 30 min for around 100000 values).  I am definitely a beginner at coding and was hoping there was some way to speed this code along.

Sub BinValues()
'binns seperation distance values for the creation of variogram

Application.ScreenUpdating = False
Application.EnableEvents = False
Application.Calculation = xlCalculationManual

Dim Cell As Object
Dim R1 As Range
Dim R2 As Range
Dim rng As Range
'define range before running
Set rng = Range("A1:A105570")
Dim K, n, L As Integer
'n is equal to the number of lags
'L is the lag size
n = 12
L = 600
For K = L To (n * L) Step 600
    For Each Cell In rng
    Dim min As Integer
    min = K - L
    'upper bound exclusive and lower bound inclusive
        If Cell.Value >= min And Cell.Value < K Then
            If R1 Is Nothing Then
                Set R1 = Range(Cell.Address)
            Else
                Set R1 = Union(R1, Range(Cell.Address))
            End If
            Cells((K / L), 5) = WorksheetFunction.Average(R1)
        End If

    Next
    Set R1 = Nothing
Next

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

End Sub

Data belongs in an Array

A worksheet *looks* like a grid of data, but there's an enourmous amount of overhead sitting behind it. Every time you do anything to a spreadsheet, events fire, formulas calculate and a million other things happen behind the scenes.

Working with Ranges is computationally expensive, and you're doing it N*105,570*2 times.

Instead, what you want is an Array. An Array is just a grid of data laid out in memory. Because it is *just* data there are no overheads, and so you can read/write to it about a Million times faster.

You can create an Array by reading in a range, like so:

Dim dataRange As Range
Set dataRange = Range("A1:A105570")

Dim dataArray As Variant
dataArray = dataRange.Value

And now, the value in "A1" is in dataArray(1, 1), "A2" in dataArray(2, 1) etc.

Let's re-write your code to use an Array:

Option Explicit

Public Sub BinValues()
    'binns seperation distance values for the creation of variogram

    Application.ScreenUpdating = False
    Application.EnableEvents = False
    Application.Calculation = xlCalculationManual

    Dim dataRange As Range
    Set dataRange = Range("A1:A105570")

    Dim dataArray As Variant
    dataArray = dataRange.Value

    Const NUM_LAGS As Long = 12
    Const LAG_SIZE As Long = 600

    Dim minValue As Double
    Dim maxValue As Double
    Dim lagCounter As Long
    Dim ix As Long

    Dim elementValue As Double
    Dim elementSum As Double
    Dim numElements As Double
    Dim elementAverage As Double

    For lagCounter = 1 To NUM_LAGS

        minValue = (lagCounter - 1) * LAG_SIZE
        maxValue = (lagCounter * LAG_SIZE) - 1

        numElements = 0
        elementSum = 0
        For ix = LBound(dataArray, 1) To UBound(dataArray, 1)
            elementValue = dataArray(ix, 1)
            If elementValue >= minValue And elementValue <= maxValue Then
                numElements = numElements + 1
                elementSum = elementSum + elementValue
            End If
        Next ix

        elementAverage = elementSum / numElements
        Cells(lagCounter, 5) = elementAverage

    Next lagCounter

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

End Sub

That alone should take your runtime from 1/2 an hour to a couple of seconds (if that).


Multiple jQuery statements in one function I've been working with jQuery for a few days now and starting to get the hang of it (it does the thing that I want to do).

I've made an accordion, and the header of each accordion item has an arrow that points up (for being open) or down (for being closed). This code works fine, does what it has to do. When I wrote it, it felt very ugly.

When I press a header of an item in the accordion, function gets called like this:

The function itself:

function leftActive(e, b) {
  $('.active').removeClass('active');
  $('.relative').eq(e).addClass('active');

  $('.arrowUp').addClass('arrowDown');
  $('.arrowUp').removeClass('arrowUp');
  $('.arrowDown', b).addClass('arrowUp');
  $('.arrowDown', b).removeClass('arrowDown');
}

HTML structure:

 Transport logistics

      <?php require 'templates/dummyBedrijven.php'; ?>

 Real Estate

      <?php require 'templates/dummyBedrijven.php'; ?>

 Salt trade Winter Maintenance

      <?php require 'templates/dummyBedrijven.php'; ?>

 Human Resources Solutions

      <?php require 'templates/dummyBedrijven.php'; ?>

First off, don't use the onclick attribute. You have jQuery, so use it to set the appropriate event handler:

$(".accordionHeadingDiv").click(function (event) {
  ...
});

That will add a click-event handler to all the headings.

Now, as far as I can tell, your code then removes the active class from all other accordion elements, adds it to the one that was clicked, and does the arrow-replacement.

This is simpler with the event handler setup above:

$(".accordionHeadingDiv").click(function (event) {
  $("#accordion .active").removeClass("active"); // remove "active" from others
  $(this).addClass("active"); // add it to the heading that was clicked
});

For the arrows, I'd advice a better class name system and more CSS. Call the class simply "arrow", and don't bother changing it via JS. Since you set the heading to active all you need is CSS like this

.accordionHeadingDiv .arrow {
  /* default style (i.e. arrow pointing down) */
}

.accordionHeadingDiv.active .arrow {
  /* active style (i.e. arrow pointing up) */
}

No need for JS.

While I'm at it, I'd advice you to make better use of semantic HTML and CSS:

Don't call a class accordionHeadingDiv. The class should be independent of the element type. If you want to style based on element and class, the basic CSS syntax of div.accordionHeading is better.
Actually, don't call it accordionHeading either. It's nested inside the #accordion element, so you don't need to prefix the class name. You can style it with just #accordion .heading
Actually, don't even use heading: Use a heading element! You've already got one, but there's no need for the DIV around it.

What you get is something like

    Transport logistics

    <?php require 'templates/dummyBedrijven.php'; ?>

  ...

Much cleaner. (P.s. I'd use include instead of require)

However...

That's all well and good, but if I understand you correctly, jQueryUI is handling all the accordion stuff. And jQueryUI adds its own "active" class to the opened accordion-element. So you can actually get rid of all you custom JS and just stick to CSS:

#accordion .ui-accordion-header .arrow {
  /* default style (i.e. arrow pointing down) */
}

#accordion .ui-accordion-header-active .arrow {
  /* active style (i.e. arrow pointing up) */
}

All you need in the JS is:

$('#accordion').accordion();

Here's a demo


Sliding marker on link hover with jQuery The following code is for a vertical menu, where I needed a marker to slide down next to a link upon hovering over said link. Therefore I needed to change CSS properties of a HTML element (the top-margin in this case).

What I've done in jQuery is add a class ('.movedown1' or '.movedown2') to the marker div ('#slide') when the mouse hovers over one of the links ('#menulink2' or '#menulink3'), and remove the class when the mouse stops hovering over the link.

It works!...But is it a clean method to add/change CSS properties of a HTML element? Does it follow best practices for jQuery? I've only used jQuery a few times in the past so I'm not familiar with best practices.

Codepen

HTML(Pug):

#slide

#menu
  a.menulink#menulink1(href="#")
    p RED
  a.menulink#menulink2(href="#")
    p BLUE
  a.menulink#menulink3(href="#")
    p GREEN

CSS (Sass)

a
  color: white
  text-decoration: none

#slide
  width: 10px
  height: 30px
  background-color: red
  position: absolute
  border-radius: 15px
  transition: 0.3s ease-in-out

.movedown1
  margin-top: 40px
  background-color: blue !important

.movedown2
  margin-top: 80px
  background-color: green !important

#menu
  margin-left: 30px

.menulink
  height: 40px
  width: 70px
  display: block

.menulink p
  height: 30px
  width: 70px
  margin: 0px
  border-radius: 15px
  display: flex
  align-items: center
  justify-content: center

#menulink1 p
  background: red
#menulink2 p
  background: blue
#menulink3 p
  background: green

jQuery

$('#menulink2').hover(
  function(){
  $('#slide').addClass('movedown1');
  },
  function(){
  $('#slide').removeClass('movedown1')

});

$('#menulink3').hover(
  function(){
  $('#slide').addClass('movedown2');
  },
  function(){
  $('#slide').removeClass('movedown2')

});

JavaScript

This code violates the Don't Repeat Yourself principle because there is some redundancy in the code to handle the hover events, but for a beginner it might be challenging to know how to abstract the common code.

One way to reduce the redundancy is to define a mapping of id values to classes to toggle. This can be done with a plain-old JavaScript Object, like below, or with a Map() object.

const targetClassMapping = {
  'menulink2': 'movedown1',
  'menulink3': 'movedown2'
};

Then the code can utilize .hover() on any element with the class name menulink with a single event handler that checks the id attribute (using the jQuery .attr() method) of the event target and if the id matches a key in that mapping it then calls .toggleClass() on the slider element. Additionally, $('slider') can be stored in a variable and used when needed instead of querying the DOM each time.

const slide = $('#slide');
$('.menulink').hover(function(){
  const id = $(this).attr('id');
  if (id && id in targetClassMapping) {
    slide.toggleClass(targetClassMapping[id]);
  }
});

This could also be done without jQuery, though the code wouldn't be quite as succinct.

const slider = document.getElementById('slide');
const targetClassMapping = {
  'menulink2': 'movedown1',
  'menulink3': 'movedown2'
};
['mouseover', 'mouseout'].forEach(type => document.addEventListener(type, handleMouseEventOnMenuLink));
function handleMouseEventOnMenuLink(event) {
  if (event.target.parentNode) {
    const id = event.target.parentNode.id;
    if (id in targetClassMapping) {
      const method = event.type === 'mouseover' ? 'add' : 'remove';
      slider.classList[method](targetClassMapping[id]);
    }
  }
}

CSS / SASS

I see a couple lines with !important.

.movedown1
  margin-top: 40px
  background-color: blue !important

.movedown2
  margin-top: 80px
  background-color: green !important

It is wise to not depend on !important if possible, to avoid confusion. You can remove the !important from those background color styles if you make the selectors more specific than just #slide - e.g. #slide.movedown1 and #slide.movedown2.


73 Lines of Mayhem - Parse, Sort and Save to CSV in PHP CLI Inside of a folder named txt I have 138 text files (totaling 349MB) full of email addresses. I have no idea (yet) how many addresses there are. They are separated from one another by line breaks. I created the following script to read all of these files into an array, dismiss the duplicates, then sort alphabetically and save in groups of 10K per csv file. It works correctly, but it has also been running for over 8 hours (dual core i3 w/ 4 gigabizzles of ram, sata 7200 hdd) which seems excessive to me. Top also tells me that my program's CPU usage is 100% and it's been like that the whole while it's been running. Give my script a looksie and advise me on where I've gone so terribly wrong.

function writeFile($fileName, $fileData)
{

    $writeFileOpen = fopen('csv/' . $fileName, 'w');
    fwrite($writeFileOpen, $fileData) or die('Unable to write file: ' . $fileName);
    fclose($writeFileOpen);

}

function openFiles()
{

    $addressList = array();
    $preventRepeat = array();

    if ($handle = opendir('txt')) {
        while (false !== ($file = readdir($handle))) {
            if ($file != '.' && $file != '..') {
                $newList = explode("\n", trim(file_get_contents('txt/' . $file)));
                foreach ($newList as $key => $val) {
                    $val = str_replace(array(',', '"'), '', $val);
                    if (in_array($val, $preventRepeat) || !strpos($val, '@') || !$val) {
                        unset($newList[$key]);
                    }
                    $preventRepeat[] = $val;
                }
                if (empty($addressList)) {
                    $addressList = $newList;
                } else {
                    $addressList = array_merge($addressList, $newList);
                }
                unset($newList);
            }
        }
        closedir($handle);
    } else {
        echo 'Unable to Read Directory';
    }

    $lineNum = 1;
    $fileNum = 1;
    $fileData = '"Email Address"' . "\n";

    sort($addressList);

    $lastKey = count($addressList) - 1;

    foreach ($addressList as $key => $val) {

        if ($lineNum > 10000) {

            writeFile('emailList-' . $fileNum . '.csv', trim($fileData));

            $lineNum = 1;
            $fileNum++;
            $fileData = '"Email Address"' . "\n";

        } elseif ($key == $lastKey) {

            writeFile('emailList-' . $fileNum . '.csv', trim($fileData));

            echo 'Complete';

        }

        $fileData .= '"' . trim($val) . '"' . "\n";
        $lineNum++;

    }

}

openFiles();

PS. I wrote this script in haste to accomplish a task. I can understand if you wouldn't consider this script distribution worthy.

EDIT: So I arrived at work today to find my script at some point exceeded 536MB of memory and quit on fatal error. I had already increased my ini file's memory_limit parameter from 128MB to 512MB. This doesn't necessarily reflect an issue with the script itself. I just wanted to share my frustration with the task.

This will be much more efficient:

$result = array();

if (($handle = opendir('./txt/')) !== false)
{
    set_time_limit(0);
    ini_set('memory_limit', -1);

    while (($file = readdir($handle)) !== false)
    {
        if (($file != '.') && ($file != '..'))
        {
            if (is_resource($file = fopen('./txt/' . $file, 'rb')) === true)
            {
                while (($email = fgets($file)) !== false)
                {
                    $email = trim(str_replace(array(',', '"'), '', $email));

                    if (filter_var($email, FILTER_VALIDATE_EMAIL) !== false)
                    {
                        $result[strtolower($email)] = true;
                    }
                }

                fclose($file);
            }
        }
    }

    closedir($handle);

    if (empty($result) !== true)
    {
        ksort($result);

        foreach (array_chunk($result, 10000, true) as $key => $value)
        {
            file_put_contents('./emailList-' . ($key + 1) . '.csv', implode("\n", array_keys($value)), LOCK_EX);
        }
    }

    echo 'Done!';
}


Improve HTML structure? I am not new to html however would like to know about best practices. I need to develop a layout like this:

This is what I have proposed:

                <image></image>

                Text goes here
                <image></image>

            <image></image>

            <image></image>

            Text goes here
            <image></image>

http://jsfiddle.net/gdWM5/

Can this be improved in any way?

your Table is not declared correctly in the markup.

This:

Should look like this

your self closing tag
 means that the row is now closed, you shouldn't have a  outside of a row, it is bad structure and won't show correctly in browsers.

HTML is not made up of
's alone.  you should include other elements as well, even when you are only figuring out your structure.

Image tags are not <image>  they are  and as long as you have everything you need inside the attributes, they are self closing tags.

from the looks of the Graphic, you may not need a table, you probably want a list instead of a table.  If I were you I would look into an unordered list (you can format how you like with CSS)

Element tags do not have Capital letters in them at all,  they are Lowercase.

More Descriptive ID names. If you have something that is going to be styled the same as something else give them class names and declare the style once.  if you have one thing that is going to be different, create a class that will style it and the surrounding items the the same and add another class to it and declare it after the other class.

In Closing

I would say that you should figure out your CSS and then post a new question as well (with the CSS)

this is not a finished structure, it looks like something you threw together in about a half an hour.  probably took you longer to draw the picture than write the HTML


Checking if a tree is balanced or not I was writing this code to check if a tree is balanced or not and I ran into a situation where I would have liked the method to return 2 values at each step. First to signify whether the tree is balanced at that level or not, and second to return the height at the level so that I can use it to check the balanced nature of the parent. I ended up writing the method such that it returns -1 if the tree is not balanced and the actual height if the tree is balanced at that level.

Is there a better way to do it? If not, then I think I should at least have a better name for the method getHeight. Can you review my code as a whole and let me know my shortcomings?

public boolean isBalanced(Node root){
        return getHeight(root)!=-1;
    }

private int getHeight(Node node){
    if(node ==null)
        return 1;

    int leftHeight = getHeight(node.getLeft());
    int rightHeight = getHeight(node.getRight());
    if(leftHeight==-1 || rightHeight ==-1)
        return -1;

    int diff = Math.abs(leftHeight-rightHeight);
    if(diff>1)
        return -1;

    return Math.max(leftHeight,rightHeight) + 1;

}

These functions can be static, and therefore should be.  Marking them static makes it clear that this is not object-oriented code.  It also serves as a hint for the JIT to inline the helper function.

Instead of using Math.abs() and Math.max(), I suggest using Math.max() and Math.min(), which simplifies the conditionals a bit.

I would expect getHeight(null) to return 0.  As you've written it, a node with no children is at height 2, which is a weird place to start counting, even if it works.

Never omit braces like that.  You will contribute to a coding accident: [Citation 1] [Citation 2]. If you really want to omit braces, then put the entire conditional statement on the same line for safety.

private static int getHeight(Node node) {
    if (node == null) return 0;

    int leftHeight  = getHeight(node.getLeft()),
        rightHeight = getHeight(node.getRight());

    int taller  = Math.max(leftHeight, rightHeight),
        shorter = Math.min(leftHeight, rightHeight);

    if (shorter < 0 || taller - shorter > 1) {
        return -1;           // Unbalanced tree
    } else {
        return taller + 1;
    }
}


Creating controls on a page which differ slightly depending on an external state I've got a part of my application which deals with tracking the dates a particular item is worked on. It is a single aspnet page, which displays controls to either start or stop tracking, depending on the state of the work item.

There are up to three controls on the page. A calendar to enter the date that work was stopped/started. A dropdown to select a reason the work item was stopped. A text area to enter a comment.

Because a similar set of controls will be created on both possible work item states (either started or stopped), I've decided to create a single set of controls, and modify them depending on the state of the work item.

The markup I use for the controls is below:

    <asp:Calendar id="DatePickerControl" runat="server"></asp:Calendar>

    <asp:DropDownList id="DropdownControl" runat="server"></asp:DropDownList>

    <textarea id="TextReasonControl"></textarea>

And the code behind to initialise:

void Page_Load(Object sender, EventArgs e)
{
    if (this.WorkItemIsStopped())
    {
        this.SetupStartControls();
    }
    else
    {
        this.SetupStopControls();
    }
}

void SetupStartControls()
{
    this.DatePickerLabel.InnerText = "Start date";

    this.DropdownContainer.Visible = false;

    this.TextReasonLabel.InnerText = "Start comment";
}

void SetupStopControls()
{
    this.DatePickerLabel.InnerText = "Stop date";

    this.DropdownLabel.InnerText = "Stop Reason";
    this.DropdownControl.DataSource = this.CreateDropdownDataSource();

    this.TextReasonLabel.InnerText = "Stop comment";
}

Is this a reasonable approach? The alternative would be to create two similar but separate sets of controls in the markup, and simply hide the set which are not relevant to the current state of the work item.

The alternative would be to create two similar but separate sets of controls in the markup

The terms similar and separate are confusing when considering class design. So allow me to think this through...

A calendar, dropDown, and textBox. - parts of a single class

From the given information both start and stop have these. And the only difference is the labels. So you can have one class, instantiating 2 objects, one for "stopping" and one for "starting".

public class StartStopper {
   //DatePicker control
   //DropDown control
   // TextBox control

   public StartStopper (string DatePickerLableText, string dropdownText, string reasonLabelText) { //assign the strings }
}

------------------------------------------------------------------

  The alternative would be to create two similar but separate sets of controls in the markup, and simply hide the set which are not relevant to the current state of the work item.

Here is an alternative:

The Design Paradigm

All the layering that's about to be done is all about the single responsibility principle applied well.
Classes start small and focused. DropBox and Textbox have distinct behavior and business rules between themselves. So the single responsibility of a StartStopBox class is to provide the all the behavior and features of their intimate, private, interaction - and nothing else.
DropDown, TextBox interaction and rules are what they are. It does not matter if it is for "stopping" or "starting". Either way, for example, "select the dropdown, the textbox must have an entry."
Aggregate classes into a StartStopCalendar class - it's single responsibility is to make the component classes interact, giving us more complex behavior.  Now we can tell when or if the calendar requires a value.
If any component needs more behavior for it's share of the interaction, fine. But that behavior is not StartStopCalendars responsibility. Remember, it's single responsibility is to make that new behavior interact with other components.
Net result is classes are more re-usable - it carries all it's own baggage. There is less coupling - Change what "I am valid" means and the clients don't know! They just call the same method as always.

Business Rules affecting Design

All 3 controls always work interactively

If business rules apply to all 3 controls together then let's keep them in one class. The calender has rules relating to the dropdown, and separate rules relating to the text box. And the dropdown and textbox have their relation rules too.

In this case, one class to rule them all. However we assume these rules are the same; it doesn't matter if we're going to use is for "stop" or "start".  Below we'll see how we get that context through class composition.

DropDown and textbox work together. This focused interaction is independent of the calendar

The dropDown and Textbox should be one class, and the calendar another. Further the calendar requires a StartStopBox object so we aggregate them:

public class StartStopBox {
    // "start" or "stop" work the same, only the labels have changed.
    public StartSTopBox ( string dropDownLabelText, string TextBoxLabelText) {}

    // This is ugly, and not so single responsibility.
    // The client is forced to evaluate the returned string
    // to decide if the object is valid.
    // But it's expedient and we'll see this get encapsulated later.
    public string Validate() {
        string errorMessage = string.Empty;

        if (! dropdown.IsEmpty() && ! TextBox.IsEmpty() {
             errorMessage = "\nDropDown and TextBox must both have enteries";
        }
        return errorMessage;
    }
}

public class StopStartCalendar {
     public StartSTopCalendar (StartStopBox aStarterOrStopper) {  }

      public string Validate () {
          string errorMessage = string.Empty;
          bool startStopperIsValid = false;

          errorMessage += aStarterOrStopper.Validate();
          startSTopperIsValid = string.IsNullOrEmpty(errorMessage) ? true : false;

          if (string.IsNullOrEmpty (theCalendar.DateSelected() && startStopperIsValid) // or however it's done.
          { errorMessage += "\nA date must be selected"; }

          return errorMessage;
      }
}

We have business rules for a "start object" interacting with a "stop object"

For example "cannot stop earlier than we started"

public class Interval {
     public Interval ( StopStartCalendar starter, StopStartCalendar stopper)
     {
         public bool isValid() {
             return string.IsNullOrEmpty(Validate()) ? true : false
         }

         public string Validate() {
             string errorMessage = string.Empty;
             errorMessage += "Starter object errors: \n\n" + starter.Validate();
             errorMessage += "Stopper object errors: \n\n" + stopper.Validate();

             if (starter.Date > stopper.Date ) {
                errorMessage += "\n\nStop Date cannot be Earlier than Start Date";
             }
             return errorMessage;
         }
     }

Out of the Trees and into the Forest

  ... It is a single aspnet page, which displays controls to either start or stop tracking, depending on the state of the work item.

public ProjectState { Undefined, Planning, Started, Ended }

public class Project {
    protected ProjectState currentState = Undefined;

    public Project ( Interval projectDates ) { }
    ...

Wait! Now I want to be able to ask the StartStopCalendar if it is in it's proper state, and single responsibility principle guides us in making changes.

public class StartStopBox {
    // Make it simpler, less error prone for the client.
    public bool IsValidState() {
        return string.IsNullOrEmpty(this.Validate()) ? true : false;
    }
}

public class StartStopCalendar {
    // ditto
    public bool IsValidState() {
        return string.IsNullOrEmpty(this.Validate()) ? true : false;
    }
}

public class Interval{
     . . .

     public bool IsStartState() {
         return ( starter.IsValidState() && !ender.IsValidState() );
     }

     public bool IsEndState() {
         return ( !starter.IsValidState() && ender.IsValidState() );
     }
}

public class Project {
    protected ProjectState currentState = Undefined;

    public void StartProject () {
        if (startStuff.IsStartState()) {
            currentState = ProjectState.Planning;
            DoStartingStuff(true);
        }else
           currentState = ProjectState.Undefined;
           DoStartingStuff(false);
        }
    }
}

ALMOST THERE

public class ProjetManager {
   protected Project me { get; set; }

   public ProjectStatus WhereAreYouOnThatProject () {
        return me.currentState;
   }

   public string ArentYouDoneYet() {
       return me.ProjectDates.IsEndState() ? "Yes" : "My dog ate my Homework";
   }

}

// THE CLIENT

public bob = new ProjectManager ( bobsProject);

bob.WhereAreYouOnThatProject();
bob.ArentYouDoneYet();

bob.YoureFired();

" NotImplementedException was thrown.  Stack trace ....."


A simple maze generator written in Rust I have been interested in trying Rust in a long time, and I finally got around to sit down and start reading "The Book". I have not finished yet, so there are probably a few features that I could have used, but I didn't know about.

Anyway, the purpose of this program is to generate a maze (using the simplest "Randomized DFS" algorithm).

Since the main goal for me was to practice the language, I tried to make the program as "idiomatic" as possible. The main thing that still bothers me is all those type conversions between the signed and unsigned types. Since this is my first Rust program, I am mainly just looking for feedback on anything that could be considered "bad code", or parts that could be improved in any way.

extern crate rand;
extern crate image;
use std::io::Write;
use rand::Rng;

#[derive(Clone, PartialEq)]
enum Cell {
    Blocked,
    Free,
}

fn make_odd(mut t: (usize, usize)) -> (usize, usize) {
    let o = |v: &mut usize| if *v%2==0{*v+=1};
    o(&mut t.0); o(&mut t.1); t
}

fn gen(s: (usize, usize)) -> Vec<Vec<Cell>> {
    let mut t = vec![vec![Cell::Blocked; s.1]; s.0];
    let mut stack = Vec::<(isize, isize)>::new();

    let c = make_odd((s.0/2, s.1/2));
    stack.push((c.0 as isize, c.1 as isize));
    t[c.0][c.1] = Cell::Free;

    let mut dirs: [(isize, isize); 4] = [(2, 0), (-2, 0), (0, 2), (0, -2)];
    let mut rng = rand::thread_rng();
    'o: while let Some(&(x, y)) = stack.last() {
        rng.shuffle(&mut dirs);
        for i in 0..4 {
            let (dx, dy) = dirs[i];
            let (nx, ny) = (x+dx, y+dy);
            if nx < 0 || ny < 0 || nx >= (s.0 as isize) || ny >= (s.1 as isize) { continue; }
            if t[nx as usize][ny as usize] != Cell::Free {
                stack.push((nx, ny));
                t[nx as usize][ny as usize] = Cell::Free;
                t[(x+dx/2) as usize][(y+dy/2) as usize] = Cell::Free;
                continue 'o;
            }
        }
        stack.pop();
    }

    t[0][1] = Cell::Free;
    t[s.0-1][s.1-2] = Cell::Free;
    t
}

fn print_usage() -> ! {
    let _ = writeln!(std::io::stderr(), "Usage: maze-gen width height [output.png]");
    std::process::exit(1);
}

fn main() {
    let args: Vec<_> = std::env::args().collect();
    if args.len() < 3 { print_usage(); }
    let mut nums = [0; 2];
    for i in 0..2 {
        match args[i+1].parse::<usize>() {
            Err(_) => { print_usage(); },
            Ok(v) => nums[i] = v,
        }
    }
    let s0 = (nums[0], nums[1]);
    let mut s = make_odd(s0);
    if s.0 < 3 { s.0 = 3; }
    if s.1 < 3 { s.1 = 3; }
    if s != s0 {
        let _ = writeln!(std::io::stderr(),
            "Warning: Adjusting sizes to {} and {}!", s.0, s.1);
    }
    let m = gen(s);
    let mut buf = image::ImageBuffer::new(s.0 as u32, s.1 as u32);
    for (x,y,px) in buf.enumerate_pixels_mut() {
        *px = image::Luma([
            match m[x as usize][y as usize] {
                Cell::Free => 255,
                Cell::Blocked => 0,
            }
        ]);
    }
    let filename = if args.len() >= 4 { args[3].as_str() } else { "out.png" };
    let ref mut file = std::fs::File::create(&std::path::Path::new(filename)).unwrap();
    image::ImageLuma8(buf).save(file, image::PNG).unwrap();
}

Sample output: (It's quite tiny, you might have to download it and zoom in to see it properly.)

(Sorry for any bad grammar, English is not my native language.)

Learn to love rustfmt.

Spaces go around operators and after commas

-    let c = make_odd((s.0/2, s.1/2));
+    let c = make_odd((s.0 / 2, s.1 / 2));

-    for (x,y,px) in buf.enumerate_pixels_mut() {
+    for (x, y, px) in buf.enumerate_pixels_mut() {

Don't put multiple statements on one line

-    o(&mut t.0); o(&mut t.1); t
+    o(&mut t.0);
+    o(&mut t.1);
+    t

Make a type for whatever (usize, usize) is. At the very least, create a type alias.
Actually, there are three places that want to apply a change to both components of the tuple. Promote the tuple to a type and add a map method.
If you have to give your closure arguments types anyway, and there's nothing captured by the closure, just make an inline function.
There's no real reason to take mutable reference to increment an integral value.
You define stack then immediately push a value on. instead, just use the vec! macro to create it all at once..
There's no need to specify the type of dirs
Instead of re-coding the number of elements in dir and having to potentially do bounds-checking again, just iterate over dirs directly
Characters are free. Use some longer variable names.
If you don't need the program name, skip it to avoid remembering it's there and then adding numbers here and there to compensate.
There's no need for the turbofish when parsing the maze size.
Use unwrap_or_else to handle a failure to parse.
Use enumerate to avoid indexing the args slice again.
Use std::cmp::max to provide a lower value for the sizes.
Don't ignore errors. If writing to stderr is important, use expect.
Use Vec::get instead of checking the size and then using the index operator (which checks the size again)
You don't need to contruct a Path, just pass in the &str
Prefer expect to unwrap.
You can use ImageBuffer::from_fn. Note that this removes the need to make your variable mutable.
Dont' use ref in let bindings. Using & on the right hand side is more idiomatic. In this case, &mut at call is more idiomatic.
To deal with the unsigned / signed transformations, remember that a + (-2) is the same as a - (+2). Add an enum to define the directions, a method to apply those directions to a point, use checked_add / checked_sub in that method to deal with under/overflow, then add your own "overflow" check for the size of the maze.

extern crate rand;
extern crate image;

use rand::Rng;
use std::cmp::max;
use std::io::Write;

#[derive(Clone, PartialEq)]
enum Cell {
    Blocked,
    Free,
}

#[derive(Debug, Copy, Clone)]
enum Direction {
    Up,
    Down,
    Left,
    Right,
}

#[derive(Debug, Copy, Clone, PartialEq)]
struct Point(usize, usize);

impl Point {
    fn map<F>(self, mut f: F) -> Self
        where F: FnMut(usize) -> usize
    {
        Point(f(self.0), f(self.1))
    }

    fn move_by(self, amount: usize, dir: Direction) -> Option<Self> {
        use Direction::*;
        match dir {
            Left  => self.0.checked_sub(amount).map(|x| Point(x, self.1)),
            Right => self.0.checked_add(amount).map(|x| Point(x, self.1)),
            Up    => self.1.checked_sub(amount).map(|y| Point(self.0, y)),
            Down  => self.1.checked_add(amount).map(|y| Point(self.0, y)),
        }
    }
}

fn next_odd_number(n: usize) -> usize {
    n + if n % 2 == 0 { 1 } else { 0 }
}

fn generate_maze(size: Point) -> Vec<Vec<Cell>> {
    use Direction::*;

    let center = size.map(|v| v / 2).map(next_odd_number);
    let mut stack = vec![Point(center.0, center.1)];

    let mut maze = vec![vec![Cell::Blocked; size.1]; size.0];
    maze[center.0][center.1] = Cell::Free;

    let mut dirs = [Left, Right, Up, Down];
    let mut rng = rand::thread_rng();

    let ensure_in_bounds = |z: Point| {
        if z.0 >= size.0 || z.1 >= size.1 {
            None
        } else {
            Some(z)
        }
    };

    'next_odd_number: while let Some(&point) = stack.last() {
        rng.shuffle(&mut dirs);

        for &dir in &dirs {
            let new_point = match point.move_by(2, dir).and_then(&ensure_in_bounds) {
                Some(new_point) => new_point,
                None => continue,
            };

            if maze[new_point.0][new_point.1] != Cell::Free {
                stack.push(new_point);
                maze[new_point.0][new_point.1] = Cell::Free;
                let middle_point = point.move_by(1, dir)
                    .expect("Middle point cannot be out-of-bounds");
                maze[middle_point.0][middle_point.1] = Cell::Free;
                continue 'next_odd_number;
            }
        }
        stack.pop();
    }

    maze[0][1] = Cell::Free;
    maze[size.0 - 1][size.1 - 2] = Cell::Free;
    maze
}

fn print_usage() -> ! {
    writeln!(std::io::stderr(), "Usage: maze-gen width height [output.png]")
        .expect("Unable to write to stderr");
    std::process::exit(1);
}

fn main() {
    let args: Vec<_> = std::env::args().skip(1).collect();
    if args.len() < 2 {
        print_usage();
    }

    let mut dimensions = [0; 2];
    for (i, arg) in args.iter().enumerate() {
        dimensions[i] = arg.parse().unwrap_or_else(|_| print_usage());
    }
    let requested_size = Point(dimensions[0], dimensions[1]);
    let valid_size = requested_size.map(next_odd_number).map(|v| max(v, 3));

    if valid_size != requested_size {
        writeln!(std::io::stderr(),
                 "Warning: Adjusting sizes to {} and {}!",
                 valid_size.0,
                 valid_size.1)
            .expect("Unable to write to stderr");
    }

    let maze = generate_maze(valid_size);

    let buf = image::ImageBuffer::from_fn(valid_size.0 as u32, valid_size.1 as u32, |x, y| {
        let luma = match maze[x as usize][y as usize] {
            Cell::Free => 255,
            Cell::Blocked => 0,
        };
        image::Luma([luma])
    });
    let filename = args.get(2).map(String::as_str).unwrap_or("out.png");

    let mut file = std::fs::File::create(filename).expect("Couldn't open the file");
    image::ImageLuma8(buf).save(&mut file, image::PNG).expect("Coulding write the file");
}

I might next look into implementing a flat storage for the maze, one that can be directly indexed by a Point. That would allow this change:

-maze[middle_point.0][middle_point.1] = Cell::Free;
+maze[middle_point] = Cell::Free;

I'll be honest and say that I strongly dislike the usage of the labelled loop. I don't think I've ever needed them, and they are one of the rarest constructs in Rust, based on my experience. I'd probably spend a lot of time (more than I already did...) trying to rewrite that in a different manner.


Clean way to handle file not found exception BACKGROUND
I have a function where i cache file from aws s3. I provide it the path on s3 and also the local path of where i want the cached file to live. Sometimes if the file does not exist on s3 it will return a 404 error. That makes sense but i want to handle it gracefully. What i want to do is if the file i am trying to cache does not exist I want my get_overview_stream function to return None.
CODE
def get_overview_stream(*, environment, proxy_key):
    s3_overview_file_path = f"s3://{FOO_TRACK_BUCKET}/{environment}"
    overview_file = f"{proxy_key}.csv"
    local_path = cache_directory(environment)
    try:
        cache.cache_file(s3_overview_file_path, local_path, overview_file)
        overview_file_cache = local_path / f"{proxy_key}.csv"
        return overview_file_cache.open("r")
    except botocore.exceptions.ClientError as e:
        if e.response["Error"]["Code"] == "404":
            return None
        else:
            raise

Attempted Solution Issue
I am very new to python so i am not sure if this the best way to handle this exception and if there is a more cleaner way. If so i would love to hear feedback.

I would suggest not opening the file in your function but instead returning the path or None, such that the caller can open the file using a with statement.


C++ Quicksort implementation I wrote this quicksort implementation in C++

#include <iostream>
#include <vector>
#include <algorithm>
#include <cassert>

void myquicksort(std::vector<int>& A, const int left, const int right)
{
  int i = left;
  int j = right;
  int pivot = A[left + ((right - left) / 2)];

  while(i < j)
  {
    while(A[i] < pivot) ++i;
    while(A[j] > pivot) --j;

    if(i <= j)
    {
      std::swap(A[i], A[j]);
      ++i;
      --j;
    }
  }

  if(left < j) myquicksort(A, left, j);
  if(i < right) myquicksort(A, i, right);
}

void quick_sort(std::vector<int>& A)
{
  myquicksort(A, 0, A.size() - 1);
}

int main()
{
  std::vector<int> v{8, 37, 4, 12, 1, 19, 13, 6, 23, 42, 3};
  auto v2 = v;
  quick_sort(v);
  std::sort(v2.begin(), v2.end());
  assert(v2 == v);
}

Can you share your opinions?

One remark: I am curious about times where i == j on some occasions in those lines:

  if(left < j) myquicksort(A, left, j);
  if(i < right) myquicksort(A, i, right);

therefore calling myquicksort with overlapping ranges. Is that an issue?

void myquicksort(std::vector<int>& A, const int left, const int right)
{
    // .......
}

Is myquicksort() supposed to be callable by those that use your sort?  If not, hide those internal implementation details by using a namespace to signify that.

std::vector::operator[] parameter takes a pos of type size_type.  Consider using a similar type (std::vector<int>::size_type, std::size_t).

int pivot = A[left + ((right - left) / 2)];

Certain inputs can make your partitioning algorithm behave poorly, degrading from \$O(n\ log\ n)\$ to \$O(n^2)\$.  In your implementation, your pivot choice of picking the median is vulnerable to organ-pipe input sequences (\$1, 2, ..., n/2, ..., 2, 1\$).  If you want to use the median, consider using a stronger pivoting rule, such as the median-of-\$k\$ (median-of-3, median-of-9, etc).

Certain inputs also just break your code. Your implementation produces a segmentation fault when trying to sort an empty sequence.

  One remark: I am curious about times where i == j on some occasions in those lines:

if(left < j) myquicksort(A, left, j);
if(i < right) myquicksort(A, i, right);

  therefore calling myquicksort with overlapping ranges. Is that an issue?

You won't be able to parallelize it using task parallelism while doing more comparisons and utilizing more space.  Just have your two calls work over the ranges [left, j] and [j + 1, right].

Keep functions short and simple.  They become easier to understand, test, and reuse.  Ideally, your myquicksort would refactor the median calculation and partitioning into their own functions.  Example:

void myquicksort(std::vector<int>& A, const std::size_t left, const std::size_t right) {
    if (right - left <= 1) {
        return;
    }
    const auto pivot_value = range_median(A, left, right);
    const auto partition_point = hoare_partition(A, left, right, pivot_value);
    myquicksort(A, left, partition_point);
    myquicksort(A, partition_point + 1, right);
}

Now if you decide that you want to use a different pivoting or partitioning strategy, you are affording yourself the opportunity to reuse code.


Running a shell command and getting output This is the code I'm currently using to run a shell command on the current machine, intended for use only with GNU/Linux at the moment:

std::string getCmdOutput(const std::string& mStr)
{
    std::string result, file;
    FILE* pipe{popen(mStr.c_str(), "r")};
    char buffer[256];

    while(fgets(buffer, sizeof(buffer), pipe) != NULL)
    {
        file = buffer;
        result += file.substr(0, file.size() - 1);
    }

    pclose(pipe);
    return result;
}

Example use:

getCmdOutput(R"( sudo pacman -Syyuu )");
auto output(getCmdOutput(R"( echo /etc/pacman.conf )"));

I use this when making simple script-like C++ programs (mostly for personal use).

Is this the "correct" way of doing it? What can be improved?

Prefer comparing pointers to nullptr instead of to NULL in C++11. nullptr exists for exactly that reason. The NULL macro is not portable. While it exists in many different environments there is no guarantee how it will be implemented or what it is intended to be used for. It may be implemented as 0, '\0', or as some pointer type, or anything else, and every different environment may define it differently or not at all. It is non-standard. nullptr is defined by the standard and all C++11 standard compliant compilers must support it.


Generalising the data binding operation Can I create a method to do work with these five sections? I'm doing binding operations so many times.  Do you have an idea on how to make this code prettier?

ddlModelValue -> ASP.NET DropDownList

var dtModel = Session[SystemConstant.ModelSessionKey] as DataTable;
ddlModelValue.DataSource = dtModel;
ddlModelValue.DataTextField = "Name";
ddlModelValue.DataValueField = "Name";
ddlModelValue.DataBind();

var dtGroup = Session[SystemConstant.GroupSessionKey] as DataTable;
ddlGroupValue.DataSource = dtGroup;
ddlGroupValue.DataTextField = "Name";
ddlGroupValue.DataValueField = "Name";
ddlGroupValue.DataBind();

var dtFit = Session[SystemConstant.FitSessionKey] as DataTable;
ddlFitValue.DataSource = dtFit;
ddlFitValue.DataTextField = "Name";
ddlFitValue.DataValueField = "Name";
ddlFitValue.DataBind();

var dtShellType = Session[SystemConstant.ShellTypeSessionKey] as DataTable;
ddlShellTypeValue.DataSource = dtShellType;
ddlShellTypeValue.DataTextField = "Name";
ddlShellTypeValue.DataValueField = "Name";
ddlShellTypeValue.DataBind();

var dtDataTypeCode = Session[SystemConstant.DataTypeCodeSessionKey] as DataTable;
ddlDataTypeCodeValue.DataSource = dtDataTypeCode;
ddlDataTypeCodeValue.DataTextField = "Name";
ddlDataTypeCodeValue.DataValueField = "Name";
ddlDataTypeCodeValue.DataBind();

This would be a start:

Sub bindTableToControl(table as DataTable, _
                       control, _
                       Optional textField As String = "Name", _
                       Optional valueField As String = "Name")

 control.DataSource = table;
 control.DataTextField = textField;
 control.DataValueField = valueField;
 control.DataBind();

End Sub

bindTableToControl( ddlModelValue , Session[SystemConstant.ModelSessionKey] );
bindTableToControl( ddlGroupValue, Session[SystemConstant.GroupSessionKey] );
bindTableToControl( ddlFitValue, Session[SystemConstant.FitSessionKey] );
bindTableToControl( ddlShellTypeValue, Session[SystemConstant.ShellTypeSessionKey] );
bindTableToControl( ddlDataTypeCodeValue, Session[SystemConstant.DataTypeCodeSessionKey] );

Basically I removed most of the repetition. I also removed all the Hungarian prefixes in the Sub.

As a side note, I wonder why SystemConstant is part of the Session, that does not seem memory efficient?


Using Euler-Maruyama method to solve Ornstein-Uhlenbeck equation (SDE) I am trying to implement the Euler–Maruyama method and use it to solve the Ornstein–Uhlenbeck process. I am basing my code on the wikipedia page where a python implementation is shown. More generally I'm trying to get to grips with stochastic differential equations in c++.
This is my code
#include <iostream>
#include <fstream>
#include <random>
#include <vector>
#include <cmath>

using namespace std;

int num_sims = 5;
double t_init = 3;
double t_end  = 7;
int N         = 1000;
float dt      = (float)(t_end - t_init) / N;
double y_init = 0;

double c_theta = 0.7;
double c_mu    = 1.5;
double c_sigma = 0.06;

double dW ( double dt ){
    normal_distribution<double> distribution( dt , sqrt(dt) );
    random_device rd;
    default_random_engine generator(rd());

    return distribution(generator);
}

double mu( double y, double t ){
    return c_theta * (c_mu - y);
}

int main()
{
    vector<double> ts;
    for (double t = t_init; t < t_end + dt; t += dt){
        ts.push_back(t);
    }

    vector<double> ys(N+1, 0);
    ys[0] = y_init;

    for (int h = 0; h < num_sims; h+=1){
        for (int i = 1; i < ts.size(); i+=1 ){
            double t = (i - 1) * dt;
            double y = ys[i - 1];
            ys[i] = y + mu(y, t) * dt + c_sigma * dW(dt);

            cout << h << '\t' << ys[i] << endl;
        }
    }
    return 0;
}

This code seems to work well when I plot the results in python. My question is if this code can be written in a more efficient way? I feel like I should be using the boost packages on some level, and also maybe structures. I'm quite new to c++, so maybe my translation of the python code is a bit too word for word.

Make constants constexpr
You can tell C++ that some of your variables are constast by marking them const. If the constants can be computed at compile time, you can even mark them constexpr. Doing this helps the compiler generated more optimal code and will result in an error if you accidentily do try to modify a constant. So:
constexpr int num_sims = 5;
constexpr double t_init = 3.0;
...

Mark variables and functions static where appropriate
If a variable or function is not used by any other source files, you should mark them static. This again might help the compiler produce more efficient code, and can prevent conflicts with other variables and functions with identical names in other source files in your project. It looks like everything except main() can be made static.
Optimize random number generation
You are using the right functions to create normal distributed random numbers, but the way you have written it, it will instantiate a new distribution, random device and generator for each call to dW(). You only need to create them once. To do this, move everything except the return statement out of the function (and make them static):
static normal_distribution<double> distribution(dt, sqrt(dt));
static random_device rd;
static default_random_engine generator(rd());

static double dW(double dt) {
    return distribution(generator);
}

Use N consistently
You created a constant for the number of datapoints you are generating. Once you have done that, use it consistently everywhere. Don't start mixing N, ts.size() and other ways to calculate the size of the array, since this might result in slight differences that will cause problems. For example, floating point values have a limited precision, and because of that the following for-loop might actually run more or less than N iterations:
for (double t = t_init; t < t_end + dt; t += dt) {

Use N consistently:
for (int i = 0; i < N; ++i) {
    ts.push_back(t_init + i * dt);
}

...

for (int h = 0; h < num_sims; ++h) {
    for (int i = 1; i < N; ++i) {
        ...

Alternatively, you could also use ts.size() everywhere, as long as you are consistent. In that case, the line to change is:
vector<double> ys(ts.size());

Note that ys doesn't need to be one larger than ts, and you don't need to initialize it with zeroes, as you will overwrite all values anyway.
Use ++ to increment counters
It's customary in C and C++ to use the ++ and -- operators to increment and decrement counters. And since it can give a slight performance benefit in some cases, prefer to use the prefix version to the postfix version if there is no difference to the result. So:
for (int i = ...; ...; ++i)

Avoid using std::endl
You should avoid using std::endl and use '\n' instead. The former is equivalent to the latter, but it will also force a flush of the output, which is often unnecessary and might slow down your application.
Do you need std::vectors at all?
You are storing the time and y-coordinates of the datapoints in the vectors ts and ys, but you never read them back. The only thing you use is the value of y of the previous timestep, but this could easily be held in a single variable:
int main() {
    for (int h = 0; h < num_sims; ++h) {
        double y = y_init;

        for (int i = 0; i < N; ++i) {
            double t = i * dt;
            y += mu(y, t) * dt + c_sigma * dW(dt);
            cout << h << '\t' << y << '\n';
        }
    }
}

Avoid unnecessary complexity

I feel like I should be using the boost packages on some level, and also maybe structures.

This problem absolutely doesn't need Boost packages nor structs. Follow the KISS principle.


Lock-free bounded stack in C++11 Here is a classical attempt to implement a lock-free stack (based on this article). I rewrote it in C++11 and also tried to add memory orders which I would like to confirm to be correct.

template<typename T>
class stack
{
    struct node_t final
    {
        T value;
        node_t *next;
    };

    struct head_t final
    {
        uintptr_t aba;
        node_t *node;
        head_t() noexcept :aba(0),node(nullptr)
        {}
        head_t(node_t* ptr) noexcept :aba(0),node(ptr)
        {}
    };

    static_assert(sizeof(head_t) == 2*sizeof(uintptr_t), "Stack head should be 2 pointers size.");

    typedef typename std::aligned_storage<sizeof(node_t), std::alignment_of<node_t>::value>::type aligned_node_t;

    std::unique_ptr<aligned_node_t[]> buffer_ptr;
    node_t *node_buffer;
    char cache_padding_1_[64 - sizeof(node_t*)];
    std::atomic<head_t> head;
    char cache_padding_2_[64 - sizeof(std::atomic<head_t>)];
    std::atomic<head_t> free_nodes;
    char cache_padding_3_[64 - sizeof(std::atomic<head_t>)];

public:

    stack(size_t capacity)
    {
        head.store( head_t(), std::memory_order_relaxed );
        // preallocate nodes
        buffer_ptr.reset(new aligned_node_t[capacity]);
        node_buffer = reinterpret_cast<node_t*>(buffer_ptr.get());
        for(size_t i = 0; i < capacity - 1; ++i)
        {
            node_buffer[i].next = &node_buffer[i + 1];
        }
        node_buffer[capacity-1].next = nullptr;
        free_nodes.store( head_t(node_buffer), std::memory_order_relaxed );
    }

    template<class U>
    bool push(U && data)
    {
        node_t *node = _pop(free_nodes);
        if (node == nullptr)
            return false;
        node->value = std::forward<U>(data);
        _push(head, node);
        return true;
    }

    bool pop(T& data)
    {
        node_t *node = _pop(head);
        if (node == nullptr)
            return false;
        data = std::move(node->value);
        _push(free_nodes, node);
        return true;
    }

private:
    node_t* _pop(std::atomic<head_t>& h)
    {
        head_t next, orig = h.load(std::memory_order_relaxed);
        do {
            if (orig.node == nullptr)
                return nullptr;
            next.aba = orig.aba + 1;
            next.node = orig.node->next;
        } while (!h.compare_exchange_weak(orig, next,
                                        std::memory_order_acq_rel,
                                        std::memory_order_acquire));
        return orig.node;
    }

    void _push(std::atomic<head_t>& h, node_t* node)
    {
        head_t next, orig = h.load(std::memory_order_relaxed);
        do {
            node->next = orig.node;
            next.aba = orig.aba + 1;
            next.node = node;
        } while (!h.compare_exchange_weak(orig, next,
                                        std::memory_order_acq_rel,
                                        std::memory_order_acquire));
    }
};

The general push/pop logic of the stack looks fine. The memory management however is somewhat flawed:

Missing Destructor

From the perspective of the unique ptr, the stack doesn't contain Nodes, but uninitialized memory (effectively a char array). So at destruction of the stack object, it won't call the destructor of the individual nodes, which in turn won't call the destructor of value.

In order to correct that, one could write a destructor for stack that traverses the nodes and manually calls the destructor on them.

Assignment into uninitialized variables

node->value = std::forward<U>(data);

Upon construction, no actual nodes are created, but only properly aligned memory is reserved, on which you later call the assignment operator. I believe (although I'm not sure) this is OK if T is a POD, but if not, then (move) assigning values to them is definitively not allowed, because custom move assignment operators usually assume that this points to a valid, initialized object.

So in general, I see two possibilities:

You default construct all nodes upon construction of the stack and destruct them when stack is destructed, in which case std::aligned_storage is unnecessary. One can just use an array of nodes and pass it to the unique_ptr. In this case you also don't need a destructor for stack. Obviously that only works with default constructable T's and makes only sense, if T doesn't hold any resources after it has been moved from.
You actually create and delete objects upon calls to push and pop, in which case you have to use placement new (new(&node->value)T(std::forward<U>(data));) and manually call the destructor node->value.~T() (not delete &node->value)

Padding
I just want to point out that aside from the false sharing between head and free_nodes, which is avoided via padding, any thread calling push or pop will access both variables anyway and afterwards you still have false sharing between the actual nodes.

Performance
This code contains a lot of micro optimizations, which might or might not be worth the effort if the code gets reused in different contexts (its definitively  not worth for handling a worker pool). However, I'd suggest, to write at least few small benchmarks to ensure that those optimizations are in fact improving and not hurting performance.

Also I want to repeat my warning from the comments that for some reason, VS2013 (and also 2015RT) seems to be unable to implement std::atomic<head_t> in a lock-free manner, in which case the whole structure is probably much slower than a normal stack with a mutex.


Filtering long lists of images I have to filter some very long lists of images many times (thousands, even 10/15k, even much more), so performance is important.

First solution:

var image_match = [];
$('#'+id_canvas+' image').each(function(i, el) {
    if (el.attributes.x.value === x && el.attributes.y.value === y) {
        image_match.push($(this));
    };
});

Second solutions:

var image_match = $('#'+id_canvas+' image[x='+x+'][y='+y+']');

Which solution is faster? Or maybe a non-jQuery solution?

Also, I can often know how many images will pass the test so when I found that number of images I can stop the search, like this:

var image_match = [];
var target_number = 3;
$('#'+id_canvas+' image').each(function(i, el) {
    if (el.attributes.x.value === x && el.attributes.y.value === y) {
        image_match.push($(this));
        if (image_match.length === target_number) {
            return false;
        };
    };
});

Can I do the same thing in the second solution?

I suspect that performance with 15k images will be bad no matter what you do. You might want to rethink your UI.

If you want to know which is the fastest solution you will have to measure them. Though I would guess the second is faster. You could test a direct document.querySelector though jQuery should be passing the query through pretty much as is. The one suggestion I would make is instead of interpolating your image_id each time give all of the images a class and use a class selector.

As to your last question, I don't know of anyway to do it using pure css and there would probably be little advantage anyway.


Too Many Staff Meetings I recently got in really deep on a question to find the overlapping meetings and I rewrote a bit of the code and added some things to the code, and I would just like a general review of my code and what I could do to make it even better, maybe performance wise or whatever.

Here is my LINQPad 5 Code Dump, you should be able to copy paste into LINQPad and run it, I don't think I did anything that is too cutting edge.

{
    var format = System.Globalization.CultureInfo.InvariantCulture.DateTimeFormat;
    Meeting[] meetings = new Meeting[]
    {
        new Meeting(DateTime.Parse("1/1/2015 21:30", format), DateTime.Parse("1/1/2015 23:00", format)),
        new Meeting(DateTime.Parse("1/1/2015 23:10", format), DateTime.Parse("1/1/2015 23:30", format)),
        new Meeting(DateTime.Parse("1/1/2015 20:00", format), DateTime.Parse("1/1/2015 21:30", format)),
        new Meeting(DateTime.Parse("1/1/2015 22:10", format), DateTime.Parse("1/1/2015 22:20", format)),
        new Meeting(DateTime.Parse("1/1/2015 23:20", format), DateTime.Parse("1/1/2015 23:50", format)),
        new Meeting(DateTime.Parse("1/1/2015 23:20", format), DateTime.Parse("1/2/2015 00:00", format)),
        new Meeting(DateTime.Parse("1/2/2015 09:00", format), DateTime.Parse("1/2/2015 12:00", format)),
        new Meeting(DateTime.Parse("1/2/2015 09:00", format), DateTime.Parse("1/2/2015 10:00", format)),
        new Meeting(DateTime.Parse("1/2/2015 11:00", format), DateTime.Parse("1/2/2015 11:30", format))
    };

    var overlappingMeetings = meetings.Overlappings();
    var meetingsOverlap = meetings.Overlappings().Any();
    overlappingMeetings.Dump();
    meetingsOverlap.Dump();
}

public static class StaffMeetings
{
    public static IEnumerable<Meeting[]> Overlappings(this IEnumerable<Meeting> meetings)
    {
        var first = (Meeting)null;
        var orderedMeetings = meetings.OrderBy(m => m.Start);
        var checkedMeetings = new List<Meeting>();

        foreach (var meeting in orderedMeetings)
        {
            if (first != null)
            {
                checkedMeetings.Add(first);
                var uncheckedMeetings = orderedMeetings.Where(x => (x.Start >= first.Start && !(x == first)) && !checkedMeetings.Any(m => m == x));
                foreach (var meet in uncheckedMeetings)
                {
                    if (first.OverlapsWith(meet))
                    {
                        yield return new[] { first, meet };
                    }
                }
            }
            first = meeting;
        }
    }
}
public class Meeting
{
    public DateTime Start { get; private set; }
    public DateTime End { get; private set; }

    public Meeting(DateTime start, DateTime end)
    {
        if (end < start)
        {
            throw new ArgumentException("Cannot create a meeting that ends before it starts.");
        }
        this.Start = start;
        this.End = end;
    }

    public bool OverlapsWith(Meeting second)
    {
        return this.End > second.Start;
    }
    public override bool Equals(object obj)
    {
        return base.Equals(obj);
    }
    public override int GetHashCode()
    {
        return base.GetHashCode();
    }
}

here are the results of that run as well

Did I miss anything?

Are there more edge cases that I didn't consider?

Is there a nice, neat, acceptable way of streamlining the foreach statements with the if statements inside of them?  (Kind of curious if it is possible to one line it)

Just a quick shot
This

public override bool Equals(object obj)
{
    return base.Equals(obj);
}
public override int GetHashCode()
{
    return base.GetHashCode();
}

doesn't buy you anything. Overriding a method and the calling the base method is just rubbish which lays around in the codebase.


Standard primitive types vs custom structs in public API I am writing a .NET library and have a lot of properties like this:

public ??? SomeProperty { get; set; }

where valid values are 0-15 (or from 00000000 to 00001111 in binary). So I should choose a proper type of the property. I see two options:

byte

I can use byte and insert value validation in each property setter:

public byte SomeProperty
{
    get { return _someValue; }
    set
    {
        if (value > 15)
            throw new ArgumentOutOfRangeException(nameof(value), value, "Message.");

        _someValue = value;
    }
}

Advantages:

you don't need to cast literals from 0 to 15 to some another type.

Disadvantages:

value check must be inserted in each property setter;
property signature doesn't reflect valid values.

custom value type

Or I can create a struct like FourBitNumber handling invalid values in its constructor:

public struct FourBitNumber
{
    private readonly byte _someValue;

    public FourBitNumber(byte value)
    {
        if (value > 15)
            throw new ArgumentOutOfRangeException(nameof(value), value, "Message.");

        _someValue = value;
    }
}

and use this struct as type for the property:

public FourBitNumber SomeProperty { get; set; }

Advantages:

user of the API takes care about passed values by casting them to FourBitNumber so the library always gets valid values;
property type clearly says what valid values are.

Disadvantages:

you need always cast values of primitive types to FourBitNumber which is inconvenient especially with literals.

At now the second way seems better for me but necessity of casting everywhere is really annoying. What other aspects should I take into an account to take final decision?

custom value type

I would use this becasue the disadvantage that you name

  you need always cast values of primitive types to FourBitNumber which is inconvenient especially with literals.

can be easily overcome by implementing an implict operator for FourBitNumber to byte:

public static implicit operator byte(FourBitNumber value) => value._someValue;

and an explicit one for byte to FourBitNumber

public static explicit operator FourBitNumber(byte value) => new FourBitNumber(value);

so now it doesn't have any disadvanteges ;-)


Upload a directory, in parallel, to an artifact repository This code uploads an entire directory and allows for its reparenting on a remote server (an artifact repository).

Three concerns:

Copying the files locally takes a lot of space. Ideally, I could use inotify and as soon as my wget -m had a file I could begin immediately uploading and then deleting. These repositories can be hundreds of gigs
For performance reasons I fork/join everything. This definitely improves overall job performance but I get an error upload.sh: fork: retry: Resource temporarily unavailable which is the OS telling me I can't have more sub processes. My concern is that some of these processes will terminate without doing their job because of this.
Lastly, and I can't be sure of this, but several of the larger files that upload seem to never complete when I use a fork. But when I do a 1-off they finish in an appropriate time

First I run:

wget --user cbongiorno --password abc123 --mirror --no-parent https://artifactory.stback.com/artifactory/BIF-Releases &>  my.logs

Then I do the upload. It would be nice to upload and delete files as they come in

#!/usr/bin/env bash

[[ -z "$1" ]] && echo "no resource directory supplied" && exit -1
[[ -z "$2" ]] && echo "no destination repository supplied" && exit -1

upload() {
    local local_repo="$1"
    local remote_repo="$2"
    local ct
    local user=${3:-${USER:-$(read -p "Username: "  user && [ -n "${user}" ] && echo ${user})}}
    local pwd=$(read -s -p "Password: "  pwd && [ -n "${pwd}" ] && echo ${pwd})
    echo ""
    declare -a pids
    for art in $(find "${local_repo}" -not -path '*/\.*' -type f -not -name 'index.html' | sed -n 's;./\(.*\);\1;p'); do

#        file extension
        case "$(echo ${art} | rev | cut -d'.' -f1 | rev)" in
            jar)
                ct="application/java-archive"
            ;;
            md5)
                ct="application/x-checksum"
            ;;
            pom)
                ct="application/x-maven-pom+xml"
            ;;
            sha1)
                ct="application/x-checksum"
            ;;
            txt)
                ct="text/plain"
            ;;
            xml)
                ct="application/xml"
            ;;
        esac
        (
            echo "${art} -> ${remote_repo}/${art}"
            curl -v -u "${user}:${pwd}" -X PUT -d "@$art" -H "Content-Type: $ct" "$remote_repo/$art" 2&>1 &&
            curl -I -u "${user}:${pwd}" "$remote_repo/$art" 2&>1
        )  &

        pids[${i}]=$!

    done
    for pid in ${pids[*]}; do
        wait ${pid}
    done
}
upload "$@"

I think you'll find that inotify has problems of its own and that you're already on the right path.

The approach I'd use: get this working the way you want, then run it in a loop while your wget runs, removing successful files as it goes.

If you're concerned about polling overhead (I wouldn't be: the size of the directory should remain manageable so long as you're uploading/deleting along the way), have the wget process emit filenames into a log file or a named pipe, while the curl process consumes them.

You can attack the resource problem in a couple of ways. One, use exec to launch curl so that there isn't an intermediate bash process sitting around waiting for curl to exit.  Two, keep count of the number of children, and reap them when the number gets too high.  Three, check ulimit -u and consider increasing that number.

In the code below I've made a few lesser edits:

content-types in an associative array with sensible default
use [[ ]] instead of [ ] (the former is a bash builtin)
clean up the find command line a little.  I've guessed that the sed invocation is meant to remove $local_repo?
omit forked echo and curl-HEAD; rely on exit status instead
use curl's -w to get a useful line of output on success
store pids in an associative array with filenames as values, to allow meaningful response to outcomes
"Content-type" has a lowercase t
run upload in infinite loop, keep count of files found, exit when no more files are found

There may be some bugs, since I don't have your environment to test.  Proceed accordingly.

#!/usr/bin/env bash
reap() {
    for pid in ${!pid2fn[@]}
    do
        local fn=${pid2fn[$pid]}
        if wait ${pid}
        then
            echo successful upload of $fn
            # rm $fn
        else
            echo $fn upload failed with status $?
        fi
        unset pid2fn[$pid]
    done
}
upload() {
    local local_repo="${1:?no resource directory supplied}"
    local remote_repo="${2:?no destination repository supplied}"
    local default_ct=application/octet-stream
    local user=${3:-${USER:-$(read -p "Username: "  user && [[ -n $user ]] && echo $user) }}
    local pwd=$(read -s -p "Password: "  pwd && [[ -n $pwd ]] && echo $pwd )
    local max_children=100
    local found=0
    declare -A pid2fn
    declare -A types=(
        [jar]=application/java-archive
        [md5]=application/x-checksum
        [pom]=application/x-maven-pom+xml
        [sha1]=application/x-checksum
        [txt]=text/plain
        [xml]=application/xml
    )
    for art in $( find "$local_repo" -type f -name "[^.]*" -not -name index.html | sed s,^$local_repo/*,, )
    do
        let found=found+1
        local ct=${types[${art##*.}]:-$default_ct}
        local dest="$remote_repo/$art"
        local status="$art -> $dest | response %{http_code} sent %{size_upload} bytes\n"
        exec curl -u "$user:$pwd" -X PUT -d "@$art" -H "Content-type: $ct" -w "$status" "$dest" 2>&1 &
        pid2fn[$!]=$art
        [[ ${#pid2fn[@]} -ge $max_children ]] && reap
    done
    reap
    [[ $found -gt 0 ]]
}
while upload "$@"; do :; done


C getline() implementation I'm practicing my C coding and I wanted to implement my own version of the getline function in C for learning purposes. I'd like a review on coding style, correctness, improvements I can make for performance, and overall quality of the code. My main concerns are correctness and performance (in that order).

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

#define MAX_LINE_LENGTH 255

/* Retrieves a line of text from the stream provided
 * and places it into @buf until a new line character is
 * reached or the number of characters read is > @size - 1.
 * This function will null-terminate the provided buffer.
 *
 * @param[in] -- stream -- the stream
 * @param[in] -- buf -- a buffer big enough for @size chars.
 * @param[in] -- size -- the maximum number of chars to read (must
 *    include room for a null terminator
 * @return -- the number of characters read from the stream.
 */
size_t getline(FILE *stream, char *buf, size_t size)
{
    size_t count = 0;
    char c;
    while ((c = (char)getc(stream)) != '\n' && count < size - 1) {
        buf[count++] = c;
    }
    buf[count] = '\0';
    return count;
}

int main()
{
   char line[MAX_LINE_LENGTH];
   while (true) {
       size_t count = getline(stdin, line, MAX_LINE_LENGTH);
       printf("The line gotten was \"%s\" and was %zu chars long.\n", line, count);
   }
}

Your getline function looks more like a variant of fgets than getline.
If size == 0, size - 1 == SIZE_MAX, a very large number.
Your getline reads up to size bytes from the stream even though it only places only up to size - 1 into the buffer. It simply drops the last byte silently. You should switch the order of the loop condition:

while (count < size && (c = (char)getc(stream)) != '\n') { ... }

Your getline also writes to memory outside the range defined by its parameters if size is 0, when it writes the terminating null byte.
If getc(stream) == EOF, which is an error condition, your getline function will keep trying to read from the stream, always "reading" EOF, placing (char) EOF (usually '\xff') into the buffer until the end of the buffer is reached.

You can try this by pointing stdin at an empty file.
Use a for loop instead of a while loop, if you're using an iteration counter. It'll be much clearer, what is incremented where and how.

This is how I would rewrite your function considering the above points:

ssize_t fgets(FILE *stream, char *buf, size_t size)
{
  if (size == 0)
    return 0;

  size_t count;
  int c = 0;
  for (count = 0; c != '\n' && count < size - 1; count++)
  {
    c = getc(stream);

    if (c == EOF) {
      if (count == 0)
        return -1;
      break;
    }

    buf[count] = (char) c;
  }

  buf[count] = '\0';
  return (ssize_t) count;
}


Python prime number generator I'm trying to get a better grasp on generators in Python because I don't use them enough (I think generators are cool). To do so I wrote a generator for prime numbers:

def primes():
    x = 2
    while True:
        for y in xrange(2, x/2 + 1):
            if x % y == 0:
                break
        else:
            yield x
        x = x + 1

This works as expected but if I use list comprehension to create a list of primes using this generator it's really slow when compared to sieve function that generates the same list:

[prime.next() for i in xrange(1000)] # exectues in 0.212 sec, sieve in 0.001 sec

I'm assuming that this is not the intended purpose of a generator, but I thought I would see if anyone can think of a way to make this faster.

Stylistically, using itertools.count() rather than implementing your own counter would be a bit cleaner.

I also prefer using the any() over a generator rather than an explicit for loop with a break and else. I think it's easier to follow.

Algorithmically, the sieve has a big advantage because it uses the previously calculated primes to make checking the next prime faster. You don't need to check every number for divisions, just the primes. You could do something like:

past_primes = []
for x in itertools.count(2):
    if not any(x % prime == 0 for prime in past_primes):
        past_primes.append(x)
        yield x

Performing the division will still be more expensive than what the sieve has to do. You could implement the entire sieve algorithm in a generator. But I'm not sure what purpose that would serve.

Generally, generators work nicely when you don't have to maintain a lot of state for each value generated. But that's not the case for primes.


Counting sort in Swift I have this sorting algorithm which takes an array of dictionary values:

guard var imageUrlString = anyImage.value as? [String:AnyObject] else { return }

I then loop through it, adding the values with the smallest Int key to a new array to be used afterward. Thus the values in this array of AnyObjects would go from 1 to n.

I was wondering if I could make it more concise.

var values = [AnyObject]()
var keys = [String]()
var Done = false
var j = 1

 while !Done {
    for i in imageUrlString {
        let key = Int(String(i.key.last!))

        if j == key {
            values.append(i.value)
            keys.append(i.key)
            print(i, " This is the i for in if ")
            if imageUrlString.count == j {
                print("Done yet: yes", values[0], " ", values[3])
                Done = true
                break;
            }
            j+=1
        } else {
            print("No,,.")
        }
    }
}

Consider:

let dictionary = ["3": "c", "1": "a", "2": "b”]

If you want to get separate arrays of keys and values, sorted by the order of the Int values of the keys, you can do:

let (keys, values) = dictionary.map { (Int($0.key) ?? 0, $0.value) }  // get `Int` keys
    .sorted { $0.0 < $1.0 }                                           // now sort by those `Int` values
    .reduce(into: ([Int](), [String]())) { arrays, entry in           // divide that up into two arrays
        arrays.0.append(entry.0)
        arrays.1.append(entry.1)
}

That results in a keys of [1, 2, 3] and values of ["a", "b", "c"].

But a key observation is that you actually have to explicitly sort your results. Dictionaries, unlike arrays, are unordered. E.g. regardless of what order the keys appeared in a dictionary literal or JSON, when you iterate through the key-value pairs in a dictionary, the order may be different. You have to convert to an array and sort it yourself if order is important.

The above arbitrarily uses 0 for any keys (if any) that couldn’t be converted to an Int. If you just wanted to drop those entries, you could alternatively do:

let (keys, values) = dictionary.compactMap { entry in Int(entry.key).map { ($0, entry.value) } }
    .sorted { $0.0 < $1.0 }                                       // now sort by those `Int` values
    .reduce(into: ([Int](), [String]())) { arrays, entry in       // divide that up into two arrays
        arrays.0.append(entry.0)
        arrays.1.append(entry.1)
}