Generate a set of random values based on seed I am a novice when it comes to Python. I am working stuff related to Flowshop scheduling and so to generate benchmarks I used one by Taillard (Link to his homepage). The details are in the technical report.

Since I ported the code from C which I am not really good at please review my Python script:

# Format - random seed, number of jobs, number of machines
# Use a separate file and precede comments with a # symbol

from numpy import int32
import math

problem = list()

# Generate a random number uniformly between low and high
def unif(seed, low, high):
    """This function generates a random number between low\
            and high based on seed
    Seed is implemented as static for set of random values based on a series\
            of seed values"""

    if getattr(unif, "seed_value",0) == 0 or unif.seed_value is None:
        #create attribute seed_value which is static for method unif
        unif.seed_value = seed #inititalization if attribute does not exist

    # constants for random number
    a = 16807
    b = 127773
    c = 2836
    m = 2**(31) - 1

    k = math.floor(unif.seed_value / b)

    unif.seed_value = int32(a * (unif.seed_value % b) - k * c)
    # use numpy 32 bit int
    if unif.seed_value < 0: # 32 bit int overflow
        unif.seed_value = unif.seed_value + m

    value_0_1 = unif.seed_value/float(m)

    return int(low + math.floor(value_0_1 * (high - low + 1))) # random\
            # number generated

# generates a problem for given number of machines and jobs in problem tuple
def generate_flow_shop(p, output):
    """Generates a flow shop problem set for a set in problem tuple and\
            writes to file in output"""
    time_seed = int(problem[p][0])
    d = list() # list containing the problem

    for i in range( int(problem[p][2]) ):
        line = list()

        line = [ unif(time_seed, 1, 99) for j in range(int(problem[p][1])) ]

        # for j in range(0, int(problem[p][1])):
        #     line.append(unif(time_seed, 1, 99))
        # 99 - max duration of op.
        d.append(line)
    unif.seed_value = None

    for line in d:
        datum = ' '.join(map(str,line))
        output.write("%s\n" % (datum))

def main():
    name = input("Enter filename to write the generated values:\n> ")
    seedfile = input("Enter filename of the seed values:\n> ")

    with open(seedfile) as in_file:
        for i in in_file:
            if line[0] == '#':
                continue
            else:
                line = i.strip().split()
                problem.append(tuple( map(int,line) ) )

    with open("%s" % (name), "w") as out_file:
        for i in range(0, len(problem)):
            out_file.write("%s %04d %s %s\n" %\
                           ("p", i, problem[i][1], problem[i][2]))
            generate_flow_shop(i, out_file)

if __name__ == '__main__':
    main()

The link to the original C program is here.

This is the link to the output which is the expected output ie. the code is actually correct.

The main purpose of this module is to generate a set of values that can be used to test our implemented solutions ie. it is a generated problem set but not completely random but reproducible if the seeds entered are the same.

Note: The seedfile is a file that contains the values that are defined in the struct problem in the C program.

What I mainly need feedback on are:

Can the script be improved further with respect to use of Python idioms?
Since the generated file should again be parsed as input for testing, is writing to a file and then opening it again to test the scripts the best way? Is the written file format the best one for parsing again as input?

unif.seed_value is very un-Pythonic. Instead use a class.
Rather than using getattr(unif, "seed_value",0), you can make self.seed_value at object instantiation, and use that as the default, unless it's falsy, where it'd instead use seed.
math.floor is mostly the same as int. If you're using this on division, you can change it to floor divisions using //.
numpy.int32 is odd, it adds a dependency to numpy that shouldn't be needed. Instead you can use & or %, this also means that it doesn't throw an error if the number is greater than (1 << 31) - 1.
unif.seed_value/float(m) is the same as unif.seed_value/m in Python 3. In Python 2 it'd do floor division, unless one number is a float.
generate_flow_shop shouldn't need to take output, that should be implemented in main.
generate_flow_shop should take a single problem rather than the list of problems.
You can use tuple unpacking to give all values in the problem a name.
I heavily recommend str.format over the old % formatting operator. It has less bugs, and is enhanced in 3.6 with f-strings.
You can use _ as a 'throw away variable' in Python, it's somewhat a standard and lets developers ignore that variable, greatly increasing readability.

unif uses some fairly odd math, and so I decided to go for a more succinct approach.
And so I'd change your code to: (note, it's untested)

class Random:
    def __init__(self):
        self.seed_value = None

    def unif(self, seed, low, high):
        seed_value = self.seed_value or seed

        a = 16807
        b = 127773
        c = 2836
        m = (1 << 31) - 1

        seed_value = (a * (seed_value % b) - (seed_value // b) * c)

        self.seed_value = (seed_value + (seed_value < 0)) & m
        return int(low + self.seed_value / m * (high - low + 1))

    def generate_flow_shop(self, problem):
        """Generates a flow shop problem set for a set in problem tuple and\
                writes to file in output"""

        time_seed, num_jobs, num_mach = problem
        r = [
            [
                self.unif(time_seed, 1, 99)
                for _ in range(num_jobs)
            ]
            for _ in range(num_mach)
        ]
        self.seed_value = None
        return r

def main():
    name = input("Enter filename to write the generated values:\n> ")
    seedfile = input("Enter filename of the seed values:\n> ")
    r = Random()

    with open(seedfile) as in_file:
        problems = [
            tuple(map(int, line.strip().split()))
            for line in in_file
            if line[0] != "#"
        ]

    with open(name, "w") as out_file:
        for i, problem in enumerate(problems):
            out_file.write("p {0:04} {1[1]} {1[2]}\n".format(i, problem)))
            data = r.generate_flow_shop(problem)
            out_file.write(''.join(' '.join(line) + '\n' for line in data))

if __name__ == '__main__':
    main()


BST traverse as higher order function Consider the following function to traverse a BST

(defun bst-traverse (fn bst)          ; [1]
  (when bst
    (bst-traverse fn (node-l bst))
    (funcall fn (node-elt bst))       ; [2]
    (bst-traverse fn (node-r bst))))

;; [1] This is a higher order function - recieves a function which it calls on each node.
;; [2] The traverse which makes sense on a bst is the in-order traverse
;;     (i.e. left first, then node, then right, which this is.)

What are some nice uses of this function?

1) Cause a side effect at each node

(bst-traverse #'princ bst)

2) Cons each elt to a global var (also a side effect)

The idea here is this is a good way to get the BST els in descending order.

(defparameter *result* nil)
(bst-traverse (lambda (obj)
               (setf *result* (cons obj *result*)))
             nums)

This leads me to wondering...

Is this higher order function any use at all without relying on a side-effect?

It seems maybe not. Why? - because it doesn't return anything always returns nil.

So, could we or should we make it return something to solve this 'problem'?

The general question I'm seeking to explore here is the purity or non-purity of higher order functions in practical use cases (and hence, how to use them well).

Any discussion/contributions appreciated.

For the record, I started off with this

(defun in-order-traverse (bst)
  (when bst
    (in-order-traverse (node-l bst))
    (format t "~A" (node-elt bst))
    (in-order-traverse (node-r bst))))

Then, when I wanted to get the BST els in reverse order, realised how completely un-reusable that function is in comparison to bst-traverse above. So, I'd like to get a better understanding of what the real flexibility of bst-traverse actually is and how to use it...

[I've realised - the function passed could return something - but how would the higher order function pick which invocation of the lambda to return the return value of if any?!]

Update

Wondering if this would be along the right lines at all

(defun bst-accumulate (fn bst)
  (let (result)
    (when bst
      (bst-accumulate fn (node-l bst))
      (cons (funcall fn (node-elt bst)) result)
      (bst-accumulate fn (node-r bst)))
  result))

Update 2

bst-accumulate above is wrong. This is not the first time I've fallen into the trap of thinking that consing was setting state - actually the return value of the expression above which does the cons is just lost...
To achieve what I wanted to achieve above, I must firstly wrap the cons in a setf so that result is actually modified. Plus there's a second problem; since bst-accumulate is recursive, it will end up redefining result on every call. To fix that, we need a helper function so that we keep the lexical var result outside of the recursive function, thusly: (this time  I tested it before posting :)

(defun bst-accumulate (fn bst)
  (let (result)
    (labels ((bst-acc (fn2 bst2)
                      (when bst2
                        (bst-acc fn2 (node-l bst2))
                        (setf result (cons (funcall fn2 (node-elt bst2)) result))
                        (bst-acc fn2 (node-r bst2)))))
            (bst-acc fn bst)
            result)))

Is this higher order function any use at all without relying on a side-effect?

The answer to this question is obviously no, since the function returns always nil.

  The general question I'm seeking to explore here is the purity or non-purity of higher order functions in practical use cases (and hence, how to use them well).

I'm not sure to understand what you are looking for, but if you want an example of recursive higher order function on binary trees that returns something (without performing side effects), consider the following one, in which the function gets another parameter that “combine” the results of the visit (assuming that an empty tree is represented with nil and that the function should return nil when an empty tree is passed to it):

(defun inorder-traverse (combine fn bin-tree)
  "in order traversal of a binary tree"
  (when bin-tree
    (funcall combine
             (inorder-traverse combine fn (node-l bin-tree))
             (funcall fn (node-elt bin-tree))
             (inorder-traverse combine fn (node-r bin-tree)))))

for instance:

(defun flatten (bin-tree)
  (inorder-traverse (lambda (x y z) (append x (list y) z))
                    #'identity
                    bin-tree))

returns the list of the leaves of a binary tree in order from left to right; if the tree is a BST, the list is sorted;

(defun reverse-flatten (bin-tree)
  (inorder-traverse (lambda (x y z) (append z (list y) x))
                    #'identity
                    bin-tree))

like the previous one, only in reverse order;

(defun map-tree (fn bin-tree)
  (inorder-traverse (lambda (left el right) (mk-tree el left right))
                    fn
                    bin-tree))

returns a new tree, with the same structure of the input one, and with the elements transformed by the function fn;

(defun my-copy-tree (bin-tree)
  (inorder-traverse (lambda (left el right) (mk-tree el left right))
                     #'identity
                     bin-tree))

makes a copy of a binary tree;

(defun reverse-tree (bin-tree)
  (inorder-traverse (lambda (left el right) (mk-tree el right left))
                    #'identity
                    bin-tree))

a symmetrical copy of a binary tree;

etc.


RadixSort implementation design and performance Credits for the original implementation I based my code on:
Quora - What is the most efficient way to sort a million 32-bit integers?

Implementation

// RadixSort - works for values up to unsigned_int_max (32-bit)
template<typename Iter>
void radix_sort(Iter __first, Iter __last){
    typedef typename iterator_traits<Iter>::value_type value_type;
    vector<value_type> out(__last - __first);

    // calculate most-significant-digit (256-base)
    value_type __mx = *max_element(__first, __last); //O(n)
    int __msb = 0;
    do {
        __msb += 8;
        __mx = __mx >> 8;
    } while(__mx);

    Iter __i, __j, __s;
    bool __swapped = false;
    for (int __shift = 0; __shift < __msb; __shift += 8) {

        // cycle input/auxiliar vectors
        if (__swapped) {
            __i = out.begin();
            __j = out.end();
            __s = __first;
        }
        else {
            __i = __first;
            __j = __last;
            __s = out.begin();
        }

        // counting_sort
        size_t count[0x100] = {};
        for (Iter __p = __i; __p != __j; __p++)
            count[(*__p >> __shift) & 0xFF]++;

        // prefix-sum
        size_t __m, __q = 0;
        for (int i = 0; i < 0x100; i++) {
            __m = count[i];
            count[i] = __q;
            __q += __m;
        }

        // filling result
        Iter __v;
        for (Iter __p = __i; __p != __j; __p++) {
            __v = __s + count[(*__p >> __shift) & 0xFF]++;
            *__v = *__p;
        }
        __swapped = !__swapped;
    }

    // if ended on auxiliar vector, copy to input vector
    if (__swapped) copy(out.begin(), out.end(), __first); //O(n)
}

Discussion

In order to implement a more general algorithm than the one used as reference, I tried to refactor the code using template and iterators. This is the main difference between my code and the original.

I can't simply swap references when using iterators, so I approached this problem by cycling the iterators used in each counting-sort loop. Doing so prevents the need to copy the output array on every loop.

Another different aspect of my code is that I find max-element and calculate the most-significant-digit on base-256. I use this information to determine how many counting-sort loops are necessary, instead of hardcoding 32 (unsigned_max) and always running the loop 4-times even if all values are less than 255. This actually adds unnecessary overhead if 4-loops are necessary, but it should increase execution time otherwise.

The temporary container used is a vector<value_type> and this is something I'm not quite sure about, I feel like this is a point where I could improve my code. I'd like to hear opinions about it.

Questions

What are possible improvements that can be made to my implementation?
How would you re-factor the iterator cycle I used? (I hate it)
Which container should I use for the temp-array?
Should I use std::copy or std::move to move the data from the temp-array to the input container?

Relevant Information

I decided to run some benchmarks to test if calculating the maximum-digit makes a huge difference. element_values defines the range of values each element can have. The maximum-digit only affects the number of loops if element_values can be represented with 24-bits or less.  Here are the results for n = 1E7:

     element_values < 256 (8-bits):
          radix_sort_msd - Average time:   51.59 ms
          radix_sort_32  - Average time:  109.03 ms

     element_values < UINT_MAX(32-bits):
          radix_sort_msd - Average time:  107.38 ms
          radix_sort_32  - Average time:   89.75 ms

While radix_sort_msd works really well when element_values is small, it really just depends on the dataset. Therefore implementing is a matter of preference.

Improvement to radix_sort_32

One of the things that you were wondering about was whether radix_sort_msd or radix_sort_32 was better.  The former called max_element() in order to determine the max width of the values, and the latter always did 4 passes even when they weren't necessary.

I took radix_sort_32 and added this code after generating the counts:

// If this byte is zero for every value, skip the byte entirely.
if (count[0] == (size_t)(__last - __first))
    continue;

This is a quick check to find out if the entire input was full of zeroes for the current byte.  If so, it moves on to the next byte instead of wasting time making an exact copy of the input to the temp area.  It still uses more time than radix_sort_msd in the "values < 256" case, but it is much faster, because for 3 passes it can skip more than 50% of the work.

On my machine, radix_sort_32 got 40% faster in the "values < 256" case compared to before the change.  This might make you reconsider which version is better.

Hybrid radix sort

Another thing you could do to improve your radix sort is to use a so-called "hybrid" radix sort, which is a hybrid of MSD and LSD radix sorts.  The idea is that use you one pass of radix sort on the most significant byte.  This breaks the input into 256 "bins".  Then you run LSD radix sort (your current sort) on each of the 256 bins.

The advantage of this hybrid sort over a plain LSD radix sort is that the hybrid version is more cache friendly.  In the plain LSD sort, if your input doesn't fit in cache, then the second counting pass can't benefit from caching because by the end of the first counting pass, the start of the array will already be evicted from the cache.

With the hybrid sort, the input is first broken up into 256 parts (which hopefully do fit in the cache).  So sorting each bin should be faster due to caching.  However, there is extra overhead, because each of the 256 smaller sorts needs to do some fixed amount of work to deal with the counting bucket.  So we should only do the hybrid sort if the input is larger than our cache size, otherwise we will only have more overhead and no benefit.

I modified your code to do the hybrid sort and it ran faster than the original version on randomized 32-bit input.  I did not attempt to optimize it for the "values < 256" case, although there are definitely possibilities for improvements there, similar to what I mentioned in the previous section.  Here is the code:

// LSD RadixSort helper function for radix_sort() below.
template<typename Iter>
static void radix_sort_lsd(Iter __first, Iter __last, Iter __out, Iter __outEnd,
        int __msb, bool needsSwap)
{
    Iter __i, __j, __s;
    bool __swapped = false;

    for (int __shift = 0; __shift < __msb; __shift += 8) {

        // cycle input/auxiliar vectors
        if (__swapped) {
            __i = __out;
            __j = __outEnd;
            __s = __first;
        }
        else {
            __i = __first;
            __j = __last;
            __s = __out;
        }

        // counting_sort
        size_t count[0x100] = {};
        for (Iter __p = __i; __p != __j; __p++)
            count[(*__p >> __shift) & 0xFF]++;

        if (count[0] == (size_t)(__last - __first))
            continue;

        // prefix-sum
        size_t __m, __q = 0;
        for (int i = 0; i < 0x100; i++) {
            __m = count[i];
            count[i] = __q;
            __q += __m;
        }

        // filling result
        for (Iter __p = __i; __p != __j; __p++) {
            *(__s + count[(*__p >> __shift) & 0xFF]++) = *__p;
        }
        __swapped = !__swapped;
    }

    // if ended on auxiliar vector, copy to input vector
    if (__swapped != needsSwap) copy(__out, __outEnd, __first); //O(n)
}

// If the input exceeds this threshold (in bytes), we do one pass of MSD
// followed by the rest of the passes done by LSD.  This number should be
// an estimate of the cache size.
#define THRESHOLD 8000000

// RadixSort - works for values up to unsigned_int_max (32-bit)
template<typename Iter>
void radix_sort(Iter __first, Iter __last)
{
    typedef typename iterator_traits<Iter>::value_type value_type;
    size_t len = (size_t) (__last - __first);
    vector<value_type> out(len);

    // First, test if the input exceeds the caching threshold.  If the input
    // is smaller than the threshold, just do a straight LSD radix sort.
    if (len * sizeof(value_type) < THRESHOLD) {
        radix_sort_lsd(__first, __last, out.begin(), out.end(),
                sizeof(value_type) * 8, false);
        return;
    }

    // Set __shift to the most significant byte.
    int  __shift = (sizeof(value_type)-1) * 8;
    Iter __s     = out.begin();
    Iter __p     = __first;

    // counting_sort
    size_t count[0x100] = {};

    for (size_t i = 0; i < len; i++) {
        count[(*__p++ >> __shift) & 0xFF]++;
    }

    // prefix-sum
    size_t __m, __q = 0;
    for (int i = 0; i < 0x100; i++) {
        __m = count[i];
        count[i] = __q;
        __q += __m;
    }

    // filling result
    __p = __first;
    for (size_t i = 0; i < len; i++) {
        *(__s + count[(*__p >> __shift) & 0xFF]++) = *__p++;
    }

    // For each of the 256 bins, do a LSD radix sort on the bin.  The input
    // and auxiliary vectors have been swapped, so we pass needsSwap = true to
    // indicate that the LSD sort should end on the auxiliary vector instead.
    int startIndex = 0;
    for (int i = 0; i < 0x100; i++) {
        int endIndex = count[i];
        radix_sort_lsd(__s + startIndex, __s + endIndex, __first + startIndex,
                __first + endIndex, __shift, true);
        startIndex = endIndex;
    }
}

Other things

I think that vector<value_type> is an appropriate container for your auxiliary array.  I can't think of anything better than that.

As far as copy vs move, they should be equivalent for numeric types.  See this Stackoverflow question and its answers for some good explanations on why that is.


Append blank value(s) to missing fields in a CSV line I have a small app that takes in CSV files. It reads the CSV file and before it inserts a line into the DB it checks certain conditions on the line.

Now, let's say the length of the line should be 5.

These are the headers:

colA,colB,colC,colD,colE

and this is one line of a CSV file the program is reading:

a, b

I want to now return back a line of length 5 (ignore the header and spacing I'm just trying to make it clearer on why I want to return it this way):

colA, colB, colC, colD, colE
"a",  "b",  "",   "",   ""

This is my current code on how to solve this:

private String buildErrorLine(String[] line) {
    if(line.length < 5) {
        return appendCommas(buildShortErrorLine(line, 5));
    } else {
        return appendCommas(line);
    }
}

private String[] buildShortErrorLine(String[] line, int lineSize) {
    ArrayList<String> arrayListLine = new ArrayList<String>(Arrays.asList(line));
    int size = lineSize - arrayListLine.size();
    for(int i=0; i<size; i++) {
        arrayListLine.add("");
    }
    System.out.println(arrayListLine.toString());
    return arrayListLine.toArray(new String[lineSize]);
}

private String appendCommas(String[] line) {
    StringBuilder sb = new StringBuilder();
    for (String n : line) {
        if (sb.length() > 0) sb.append(',');
        sb.append("\"").append(n).append("\"");
    }
    return sb.toString();
}

This will be the final output:

colA, colB, colC, colD, colE, colD(error)
"a",  "b",  "",   "",   "",   "the error"

Is there a cleaner or faster way to do this? Also, don't worry about checks for lines that are bigger than 5.

As I understand it, you'd like to make buildShortErrorLine more elegant. In that case, why not try something like this algorithm:

Initialise a String array of length equal to the number of required entries per line to all empty Strings.
Copy in the String array of the entries of the line just read into the array of empty Strings defined above, starting at the first index of each array, until the array of entries has been exhausted.
Et voila! Entry padding for (somewhat) free!

An implementation may look like:
(if Java supported real immutable lists we could have tried to cache the empty String array for hopefully better performance, but we're stuck with what we have...)
private String[] emptyStrings(int length) {
     String[] emptyStrings = new String[length];
     Arrays.fill(emptyStrings, "");
     return emptyStrings;
}

private String buildErrorLine(String[] line) {
     String[] padded = line;
     if(line.length < 5) {
          padded = emptyStrings(5);
          System.arraycopy(line, 0, padded, 0, line.length);
     }
     System.out.println(Arrays.toString(padded));
     return appendCommas(padded);
}

Note that the buildShortErrorLine method is now obsolete.
Also, the number 5 is used twice in the above code, so it appears to be a magic number. As it doesn't change according to your use-case, you could refactor it into a constant, say private static final int STANDARD_LINE_LENGTH = 5, as @mdfst13 said. However, in my opinion, this should actually be a parameter of buildErrorLine, as follows:
private String buildErrorLine(String[] line, int standardLineLength) {
     String[] padded = line;
     if(line.length < standardLineLength) {
          padded = emptyStrings(standardLineLength);
          System.arraycopy(line, 0, padded, 0, line.length);
     }
     System.out.println(Arrays.toString(padded));
     return appendCommas(padded);
}

Then call this with:
buildErrorLine(<data>, STANDARD_LINE_LENGTH);

Thus getting the best of both worlds. The parametrization is assuming the declaration and call sites of this method are in 2 different classes, but otherwise it's perfectly fine to lose the parameter and inline the constant - however, the code remains more flexible this way.
You'd notice that here we're doing reference copying (shallow copying), but that isn't a problem as Strings are immutable and interned (there is only one copy of 2 identical String constants).
Other notes:
1. Naming:
buildShortErrorLine is a pretty weird name for what this function does; why not just call it something like padRowEntries or something?
Otherwise, the rest of the naming in the code looks OK.
2. Unnecessary code:
The check you've got in buildErrorLine approach becomes irrelevant using my algorithm.
3. Notes about appendCommas:
Firstly, appendCommas could be (somewhat) better named as interleaveCommas? I'm not too sure, both are applicable given the context.
If you can use Java 8, you can significantly reduce the complicated StringBuilder logic with a simple stream map and collect. It would look like this:
private String appendCommas(String[] line) {
     return Arrays.stream(line)
           .map(entry -> "\"" + entry + "\"")
           .collect(Collectors.joining(","));
}

Note that @mdfst13's approach below also relies on Java 8, and although his is more efficient, I feel that my approach conveys the idea behind the process a bit better. By all means, please use his version in production code, mine might be too slow for that. Just reproducing his approach below:
private String appendCommas(String[] line) {
     return "\"" + String.join("\",\"", line) + "\"";
}


Rethrowing exception just as an information I have something like that:

My custom exception class:

public class SolvableException extends RuntimeException {

    protected boolean solved = false;

    public SolvableException(String message) {
        super(message);
    }

    public void setSolved() {
        this.solved = true;
    }

    public boolean isSolved() {
        return this.solved;
    }
}

Class for some very specific calculations which in some unpredictable cases throws my exception:

class VerySpecificCalculator {
    public void makeSomeSpecificCalculations(double someParameter){
        // make some actions...
        // Now I have to make some specific calculations, but sth wrong happend and I do know what to do with it in this place...
        throw new SolvableException("Something wrong happend... Problem with XYZ");
    }
}

My very specific calculator is used in other class and provides some part of its logic:

class Calculator {
    public void makeSomeCalculations(double someParameter){
        // Some variuos calculations...
        VerySpecificCalculator calc = new VerySpecificCalculator();
        try {
            calc.makeSomeSpecificCalculations(4.34);
        } catch (SolvableException e){
            // I know what to do here, so I'm fixing a bug on this level:
            // some fixing code, some rollback whatever...
            e.setSolved();
            // But I want to inform, that the fixing process took place...
            throw e;
        }
    }
}

I use the entire stuff as follows:

class Test {
    public static void main(String[] args) {
        Calculator calculator = new Calculator();
        try {
            calculator.makeSomeCalculations(344.47);
        }catch (SolvableException e){
            if(e.solved){
                System.out.println("Ok, there was a problem but it has been solved... You can display some JOptionPane to inform a user if you want...");
            } else {
                System.out.println("Buuu, there was a problem an it's still not solved... Action is interrupted ");
                return;
            }
        }
    }
}

Is this approach reasonable? I mean, is it ok to rethrow an exception if the problem has been already solved, and rethrowing plays only some informative role like in my example?

Let's consider such a situation (without rethrowing):

public void f(){
        try {
            methodCausingException();
        } catch (SolvableException e) {
            // fixing code...
        }
        //
        // Code executed when problem is fixed...
        //
    }

But with rethrowing I need to duplicate my code:

public void f(){
        try {
            methodCausingException();
        } catch (SolvableException e) {
            // fixing code...
            e.setSolved();
            //
            // Code executed when problem is fixed...
            //
            throw e;
        }
        //
        // Code executed when problem is fixed...
        //
    }

I can use a finally block, but I don't find this place as good for business logic code.

Especially if you expect these exceptions to occur (relatively) often, I think that using exceptions is a bad idea here. Exceptions should not be used for regular flow of control, this applies to Java as well. Creating and throwing exceptions is among the most costly operations there is in many programming languages.

One comment especially that caught my attention was: "Now I have to make some specific calculations, but sth wrong happend and I do know what to do with it in this place..." If you know what to do with it, then why are you throwing an exception? If you look in the dictionary an exception is:

  special case, departure, freak, anomaly, inconsistency, deviation, quirk, oddity, peculiarity, irregularity" (+ exclusion, omission, objection).

Is your case really an anomaly? (Doesn't sound like it but I can't make this decision for you)

Instead, consider creating a CalculationResult class, which can be returned from your methods. The class can contain your solved variable (possibly among other things). Then when a method has returned a CalculationResult to it's caller, you can read the CalculationResult and decide what to do with it.


Cleaning a text field How can I optimize the if condition in this snippet? The only one difference is && [self isCurrentPosition:i]. How can I make it a single if, including the condition val?

Note: self is a category of NSArray.

- (void) cleanTextfieldExcluding:(int)current checkPosition:(BOOL)val {
  for ( int i=0; i<[self count]; i++ ) {

    // -----------
    if ( val ) {  // this IF block is very bad
      if ( i != current && [self isCurrentPosition:i] )
        [self replaceObjectAtIndex:i withObject:@""];
    } else {
      if ( i != current )
        [self replaceObjectAtIndex:i withObject:@""];
    }
    // -----------

  }
}

the (val · ¬cur · pos) | (¬val · ¬cur) = ¬cur (val·pos | ¬val) = ¬cur (pos | ¬val). So, the condition is

if ( (i != current) && (!val || [self isCurrentPosition:i]) ) {
        [self replaceObjectAtIndex:i withObject:@""];
    }

You just must practise factoring out the common subexpressions and subprograms and learn how to use the Carnot maps for boolean minimization.


A web crawler in Python The crawler crawls for a set of keywords and saves the count in a database:

import re
import time
from bs4 import BeautifulSoup
from bs4 import SoupStrainer
import os
import httplib2
#import Links
#import Keywords
import MySQLdb
import peewee
from peewee import *
from datetime import datetime
import argparse
import logging
logger = logging.getLogger(__name__)
logger.setLevel(logging.DEBUG)
fh = logging.FileHandler('crawler.log')
fh.setLevel(logging.DEBUG)
#ch = logging.StreamHandler()
#ch.setLevel(logging.DEBUG)
formatter = logging.Formatter('%(asctime)s - %(name)s - %(levelname)s - %(message)s')
#ch.setFormatter(formatter)
#logger.addHandler(ch)
fh.setFormatter(formatter)
logger.addHandler(fh)

parser = argparse.ArgumentParser()

parser.add_argument('-l', '--url', help="The base link to be crawled", required=True)
parser.add_argument('-k', '--keywords', help="Keywords to search", required=True)

args = parser.parse_args()

keywords = (args.keywords).split(',')

mapping  = dict()

mapping[args.url] = keywords

logger.info(mapping)

db = MySQLDatabase('WebSpider', user='ruut', passwd='ruut')

parsed = set()

class DATA(peewee.Model):
    parent_link = peewee.CharField()
    sub_link  = peewee.CharField()
    keyword = peewee.CharField()
    count =  peewee.IntegerField()
    class Meta:
        database = db
        db_table = 'DATA'

def make_soup(s):
   match=re.compile('https://|http://')
   if re.search(match,s):
    try:
         http = httplib2.Http()
             status, response = http.request(s)
             page = BeautifulSoup(response,'lxml')
         return page
    except:
         return None
   else:
     return None

def get_list_of_urls(url):
    match = re.compile('(https?:\/\/(?:www\.|(?!www))[a-zA-Z0-9][a-zA-Z0-9-]+[a-zA-Z0-9]\.[^\s]{2,}|www\.[a-zA-Z0-9][a-zA-Z0-9-]+[a-zA-Z0-9]\.[^\s]{2,}|https?:\/\/(?:www\.|(?!www))[a-zA-Z0-9]\.[^\s]{2,}|www\.[a-zA-Z0-9]\.[^\s]{2,})')
    soup = make_soup(url)
    l = set()
    try:
        for a in soup.find_all('a'):
                try:
                if '?' not in a['href'] and re.search(match,a['href']) and re.search(re.compile(url),a['href']) and a['href']!=url:
                    l.add(str(a['href']))
                except Exception as e:
                logger.info('Exception ' + str(a)+' has no href')
                logger.info(e)
                continue
    except Exception as e:
        logger.info('Exception ' + url+'    has no links')
        logger.info(e)
        pass
    return l

def get_all_the_urls(base,list_of_urls,depth):
    logger.info(depth)
    if depth == 10:
        return
    else:
        depth = depth  + 1
        for i in list_of_urls: #scan the list of urls
            s = get_list_of_urls(i)
            get_all_the_urls(base,s,depth)
            for j in s: #scan the sublinks
               try:
                    if j in parsed:
                   continue
                soup = make_soup(j)
                logger.info('url is     '+ j)
                for k in mapping[base]:  #look for keys on the webpage
                    key_count = len(soup(text=re.compile(k, re.IGNORECASE)))
                    logger.info('Key count is '+str(key_count))
                    if(key_count>0):
                        record = DATA(parent_link = base,sub_link = j ,keyword = k ,count = key_count) #i,j,k,key_count
                            record.save()
                        parsed.add(j)
                        logger.info('saved data successfully ' +str(key_count))
               except Exception as e:
                logger.info('Exception ' +str(e)+'  in keywords searching')
                continue

def populate_db():
    k = set()
    k.add(args.url)
    temp = time.time()
    logger.info(str(datetime.now()))
    get_all_the_urls(args.url,k,0)
    logger.info('time taken '+str(time.time()-temp))
populate_db()

Some of the general things I would work on:

split the code into separate modules logically. Currently, you have all the code mixed up in single file - you have argument parsing, database interactions, web-scraping code blocks in one place
consistent indentation. Use 4 spaces for indentation
variable naming. Use descriptive variable names. Variable names like l, i or j are not meaningful and raise questions when reading the code

Code Style

avoid handling broad exceptions with a bare except
remove unused imports and re-group them based on PEP8 recommendations
make sure to properly use whitespaces in expressions in statements
put the main execution logic of the program into the if __name__ == '__main__':
you don't need that pass in the get_list_of_urls() function
depth = depth + 1 could be shortened to depth += 1

Performance

since you are requesting the pages from the same host multiple times, consider switching to requests making use of a single session instance which allows to re-use an underlying TCP connection making subsequent requests to the same host faster
importing SoupStrainer was actually a good idea. You can use it and scope the parsing to only the desired parts of the HTML
since you are using regular expressions checks here and there, consider pre-compiling them and using the compiled patterns for searching and matching


Inefficient usage of string.format? I have some sample code where I used string.Format but after reading about benchmark performances by other people I believe that it may be better to do multiple appends by a StringBuilder

Here is the scenario:

In my gridView, I have a function called before a databind. The function looks like such:

public string getColumnText(String myParam)
{
    StringBuilder sb = new StringBuilder();
    return sb.Append(string.Format("{1}", myParam).ToString();
}

Is this a better alternative?

StringBuilder sb = new StringBuilder();
sb.Append("<a href = 'javascript:Global.someFunction(\"");
sb.Append(myParam"); etc...

Let's assume that this function will be called 10,000 times.

A StringBuilder is useful when you're constructing a string where a loop will be directly effecting it, i.e. if you were appending several strings together within a loop, I'd recommend using a StringBuilder. Here's an example of where you might use a StringBuilder.

StringBuilder sb = new StringBuilder()
for (int i = 0; i < someItems.length; i++)
{
    sb.AppendFormat("{0} is item index {1}\n", someItems[i], i);
}
return sb.ToString();

There are some overheads with string.Format, however they aren't huge, especially not in your case. Unless you're concerned about micro-optimisations, I think in this scenario it all falls down to readability, and which you prefer.

An alternative method, which would be slightly faster than string.Format is to simply concatenate the string together, like so:

public string getColumnText(String myParam)
{
    return "" + myParam + "";
}

You may also want to consider ensuring it's safe for your desired output, i.e. do you need to escape quotation marks for myParam, or does it need to be HTML encoded?


CodeChef Chewing challenge The ZCO is today and I did 5 problems to prepare for it overnight.  One of the problems is the CodeChef Chewing problem.

The code takes an integer N that specifies the number of chewing gums, integer K that is the maximum hardness quotient of a pair the chewing gums possible to be eaten. This is followed by N integers that specify the hardness quotient of each type of gum. My code selects only gums with a quotient less than K and adds them to a vector. A nested for loop is used to count the number of pairs for which hardness quotient of Gum_Type1+Gum_Type2 < K.

#include <iostream>
#include <vector>
#include <algorithm>
#define ll unsigned long long int
#define speedup ios::sync_with_stdio(false); cin.tie(NULL);
using namespace std;

int main(){
  speedup;
  ll N,K,hq,numPairs=0;
  vector<ll> hardness;
  cin >> N >> K;
  for(ll i=0; i< N; i++){
    cin >> hq;
    if (hq< K) hardness.push_back(hq);
  }
  sort(hardness.begin(), hardness.end());
  for(vector<ll>::iterator it=hardness.begin(); it<hardness.end();it++){
    for(vector<ll>::iterator it2=it+1;it2<hardness.end();it2++){
      if (*it+*it2 < K) numPairs+=1;
      else hardness.erase(it2);
    }
  }
  cout << numPairs<<endl;
}

For subtask 2, I'm getting TLE by 0.01s.  Could somebody help me optimize this code? I don't want to get TLEs in the exam today.

Do not use namespace std; It is a bad practice that you should stop as soon as possible.
Do not trust magic macros like speedup. For such small code fragments that are repeated only few time it is often way better to write it out.
use std::size_t instead of unsigned long long int
You already know the maximum number of chewing gums, So reserve space for your vector early to avoid reallocations. You can always later call shrink_to_fit to free the unneded memory.
Use descriptive names even in challenges that only call them N or K. N is the number of gums so call it numGums or whatever you want. This improves readability quite a bit. This also goes for example hardness, which is a vector so it should be plural e.g. hardnesses.
Declare variables when you need them. This creates more or less independent code paths that might late be put into functions more easily.
In your loop you use it++. This requires a copy of it before the increment. Especially for iterators and friends ++it is more efficient as it does not involve the copy.
Try to separate command flow with a whitespace, e.g. for (). This helps to discriminate it from normal functions.
EDIT C++11 knows the language construct nullptr, use it rather than NULL

The early portion of our code would then look like this

std::ios::sync_with_stdio(false);
std::cin.tie(nullptr);
std::size_t numGums, chewingThreshold;
std::cin >> numGums >> chewingThreshold;

std::vector<std::size_t> hardnesses;
hardness.reserve(numGums);
for (std::size_t gum = 0; gum < numGums; ++gum) {
    std::size_t hardness;
    std::cin >> hardness;
    if (hardness < chewingThreshold) {
        hardnesses.push_back(hardness);
    }
}
hardnesses.shrink_to_fit();

If you look at this code in a year, you will still understand most of its meaning, without having to search for documentation.

Avoid single line command flow statements like if (*it+*it2 < K) numPairs+=1; as much as possible. This completely breaks readability of your code as you know have to check every for and if statement if it has a coomplicated condition or hidden code or nothing at all.
Your code does work, but it is highly inefficient from a mathematical point of view. As a hint I would suggest you think about how to utilize a std::map for this problem.


Determining if a connection has been made to a communications device I am unsure if my use of Monitor.Wait and Monitor.Pulse is correct. It seems to work alright, but there is a nagging doubt I am doing something wrong.

Am I dealing with a timeout OK, or is there a better way to do it?

All examples I have seen use Monitor.Wait with a while loop, and not with an if to re-check the blocking condition after Pulse, but I need to determine if a timeout occurred.

_cmdDispatcher.EnqueueCommand(cmd) sends a command to the device, and it responds with an event that executes the CommResponseReceived method on another thread.

private readonly object _connectLock = new object();

public bool Connect()
{
    if (this._connected) throw new InvalidOperationException("Plc is already connected.");

    ICommand cmd = new Command(MessageIdentifier.Name);

    try
    {
        if (this._channel.Open() && !this._connected)
        {
            // Wait for communications to be fully established or timeout before continuing.
            lock (this._connectLock)
            {
                this._cmdDispatcher.EnqueueCommand(cmd);

                if (!this._connectSignal)
                {
                    if (Monitor.Wait(this._connectLock, this._timeout))
                    {
                        this._connected = true;
                        this._connectSignal = false;
                        Debug.WriteLine("Connection succeeded.");
                    }
                    else
                    {
                        //TODO Log timeout.
                        Debug.WriteLine("Connection timed out.");
                    }
                }
            }

            if (this._connected) this.OnConnectionChangedEvent(ConnectionStatus.Connected);
        }
    }
    catch (Exception ex)
    {
        //TODO Log errors.
        Debug.WriteLine("Connection failed.");
    }

    return this._connected;
}

Executed in another thread when the device responds:

private void CommResponseReceived(object sender, CommsResponseEventArgs e)
{
    //

    // Signal that communications are successfully established.
    lock (this._connectLock)
    {
        this._connectSignal = true;
        Monitor.Pulse(this._connectLock);
    }
}

I agree with Alex that ManualResetEvent may be a little cleaner simply because you don't need to perform manual signal tracking to ensure you don't wait due to the Monitor already pulsing but, as I understand it, WaitHandle (used by ManualResetEvent) has more overhead. It's also quite possible to use memory barriers and Application.DoEvents/Thread.Sleep in a time-limited loop to achieve a lockless solution, but none of this matters unless you have very strict resource/performance concerns.

Regardless, there's always more than one way to skin a cat and your usage is indeed safe and correct.

If the Connect method is running on your UI thread be sure your timeout isn't really long or windows may think your application is not responding because blocking in Monitor.Wait does not resume the window's message pumping; if you need a long timeout or it is not running on the UI thread you will need to ensure the Connect method cannot be called again before the last has finished (if this._channel.Open() returns false if it is already open that should suffice). In the case of it being on the UI thread, besides preventing double execution, you would want to switch to a small timeout in a time-limited loop that calls Application.DoEvents(). You may already be aware of these concerns but I'm having to make a lot of assumptions to review the code. While I'm at it, it's probably trivial, but you can also move you Command instantiation to just before the lock to save yourself unnecessary instance creation in some cases.


String reorder by integers The aim is to have any strings with integers within to be reorder by these integers.

Let's say you have the following string:

re5 Forty1 cash2 Man3 Booker4

The aim is to have the string order by the integer contained within the string:

Forty1 cash2 Man3 Booker4 re5

I created this function:

def reorder(wordchain):
    from re import findall
    splitchain = wordchain.split()
    entier = findall('\d+',wordchain)
    chaineok = [(a,b) for a,b in zip(entier,splitchain)]
    chaineok.sort(reverse = False)
    chaineok1 = [b[1] for b in chaineok]
    chaineok2 = " ".join(chaineok1)
    return chaineok2

The function is fine, however, there are a few things I would like to improve:

The number of variable; I would like to have less. What should I do?
The number of lines; I feel there can be fewer lines and definitely fewer operations.

You should use sorted() for sorting and give your function and variables better names.
Something like this:

import re

find_int = re.compile('(\d+)')

def reorder(sentence):
    def num_part(word):
        return int(find_int.search(word).group(0))

    return ' '.join(sorted(sentence.split(), key=num_part))


Dynamically create Javascript Object Given a strings like 'param1.param2.param3' and a value that should be set on the last parameter, is there a better way than the following to dynamically create an Object and reuse the function to create more parameters on the Object, some which may share a parent parameter? This function also assumes every parameter on the parent Object is an Object with parameters. The purpose is to construct a JSON Object dynamically for a PUT/PATCH request.

function(o,prop,val) {
            prop = prop.split('.');
            prop.forEach(function(property,i){
                if(i===0 && typeof(o[property]) === 'undefined'){
                    o[property] = {};
                    if(prop.length === 2){
                        o[prop[0]][prop[1]] = val;
                    }
                }
                else if(i===1 && typeof(o[prop[0]][property]) === 'undefined'){
                    o[prop[0]][property] = {};
                    if(prop.length === 3){
                        o[prop[0]][prop[1]][prop[2]] = val;
                    }
                } else if(i===1 && typeof(o[prop[0]][property]) === 'object'){
                    if(prop.length === 3){
                        o[prop[0]][prop[1]][prop[2]] = val;
                    }
                }
                else if(i===2 && typeof(o[prop[0]][prop[1]][property]) === 'undefined'){
                    o[prop[0]][prop[1]][property] = {};
                    if(prop.length === 4){
                        o[prop[0]][prop[1]][prop[2]][prop[3]] = val;
                    }
                } else if(i===2 && typeof(o[prop[0]][prop[1]][property]) === 'object'){
                    if(prop.length === 4){
                        o[prop[0]][prop[1]][prop[2]][prop[3]] = val;
                    }
                }
            });
            return o;
        };

Here is a Fiddle https://jsfiddle.net/cn25o1vf/

It seems odd to special-case the string "object" when preparing an object for serialization, so I came up with the following code:

var createObjectFromParam = function (o, prop, val) {
    var parts = prop.split('.');
    var last = o;
    while (parts.length) {
        var part = parts.shift();
        if (parts.length > 0) {
            last[part] = last[part] || {};
            last = last[part];
        } else {
            last[part] = val; // Add conditional expressions here
        }
    }
    return last;
}

The code does the following:

Creates a temporary variable (last) to contain the current "level" of the object hierarchy.
Loops through the array. Note that while (parts.length) will stop the loop when there are no longer parts in the array.
Take the next part of the prop. shift pulls the first element of the array out.
If we're not at the last element

If the element with the specified name exists at the level, use that
If not, create an object at that level

Otherwise, add a property with the specified value to the object.
Finally, return the created object.

Given the example in the jsFiddle (updated here), this would produce the following structure:

{
    "ready": {
        "set": "go"
    },
    "another": "object",
    "happy": {
        "happy": {
            "joy": "joy",
            "foo": "bar",
            "happy": "happy"
        }
    }
}

To keep that original special-casing, just replace the commented line with something like:

last[part] = val === 'object' ? {} : val === 'undefined' ? : undefined : val;


Common Lisp macro: review for code quality & leaky name escaping (defmacro with-condition-retries(retries expected-errors fail-function &body body)
  "Attempts to execute `body` `retries` times, watching for
`expected-errors`.  Optionally, if `fail-function` is set, each time a
failure occurs, prior to retrying, `fail-function` is executed.

Other conditions beside `expected-errors` will exit out of this macro"
   (let ((counter (gensym))
     (result (gensym))
     (start-tag (gensym))
     (error-handler
      #'(lambda (c)
        (let ((r (find-restart 'handler c)))
          (when r
        (invoke-restart r c))))))

     `(let ((,counter 0)
        (,result))
        (tagbody
           ,start-tag
           (restart-case
           (handler-bind
            ,(mapcar #'(lambda (error-val)
                     (list error-val   error-handler))
                 expected-errors)
             (setf ,result ,@body))
         ;;the restart pointed at by the error-handler lambda
         (handler (&rest args)
           (declare (ignore args))
           (incf ,counter)
           (unless (> ,counter ,retries)
             (when ,fail-function
               (funcall ,fail-function))
             (go ,start-tag)))))
    ,result)))

This is the sort of thing you want to get as many eyes on as possible. My gut tells me that you'd want to break it up into smaller functions, but I'm not entirely sure where I'd cut. Any chance of getting an example use or test case? Could you explain the reasoning behind using tagbody with a single go rather than a loop?

Preliminary stuff I've noticed

with-gensyms can save you a few lines at the beginning, and you should probably define it in a macro-heavy package.

(defmacro with-gensyms (syms &body body)
  `(let ,(loop for s in syms collect `(,s (gensym)))
     ,@body))

The expression (setf ,result ,@body) might do some odd things if body is composed of multiple expressions. That should probably be (setf ,result (progn ,@body)) instead.


employing AES algorithm in Java I have employed an AES algorithm in order to encrypt files within a GUI. I was wondering if my algorithm is secure. Could I change anything to improve it? Is my IV secure and is it producing different random numbers each time? Am I transferring the salt successfully?

I know that I could improve this by making separate classes for the different methods however I am just doing this within the main for now. I'm quite new to cryptography so any feedback would be greatly appreciated!
    // author @Alex Matheakis
    public class AESFileEncryption {

// password to encrypt the file - how long should password be?
private static final String password = "UxIpOqSdNmSTuxZaShPu";

public static void main(String args[]) throws Exception {

    // file to be encrypted
    FileInputStream inF = new FileInputStream(GUI.AESinFile); // 'AESinFile' is a JFileChooser method from my GUI class

    // encrypted file
    FileOutputStream outF = new FileOutputStream("encrypted_file.des");

    // generate and write the salt
    SecureRandom sr = new SecureRandom();
    byte[] salt = new byte[16];
    sr.nextBytes(salt);
    outF.write(salt);

    // generate key
    SecretKeyFactory skf = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA1");
    KeySpec keySpec = new PBEKeySpec(password.toCharArray(), salt, 65536, 256); // salt, iteration count, key strength
    SecretKey tmp = skf.generateSecret(keySpec);
    SecretKey secretKey = new SecretKeySpec(tmp.getEncoded(), "AES"); // returns key

    // initialise the cipher with secure padding
    Cipher cipher = Cipher.getInstance("AES/CBC/PKCS5Padding");
    cipher.init(Cipher.ENCRYPT_MODE, secretKey);
    AlgorithmParameters p = cipher.getParameters();

    // iv used when initializing the cipher to make text more random

    byte[] iv = p.getParameterSpec(IvParameterSpec.class).getIV();
    outF.write(iv);

    // file encryption
    byte[] input = new byte[64];
    int bytesRead;

    while ((bytesRead = inF.read(input)) != -1) {
        byte[] output = cipher.update(input, 0, bytesRead);
        if (output != null)
            outF.write(output);
    }

    byte[] output = cipher.doFinal();
    if (output != null)
        outF.write(output);
        System.out.println("file encrypted");

    inF.close();
    outF.flush();
    outF.close();
    // inputScanner.close();

}

}

Line 105 is a typo. The indentation suggests its only executed if output is not null.

But it's actually unrelated to the if statement.

You should always use curly braces even if execution is only 1 line long. This goes for all statements: while, if, do, etc.


Program for finding longest run of zeros from a list of 100 random integers which are either 0 or 1 I have written an answer to this question on Stack Overflow. To make it easier for you, I have copy-pasted the main question below.

  Write a program that generates 100 random integers that are either 0
  or 1.

  Then find the:

  longest run of zeros, the largest number of zeros in a row. For instance, the longest run of zeros in [1,0,1,1,0,0,0,0,1,0,0] is
  4.

Here is my answer to this:

import random

l = []

def my_list():
    for j in range(0,100):
        x = random.randint(0,1)
        l.append(x)
    print (l)
    return l

def largest_row_of_zeros(l):

    c = 0
    max_count = 0

    for j in l:
        if j == 0:
            c += 1
        else:
            if c > max_count:
                max_count = c
            c = 0
    return max_count

l = my_list()
print(largest_row_of_zeros(l))

NOTE: I have changed zero_count to max_count as it sounds more sensible. It keeps track of the max_count (or the largest number of zeros in a row) seen so far, and if a number is not 0, it resets the value of c (count) to 0 after updating the value of max_count with a new value.

So, I would like to know whether I could make this code shorter and more efficient.

Any help would be highly appreciated.

Generating a random list

Instead of defining a global variable that will be modified by your generation function, you should instead define that variable inside the function and return it. This way, you will be able to call my_list a second time without having l being 200 items long. It will make the code easier to test.

Also note that l as a variable name is a poor choice as certain fonts make it hard to distinguish from 1.

You also use the "empty list + for loop + append" pattern which can be converted to a more efficient list-comprehension:

def random_list(length=100):
    return [random.randint(0, 1) for _ in range(length)]

Note that, as suggested by @jpmc26 in the comments, and starting with Python 3.6, you can simplify further using random.choices:

def random_list(length=100):
    return random.choices((0, 1), k=length)

Finding the longest sequence

Your manual counting is not that bad, but usually counting a number of element can be done using either sum or len depending on the iterable at play. And finding the longest count can be delegated to max. So you just need to group zeroes together, count how much there is in each group and find the max of these counts.

itertools.groupby will happily do the grouping for you. But you won't be able to use len on the resulting groups, so you can add 1 for each element in said group.

Lastly, if there is no sequence of zeroes, you'll get no groups, and thus no lengths to take the maximum from, so you need to instruct max that the longest count is 0 in such cases:

def largest_row_of_zero(iterable):
    return max((sum(1 for _ in group) for value, group in itertools.groupby(iterable) if value == 0), default=0)

Testing code

Instead of putting the testing code at the top-level of the file, you should take the habit of using an if __name__ == '__main__': guard:

if __name__ == '__main__':
    l = random_list()
    print(l, largest_row_of_zero(l))


Expandable integer List in C I just started learning C fundamentals and started to write a List that is limited to integers for the beginning and has dynamic memory allocation according to its content.

I've written custom simple assertion functions that revealed, that upon reallocating less memory when shrinking the list, it is still possible to access the removed element - my guess is that there's no need to move the pointer when shrinking its allocation and realloc() does not free the memory? Is it right? Is this code C-ish? Thanks for your reviews!

list.h

typedef struct {
    long size;
    int* elements;
} intlist;

/**
 * Creates an empty list
 */
intlist il_create();

/**
 * Appends a value to a list and returns its index
 *
 * @return index of the item
 */
long il_push(intlist*, int);

/**
 * Retrieves a value by its index
 *
 * @return item of the list
 */
int il_get(intlist*, long);

/**
 * Removes the last element
 */
void il_pop(intlist*);

main.c

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

void assert(char* msg, long actual, char expression) {
  static int assert_nr = 0;
  ++assert_nr;

  if(!expression) {
    fprintf(stderr, "[ERROR] Assertion %d failed\n", assert_nr);
    fprintf(stderr, "[ERROR] Expected: %s\n", msg);
    fprintf(stderr, "[ERROR] Actual:   %ld\n", actual);
  }
}

void assertEquals(char *msg, long actual, long expected) {
  assert(msg, actual, actual == expected);
}

void assertNotEquals(char *msg, long actual, long expected) {
  assert(msg, actual, actual != expected);
}

int main(int argc, char const *argv[]) {
  // Create an empty list
  intlist list = il_create();

  { // List is initially empty
    assertEquals("Size is 0", list.size, 0);
  }

  { // The item is added to the list
    int index = il_push(&list, 123);
    int item = il_get(&list, 0);

    assertEquals("Size is 1", list.size,  1);
    assertEquals("First index is 0", index, 0);
    assertEquals("First element is 123", item, 123);
  }

  { // Another item is added to the list
    int index = il_push(&list, 456);
    int item2 = il_get(&list, 1);

    assertEquals("Size is 2", list.size, 2);
    assertEquals("Second index is 1", index, 1);
    assertEquals("Second element is 456", item2, 456);
  }

  { // Last element is removed and thus last = first
    il_pop(&list);

    assertEquals("Size is 1", list.size, 1);

    int first = il_get(&list, 0);
    int last = il_get(&list, list.size-1);

    assertEquals("First item is 123", first, 123);
    assertEquals("Last element is the first one", last, first);

    int deleted = il_get(&list, 1);
    assertNotEquals("Deleted item is not 456", deleted, deleted != 456);
  }

  { // Removed element is overwritten when pushing a new one
    int index = il_push(&list, 789);
    int item = il_get(&list, index);

    assertEquals("Index is 1", index, 1);
    assertEquals("Element is 789", item, 789);
  }

  return 0;
}

intlist il_create() {
  return (intlist){.size=0};
}

long il_push(intlist* list, int value) {
  int index = list->size;

  list->elements = realloc(list->elements, ++list->size * sizeof(int));
  *(list->elements + index) = value;

  return index;
}

int il_get(intlist* list, long index) {
  return *(list->elements + index);
}

void il_pop(intlist* list) {
  list->size -= 1;
  *(list->elements + list->size) = 0;
  list->elements = realloc(list->elements, list->size * sizeof(int));
}

It does. You directly assign list->elements to the result of realloc which can return NULL, in which case your original array is leaked (but it's okay since you immediately assign a value to a NULL-based array after that, which is UB).

Few more observations.

The list itself can only be operated via its public interface that consists of methods that accept list object by pointer. Perhaps it makes sense to return by pointer from constructor, too; then you can make list's implementation an opaque struct, all its fields will be private.
You should define a destructor to release the array, that otherwise (currently) leaks.
Your current allocation strategy is inefficient. Reallocating the whole array per each new element raises push's complexity to O(N), so creating an array of N elements takes O(N²) time. A traditional approach to deal with it is adding capacity to the list object; capacity equals to the size of allocated space and grows exponentially: when list's size reaches capacity, on next element push capacity doubles, thus reducing push complexity to ammortized O(1).
Elements are stored in a contiguous array, perhaps a better descriptive name? Traditionally, list is associated with linked lists in C family.


Image/content slider I've built an image/content slider using JavaScript and as I've added to it and built in new functionality I've found myself writing several if statements in order to achieve the required response.

I'm now going over it again and I want to refactor it to make it shorter (if possible) and to increase performance (again if possible).

Has anyone any ideas on how I can refactor this? I'm thinking along the line of switch statements, but I'm not sure if that's the right route to go down or whether it would actually improve anything.

/*
 * animate margin left and store current position
 */

slider.prototype.transition = function (position, offset) {
    //'use strict';

    this.position = position;

    this.offset = offset;
    // update new slide position if deeplinking

    this.slide_pos = parseInt(this.position) || this.slide_pos;

    //check slide no exists in dl_urls object
    //this.deep_link_slide_no in window ? this.deep_link_slide_no = 0 : this.deep_link_slide_no = deep_link_slide_no;
    if (window.hasOwnProperty(this.deep_link_slide_no)) {
        this.deep_link_slide_no = 0;
    } else {
        this.deep_link_slide_no = position;
    }

    if(this.position === 0 || this.slide_pos === 0) {

        if(supportCSSTransitions && this.options.hardware_accelerated) {

          if(this.options.centered) {
            this.container.css({
              'margin-left': - (0.5 * this.slideWidth),
              transition :  'margin 0.8s ease'
            });
          } else {
            this.container.css({
              'margin-left': this.offset || 0,
              transition :  'margin 0.8s ease'
            });
          }
        } else {

          if(this.options.centered) {
            this.container.velocity({
              'margin-left': - (0.5 * this.slideWidth),
            }, {
                duration: this.options.slide_delay,
                easing: this.options.easing
            });
          } else {
            this.container.velocity({
              'margin-left': this.offset || 0,
            }, {
                duration: this.options.slide_delay,
                easing: this.options.easing
            });
          }
        }

        this.slide_pos = 0;

    } else {

      if(supportCSSTransitions && this.options.hardware_accelerated) {
        //console.log ("css and hardware");

        if(this.options.centered) {

          this.container.css({
            'margin-left': - (this.slide_pos * this.slideWidth) - (0.5 * this.slideWidth),
            transition :  'margin 0.8s ease'
          });

        } else if (typeof this.offset != "undefined") {
          this.container.css({
              'margin-left': (((this.slide_pos * this.slideWidth) * -1) + this.offset),
              transition : 'margin 0.8s ease'
          });
        } else {
          this.container.css({
              'margin-left': ((this.slide_pos * this.slideWidth * -1)) || (this.current_margin + this.direction),
              transition : 'margin 0.8s ease'
          });
        }

      } else {

        if(this.options.centered) {
          this.container.velocity({
            'margin-left': - (this.slide_pos * this.slideWidth) - (0.5 * this.slideWidth),
          });
        } else if (typeof this.offset != "undefined") {
          this.container.velocity({
              'margin-left': (((this.slide_pos * this.slideWidth) * -1) + this.offset)
          });
        } else {
          this.container.velocity({
              'margin-left': ((this.slide_pos * this.slideWidth * -1)) || (this.current_margin + this.direction)
          });
        }
      }
    }

    // store and update margin each time
    this.current_margin = (this.current_margin + this.direction);

    // set button state
    this.set_button_state();

    // set active states
    this.set_active_states();
};

There are a few things I can spot in your code that seem off to me.

Your code could realy do with some more comments. You put the comments on the lines that seem quite obvious to me, but when you start calculating the new position things get a little more complicated and you don't comment any more. I believe it should be the other way around.
Animating margins is not realy such a good idea when it comes to performance. You are much better of using the cheaper transform: translate3d() as I tried to explain here. It should make your animation a lot smoother.
There is a lot of repetition in your code, which is never a good idea. You set that 'margin-left' on 10 different lines for example. Why not first calculate it, and then set it just once? If you decide, for example, to work with transforms somewhere in the future, you would have to update all 10 of those lines, while it could be just a single line. And it would make your code a lot shorter, which is always nice.
There are a lot of nested ifs in there, which does not improve the readability of your code. I try to never nest ifs. If you really have to, you are probably doing something wrong. Most often you can avoid this by splitting your code into multiple functions (single responsibility), working with early returns or just rethinking your logic.
When you assign just a single variable in an if / else block, you are better of using the ternary operator. It is more compact (which is always good in js) and I think it makes your code better readable.

I went ahead and tried to refactor your code with those tips in mind (not the translate, I leave that up to you to decide). Note that this code is not tested (it's a bit hard with only part of your plugin) but I believe it should do the same as your code does. It is a bit shorter and a lot better readable imo.

slider.prototype.transition = function(position, offset) {
    this.position = position;
    this.offset = offset;

    // update new slide position if deeplinking
    this.slide_pos = parseInt(this.position) || this.slide_pos;

    //check slide no exists in dl_urls object
    this.deep_link_slide_no =
        window.hasOwnProperty(this.deep_link_slide_no)
        ? 0
        : position;

    // -- calculate new margin left
    var marginLeft = 0;
    // position == 0
    if (this.position === 0 || this.slide_pos === 0) {
        marginLeft =
            this.options.centered
            ? -(0.5 * this.slideWidth)
            : this.offset || 0;
    }
    // position != 0
    else {
        marginLeft =
            this.options.centered
            ? -(this.slide_pos * this.slideWidth) - (0.5 * this.slideWidth)
            : this.offset == 'undefined'
                ? (((this.slide_pos * this.slideWidth) * -1) + this.offset)
                : ((this.slide_pos * this.slideWidth * -1)) || (this.current_margin + this.direction);
    }

    // -- animate
    // transitions supported
    if (supportCSSTransitions && this.options.hardware_accelerated) {
        this.container.css({
            'margin-left': marginLeft,
            transition: 'margin 0.8s ease'
        });
    }
    // transitions not supported
    else {
        this.container.velocity({
            'margin-left': marginLeft,
        }, {
            duration: this.options.slide_delay,
            easing: this.options.easing
        });
    }

    this.current_margin = (this.current_margin + this.direction);
    this.set_button_state();
    this.set_active_states();
};


Doing an ROP style bind for two functions on the same input in a pipeline I am working on understanding Railway Oriented Programming (Scott Wlaschin style) in F#.  In my example I want to create a pipeline which does some calculation, applies two different functions to the output of that initial calculation, then hands off a tuple of its results to a final function.

In real code the 'initial calculation' might be getting some data, the 'two different functions' might produce a textual summary and a chart of the data, and the final function might be rendering some output containing both the text and the chart.

I've invented a bind2 function to achieve this, but I can't help thinking that if this was a good idea it would already be common practice and must appear (in disguise) in existing libraries such as Scott's.

Please let me know how I would achieve what I want in an idiomatic style.

Here's my code.  All above bind2 is taken broadly from Scott's code, the remainder is my bind2 function and a simple demo.

type Result<'S, 'F> =
| Success of 'S
| Failure of message : 'F list

let bind f x =
    match x with
    | Success s -> f s
    | Failure f -> Failure f

let bind2 (f1 : 'A -> Result<'B, 'F>)
          (f2 : 'A -> Result<'C, 'F>) (x : Result<'A, 'F>) : Result<'B * 'C, 'F> =
    match x with
    | Success _ ->
        let r1, r2 = (bind f1) x, (bind f2) x
        match r1, r2 with
        | Success s1, Success s2 ->
            Success(s1, s2)
        | Failure f1, _ ->
            Failure f1
        | _, Failure f2 ->
            Failure f2
    | Failure f -> Failure f

let demo1 (a: int) : Result<int, string> =
    Success a

let demo2 (a: int) : Result<string, string> =
    a |> string |> Success

let demo3 (a: int) : Result<string, string> =
    a |> (*) -1 |> string |> Success

let demo4 (s1 : string, s2 : string) =
    sprintf "%s - %s" s1 s2 |> Success

let Demo =
    bind demo1
    >> bind2 demo2 demo3
    >> bind demo4

I actually wouldn't bother defining a bind2 as this can lead to an explosion, bind3, 4, 5, 6. Instead I would define a simple computation builder to do this. So using your code from above.

 type DemoBuilder() =
     member x.Bind(m,f) = bind f m
     member x.Return(s) = Success s

 let demo = DemoBuilder()

which you can then use like so

 let DemoB x =
    demo {
        let! input = x
        let! a = demo1 input
        let! b = demo2 a
        let! c = demo3 a
        return (b,c)
    }

 DemoB (Success 1)


Performant Sort function for big arrays In sort(arr), I want to sort an array. Children must be beneath their parent. And children of the same parent are sorted using orderPos value.
I guess the complexity of this algorithm is quite high, is there a way to make it more performant for very long lists?
(the language used here is typescript)
     interface data { id:number; parentPK:number; orderPos:string; children: data[] }

 function sort(arr:data[]) {  const sorted : data[] = [];
  let parent: data = {children: [] , id : 0 , orderPos:'0' , parentPK:0} ;   let i = 1;

  // detect parent of all , parent pk is -1 , assumes theres only one root element whose parent id is -1
    arr.forEach(x => {

    if (x.parentPK == -1) {
      parent = x;
      sorted.push(x);
      arr.splice(arr.indexOf(x), 1);
      return;
    }   });   // root element is now first in the array   // get root's children and place them beneath their parent (sorted by orderPos)

  let temp: data[] = arr
    .filter(x => x.parentPK === (parent && parent.id))
    .sort((a, b) => parseInt(a.orderPos, 10) - parseInt(b.orderPos, 10));   sorted.push(...temp);   temp.forEach(x => parent.children.push(x));   while (i < arr.length - 1) {
    // new parent
    parent = sorted[sorted.indexOf(parent) + 1];
    // place children beneath parent
    temp = arr.filter(x => x.parentPK === (parent && parent.id)).sort((a, b) => parseInt(a.orderPos, 10) - parseInt(b.orderPos, 10));
    temp.forEach(x => parent.children.push(x));
    sorted.splice(sorted.indexOf(parent) + 1, 0, ...temp);
    i++;   }

  return sorted; }

  let array : data[] =  [ { id: 4, parentPK : 1, orderPos:'100' , children:[]}, {id:1, parentPK : -1, orderPos:'1', children:[] }, { id:2, parentPK : 1, orderPos:'51',children:[] }, {id:3, parentPK : 2, orderPos:'1',children:[] }, {id:5, parentPK : 2, orderPos:'10',children:[] } ]
let sortedArray = sort(array)
console.log('sorted array : ',sortedArray.map(element => [element.id, element.parentPK,element.orderPos] ))

The main improvement that can be made in the algorithm is recognizing that you can reduce more expensive sorting (and make the algorithm a lot easier to understand at a glance) by arranging the data into a structure indexed by the parent ID first. This operation is O(n). (In contrast, sorting is O(n log n)) The data structure will look like:

// This is not runnable, I'm just using a snippet to hide this long code
const itemsByParentId = {
  // Key: parent ID
  "1": [
    // Value: Array of children which have that ID as a parent (parentPK)
  {
    "id": 4,
    "parentPK": 1,
    "orderPos": "100",
    "children": []
  }, {
    "id": 2,
    "parentPK": 1,
    "orderPos": "51",
    "children": []
  }],
  "2": [{
    "id": 3,
    "parentPK": 2,
    "orderPos": "1",
    "children": []
  }, {
    "id": 5,
    "parentPK": 2,
    "orderPos": "10",
    "children": []
  }],
  "-1": [{
    "id": 1,
    "parentPK": -1,
    "orderPos": "1",
    "children": []
  }]
}

Then:

For each subarray (where all elements with the same parent are grouped together), sort it by orderPos (there's no getting around the O(n log n) complexity here)

Make a function that, given a parent, finds child elements in the data structure with that parent ID (O(1)), pushes them to the output array (O(n)), and recursively iterates over its children, depth-first, as the algorithm requires.

function sort(arr: data[]) {
  const itemsByParentId: {
    [id: number]: Array<data>;
  } = {};
  // (1) Group by parent ID
  for (const item of arr) {
    if (!itemsByParentId[item.parentPK]) {
      itemsByParentId[item.parentPK] = [];
    }
    itemsByParentId[item.parentPK].push(item);
  }
  // (2) Sort each subarray
  for (const childrenArr of Object.values(itemsByParentId)) {
    childrenArr.sort((a, b) => Number(a.orderPos) - Number(b.orderPos));
  }
  // (3) Recursively add parents and their children, depth-first
  const sorted: data[] = [];
  const addParent = (parent: data) => {
    sorted.push(parent);
    if (!itemsByParentId[parent.id]) {
      return;
    }
    for (const child of itemsByParentId[parent.id]) {
      parent.children.push(child);
      addParent(child)
    }
  };
  addParent(itemsByParentId[-1][0]);
  return sorted;
}

Live snippet of the compiled code:

function sort(arr) {
    const itemsByParentId = {};
    for (const item of arr) {
        if (!itemsByParentId[item.parentPK]) {
            itemsByParentId[item.parentPK] = [];
        }
        itemsByParentId[item.parentPK].push(item);
    }
    for (const childrenArr of Object.values(itemsByParentId)) {
        childrenArr.sort((a, b) => Number(a.orderPos) - Number(b.orderPos));
    }
    const sorted = [];
    const addParent = (parent) => {
        sorted.push(parent);
        if (!itemsByParentId[parent.id]) {
            return;
        }
        for (const child of itemsByParentId[parent.id]) {
                parent.children.push(child);
            addParent(child);
        }
    };
    addParent(itemsByParentId[-1][0]);
    return sorted;
}
let array = [{ id: 4, parentPK: 1, orderPos: '100', children: [] }, { id: 1, parentPK: -1, orderPos: '1', children: [] }, { id: 2, parentPK: 1, orderPos: '51', children: [] }, { id: 3, parentPK: 2, orderPos: '1', children: [] }, { id: 5, parentPK: 2, orderPos: '10', children: [] }];
let sortedArray = sort(array);

console.log('sorted array : ',sortedArray.map(element => [element.id, element.parentPK,element.orderPos] ))

For a review of your existing code:
Put each statement on its own line, usually - this significantly improves readability and means that readers of the code don't have to scroll horizontally.
Indent when starting a new block, and indent when chaining methods off an expression above - for similar reasons to above, it makes it a lot easier to pick up on the logic at a glance. (Also use the same indentation across the same block.) There are many IDEs which will automatically format your code to adhere to some sort of standard like this. They're well worth using.
parentPK? Currently, an object's parentPK value links to another object's id value. I'm not sure what these objects represent in your wider script, but you might consider if it would be clearer if the parentPK property was named parentID or idOfParent instead, to make the link clearer.
Declare variables close to where they'll be used, to reduce cognitive overhead. For example, rather than:
let parent;
let i = 1;
// many lines of code that doesn't reference i
while (i < arr.length - 1) {

you'll usually want to do
let parent;
// many lines of code that doesn't reference i
let i = 1;
while (i < arr.length - 1) {

You could also consider a for loop, since you increment i at the end:
for (let i = 1; i < arr.length - 1; i++) {

Use strict equality with === and !== - avoid != and ==, since they have strange coercion rules a reader of the script should not have to have memorized in order to understand the logic.
return does nothing in a forEach callback except terminate the current callback. It doesn't stop further callbacks from running. If you want to find the index of an element with a particular property in an array, use findIndex:
const rootIndex = arr.findIndex(item => item.parentPK === 1);
sorted.push(arr[rootIndex]);
arr.splice(rootIndex, 1);

Unnecessary comparison? I think parent will always exist, so the .filter(x => x.parentPK === (parent && parent.id)) check of parent first is superfluous. (If it might not exist, then you could simplify with optional chaining: x.parentPK === parent?.id)


Pivot and annotate Pandas DataFrame Given the CSV data:

,fan1,fan2,foil1,foil2
0,0.0,0.0,0.0,0.125
1,0.0625,0.0,0.0625,0.125
2,0.0625,0.0,0.0,0.3125

Which I want to turn into a kind of annotated pivot-table which can be plotted as a bar-plot:

,Err,PairType,StimType
0,0.0,Target,1
1,0.0625,Target,1
2,0.0625,Target,1
0,0.0,Target,2
1,0.0,Target,2
2,0.0,Target,2
0,0.0,RPFoil,1
1,0.0625,RPFoil,1
2,0.0,RPFoil,1
0,0.125,RPFoil,2
1,0.125,RPFoil,2
2,0.3125,RPFoil,2

I currently accomplish this with the following code:

import numpy as np
import pandas as pd

def df_plotable(model_err: pd.DataFrame):
    t_len = len(model_err.fan1)
    cols = ("Err", "PairType", "StimType")

    fan1_df = pd.DataFrame(np.array([model_err.fan1, ["Fan"]*t_len, [1]*t_len]).T,
                           columns=cols)
    fan2_df = pd.DataFrame(np.array([model_err.fan2, ["Fan"]*t_len, [2]*t_len]).T,
                           columns=cols)
    foil1_df = pd.DataFrame(np.array([model_err.foil1, ["Foil"]*t_len, [1]*t_len]).T,
                            columns=cols)
    foil2_df = pd.DataFrame(np.array([model_err.foil2, ["Foil"]*t_len, [2]*t_len]).T,
                            columns=cols)

    new_model_err = pd.concat((fan1_df, fan2_df, foil1_df, foil2_df))
    new_model_err["Err"] = new_model_err["Err"].astype(float)
    new_model_err["StimType"] = new_model_err["StimType"].astype(int)

    return new_model_err

Such that:

df = pd.read_csv("in.csv", "r", delimiter=",", index_col=0)
df_plotable(df).to_csv("out.csv")

Is there a way to do this more cleanly?

Don't hard-code your transformation

With your current approach, as soon as you are faced with a new PairType or StimType you will have to adjust your function accordingly to account for them. What your current code is doing is really a hard-coded version of a wide-form to long-form conversion of the column data - and Pandas has methods to allow you to do that in an automated way.

Two options would be df.melt, or a combination of df.unstack and reset_index.

Either way, after this step you'll nearly be all the way there.

>>> model_err.melt(var_name='PairStim', value_name='Err')

   PairStim     Err
0      fan1  0.0000
1      fan1  0.0625
2      fan1  0.0625
3      fan2  0.0000
4      fan2  0.0000
5      fan2  0.0000
6     foil1  0.0000
7     foil1  0.0625
8     foil1  0.0000
9     foil2  0.1250
10    foil2  0.1250
11    foil2  0.3125

The only other step in automation is to split the PairStim column up into its components, and do some cleanup. Wrapping this up in your function:

def longform_model_error(model_err: pd.DataFrame):
    melted = model_err.melt(var_name='PairStim', value_name='Err')
    melted[['PairType', 'StimType']] = (
        melted['PairStim'].str.extract('(\w+)(\d+)', expand=True)
    )
    melted['PairType'] = melted['PairType'].str.capitalize()
    melted['StimType'] = melted['StimType'].astype(int)
    return melted.drop('PairStim', axis='columns')

Comments on your current approach

Consider using the .size attribute on a Series (or the .shape attribute) instead of len. This makes it more clear to me that model_err.fan1 is indeed a Pandas type.
If you did want to hard-code the transformation for some reason, creating a DataFrame for each group and concat-ing them together is not ideal - you might as well remain in NumPy-land for as long as possible before turning it into a DataFrame. You would also benefit from hard-coding the separate columns instead of separate rows, as the dtypes of the columns are homogeneous, so you'd avoid the subsequent casting. Perhaps something like

def longform_model_error_hardcode(model_err: pd.DataFrame):
    n_per_type = model_err.shape[0]
    pair_types = ('Fan', 'Foil')
    stim_types = (1, 2)
    n_stims = len(stim_types)

    cols = {
        'Err': model_err.values.ravel('F'),
        'PairType': np.repeat(pair_types, n_per_type*n_stims),
        'StimType': np.tile(np.repeat(stim_types,
                                      n_per_type),
                            n_stims)
    }
    return pd.DataFrame(cols)

This will be faster, though if you were to go down the hard-coding avenue there isn't really much point in using Pandas at all if you're not performing other operations on the data - might as well just stick to NumPy in that case.
Nitpicking, df_plotable and new_model_err don't really help me understand what the function is supposed to do, or how new_model_err is different from the input DataFrame. Try and use some more descriptive variable names.


Split large log file like a dictionary I have a large log file in the following format. Do note that this is already sorted alphabetically. I wish to split it into smaller pieces, and with file names reflecting the content of the file (first three char of the row).

I wish for the code to be efficient. Speed and simplicity is the key. The log file is in 10s of GB in size, and there are 1000s of such logs.

Secondly, for large output files (only if the output file is greater than 4 lines), the script should sub-split the file into smaller pieces, using the next alphabet. I am aware this can be done by 2 separate awk commands, but that would not be efficient. Can you pls help me update the existing code?

Input file:

Jon1,details
Jon2,details
Jon4,details
Ron1,detailsaa
Ron1,detailsbb
Ron1,detailscc
Ron1,detailsdd
Ron1,detailsee
Ron2,detailsff

Output I wish for:

cat Jon
Jon1,details
Jon2,details
Jon4,details

cat Ron1
Ron1,detailsaa
Ron1,detailsbb
Ron1,detailscc
Ron1,detailsdd
Ron1,detailsee

cat Ron2
Ron2,detailsff

Current code

awk '
  fname != $1{
    close(fname)
    fname=$1
  }
  {print $0 > fname}' FS="[[:digit:]]+" ./hugeFile

Since you are concerned about execution speed, you should write this program in some other programming languages as well and compare their performance. I'd take Java, C++, Go, Perl, Python and Rust. Traditionally, AWK gets the job done but is not optimized for processing huge amounts of data.

Instead of setting FS on the command line, it should be in a BEGIN clause since it is an essential part of the program:

BEGIN {
    FS = /[[:digit:]]+/;
}

Asking for code that is not yet implemented is off-topic on this site, so I won't comment much on that. The code will become much larger by that requirement though and will require more thought. Be prepared to have a collection of small sample files to test the edge cases automatically.


Basic SQL aggregate query I have an employee table, employees may have multiple peripherals. I want to bring back all employees that have a peripheral of type 1 or 2, but that don't have any others, and vice versa (i.e. have a peripheral of type other than 1 or 2 but not have any peripherals of 1 or 2). For each of these employees, there should be a flag to show which of the two categories/product types they have.

This query works, but feels a bit verbose:

SELECT employee_id,
       has_screen,
       has_keyboard
FROM
(
    SELECT e.employee_id,
    CASE WHEN SUM(CASE WHEN ep.type IN (1,2) THEN 1 ELSE 0 END) > 0 THEN 1 ELSE 0 END has_screen,
    CASE WHEN SUM(CASE WHEN ep.type > 2 THEN 1 ELSE 0 END) > 0 THEN 1 ELSE 0 END has_keyboard
    FROM employee e
    JOIN employee_peripheral ep
    ON e.employee_id = ep.employee_id
    GROUP BY e.employee_id
) T
WHERE has_screen <> has_keyboard

BOOL values

Your outer case statements convert a value to itself. SUM(CASE WHEN type IN (1,2) THEN 1 ELSE 0 END) > 0 is TRUE or FALSE, which are equivalent to 1 and 0.

WHERE vs HAVING

WHERE will filter rows from the input, which means that you can't use an aggregate. HAVING filters after the aggregation, which means you can drop the outer query.

Unnecessary JOIN

You are only using the employee_id from employee, which is present in employee_peripheral

Resulting query

SELECT employee_id,
SUM(CASE WHEN type IN (1,2) THEN 1 ELSE 0 END) > 0 AS has_screen,
SUM(CASE WHEN type > 2 THEN 1 ELSE 0 END) > 0 AS has_keyboard
FROM employee_peripheral
GROUP BY employee_id
HAVING (SUM(CASE WHEN type IN (1,2) THEN 1 ELSE 0 END) > 0) <> (SUM(CASE WHEN type > 2 THEN 1 ELSE 0 END) > 0)


Checking lines for a certain string I currently have a sorting script that will check line by line for a certain string. If the string matches, then that line will be copied to that worksheet and the line will be deleted. However, sometimes I need to sort tens of thousands of rows and it can take a long time or will crash Excel. Is there a way to rewrite it to be able to sort more efficiently?

In my code, I have the With ws2 block copied multiple times with different sheet names

Dim ws1 As Worksheet, ws2 As Worksheet
Dim firstRowWs1 As Long
Dim lastRowWs1 As Long
Dim lastRowWs2 As Long
Dim searchColumnWs1 As Integer
Dim i As Integer
Dim check As Variant
Dim strSearch As Variant
Dim foundMatches As Boolean
Dim sDate As String
sDate = Format(Now(), "mm/dd/yyyy")
Set ws1 = Worksheets("Sort")
Set ws3 = Worksheets("Overview")

Set ws2 = Worksheets("sheet2")
With ws2
lastRowWs1 = ws1.UsedRange.Rows.Count
lastRowWs2 = ws2.UsedRange.Rows.Count
firstRowWs1 = 1
searchColumnWs1 = 10
strSearch = Array("john")

For i = firstRowWs1 To lastRowWs1
    For Each check In strSearch
        If check = ws1.Cells(i, searchColumnWs1).Value Then
            ws1.Rows(i).Copy (ws2.Rows(lastRowWs2 + 1))
            ws2.Rows(lastRowWs2 + 1).Columns("A:B").Insert xlToRight
            lastRowWs2 = lastRowWs2 + 1
            ws1.Rows(i).Delete shift:=xlUp
            i = i - 1
            Exit For
        End If
    Next check
Next i
End With

Some of your declared variables aren't used in your posted code - ws3, foundMatches, sDate.  Maybe they are used in code that you haven't included.  Even if you think your code may be too long to post in full, it would be helpful for us to be able to see everything between Sub and End Sub.

Your With ws2 End With is ineffective in so far as you continue to use ws2 explicitly in your code.  For example, lastRowWs2 = ws2.UsedRange.Rows.Count should be lastRowWs2 = .UsedRange.Rows.Count.

I'm not sure why you are looping through a single element array strSearch.  In fact, you could do away with strSearch and check by simply testing If ws1.Cells(i, searchColumnWs1).Value = "john" Then.

Looping through cells is inefficient.  So is copying entire rows.  I'm assuming that you actually want to paste into column C of lastRowWs2 + 1 which is why you insert new columns A:B.

While I haven't time tested filtering column J = "john" and copying/pasting the resulting list into .Cells(lasrRowWs2 + 1, 3) I'm confident that it would be much faster.

Avoid declaring variables as Variant because they have an overhead in terms of size and time taken for VBA to work out what sub-type they are.

.UsedRange.Rows.Count can return unexpected results if the last cell on the sheet is not where you think it is.  I've seen cases where Excel thinks the UsedRange extends down to row 1048576.


Pig Latin converter in Python How is it and can it be optimised?

sentence = input("Enter a sentence you want to convert to pig latin: ")

sentence = sentence.split()
for i in range(len(sentence)):
    if sentence[i][0] in "aeiou":
        sentence[i] += 'yay'
    else:
        sentence[i]=sentence[i][1:]+sentence[i][0]
        sentence[i]+='ay'
sentence = ' '.join(sentence)

print(sentence)

You don't actually care about indexes during the iteration; you just want the contents. The loop would be better written as

sentence = input(…)
for word in sentence.split():
    if word[0] in "aeiou":
        …

How would you reassemble the results?  Noting that each word can be translated independently, I would define a function for that, then use a generator expression.

def pig_latin(word):
    if word[0] in "aeiou":
        return word + 'yay'
    else:
        return word[1:] + word[0] + 'ay'

sentence = input("Enter a sentence you want to convert to pig latin: ")
print(' '.join(pig_latin(word) for word in sentence.split()))


Refactoring after grouped in Linq Is there any way to refactor this?

objLeadTimeReportResult =
    (from prop in propList
     join book in booking on prop.PropertyId equals book.PropertyId into j
     from book in j.DefaultIfEmpty()
     group book by new { prop.PropertyId, prop.PropertyName } into groupedLeadTime
         select new LeadTimeReportResult
         {
             PropertyId = groupedLeadTime.Key.PropertyId,
             PropertyName = groupedLeadTime.Key.PropertyName,
             Today = groupedLeadTime.Sum(item => ((item.CheckInDate.Date - item.BookingDate.Date).Days == 0) ? 1 : 0),
             Days1_2 = groupedLeadTime.Sum(item => ((item.CheckInDate.Date - item.BookingDate.Date).Days >= 1 && (item.CheckInDate.Date - item.BookingDate.Date).Days <= 2) ? 1 : 0),
             Days3_6 = groupedLeadTime.Sum(item => ((item.CheckInDate.Date - item.BookingDate.Date).Days >= 3 && (item.CheckInDate.Date - item.BookingDate.Date).Days <= 6) ? 1 : 0),
             Days7_10 = groupedLeadTime.Sum(item => ((item.CheckInDate.Date - item.BookingDate.Date).Days >= 7 && (item.CheckInDate.Date - item.BookingDate.Date).Days <= 10) ? 1 : 0),
             Days11_15 = groupedLeadTime.Sum(item => ((item.CheckInDate.Date - item.BookingDate.Date).Days >= 11 && (item.CheckInDate.Date - item.BookingDate.Date).Days <= 15) ? 1 : 0),
             Days16_30 = groupedLeadTime.Sum(item => ((item.CheckInDate.Date - item.BookingDate.Date).Days >= 16 && (item.CheckInDate.Date - item.BookingDate.Date).Days <= 30) ? 1 : 0),
             Days31_45 = groupedLeadTime.Sum(item => ((item.CheckInDate.Date - item.BookingDate.Date).Days >= 31 && (item.CheckInDate.Date - item.BookingDate.Date).Days <= 45) ? 1 : 0),
             Days46_60 = groupedLeadTime.Sum(item => ((item.CheckInDate.Date - item.BookingDate.Date).Days >= 46 && (item.CheckInDate.Date - item.BookingDate.Date).Days <= 60) ? 1 : 0),
             Days60Plus = groupedLeadTime.Sum(item => ((item.CheckInDate.Date - item.BookingDate.Date).Days > 60) ? 1 : 0)
         }).ToList();

Additional Information

My senior programmer told me to use some instead of count, he claimed that it would make better performance when using on MSSQL Server.

(1). You can create an exntession method to check that item is within the range.

(2). I'd use Count instead of Sum because you just count the number of days in specified interval. It's more natural.

(3). I'd check performance of this query and try to cache result of this join and/or grouping by using some method that requires eager evaluation. ToList() for example. It can help with a lot of traverals of every group in Sum()/Count() methods.

(4). Note for additional information. Even if Sum() in Linq To SQL is a lot faster than Count() you still can have MyCount() implemented as Sum() inside. This will allow you to have a good performance as your senior programmer mentioned and hide this "? 1 : 0" conidition.

 var tempQuery = from prop in propList
                                       join book in booking on prop.PropertyId equals book.PropertyId into j
                                       from book in j.DefaultIfEmpty()
                                       group book by new { prop.PropertyId, prop.PropertyName }.ToList();

 objLeadTimeReportResult = from tempVar in tempQuery
                                 select new LeadTimeReportResult
                                       {
                                           PropertyId = tempVar.Key.PropertyId,
                                           PropertyName = tempVar.Key.PropertyName,
                                           Today = tempQuery.Count(item => (item.WithinRange()),
                                           Days1_2 = tempQuery.Count(item => (item.WithinRange(1,2)),
                                           Days3_6 = tempQuery.Count(item => (item.WithinRange(3,6)),
                                           Days7_10 = tempQuery.Count(item => (item.WithinRange(7,10)),
                                           Days11_15 = tempQuery.Count(item => (item.WithinRange(11,15)),
                                           Days16_30 = tempQuery.Count(item => (item.WithinRange(16,30)),
                                           Days31_45 = tempQuery.Count(item => (item.WithinRange(31,45)),
                                           Days46_60 = tempQuery.Count(item => (item.WithinRange(46,60)),
                                           Days60Plus = tempQuery.Count(item => (item.GreaterThan(60));
}.ToList();