Calculating the joint probability of n events from a sample sequence of occurrences I'm writing an algorithm to take in a sample list of sequences of events, calculate 1-step transitional probabilities from the sequences, forward or in reverse, then calculate the joint probability of n events occurring from those conditional probabilities.

Eg. if the sample list of sequences is:

  ['TTAGCAGTCAGT', 'TCAGCGTTCGACTGA']

cond_probs() produces a dictionary {'AT': n1, 'AG': n2, 'AA': n3, 'AC': n4, ... , 'TC': n16}, where, for one, n1 + n2 + n3 + n4 = 1, from those sequences. In reverse, the transitional probabilities are calculated with the sequences in reverse.

With that dictionary, exp_joint_probs() produces another dictionary which, when n = 3, looks like {'TTA': m1, 'TTC': m2, ..., 'GAC': m64}, where 'TTA' is calculated by P('T') * P('TT') * P('TA'). In reverse, it is P('A') * P('AT') * P('TT'), but the dictionary entry should still be 'TTA'.

I'm wondering if my algorithm could be cleaner. I feel that perhaps using Numpy would make it much nicer, so a word from a Numpy expert would be great. I'm also looking for help double-checking the math.

from collections import Counter
from itertools import product

def exp_joint_probs(seqList, n, reverse = 0):
    probs = {}
    counts = Counter(''.join(seqList))
    states = counts.keys()
    sumv = sum(counts.values())
    initProbs = {c: counts[c] / float(sumv) for c in counts}
    condProbs = cond_probs(seqList, reverse)
    paths = product(counts.keys(), repeat = n)
    for p in paths:
        p = ''.join(p)
        if reverse == 0:
            probs[p] = initProbs[p[0]]
            for i in range(n - 1):
                probs[p] = probs[p] * condProbs.get((p[i], p[i + 1]), 0)
        else:
            probs[p] = initProbs[p[2]]
            for i in range(n)[::-1][:-1]:
                probs[p] = probs[p] * condProbs.get((p[i], p[i - 1]), 0)
   return probs

def cond_probs(seqList, reverse = 0):
    counts = {}
    probs = {}
    for seq in seqList:
        if reverse:
            for i in range(len(seq))[::-1][:-1]:
                counts[seq[i],seq[i - 1]] = counts.get((seq[i], seq[i - 1]), 0) + 1
        else:
            for i in range(len(seq) - 1):
                counts[seq[i],seq[i + 1]] = counts.get((seq[i], seq[i + 1]), 0) + 1
    states = list(set(k[0] for k in counts.keys()))
    for s in states:
        if reverse:
            sCounts = dict((k,v) for (k,v) in counts.items() if k[1] == s)
        else:
            sCounts = dict((k,v) for (k,v) in counts.items() if k[0] == s)
        sumv = sum(sCounts.values())
        probs.update({(k,float(v)/sumv) for (k, v) in sCounts.items()})
    return probs

I'm just going to look at cond_probs here. You'll see that there's plenty in this function for one answer.

1. Comments on your code

The line return probs has the wrong indentation. Copy/paste error?
There are no docstrings. What do these functions do and how do I call them? (The text from your question would be a good starting-point for docstrings.)
There are no test cases. This is the kind of code that would benefit from some unit tests and/or doctests.
You write, "cond_probs() produces a dictionary {'AT': n1, 'AG': n2, 'AA': n3, 'AC': n4, ... , 'TC': n16}" but when I run it the dictionary has tuples for keys, not strings:

>>> cond_probs(['AAC'])
{('A', 'C'): 0.5, ('A', 'A'): 0.5}

I don't understand the behaviour when reverse=True. You write, "In reverse, the transitional probabilities are calculated with the sequences in reverse" but this doesn't seem to be the case:

>>> cond_probs(['CAA'], reverse=True)
{('A', 'A'): 1.0}

What has happened to C? This looks wrong to me.

If I were writing this, I'd implement cond_probs to run only in the forward direction; to do the reverse direction I would pass in reversed sequences.
Prefer using True and False for Booleans, not 1 and 0. So the second parameter to cond_probs should be reverse=False.
Instead of:

counts = {}
# ...
counts[X] = counts.get(X, 0) + 1

use collections.Counter and write:

counts = Counter()
# ...
counts[X] += 1

(But see below for how to use collections.Counter.update.)
Instead of:

range(len(seq))[::-1][:-1]

write:

range(len(seq) - 1, 0, -1)

(But see below for how to avoid this.)
Prefer iteration over elements of sequences instead of iteration over their indexes. So instead of:

for i in range(len(seq) - 1):
    counts[seq[i],seq[i + 1]] = counts.get((seq[i], seq[i + 1]), 0) + 1

use zip:

for t in zip(seq, seq[1:]):
    counts[t] += 1

or, even more simply, using collections.Counter.update:

counts.update(zip(seq, seq[1:]))

(If you are concerned about memory use, then you could use  itertools.islice and write islice(seq, 1, None) instead of seq[1:] to avoid the copy.)
In this code:

for s in states:
    sCounts = dict((k,v) for (k,v) in counts.items() if k[0] == s)

you iterate over the whole of counts for every state, collecting the ones you want. It would be more efficient to iterate only over the transitions from s. See below for how this would look.

2. Revised code

def cond_probs_2(sequences):
    """Calculate 1-step transitional probabilities from sequences.
    Return dictionary mapping transitions to probabilities.

    >>> sequences = ['ACABC', 'ABCAB']
    >>> sorted(cond_probs_2(sequences).items())
    ... # doctest: +NORMALIZE_WHITESPACE
    [(('A', 'B'), 0.75),
     (('A', 'C'), 0.25),
     (('B', 'C'), 1.0),
     (('C', 'A'), 1.0)]

    """
    counts = Counter()
    entries = set()
    for seq in sequences:
        entries.update(seq)
        for a, b in zip(seq, islice(seq, 1, None)):
            counts[a, b] += 1
    probs = {}
    for a in entries:
        suma = float(sum(counts[a, b] for b in entries))
        if suma != 0:
            probs.update(((a, b), counts[a, b] / suma) for b in entries
                         if counts[a, b])
    return probs

This is about 30% faster than your code:

>>> from timeit import timeit
>>> from random import choice
>>> data = ''.join(choice('ACGT') for _ in range(1000000))
>>> cond_probs([data]) == cond_probs_2([data])
True
>>> timeit(lambda:cond_probs([data]), number=1) # yours
1.4706195190083236
>>> timeit(lambda:cond_probs_2([data]), number=1) # mine
0.9721288200234994

3. Rewriting in NumPy

Here's how I'd go about this kind of task in NumPy. NumPy works best with fixed-size numeric data, so I'd encode the input as small integers:

>>> import numpy as np
>>> data = 'TTAGCAGTCAGTTCAGCGTTCGACTGA'
>>> distinct = set(data)
>>> coding = {j:i for i, j in enumerate(distinct)}
>>> coding
{'A': 0, 'C': 1, 'T': 2, 'G': 3}
>>> coded_data = np.fromiter((coding[i] for i in data), dtype=np.uint8)
>>> coded_data
array([2, 2, 0, 3, 1, 0, 3, 2, 1, 0, 3, 2, 2, 1, 0, 3, 1, 3, 2, 2, 1, 3, 0,
       1, 2, 3, 0], dtype=uint8)

(I've chosen the type np.uint8 here because there are only four different characters in the input strings. You might need to choose a different type.)

Then you can encode adjacent pairs of numbers from 0–3 as a single number from 0–15, count the number of occurrences of each pair using numpy.bincount, and decode the result using numpy.reshape:

>>> n = len(distinct)
>>> pairs = coded_data[:-1] + n * coded_data[1:]
>>> counts = np.bincount(pairs, minlength=n*n).reshape(n, n)
>>> counts
array([[0, 3, 1, 2],
       [1, 0, 3, 2],
       [0, 1, 3, 3],
       [4, 2, 1, 0]])

This array represents the transition table:

          source
       │ A  C  T  G
     ──┼───────────
     A │ 0, 3, 1, 2
dest C │ 1, 0, 3, 2
     T │ 0, 1, 3, 3
     G │ 4, 2, 1, 0

You can then compute the transition probabilities by dividing each element by the sum of its column:

>>> counts / counts.sum(axis=0)
array([[ 0.        ,  0.5       ,  0.125     ,  0.28571429],
       [ 0.2       ,  0.        ,  0.375     ,  0.28571429],
       [ 0.        ,  0.16666667,  0.375     ,  0.42857143],
       [ 0.8       ,  0.33333333,  0.125     ,  0.        ]])

Here's the above as a function:

from itertools import chain
import numpy as np

def cond_probs_np(sequences):
    """Calculate 1-step transitional probabilities from sequences.
    Return dictionary mapping transitions to probabilities.

    >>> sequences = ['ACABC', 'ABCAB']
    >>> sorted(cond_probs_np(sequences).items())
    ... # doctest: +NORMALIZE_WHITESPACE
    [(('A', 'B'), 0.75),
     (('A', 'C'), 0.25),
     (('B', 'C'), 1.0),
     (('C', 'A'), 1.0)]

    """
    distinct = set(chain.from_iterable(sequences))
    n = len(distinct)
    assert(n < 256)             # so that they will fit in an np.uint8
    coding = {j:i for i, j in enumerate(distinct)}
    counts = np.zeros((n, n))
    for seq in sequences:
        coded_seq = np.fromiter((coding[i] for i in seq), dtype=np.uint8)
        pairs = coded_seq[:-1] + n * coded_seq[1:]
        counts += np.bincount(pairs, minlength=n*n).reshape(n, n)
    totals = counts.sum(axis=0)
    totals[totals == 0] = 1     # avoid division by zero
    probs = counts / totals
    return {(a, b): p for a, b in product(distinct, repeat=2)
            for p in (probs[coding[b], coding[a]],) if p}

And this is about seven times faster than your original code:

>>> cond_probs([data]) == cond_probs_np([data])
True
>>> timeit(lambda:cond_probs_np([data]), number=1)
0.2206433709943667


Shuffling algorithm suitable? Just checking that this shuffling algorithm is suitable. Or maybe there could be improvements?

public static void shuffleArray(Object[] arr, Random rnd){
    int lastIndex = arr.length - 1;
    for (int i=0; i<=lastIndex; i++){
        int k = nextInt(i,lastIndex+1, rnd);
        Object a = arr[i];
        Object b = arr[k];
        arr[i] = b;
        arr[k] = a;
    }
}

//Returns an integer between start(inclusive) and end(exclusive) [start..end)
public static int nextInt ( int start, int end, Random rnd);

One small thing is during the last iteration of the loop, the last element is swapped with itself.

Why not just use the Collections.shuffle as a guide?

public static void shuffle(List<?> list, Random rnd) {
    int size = list.size();
    if (size < SHUFFLE_THRESHOLD || list instanceof RandomAccess) {
        for (int i=size; i>1; i--)
            swap(list, i-1, rnd.nextInt(i));
    } else {
        Object arr[] = list.toArray();

        // Shuffle array
        for (int i=size; i>1; i--)
            swap(arr, i-1, rnd.nextInt(i));

        // Dump array back into list
        ListIterator it = list.listIterator();
        for (int i=0; i<arr.length; i++) {
            it.next();
            it.set(arr[i]);
        }
    }
}

private static void swap(Object[] arr, int i, int j) {
    Object tmp = arr[i];
    arr[i] = arr[j];
    arr[j] = tmp;
}

This could be altered to:

public static void shuffleArray(Object[] arr, Random rnd) {
    for (int i=arr.length; i>1; i--) {
        int j = rnd.nextInt(i);
        Object tmp = arr[i];
        arr[i] = arr[j];
        arr[j] = tmp;
    }
}


Codeigniter models optimization I have two tables:

tableA

aid name age sex email password

tableB

bid idno aid

The steps should be

Enter, verify email and password
If email and password are correct, insert random number into tableB as idno
Select name, age, sex, email, idno from both tables and return an array as result.
If email and password are wrong, do nothing.

I wrote the following code in Codeigniter model

function login($data) {
    $email = $data["email"];
    $password = $data["password"];
    $sql = "select aid from tableA where email='".$email."' and password='".$password."'";
    $query = $this->db->query($sql);
    if ($query->num_rows() > 0) {
        $ret = $query->row();
        $idno = random();
        $aid = $ret->aid;
        $sql = "insert into tableB (idno, aid) values('".$idno."','".$aid."')";
        $query = $this->db->query($sql);
        if ($query) {
            $sql = "select '".$idno."' as idno, name, age, sex, email from tableA where aid='".$aid."'";
            $query = $this->db->query($sql);
            if ($query->num_rows() > 0) {
                return $query->result_array();
            }
            return $query->num_rows();
         } else {
            return false;
         }
         return false;
    }
    return $query->num_rows();
}

The code works, but I think the code is so ugly and not efficient. How can I optimize the code?

The biggest thing I see are the numerous SQL injection vulnerabilities enabled by concatenating variables with SQL together. Use Prepared Statements instead.

You can reduce nesting in code by returning early from the function.

$query = $this->db->query($sql);

if ($query->num_rows() == 0)
    return false;

$sql = "insert into tableB (idno, aid) values('".$idno."','".$aid."')";
$query = $this->db->query($sql);
...

if (!isset($query))
    return false;

$sql = "select '".$idno."' as idno, name, age, sex, email from tableA where aid='".$aid."'";
$query = $this->db->query($sql);

if ($query->num_rows() == 0)
    return false;

return $query->result_array();

Pass the e-mail and password values as explicit parameters, instead of an array. You only need two values from this array. That's not too many arguments for a method:

public function login($email, $password) {
    ...
}

Some code style improvements:

Put a blank line above the if statements
Put a blank line above the return statements

Whitespace is not just a useful design element for graphic designers. It is useful for code authors as a means for grouping related statements together, or calling out a statement as something more important than the others. A return statement is pretty important in knowing what this method does. Furthermore blank lines above and below code elements give natural "breaks" in the look of the code, which guides your eye and mind to realize they are related — like paragraphs of text.


Password hashing and matching I took information from a series of posts and some prior knowledge to implement the following hashing algorithm.

However, there is a lot of talk about what implementations are secure and not secure.  How does my method measure up?  Is it secure? Are there more secure methods in PHP for hashing tokens and matching with tokens later on?

public static function sha512($token,$cost = 50000,$salt = null) {
        $salt = ($salt == null) ? (generateToken(32)) : ($salt);
        $salt = '$6$rounds=' . $cost . '$' . $salt . ' $';
        return crypt($token, $salt);
}

public static function sha512Equals($token,$hash) {
    return (crypt($token,$hash) == $hash);
}

public static function generateToken($length,$characterPool = '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ') {
    $token = '';
    $max = mb_strlen($characterPool);

    for ($i = 0;$i < $length;$i++){
        $token .= $characterPool[cryptorand(0,$max)];
    }

    return $token;
}

public static function cryptorand($min, $max) {
    $range = $max - $min;

    if ($range < 0)
        return $min;

    $log = log($range, 2);
    $bytes = (int) ($log / 8) + 1; // length in bytes
    $bits = (int) $log + 1; // length in bits
    $filter = (int) (1 << $bits) - 1; // set all lower bits to 1

    do {
        $rnd = hexdec(bin2hex(openssl_random_pseudo_bytes($bytes)));
        $rnd = $rnd & $filter; // discard irrelevant bits
    } while ($rnd >= $range);

    return $min + $rnd;
}

However, there is a lot of talk about what implementations are secure and not secure. How does my method measure up? Is it secure? Are there more secure methods in PHP for hashing tokens and matching with tokens later on?

Since you're specifically asking about security, I think reviewing your security instead of your code is a valid answer in this case.
The rule in Security-land is "don't do it yourself if others have done it better". It's basically a re-wording of "Use the right tool for the job" and "It isn't encryption until you've taken the time to fully understand the concepts behind the encryption code".
If you want to do hashing right, make sure you read this  answer on Security.SE.
Basically, if you want to do it secure, use bcrypt. Since PHP 5.5 can be called with password_hash.
PHP itself also recommends bcrypt. Their example usage states something like that in the comments:
<?php
/**
* We just want to hash our password using the current DEFAULT algorithm.
* This is presently BCRYPT, and will produce a 60 character result.
 *
 * Beware that DEFAULT may change over time, so you would want to prepare
 * By allowing your storage to expand past 60 characters (255 would be good)
 */
echo password_hash("rasmuslerdorf", PASSWORD_DEFAULT)."\n";
?>

If you don't want to be surprised by a changed default value, change PASSWORD_DEFAULT to PASSWORD_BCRYPT.
Note they have more examples with varying complexity on that site, but this should convey the basics. It's perfectly possible to hand a whole sleeve of options to the function.
As a last note, you want to handle tokens. Do not store tokens in your database, only store hashed tokens. See also Part II of this answer.
Update:
In the above I assume you're using PHP 5.5 or higher. See this answer for implementations when using a lower version.


Finding the prime factors of a number I am trying to write a program which, given a number as an input, prints the prime factors AND the exponents of the prime factors of that number.

Example:

Input:

20

Output:

2^2 5^1

And this is the code that I've come up with:

#include <cstdio>
#include <vector>
#include <map>
#include <algorithm>

std::vector<int>factors;
std::map<int,int>factormap;

int main()
{
    int n;
    scanf("%d",&n);

    for(int i = 2; i < n + 1; i++)
    {
        while(n%i < 1)
        {
            //Populate a vector of integers with the prime factors of the given number
            factors.push_back(i);
            n /= i;
        }
    }

    for(int i = 0; i < factors.size(); i++)
    {

        if(factormap.count(factors[i]) == 0)
        {
            //If the number hasn't been added yet, add it to the std::map
            //map is designed this way: <number, how many times it appears>
            factormap.insert(std::pair<int,int>(factors[i],std::count(factors.begin(), factors.end(), factors[i])));
        }
    }

    for(std::map<int,int>::iterator it=factormap.begin(); it != factormap.end(); ++it)
    {
        printf("%d^%d ", it->first, it->second);
    }
    printf("\n");

}

And it works as I intended but I feel like I'm putting too much effort into this. Is there an easier and more elegant way to accomplish this?

My first thought was that there wasn't much that you could do.  However, it does occur to me that you don't need to have both the vector and map variables.

private std::map<int,int> count_for;

protected void count_factors(int &n, int factor) {
    // if factor is not actually a factor of n, nothing to do
    if ( n % factor != 0 ) {
        return;
    }

    // initialization only needs to be done once,
    // so do it before the loop
    // we already checked that this is a factor and should be in the map
    count_for[factor] = 0;
    do {
        count_for[factor]++;
        n /= factor;
    } while ( n % factor == 0 );
}

public void generate_factors(int n) {
    // handle the only even prime separately
    count_factors(n, 2);

    // note that if a particular value of i is not prime,
    // then it won't divide n evenly
    // as we will have removed all its factors from n already
    for ( int i = 3; i <= n; i += 2 ) {
        count_factors(n, i);
    }
}

public ostream & display(ostream &out) {
    for ( const auto &p : count_for ) {
        out << p.first << "^" << p.second << " ";
    }
    out << endl;
}

I don't have a C++ compiler at the moment, so I haven't tested any of this.

Some other comments.  Note that I put those functions in a class.  That gets us away from the global variables of the original program and puts the code in a more sustainable location.  We can reuse the code in other programs easily.

When doing comparisons in a loop, you want to minimize the number of operations.  Thus, i <= n is better than i < n + 1.  Similarly, try to avoid making function calls as in i < factors.size().  If the result won't change during the loop, do the function call in the loop initialization, like so

for ( int i = 0, m = factors.size(); i < m; i++ )

When doing a modulus operation, there's one special result 0.  I don't know that there's a way to get a negative result, but if there is, you don't want one.  So write it as you mean it n % i == 0 not n % i < 1.  Note that this would be different if you were doing something like

for ( int i = n; i >= 0; i-- ) {
    // do stuff with i
}

Then you'd want to guard against i changing in addition to the decrement, so you use the inequality.

        factormap.insert(std::pair<int,int>(factors[i],std::count(factors.begin(), factors.end(), factors[i])));

That's an awful lot of things happening in one line.  It's often easier to read something like that if you assign the results of the function calls to variables rather than chaining them like that.  Also, some errors won't do a good job of identifying what prompted them, but they'll usually tell you the right line.

If you're using a C++11 compiler, then you can use range-based for loops.  If not, then stick with your original iterator approach.

The printf function comes from C.  While it will work in C++, the C++ way is to use std::cout.  I used a display function, but you could also overload the << operator to similar but more flexible effect.


Simple dataset class in C++ As part of my training, I implemented a dataset class in C++ that allows me to create synthetic datasets to get more familiar with features that C++ offers such as object oriented programming.
This implementation uses a simple method to create two intertwined spirals as shown in the following image.

The output looks as follows:
x y label
-3.85616 -0.987058 0
1.16 4.6764 0
2.66279 2.13692 1
-0.643551 3.81989 0
-0.252046 -0.888411 0

Please be as hard as possible with this implementation and give me constructive feedback.
I would appreciate advice especially in the following areas:

Code style (readability, naming conventions, etc...)
Class design
Efficiency (how to avoid unnecessary complexity)
Reinventing the wheel (does the STL offer functionality that I should use?)

main.cpp
#include <iostream>
#include <vector>
#include "data.hpp"

int main() {
    unsigned int n_points = 3000;
    Data data (n_points);
    std::vector<Point> d;
    d = data.get();

    for (std::vector<Point>::iterator it = d.begin(); it != d.end(); ++it) {
        std::cout << it->x << " " << it->y << " " << it->label << std::endl;
    }
    return 0;
}

data.hpp
#ifndef DATA_H
#define DATA_H

#include <vector>
#include <cmath>
#include "util.hpp"

class Data {
    public:
        Data (unsigned int);
        unsigned int n_samples;
        unsigned int n_dims = 2;
        unsigned int n_classes = 2;
        std::vector<Point> get();

    private:
        Random random;
        std::vector<Point> data;
        std::vector<Point> generate_data();
};

Data::Data (unsigned int _n_samples) {
    n_samples = _n_samples;
}

std::vector<Point> Data::generate_data() {
    std::vector<Point> data(n_samples);
    std::vector<Point>::iterator it;

    double noise = 1.0;
    double spread = 4.0;
    for (it = data.begin(); it != data.end(); ++it) {
        double r = 3.14 * spread * random.rand();
        double r1 = -cos(r) * r + noise * random.rand();
        double r2 = sin(r) * r + noise * random.rand();
        if (random.rand() < 0.5) {
            it->x = r1;
            it->y = r2;
            it->label = 1;
        } else {
            it->x = -r1;
            it->y = -r2;
            it->label = 0;
        }
    }

    return data;
}

std::vector<Point> Data::get() {
    std::vector<Point> data(n_samples);
    std::vector<Point>::iterator it;
    data = generate_data();
    return data;
}

#endif /* DATA_H */

util.hpp
#ifndef UTIL_H
#define UTIL_H

#include <cstdlib>
#include <ctime>

struct Point {
    double x;
    double y;
    unsigned int label;
};

class Random {
    public:
        Random ();
        double rand();
};

Random::Random () {
    std::srand(static_cast<unsigned int>(std::time(nullptr)));
}

double Random::rand() {
    return static_cast<double> (std::rand()) / static_cast<double> (RAND_MAX + 1U);
}

#endif /* UTIL_H */

Here are some things that may help you improve your program.
Separate interface from implementation
The interface goes into a header file and the implementation (that is, everything that actually emits bytes including all functions and data) should be in a separate .cpp file.  The reason is that you might have multiple source files including the .h file but only one instance of the corresponding .cpp file.  In other words, split your existing data.hpp and util.hpp files into a .h file and a .cpp file.
Understand the use of namespaces
Your code is correctly using <cmath> instead of <math.h>.  The difference between the two forms is that the former defines things within the std:: namespace versus into the global namespace.  So this implies that instead of using cos and sin, the code should be using std::cos and std::sin
Use const where practical
There are a number of named constants in the program such as unsigned int n_dims = 2;.  It's very good practice to name these, but even better is to declare them const or better still constexpr.  The reason is that it allows the compiler to perform better optimizations without sacrificing readability.
Don't use std::endl if you don't really need it
The difference betweeen std::endl and '\n' is that '\n' just emits a newline character, while std::endl actually flushes the stream.  This can be time-consuming in a program with a lot of I/O and is rarely actually needed.  It's best to only use std::endl when you have some good reason to flush the stream and it's not very often needed for simple programs such as this one.  Avoiding the habit of using std::endl when '\n' will do will pay dividends in the future as you write more complex programs with more I/O and where performance needs to be maximized.
Use "range for" and simplify your code
The code currently includes these lines:
for (std::vector<Point>::iterator it = d.begin(); it != d.end(); ++it) {
    std::cout << it->x << " " << it->y << " " << it->label << std::endl;
}

Since C++11, however, we have a range for which simplifies it to this:
for (const auto& point : d) {
    std::cout << point.x << " " << point.y << " " << point.label << '\n';
}

Write a stream inserter for your class
We can further refine the for loop above by creating a custom inserter for the Point class:
std::ostream& operator<<(std::ostream& out, const Point& point) {
    return out << point.x << " " << point.y << " " << point.label;
}

Now our loop in main is even simpler:
for (const auto& point : d) {
    std::cout << point << '\n';
}

Use standard library algorithms
We can use std::copy to copy from the vector to std::cout, replacing the loop with a single statement:
std::copy(d.begin(), d.end(), std::ostream_iterator<Point>(std::cout, "\n"));

Consider using a better random number generator
If you are using a compiler that supports at least C++11, consider using a better random number generator.  In particular, instead of rand, you might want to look at std::uniform_real_distribution and friends in the <random> header.  Also, instead of doing this:
if (random.rand() < 0.5) {
    it->x = r1;
    it->y = r2;
    it->label = 1;
} else {
    it->x = -r1;
    it->y = -r2;
    it->label = 0;
}

I would suggest using a std::bernoulli_distribution.
Rethink the classes
Rather than having the vaguely named Data class creating Points, I'd suggest moving that functionality into the Point class.  Since this is actually creating a Point, it makes sense to me to create a custom constructor with this functionality:
Point::Point() {
    static std::random_device rd;
    static std::mt19937 gen(rd());
    static std::uniform_real_distribution<> spread(0.0, 4.0*3.14);
    static std::uniform_real_distribution<> noise(0.0, 1.0);
    static std::bernoulli_distribution type(0.5);
    static constexpr double multiplier[]{-1.0, +1.0};

    label = type(gen);
    double r = spread(gen);
    x = multiplier[label] * (-std::cos(r) * r + noise(gen));
    y = multiplier[label] * (+std::sin(r) * r + noise(gen));
}

Minimize memory use
It's not critical for this program, but rather than storing everything in a vector only to print it and then destroy the vector, a more memory efficient approach would be to generate each point as needed and emit it directly.  Using the Point constructor and stream inserter as defined above, this becomes trivial.
Result
Here's the entire program as rewritten with all of these ideas:
#include <random>
#include <iostream>
#include <iterator>
#include <cmath>

struct Point {
    Point();
    double x;
    double y;
    unsigned int label;
};

Point::Point() {
    static std::random_device rd;
    static std::mt19937 gen(rd());
    static std::uniform_real_distribution<> spread(0.0, 4.0*3.14);
    static std::uniform_real_distribution<> noise(0.0, 1.0);
    static std::bernoulli_distribution type(0.5);
    static constexpr double multiplier[]{-1.0, +1.0};

    label = type(gen);
    double r = spread(gen);
    x = multiplier[label] * (-std::cos(r) * r + noise(gen));
    y = multiplier[label] * (+std::sin(r) * r + noise(gen));
}

std::ostream& operator<<(std::ostream& out, const Point& point) {
    return out << point.x << " " << point.y << " " << point.label;
}

int main() {
    for (auto i{3000}; i; --i) {
        std::cout << Point() << '\n';
    }
}


Formatting a date range, with shortening if the interval falls entirely within a calendar month The graphic guy wants to display dates like

16. February
17. - 24. February, or
25. February - 02. March.

Here is my solution:

$d1 = new \DateTime('2015-02-06');
$d2 = new \DateTime('2015-03-02');

$dateDiff = $d1->diff($d2);
if (intval($dateDiff->d) != 0) {
    $d = $d1->format('d.') . ' ' . ($d1->format('m') != $d2->format('m') ? $d1->format('F') : '') . ' - ' . $d2->format('d. F');
} else {
    $d = $d1->format('d. F');
}

I'm wondering if the this comparison:

$d1->format('m') != $d2->format('m')

is the best way to check whether $d1 and $d2 are in the same or different months.

($dateDiff = $d1->diff($d2) tells me that the two dates are 24 days apart, but it doesn't tell me if they are in different calendar months.)

This is a self-contained problem, with well-defined inputs and outputs.  To be maintainable, the code should reside in a function.

You have three distinct cases, and maybe more:

Start and end dates are the same day
Start and end dates fall in the same calendar month
Start and end dates span calendar months

Your solution should therefore be written to make it obvious that there are more than two cases.

Bugs in your implementation

intval($dateDiff->d) != 0 is an invalid check.  You would be treating the range from 2015-01-22 to 2015-03-22 as if it were one day.

Similarly, $d1->format('m') != $d2->format('m') is not quite right either.  It would be deceptive to format the range from 2014-12-26 to 2015-12-25 as 26. – 25.  December.  It might be acceptable to say 26. December – 25. December.  But really, it would be clearest to include the years as well.

Suggested solution

function formatDateRange($d1, $d2) {
    if ($d1->format('Y-m-d') === $d2->format('Y-m-d')) {
        # Same day
        return $d1->format('d. F');
    } elseif ($d1->format('Y-m') === $d2->format('Y-m')) {
        # Same calendar month
        return $d1->format('d.') . $d2->format(' – d. F');
    } elseif ($d1->format('Y') === $d2->format('Y')) {
        # Same calendar year
        return $d1->format('d. F') . $d2->format(' – d. F');
    } else {
        # General case (spans calendar years)
        return $d1->format('d. F Y') . $d2->format(' – d. F Y');
    }
}


Checking for violation of primary key in a DataTable I coded this function to check for a violation of a primary key in a DataTable. The main idea is to retrieve a list of every row that the same value of the column in input and set an error to it.

Private Sub PrimaryKeyCheck(Of ColumnType)(dt As DataTable, ColumnName As String)

    Dim violatingGroups =
        dt.AsEnumerable() _
        .Where(Function(i) i.RowState <> DataRowState.Deleted) _
        .GroupBy(Function(i) i.Field(Of ColumnType)(ColumnName)) _
        .Where(Function(g) g.Count() > 1).ToList()

    For Each violatingGroup In violatingGroups
        For Each violatingRow In violatingGroup
            violatingRow.SetColumnError(ColumnName, "Error, Violating PK") ' TODO Traduzione
        Next
    Next

End Sub

I'm a newbie of Linq, so I'm asking. Is it possible to improve this function in terms of performance and correctness? Is it possible to remove the double for each?

According to the .NET Naming Guidelines, parameter names should be written in lower Camel case and be descriptive.

Private Sub PrimaryKeyCheck(Of ColumnType)(table As DataTable, columnName As String)

  "Is it possible to remove the double for each?"

Yes! There's a handy method in the List<T> class named ForEach.  The provided action will be invoked for each element in the list.

I advice you to not use the name i in this context as this name is widely used as an abbreviation for index (usually a number). A better name is r or row. And g could be renamed to group.

You should also move the magic string "Error, Violating PK" to a constant.

Const errorMessage As String = "Error, Violating PK"

table _
    .AsEnumerable() _
    .Where(Function(r) r.RowState <> DataRowState.Deleted) _
    .GroupBy(Function(r) r.Field(Of ColumnType)(columnName)) _
    .Where(Function(g) g.Count() > 1) _
    .ToList() _
    .ForEach(Sub(g) g.ToList().ForEach(Sub(r) r.SetColumnError(columnName, errorMessage)))


Simple form validation script This is a simple form validation script.

I'd like to:

improve the jQuery validation
simplify the jQuery code

Questions:

Should I be exporting pure JS validation to avoid potential conflicts with other libraries that users might have installed?
Is it worth the effort or should I stick with my jQuery code?
Is there a way to reduce the chances for conflict with the jQuery code without having to reworking it to JS?

To avoid conflict with any other libraries, wrap your code using the jQuery function, and additionally call jQuery.noConflict();

The only conflict you may have is if somebody else imported an object named jQuery into the global namespace.

Reference: http://docs.jquery.com/Using_jQuery_with_Other_Libraries

Example:

jQuery.noConflict();
jQuery(document).ready(function($){
 //This is a jQuery function
 $('.myClass');
});


Adding a List to SQL database (Entity Database) I got a database with members, each member has a list of sports they do.
now I want to loop through a listbox and add every selected item to my database.

This is my database :

And this is my code :

foreach (var item in sportCheckedListBox.CheckedIndices)
            {
                int sportindex = Convert.ToInt32(item.ToString()) + 1;
                var queryResult = from sp in context.Sports
                                  where sp.sportnr == sportindex
                                  select sp;
                foreach (var sport in queryResult)
                {
                    myMember.Sports.Add(sport);
                }
            }

This looks kinda 'shady', how could I do this better ?

You're doing a lot of unnecessary iterating with your nested for loops. I would suggest using CONTAINS over a list of ints for your SQL query, this will eliminate your outer loop. Your inner loop is adding each individual item whereas you can use AddRange to just add them all.

//Generate a List of Integers from your Indices:
int[] indexes = myCheckedListBox.CheckedIndices.Cast<int>().ToArray()

//Use that list of Integers to get your list of Sports Objects
var queryResult = from sp in context.Sports
                  where indexes.Contains(sp.sportnr)

//Add your results.
myMember.Sports.AddRange(queryResult);


Using a form with formsets in Django I'm learning Django and making my (technically second) project.

The project features several apps, including player_interface and manager_interface.

Here's the player_interface/models.py:

from django.db import models
from django.contrib.auth.models import User

class PlayerProfile(models.Model):
    user = models.OneToOneField(User, on_delete=models.PROTECT,
                                limit_choices_to={'is_staff': False})
    balance = models.PositiveIntegerField(default=0)

    def __str__(self):
        return f'PlayerProfile user_id={self.user_id} balance={self.balance}'

In the manager_interface app, I want to create users together with corresponding player profiles. So I made a form (in manager_interface/forms.py):

from django import forms
from django.contrib.auth.models import User
from django.db import transaction

from player_interface.models import PlayerProfile

PlayerProfileUserFormset = forms.inlineformset_factory(
    User, PlayerProfile, fields=('balance',),
    extra=1, max_num=1, can_delete=False)

class PlayerUserForm(forms.ModelForm):
    def __init__(self, data=None, instance=None, **kwargs):
        super().__init__(data=data, instance=instance, **kwargs)
        self.formset = PlayerProfileUserFormset(data=data,
                                                instance=self.instance)

    def is_valid(self):
        return super().is_valid and self.formset.is_valid()

    @transaction.atomic
    def save(self):
        super().save()
        self.formset.save()
        return self.instance

    class Meta:
        model = User
        fields = ('username', 'password')

And this is how I use this form in manager_interface/views.py:

<...>

class CUPlayerMixin:
    template_name = 'manager_interface/players_form.html'
    form_class = PlayerUserForm
    success_url = reverse_lazy('players_list')

class CreatePlayerView(CUPlayerMixin, edit_views.CreateView):
    pass

class UpdatePlayerView(CUPlayerMixin, edit_views.UpdateView):
    queryset = User.objects.filter(is_staff=False)
    slug_field = 'username'
    slug_url_kwarg = slug_field

And the relevant template bit is:

{% block content %}
    <form method="post">
        {% csrf_token %}
        {{ form.as_p }}
        <div>{{ form.formset.as_p }}</div>
        <button type="submit">Submit</button>
    </form>
{% endblock %}

The code above works.

My question is: is this a good, Django way to handle a form for both a model and its association?

In this case, I edit both the User model with the associated PlayerProfile. That's why I decided to use the formset: it would automatically populate the subform for the player profile and then save it with the self.formset.save() part, keeping track of the association and all.

Thank you very much!

You have the right approach of creating two separate forms for your two models involved (User and PlayerProfile) but I think the formset is overkill and not the right tool for the job anyway.

I’d rather create the ModelForms more naturally:

class UserForm(forms.ModelForm):
    class Meta:
        model = User
        fields = ('username', 'password')

class PlayerProfileForm(forms.ModelForm):
    class Meta:
        model = PlayerProfile
        fields = ('balance',)

And glue them together with an ad-hoc "form" along the lines of:

class PlayerUserForm:
    def __init__(self, *args, **kwargs):
        self.user_form = UserForm(*args, **kwargs)
        self.player_form = PlayerProfileForm(*args, **kwargs)

    def is_valid(self):
        # Make sure to **not** short-circuit so as
        # to populate both forms errors, if any.
        user_valid = self.user_form.is_valid()
        player_valid = self.player_form.is_valid()
        return user_valid and player_valid

    @transaction.atomic
    def save(self):
        user = self.user_form.save()
        player = self.player_form.save(commit=False)
        player.user = user
        player.save()
        return player

And you can add template helpers for good measure:

    def __str__(self):
        return self.as_table()

    def as_table(self):
        return mark_safe(f'{self.user_form.as_table()}\n{self.player_form.as_table()}')

    def as_ul(self):
        return mark_safe(f'{self.user_form.as_ul()}\n{self.player_form.as_ul()}')

    def as_p(self):
        return mark_safe(f'{self.user_form.as_p()}\n{self.player_form.as_p()}')

This is untested but should work as a drop-in replacement for your PlayerUserForm.


Ray→plane and ray→quad intersection This checks the intersection between a Ray and a Plane and between a Ray and a Quad (in 3D):

import Linear

data Ray a   = Ray {rayPos :: V3 a, rayDir :: V3 a}
data Plane a = Plane {planePos :: V3 a, planeNorm :: V3 a}
type Quad a  = V4 (V3 a)

hitPlane :: (Num a, Fractional a) => Ray a -> Plane a -> V3 a
hitPlane (Ray rPos rDir) (Plane pPos pNorm) = rPos + dot (pPos - rPos) pNorm / dot rDir pNorm *^ rDir

hitQuad :: (Ord a, Epsilon a, Num a, Floating a) => Ray a -> Quad a -> Maybe (V3 a)
hitQuad ray (quad@(V4 a b c d)) = if hitInsideQuad then Just hitPoint else Nothing
    where hitInsideQuad = insideQuad hitPoint quad
          planeNormal   = cross (b - a) (d - a)
          hitPoint      = hitPlane ray (Plane a planeNormal)

insideQuad :: (Num a, Ord a) => V3 a -> Quad a -> Bool
insideQuad pos (V4 a b c d) = all inside borders where
    borders      = [(a,b),(b,c),(c,d),(d,a)]
    inside (a,b) = dot (b - a) (pos - a) > 0

Please, analyze if:

(Most importantly) I'm using instances correctly (I'm suspect something is wrong with the amount of instances on the types).
My design of types is correct and linguistic.
My code is comprehensible.
If I did something stupid in general.

I'll preface this by saying that I'm not a Haskell programmer, so take my comments advisedly.

I would appreciate a short comment specifying the behaviour of each type and function. For example:

-- The point of intersection between a ray and a plane.
hitPlane :: (Fractional a) => Ray a -> Plane a -> V3 a

This allows a reader to quickly understand the purpose of a function without having to read the code to find out. Also it provides a specification that can be checked against the implementation.
In geometry the term ray usually refers to the half-line with a starting point and direction. So a ray does not necessarily hit a plane, and the type of hitPlane ought to be:

hitPlane :: (RealFrac a) => Ray a -> Plane a -> Maybe (V3 a)

If you intend your Ray type to be a full line, then it ought to be called Line instead, to avoid confusion. But even a full line might not have a point of intersection with a plane, because it might be parallel to the plane. In this case there will be a division by zero error: it would be better to avoid this and return Nothing.
I don't understand all the details of the class constraints. hitPlane requires both Num a and Fractional a but if I understand the standard Haskell type documentation, the latter implies the former, so the Num a is redundant. Similarly for hitQuad, where Num a is redundant given that you have Floating a.
hitQuad requires Epsilon a and Floating a, but I don't see how either of these classes is necessary. I would have expected just Ord a, Fractional a, and this combination is the same as RealFrac a.
Some of the helper functions could be usefully made into top-level functions. For example, the implementation of hitQuad finds the plane containing a triangle of points. But this operation is generally useful:

-- The plane containing three points, with a normal chosen so that
-- the points are clockwise when looking in the normal direction.
planeContaining :: (Num a) => (V3 a, V3 a, V3 a) -> Plane a
planeContaining (a, b, c) = Plane a (cross (b - a) (c - a))


String length validation predicate I partially refactored the code so okFields uses the extracted function checkStringLength.

Now I also want problemFields() to use the same refactored function, but there is a difference between how okFields and problemFields use the function: you see that the problemFields has a little ! in front of validator, and I don't know how to have this vary from the same extracted function. How would you avoid the duplicate code?

function stringLengthValidation( mergedModelAndFormFields ) {
    let fieldsToValidate = mergedModelAndFormFields.okFields

    // extracted function
    function checkStringLength() {
        return _.pickBy( fieldsToValidate, ( value ) => {
            return validator.isLength( value[ 'content' ], {
                'min': value[ 'validation' ].minLength,
                'max': value[ 'validation' ].maxLength
            } )
        } )
    }

    let okFields = checkStringLength()

    // I want to replace this with checkStringLength()
    let problemFields = _.pickBy( fieldsToValidate, ( value ) => {
        // see the '!' here that is the variation
        return !validator.isLength( value[ 'content' ], {
            'min': value[ 'validation' ].minLength,
            'max': value[ 'validation' ].maxLength
        } )
    } )

    return {
        'okFields'     : okFields,
        'problemFields': problemFields
    }
}

It seems that the ok fields plus the problem fields equal all the fields.

In that case, you should consider deriving problem fields from all fields and ok fields with the Underscore difference method.

So in short:

function stringLengthValidation( mergedModelAndFormFields ) {
    let fieldsToValidate = mergedModelAndFormFields.okFields

    // extracted function
    function checkStringLength() {
        return _.pickBy( fieldsToValidate, ( value ) => {
            return validator.isLength( value[ 'content' ], {
                'min': value[ 'validation' ].minLength,
                'max': value[ 'validation' ].maxLength
            } )
        } )
    }

    let okFields = checkStringLength()

    return {
        'okFields'     : okFields,
        'problemFields': _.difference( fieldsToValidate, okFields )
    }
}

Actually, I would probably not even encapsulate what is in essence 1 statement into a separate function, it seems subjectively overkill to me:

function stringLengthValidation( mergedModelAndFormFields ) {

    let fieldsToValidate = mergedModelAndFormFields.okFields
    let okFields = _.pickBy( fieldsToValidate, ( value ) => {
        return validator.isLength( value[ 'content' ], {
            'min': value[ 'validation' ].minLength,
            'max': value[ 'validation' ].maxLength
        } )
    });

    return {
        'okFields'     : okFields,
        'problemFields': _.difference( fieldsToValidate, okFields )
    }
}


Validating user credentials and logging into a Symfony site I have the following code in a Symfony site that validates a user's credentials and logs them in via the Session class:

/**
 * Validates an Affiliate
 */
public function validateUser($username, $password, $session, $securityContext)
{
    $repo = $this->em->getRepository('JCMainBundle:Affiliate');
    $user = $repo->findOneBy(["username" => $username]);

    if (!is_object($user)) return false;

    $passwordValidated = password_verify($password, $user->getPassword());

    if (!$passwordValidated) return false;

    // Create token for user
    $token = new UsernamePasswordToken($user, null, "affiliate_dashbaord", ['ROLE_AFFILIATE']);
    $securityContext->setToken($token);
    $session->set('_security_affiliate_dashboard', serialize($token));

    return true;
}

As you can see, the function has very simple return values--just booleans. The function that calls this function looks at the return value of this function and determines whether or not to redirect the user to the affiliate dashboard.

How can I handle the return values better? I have two possible paths than can return false, but the calling function will have no idea what caused "false" to be returned.

In the past I've done something like this:

$response = ["success" => false, "message" => "error message here"];

I return the response with the success and a message. Using this design, I can return false AND return what made it return false. However, this seems a bit unorthodox for use on the backend and seems to fit more in a REST API.

I could use an exception, but the role of the function is to validate, so it wouldn't make sense to throw an exception if the user credentials are incorrect.

Note: the suggestion below is founded on a certain vision about readability, that may not be shared by everybody, as illustrated by the totally opposite approach proposed by @Thijs Riezebeek.

A pretty simple, light, and though efficient solution I frequently use in such cases is to merely:

Return the appropriate explainatory message when there is an error.
Do nothing otherwise.

First it makes your function as simple as possible (when successfull you don't even need to return anything).

Then from the app which invokes the function it's easy to use while being readable, assumed you change the function name for something like invalidUser(). So the app can work like this:

if ($errorMessage = invalidUser(...)) {
  // take any appropriate action, using $errorMessage
}
// otherwise merely continue


Workaround for the precision limitation for Python's round() function I'm trying to come up with a general way to correctly round floats with Python, given the known limitation of round():

  Note: The behavior of round() for floats can be surprising: for example, round(2.675, 2) gives 2.67 instead of the expected 2.68. This is not a bug: it’s a result of the fact that most decimal fractions can’t be represented exactly as a float. See Floating Point Arithmetic: Issues and Limitations for more information.

I thought that a simple way to correct for this effect is to add a small value to every float that ends with a 5, like so:

a = 7.85
b = a + 0.001
round(b, 1)
> 7.9

Below is the function I've written to do this. I've tested it and it seems to work fine, but I'd like some input on possible caveats I might be overlooking (or ways to improve it)

import numpy as np
from decimal import Decimal
from random import randint

def floats_5(N):
    """
    Generate random floats between a random range, where all floats
    end with a '5'.
    """
    rang = randint(1, 10000)
    flts = np.random.uniform(-1.*rang, 1.*rang, N)

    # Add '5' to the end of each random float, with different lengths.
    fl_5 = []
    for f in flts:
        # Trim float.
        i = randint(2, len(str(f).split('.')[1]))
        # Create trimmed float that ends with a '5' .
        f = Decimal(str(f).split('.')[0] + '.' + str(f).split('.')[1][:i] +
                    '5')
        fl_5.append(f)

    return fl_5

def format_5(f):
    """
    Take float and add a decimal '1' to the end of it.

    Return the number of decimal paces and the new float.
    """
    # Count number of decimal places.
    n = len(str(f).split('.')[1])

    # Decimal '1'  to add to the end of the float.
    d = '0.' + '0'*n + '1'

    # Add or subtract depending on the sign of the float.
    c = -1. if str(f)[0] == '-' else 1.

    # New augmented float.
    new_float = f + Decimal(c)*Decimal(d)

    # Return number of decimals and float with the small value added.
    return n, new_float

# Get some random floats.
fl_5 = floats_5(10)

# Compare float, round(float), and round(new_float)
print 'Original   round(f)   round(new_f)'
for f in fl_5:
    n, new_float = format_5(f)
    print f, round(f, n-1), round(new_float, n-1), '\n'

Following ferada's advice I looked up the decimal module and it simplifies things quite a bit.

This is a function that does what format_5() does but much succinctly:

def format_5_dec(f):
    new_float = Decimal.quantize(f, Decimal(str(f)[:-1]), rounding='ROUND_UP')
    return new_float


Present database results based on form submission When we select the 'info' list in the select field, the article with type 'info' characteristic automatically showed up, and when we select 'Berita' list in the select field, the articles with type 'Berita' characteristic automatically show up.
This is a long script that uses a post method.  Can you help me make it simpler, perhaps using functions?
<form method="post" name="form2">
  <select name="type" id="type">
    <option value="Semua">Semua</option>
    <option value="Berita">Berita</option>
    <option value="Info">Info</option>
  </select>
  <input type="submit" id="type" />
</form>

    <?php
        $i=0;
        $no=0;
            if($_POST['type'] == 'Semua')
            {
                $query_art = mysql_query("SELECT * FROM artikel_tbl ORDER BY id") or die(mysql_error());
            }
            else if ($_POST['type'] == 'Berita')
            {
                $query_art = mysql_query("SELECT * FROM artikel_tbl WHERE type = 'Berita' ORDER BY id") or die(mysql_error());
            }
            else if ($_POST['type'] == 'Info')
            {
                $query_art = mysql_query("SELECT * FROM artikel_tbl WHERE type = 'Info' ORDER BY id") or die(mysql_error());
            }
            else
            {
                $query_art = mysql_query("SELECT * FROM artikel_tbl ORDER BY id") or die(mysql_error());
            }
            while($show=mysql_fetch_array($query_art))
            {
                $no++;
                if(($no%2)==0)
                    $color = '#f2f2f2';
                else
                    $color = '#f9f9f9';
    ?>
  <tr bgcolor="<?php echo $color; ?>" class="rows">
    <input type="checkbox" name="checked<?php echo $i; ?>" value="<?php echo $show['id']; ?>"/>
    <?php echo $no; ?>
    <?php echo $show['judul']; ?>
    <?php echo $show['penulis']; ?>
    <?php echo $show['type']; ?>
    <img src=".././upload/artikel/<?php echo $show['foto']; ?>" width="144" height="88"/>
    <?php echo $show['tanggal']; ?>

<a href="table_artikel.php?edit=<?php echo $show['id']; ?>">

<a href="table_artikel.php?delete=<?php echo $show['id']; ?>" onclick="return confirm('Hapus artikel?')";>

    <?php
        $i++;
        }
    ?>
    <input type="hidden" name="n" value="<?php echo $i; ?>" />

Well... First we can shorten your if-construct a lot:

$sql = 'SELECT * FROM artikel_tbl';
if ($_POST['type'] == 'Info' || $_POST['type'] == 'Berita')
  $sql .= " WHERE type = '{$_POST['type']}'";
$sql .= ' ORDER BY id';

In my opinion this also increases readability, because it is clear that the query is basically everytime the same, you are just including a condition in one case.

Next thing the if(($no%2)==0)-part. Do you know about the nth-child of css? Here is an example for that: http://jsfiddle.net/2ZR2U/

For these things: <?php echo $show['judul']; ?> change this to read <?php echo htmlspecialchars($show['judul']); ?>. Unless you are very sure that the input is just a number or something that is guaranteed to not contain any of [<>"] this is very important to prevent display bugs or in worst case cross site scripting attacks.


ELisp function to allow file renaming in Emacs As a disclaimer, this is the first function I have EVER written in Lisp or ELisp. Please, bash it as much as you would any other piece, but don't judge me for it please! :)

So, I realized I wanted a way to rename files without all the hassle of reopening the file etc. I googled and found a function at https://stackoverflow.com/questions/384284/can-i-rename-an-open-file-in-emacs (credit Steve Yegge).

However, it did not seem to work well with uniquify. Which was a problem. Having two files open and trying to rename the first one to the same name as the second one (for example CMakeLists.txt, which there might be many of) caused it to bail since it only checked the name of the buffer (which wasn't unique yet, as uniquify doesn't do it's thing before something needs uniquifying).

The only way I found to force a "re-uniquification" was to close the previous buffer and open the new file. This seems very inelegant...

(defun is-unique-name-for-buffer (new-name)
  (let ((file-name (buffer-file-name))
    (dir-name (file-name-directory buffer-file-name)))
  (let ((new-complete-name (concat dir-name new-name)))
  (progn (not (string-equal file-name new-complete-name))))))

(defun rename-file-and-buffer (new-name)
  "Renames both current buffer and file it's visiting to NEW-NAME."
  (interactive "sNew name: ")
  (let ((name (buffer-name))
    (filename (buffer-file-name))
    (dir-name (file-name-directory (buffer-file-name))))
    (if (not filename)
    (message "Buffer '%s' is not visiting a file!" name)
      (if (not (is-unique-name-for-buffer new-name))
      (message "A buffer named '%s' already exists in that location!" new-name)
    (progn
      (rename-file (file-name-nondirectory filename) new-name 1)
      (kill-buffer)
      (set-buffer (find-file (concat dir-name new-name))))))))

Judgment circuits deactivated; you have nothing to fear.

I was going to suggest that there must already be a function that you can call to force uniquify a buffer, but M-x apropos RET ^uniq doesn't show any documented functions that would do that for you (take a look and try a few out if you like, you may find a more elegant solution).

Lisp (actually, Elisp and Common Lisp; Scheme does its own thing) convention is to end predicate functions with -p or p rather than begin them with is-.

(defun unique-name-for-buffer-p (new-name)
  (let ((file-name (buffer-file-name))
        (dir-name (file-name-directory buffer-file-name)))
    (let ((new-complete-name (concat dir-name new-name)))
      (progn (not (string-equal file-name new-complete-name))))))

Your predicate seems to be nesting lets only because you need new-complete-name to refer to dir-name. When you're in a situation like that, you can instead use let*. It's the same as let except that its clauses are guaranteed to be evaluated in order (it's still good style to use let when you can, but one let* is better than many nested lets).

(let* ((file-name (buffer-file-name))
       (dir-name (file-name-directory buffer-file-name))
       (new-complete-name (concat dir-name new-name)))
    (progn (not (string-equal file-name new-complete-name))))

You only need explicit progn when you want to evaluate multiple expressions as one (you use it correctly in your nested ifs, but read on).

(defun unique-name-for-buffer-p (new-name)
  (let* ((file-name (buffer-file-name))
         (dir-name (file-name-directory buffer-file-name))
         (new-complete-name (concat dir-name new-name)))
    (not (string-equal file-name new-complete-name))))

The nested ifs in your second function can be expressed as a single cond (which also gets rid of that otherwise legitimate progn).

(defun rename-file-and-buffer (new-name)
  "Renames both current buffer and file it's visiting to NEW-NAME."
  (interactive "sNew name: ")
  (let ((name (buffer-name))
        (filename (buffer-file-name))
        (dir-name (file-name-directory (buffer-file-name))))
     (cond ((not filename)
            (message "Buffer '%s' is not visiting a file!" name))
           ((not (unique-name-for-buffer-p new-name))
            (message "A buffer named '%s' already exists in that location!" new-name))
           (t (rename-file (file-name-nondirectory filename) new-name 1)
              (kill-buffer)
              (set-buffer (find-file (concat dir-name new-name)))))))


Linq group by with multiple where, sum clauses in select causes a timeout I have  linq query that extracts a summary of my sales quantities item group wise. This query causes an sql timeout. From the what i have found out, timeout indicates that my query is poorly designed and needs to be optimized.

Below is the linq query:

 (from inv in Dbcontext.tblSalesInvoices
    join det in Dbcontext.tblSalesInvoiceDetails on inv.SalesInvoiceId equals det.SalesInvoiceId
    join item in Dbcontext.tblItems on det.ItemId equals item.ItemId
    join itemGroup in Dbcontext.tblItemGroups on item.ItemGroupId equals itemGroup.ItemGroupId
    join loc in Dbcontext.tblLocations on inv.LocationId equals loc.LocationId
    where inv.InvoiceDate >= FeedSearchCriteria.FromDate && inv.InvoiceDate <= FeedSearchCriteria.ToDate
    && inv.InvoiceStatusId != Constants.REVERSED_INVOICE
    group new { det.Quantity, itemGroup.ItemGroupCode, item.Weight, item.ItemCode, det.LineTotal } by loc.LocationName into g
    select new LocationCount()
    {
    LocationName = g.Key,
    FeedBagCount = (int?)g.Where(x => x.ItemGroupCode == "FEED").Sum(x => x.Quantity),
    SupremeKg = (decimal?)g.Where(x => x.ItemGroupCode == "ABATTOIR").Sum(x => x.Weight * x.Quantity),
    SupremeValue = (decimal?)g.Where(x => x.ItemGroupCode == "ABATTOIR").Sum(x => x.LineTotal),
    DocCount = (int?)g.Where(x => x.ItemCode == "DOC").Sum(x => x.Quantity)
    }).OrderBy(x=>x.LocationName).ToListAsync();

I think my issue arises because of the where/sum clause in select. How can i re-write below query in more optimized manner.

Edit

Time out occurs when a date range is more than or equal to 1 month of transactions.

You could try to set the joins on the already filtered table:

(from inv in Dbcontext.tblSalesInvoices.Where(inv => inv.InvoiceDate >= FeedSearchCriteria.FromDate && inv.InvoiceDate <= FeedSearchCriteria.ToDate
    && inv.InvoiceStatusId != Constants.REVERSED_INVOICE)
    join det in Dbcontext.tblSalesInvoiceDetails on inv.SalesInvoiceId equals det.SalesInvoiceId
    join item in Dbcontext.tblItems on det.ItemId equals item.ItemId
    join itemGroup in Dbcontext.tblItemGroups on item.ItemGroupId equals itemGroup.ItemGroupId
    join loc in Dbcontext.tblLocations on inv.LocationId equals loc.LocationId
    group new { det.Quantity, itemGroup.ItemGroupCode, item.Weight, item.ItemCode, det.LineTotal } by loc.LocationName into g
    select new LocationCount()
    {
    LocationName = g.Key,
    FeedBagCount = (int?)g.Where(x => x.ItemGroupCode == "FEED").Sum(x => x.Quantity),
    SupremeKg = (decimal?)g.Where(x => x.ItemGroupCode == "ABATTOIR").Sum(x => x.Weight * x.Quantity),
    SupremeValue = (decimal?)g.Where(x => x.ItemGroupCode == "ABATTOIR").Sum(x => x.LineTotal),
    DocCount = (int?)g.Where(x => x.ItemCode == "DOC").Sum(x => x.Quantity)
    }).OrderBy(x=>x.LocationName).ToListAsync();


Count substring in string With the comments on the same line as the code, it refers to. e.g

NumBob = 0
s = 'sexbobombob'
i = range(len(s)+1) #Generates a list from 0 to the length of s
u = 0 # This would be the variable storing each value of the list
for ans in i: # this allows me to extract each value of i
    if ans > 0: # Completely arbitrary. i just used any condition that allows me to pick up all values in the list.
        u = ans
    x = s[u:u+3] #By using indexing to pick u, u+1, and u +2 i can pick up 3 letters at a time
    if "bob" in x: # Checking if the term i picked up is a Bob
        numBob += 1 # increasing the Bob count
print "Number of times bob occurs is:" + str(numBob) # Printing the number of Bobs

With the comments beneath the line of code, it refers to. e.g

numBob = 0
s = 'sexbobombob'
i = range(len(s)+1)
#Generates a list from 0 to the length of s
u = 0
# This would be the variable storing each value of the list
for ans in i:
    # this allows me to extract each value of i
    if ans > 0:
        # Completely arbitrary. i just used any condition that allows me to pick up all values in the list.
        u = ans
    x = s[u:u+3]
    #By using indexing to pick u, u+1, and u +2 i can pick up 3 letters at a time
    if "bob" in x:
        # Checking if the term i picked up is a Bob
        numBob += 1
        # increasing the Bob count
print "Number of times bob occurs is:" + str(numBob)
#Printing the number of Bobs

Or with the comments preceding the line of code. e.g

numBob = 0
s = 'sexbobombob'
#Generates a list from 0 to the length of s
i = range(len(s)+1)
# This would be the variable storing each value of the list
u = 0
# this allows me to extract each value of i
for ans in i:
    # Completely arbitrary. i just used any condition that allows me to pick up all values in the list.
    if ans > 0:
        u = ans
    # By using indexing to pick u, u+1, and u +2 i can pick up 3 letters at a time
    x = s[u:u+3]
    # Checking if the term i picked up is a Bob
    if "bob" in x:
        # increasing the Bob count
        numBob += 1
        # Printing the number of Bobs
print "Number of times bob occurs is:" + str(numBob)

Using this kind of all-top-level short-named fully imperative style forces you to rely heavily on comments to make the code readable.

Comments everywhere repeating the code violate the DRY principle:

    # increasing the Bob count
    numBob += 1

You just doubled reading / maintaining time here.

Descriptive names, examples of usage and sometimes functional style make "how"-kind comments unnecessary:

def count(substring, string):
    """
    >>> count("bob","bobhejrhaabobfqbqsanbobb")
    3
    """
    return sum(substring == group for group in groups(string, len(substring)))

def groups(xs, n):
    """
    >>> groups([1, 2, 3, 4, 5], 3)
    [[1, 2, 3], [2, 3, 4], [3, 4, 5]]
    """
    for i in range(len(xs)-n):
        yield xs[i:i+n]

The code looks self explaining to me, but feel free to ask about any doubts about it.


CodeForces Freshman exercise There are 60 submissions from me to the very first beginner-freshman exercise at codeforces.com. Problem: 100985A - MaratonIME helps Pablito

I don't pass the tests because of Time limit exceeded (tle). I'm just thinking about the test can't be passed using java because of performance. But really, I think a crappy error in my algorithm is more likely. Or my approach is completely wrong. So feel free to comment my approach.

Excercise:

  As is well known by any cultured person, rats are the smartest beings
  on earth. Followed directly by dolphins.

  MaratonIME knows about the species hierarchy and uses this knowledge
  in it's regard. Usually, when they need some resource, they know it's
  always useful to have a smart rat available. Unfortunately, rats are
  not very fond of us, primates, and will only help us if they owe us
  some favour.

  With that in mind, MaratonIME decided to help a little rat called
  Pablito. Pablito is studying rat's genealogy, to help with cloning and
  genetic mapping. luckily, the way rats identify themselves make the
  job much easier.

  The rat society is, historically, matriarchal. At first, there were
  little families, each of which had it's own leading matriarch. At that
  time, it was decided that rats would organize themselves according to
  the following rules:

  Each martiarch had an id number greater than one. Each of these ids
  were chosen in a way such that they would have the least amount of
  divisors possible. Each family member had the same id as the
  matriarch. The id of any newborn rat would be the product of its
  parents id's. For instance, the offspring of a rat with id 6 and
  another with id 7 is 42.

  Pablito needs to know if two given rats have a common ancestor, but
  his only tool is the id number of each of the two rats, which is
  always a positive integer greater than 1 with no more than 16 digits.
  Can you help him?

  Create a program that decides if a pair of rats have some common
  ancestor.

  Input The input begins with a positive integer t ≤ 105, the number of
  test cases.

  After that, follows t lines, each with two integers ai e bi
  identifying two rats.

  Every rat's id is a positive integer greater than 1 and with no more
  than 16 digits.

  Output For each test case, print "Sim" if the rats ai and bi share a
  common ancestor and "Nao" otherwise.

My latest approach before using multiple threads. (Yeah, TLE there as well).

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;

public class A {
    public static void main(String[] args) throws IOException{
        BufferedReader scan = new BufferedReader (new InputStreamReader(System.in));
        int amountTests = Integer.parseInt(scan.readLine());

        for (int i = 0; i<amountTests; i++){
            String[] tmp = scan.readLine().split(" ");
            long l1 = Long.parseLong(tmp[0]);
            long l2 = Long.parseLong(tmp[1]);
            if (l1 < l2){
                searchPrims(l1,l2);
            }else{
                searchPrims(l2,l1);
            }
        }
    }

    static void searchPrims(long n, long n2){
        int d = 2;
        while (true){
            if(n%d==0){
                if(n2%d==0){
                    System.out.println("Sim");
                    return;
                }
                n/=d;
                d = 2;
            }
            else if(n==1){
                System.out.println("Nao");
                return;
            }
            else{
                if(d==2)
                    d=3;
                else{
                    d+=2;
                }
            }
        }
    }
}

Improving what you have

                d = 2;

Part of the problem is this line.  Each time you find a matching divisor, you restart from the beginning.  This isn't necessary.  The code works without this.  Once you've removed all the 2s, nothing will put them back.

    static void searchPrims(long n, long n2){
        int d = 2;
        while (true){
            if(n%d==0){
                if(n2%d==0){
                    System.out.println("Sim");
                    return;
                }
                n/=d;
                d = 2;
            }
            else if(n==1){
                System.out.println("Nao");
                return;
            }
            else{
                if(d==2)
                    d=3;
                else{
                    d+=2;
                }
            }
        }
    }

I would find this easier to read as

    static boolean searchCommonFactors(long n, long n2){
        for (int d = 3; n >= d; d += 2) {
            if (n % d == 0) {
                if (n2 % d == 0) {
                    return true;
                }

                do {
                    n /= d;
                } while (n % d == 0);
            }
        }

        return n % 2 == 0 && n2 % 2 == 0;
    }

This returns a boolean value instead of printing.  So we would have to print outside it.  Something like

    static void printAnswer(boolean answer) {
        System.out.println(answer ? "Sim" : "Nao");
    }

This way we can split up the returns more naturally.  Also, this would be reusable in other problems with different output requirements.

This way we don't keep checking if n2 is divisible by a factor.  We check once.  If it is, we return immediately.  If it isn't, we know it never will be.  We discard all of that factor to reduce the maximum size of d.

Putting the even check at the end takes the special behavior out of the loop.  This way we can simplify the loop iteration.

We could get back some efficiency by putting that check first and then removing all the 2s from n.

        if (n % 2 == 0 && n2 % 2 == 0) {
            return true;
        }

        while (n % 2 == 0) {
            n /= 2;
        }

Alternative approaches

Your approach basically factors the smaller number.  If one of its factors is also a factor of the other number, you say that they are related.

A more efficient approach might be to use the Euclidean algorithm to see if the two numbers have a greatest common divisor (GCD) larger than 1.  So you'd say something like

    printAnswer(gcd(l1, l2) > 1);

to get the answer.

A little searching finds a gcd implementation:

public static long gcd(long a, long b) {
    while ( 0 != b ) {
        long temp = a;
        a = b;
        b = temp % b;
    }

    return a;
}

There's a relatively elegant recursive solution.

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

But the iterative version should normally be faster.

I would expect this to be faster than your code, as it reduces a and b to the greatest common factor rather than searching out every factor of a only to find that none of them match.


Modelling user preference system with categories I am modelling a user preference system with the following requirements

a user can have attributes (foods they eat)
a user can have dynamic categories of preferences
users which have all their categories with at least one intersect can match

So far I have used Postgres's arrays to build a system to avoid joins. NB. I'm not using proper keys at the moment for simplicity.

CREATE TABLE public.users (
    name text,
    food text[]
);

CREATE TABLE public.preferences (
    name text,
    category text,
    food text[]
);

INSERT INTO public.preferences VALUES ('John', 'Breakfast', '{toast,cereal}');
INSERT INTO public.preferences VALUES ('John', 'Dinner', '{ham}');
INSERT INTO public.preferences VALUES ('Jane', 'Breakfast', '{toast,eggs}');
INSERT INTO public.preferences VALUES ('Jack', 'Breakfast', '{grapefruit}');

INSERT INTO public.users VALUES ('John', '{peas,ham,"ice cream",toast}');
INSERT INTO public.users VALUES ('Jane', '{eggs,ham,"ice cream",cereal,toast}');
INSERT INTO public.users VALUES ('Jack', '{toast,cereal,eggs,peas}');

I have a query that seems to work, however was wondering if anyone had any feedback. It works by asserting that there "are not any categories, which don't match".

select * from users u where name <> 'Jane'
and not exists (select from preferences p where name = 'Jane' and not u.food && p.food)
and not exists (select from preferences p where u.name = p.name
                and not (select u.food from users u where u.name = 'Jane') && p.food);

 name |             food
------+------------------------------
 John | {peas,ham,"ice cream",toast}
 (1 row)

I have used Postgres's arrays to build a system to avoid joins. NB. I'm not using proper keys at the moment for simplicity.

One person's simplicity is another person's denormalized nightmare. Are you avoiding joins because of some already-analysed performance rationale? If so, fine. But if it's based on superstition, this is probably premature optimization.

All of that is to say, the "traditional" normalized relational approach to this would be:

A user table with an integer primary key
A food table with an integer primary key
A meal table with an integer primary key
A preference table with foreign keys to user, food and meal

Don't be afraid of join. Or if you are, consider using a denormalized or "tabular" database like Mongo.


Basic JavaScript counter I am new to JavaScript and wondered if there is a more streamlined way of writing this code:

var choices = "ABBCD" // A string containing the letters A-D
var total = 0

function calculate() { // Iterate through choices and add to total depending on the letter
  for (var i=0; i<choices.length; i++) {
    if (choices[i]=="A") {
      total+=1;
    } if (choices[i]=="B") {
      total+=2;
    } if (choices[i]=="C") {
      total+=3;
    } if (choices[i]=="D") {
      total+=4;
    }
  }
}

Good: Use if-elseif

You are calling 4 if statements with your code even if the 1st one is true. These are all mutually exclusive statements so use if-elseif

for (var i=0; i<choices.length; i++) {
    if (choices[i]=="A") {
        total+=1;
    } else if (choices[i]=="B") {
        total+=2;
    } else if (choices[i]=="C") {
        total+=3;
    } else if (choices[i]=="D") {
        total+=4;
    }
}

Better: Use a switch statement

for (var i=0; i<choices.length; i++) {
    switch (choices[i]) {
        case 'A':
            total += 1;
            break;
        case 'B':
            total += 2;
            break;
        case 'C':
            total += 3;
            break;
        case 'D':
            total += 4;
            break;
    }
}

Best: Convert the character to its numeric equivalent using its ASCII value

for (var i = 0; i < choices.length; i++) {
    //omitted validation that character is a legal value...
    total += choices[i].charCodeAt(0) - 64;
}


Automatic IEqualityComparer There are APIs like the Except extension that require the IEqualityComparer<T> to work. I find it's too much work for such a simple task to implement an interface so I thought why not automate it.

I implemented this interface in a reusable fashion so that I can use it with any value and any number of properties.

internal class AutoEqualityComparer<T> : IEqualityComparer<T>
{
    public AutoEqualityComparer(IEnumerable<Func<T, object>> selectors)
    {
        Selectors = selectors;
    }

    private IEnumerable<Func<T, object>> Selectors { get; }

    public bool Equals(T left, T right)
    {
        return
            !ReferenceEquals(left, null) &&
            !ReferenceEquals(right, null) &&
            Selectors.All(selector => selector(left).Equals(selector(right)));
    }

    public int GetHashCode(T obj)
    {
        unchecked
        {
            return Selectors
                .Select(selector => selector(obj).GetHashCode())
                .Aggregate(17, (hashCode, subHashCode) => hashCode * 31 + subHashCode);
        }
    }
}

I then build a new extension which also can accept any number of selectors to compare:

internal static class Enumerable
{
    public static IEnumerable<T> Except<T>(
        this IEnumerable<T> first,
        IEnumerable<T> second,
        params Func<T, object>[] compare)
    {
        var mec = new AutoEqualityComparer<T>(compare);
        return first.Except(second, mec);
    }
}

Example:

Select all properties of an exception that are not in the base exception:

var exceptionProperties =
    typeof(ArgumentException)
    .GetProperties()
    .Except(typeof(Exception).GetProperties(), x => x.Name);

// result: ParamName is the only property

I think if you created a projection and pass that in the class would be easier to use and more readable for anyone coming after you.

Instead of the IEnumerable<Func<T, object>> I would change it to be Func<T, K> projection and lose the IEnumerable.  Which would need to make the class now take two generics

public class AutoEqualityComparer<T, K> : IEqualityComparer<T>
{
    private readonly Func<T, K> _projection;

    public AutoEqualityComparer(Func<T, K> projection)
    {
        _projection = projection;
    }

Since we now have the strong type of class in equals we can use the EqualityComparer class for both the Equals and GetHashCode methods.

    public virtual bool Equals(T x, T y)
    {
        if (x == null && y == null)
        {
            return true;
        }
        if (x == null)
        {
            return false;
        }
        if (y == null)
        {
            return false;
        }

        var xData = _projection(x);
        var yData = _projection(y);

        return EqualityComparer<K>.Default.Equals(xData, yData);
    }

    public virtual int GetHashCode(T obj)
    {
        if (obj == null)
        {
            return 0;
        }

        var objData = _projection(obj);

        return EqualityComparer<K>.Default.GetHashCode(objData);
    }

I would create a class to create the AutoEqualityComparer to make it easier to use

public class EqualityProjectionComparer<T>
{
    public static AutoEqualityComparer<T, K> Create<K>(Func<T, K> projection)
    {
        return new AutoEqualityComparer<T, K>(projection);
    }
}

Now with the extension method you can create Tuples or anonymous classes for the IEqualityComparer

Example could be

var comparer = EqualityProjectionComparer<ArgumentException>.Create(arg => new
{
    arg.ParamName,
    arg.Message
});

Now to me this is more clear on what we are comparing and anonymous type compare the properties to see if they are equal and not reference.


Find the number of all specific substrings in a string I would like to ask a code review  regarding a concrete exercise. Let's suppose I have to get the number of all specific substrings in a string.
We call something specific if any of these conditions are is true:

The string consists of similar characters. For example: "aaaaa"

The string consists of similar characters except the middle one, which can be anything. For example: "aabaa"

To do this, I first decided to get all combinations into a list. I did it in an O(n³) solution just with 2 basic for loops and using substring() (see more), this way:
private static List<String> allCombinations(String s) {
    List<String> output = new ArrayList<>();
    for (int i = 1; i <= s.length(); i++) {
        for (int j = 0; j <= s.length() - i; j++) {
            output.add(s.substring(j, j + i));
        }
    }
    return output;
}

Subsequently, I used this method to count how many of these are special:
static long substrCount(String s) {
    long res = 0L;
    List<String> output = allCombinations(s);
    for (String x : output) {
        if(isSpecial(x)){
            res++;
        }
    }
    return res;
}

isSpecial() looks like this:
private static boolean isSpecial(String input) {
    Set<Character> occurrences = new HashSet<>();
    for (int i = 0; i < input.length(); i++) {
        occurrences.add(input.charAt(i));
    }
    if (occurrences.size() > 2) {
        return false;
    }
    if (occurrences.size() == 1) {
        return true;
    }
    return input.length() % 2 == 1 && input.charAt(0) == input.charAt(input.length() - 1) && input.charAt(input.length() / 2) != input.charAt(0);
}

I've got 2 questions:

This is a practice exercise with provided tests, and most of the test cases failed due to time complexity problems. How could I reduce the time complexity of my solution?

If you could give me any general feedback what to improve in - based on my code - I would be really thankful.

Thank you in advance.

First off, I believe you are confusing the terms "specific" and "special" in the description.

Method isSpecial

You could move the if (occurrences.size() > 2) statement into the for loop, because once you know that there are more than 2 different characters, there is no need to continue adding more characters to the Set. Then you can also initialize the HashSet to an initial size of 3, because it never can grow larger than that.
You don't need the part input.charAt(0) == input.charAt(input.length() - 1) in the final expression, because when the length is more than two and if there are exactly two different characters, it can never be
false when input.charAt(input.length() / 2) != input.charAt(0) is true.
Finally I'd put input.length into a local variable. That will speed up the for loop by a tiny amount by avoiding the method call and it will make the final expression a bit shorter and thus better to read

private static boolean isSpecial(String input) {
    int len = input.length();
    Set<Character> occurrences = new HashSet<>(3);
    for (int i = 0; i < len; i++) {
        occurrences.add(input.charAt(i));
        if (occurrences.size() > 2) {
            return false;
        }
    }
    if (occurrences.size() == 1) {
        return true;
    }
    return len % 2 == 1 && input.charAt(len / 2) != input.charAt(0);
}

Method substrCount

This method can be shorten significantly by using Java 8's Stream API:

static long substrCount(String s) {
   return allCombinations(s).stream().filter(ClassName::isSpecial).count();
}

(ClassName is the name of the class isSpecialis in.)


Endianness conversion in C I have written a simple C header for converting the endianness of short integers and long integers. It uses the GCC macro __BYTE_ORDER__ to check the system's byte order and define the macros based on that.

The header creates the macros LITTLE_ENDIAN_SHORT(n), LITTLE_ENDIAN_LONG(n), BIG_ENDIAN_SHORT(n), BIG_ENDIAN_LONG(n) which convert the value n from host endianness to the endianness specified.

Here is the source for endian.h:

#ifndef ENDIAN_H
#define ENDIAN_H

#define REVERSE_SHORT(n) ((unsigned short) (((n & 0xFF) << 8) | \
                                            ((n & 0xFF00) >> 8)))
#define REVERSE_LONG(n) ((unsigned long) (((n & 0xFF) << 24) | \
                                          ((n & 0xFF00) << 8) | \
                                          ((n & 0xFF0000) >> 8) | \
                                          ((n & 0xFF000000) >> 24)))

#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
#    define LITTLE_ENDIAN_SHORT(n) (n)
#    define LITTLE_ENDIAN_LONG(n) (n)
#    define BIG_ENDIAN_SHORT(n) REVERSE_SHORT(n)
#    define BIG_ENDIAN_LONG(n) REVERSE_LONG(n)
#elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
#    define LITTLE_ENDIAN_SHORT(n) REVERSE_SHORT(n)
#    define LITTLE_ENDIAN_LONG(n) REVERSE_LONG(n)
#    define BIG_ENDIAN_SHORT(n) (n)
#    define BIG_ENDIAN_LONG(n) (n)
#else
#    error unsupported endianness
#endif

#endif

Is this a good way to implement the macros or is there a better way?

First off, Jean-François is absolutely right: you cannot assume any particular bit widths for the built-in types, short, int, long, etc. Use the types defined in stdint.h that have explicit bit widths to ensure that the code is correct and portable.
Otherwise, your code looks pretty good, and this is a reasonable implementation. But…

Is this a good way to implement the macros or is there a better way?

There is indeed a better way: to use inline functions rather than macros. :-)
After an optimizing compiler finishes, the resulting code will be equivalent, but the advantages of functions are numerous: type safety, the ability to use expressions as arguments, no insidious parenthesization bugs, etc.
You will still need some preprocessor magic to avoid generating code when it is not necessary, but this is still much cleaner than implementing the whole header as a mess of macros.

Define a couple of inline helper functions that do the conversion, like so:
inline uint16_t Reverse16(uint16_t value)
{
    return (((value & 0x00FF) << 8) |
            ((value & 0xFF00) >> 8));
}

inline uint32_t Reverse32(uint32_t value)
{
    return (((value & 0x000000FF) << 24) |
            ((value & 0x0000FF00) <<  8) |
            ((value & 0x00FF0000) >>  8) |
            ((value & 0xFF000000) >> 24));
}

Although it is not strictly necessary, I have kept your explicit masking because I think it increases the clarity of the code, both for human readers and for the compiler. Others may feel that simpler is better. For example, Reverse16 could simply be implemented as return ((value << 8) | (value >> 8));.

However, I've chosen to format your code slightly differently for nice vertical alignment. I think this makes it easier to read, and easier to audit for correctness at a glance.

On most compilers, there are byte-swapping intrinsics that you could have used to implement the body of these functions. For example, GNU compilers (including GCC and Clang) have __bswap_32 and __bswap_16 macros in the <byteswap.h> header, and Microsoft's compiler offers the _byteswap_ushort and _byteswap_ulong intrinsics in the <intrin.h> header.
While intrinsics can often result in better code than writing out the C code long-form, all compilers I tested here are exceptionally smart: GCC, Clang, and ICC all recognize the bit-twiddling code used above and compile it to identical object code as if we had used the intrinsic—a single BSWAP instruction on the x86 architecture! Microsoft's compiler makes this optimization for the 32-bit version, but not for the 16-bit version. However, its output either way is still perfectly reasonable, and if you're seeking portable code, there is no compelling interest in using the intrinsics. For once, you don't need them to get optimal code!

Now that we have those helper functions, we need to define some more inline functions that work like your macros, except that the conditional logic will be encapsulated within the body of the functions, rather than surrounding the macro definitions.
Naturally, since these are functions, not macros, we'll use a different (title-case) naming convention. We don't need SCREAMING_CASE because they're not scary anymore:
inline uint16_t LittleEndian16(uint16_t value)
{
#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
    return value;
#elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
    return Reverse16(value);
#else
#    error unsupported endianness
#endif
}

inline uint16_t BigEndian16(uint16_t value)
{
#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
    return Reverse16(value);
#elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
    return value;
#else
#    error unsupported endianness
#endif
}

inline uint32_t LittleEndian32(uint32_t value)
{
#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
    return value;
#elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
    return Reverse32(value);
#else
#    error unsupported endianness
#endif
}

inline uint32_t BigEndian32(uint32_t value)
{
#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
    return Reverse32(value);
#elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
    return value;
#else
#    error unsupported endianness
#endif
}

The complete code is slightly longer than the macro-based version, but the safety and other benefits of functions more than justify this expanded length. The compiler doesn't care, and it only takes a few minutes longer to write. You will more than make up for it later in the time you don't have to spend debugging macros used in expressions.
You'll use them in basically the same way as the macros. Instead of LITTLE_ENDIAN_SHORT(value), you'd call LittleEndian16(value). Note that I've also used the explicit bit-width in the function's names, instead of the ambiguous short and long type names.


Pythonic way to flatten nested dictionarys Here's my attempt to write a function that flattens a nested dictionary structure in Python 3.6

This came about because I had a nested dictionary structure of data I wanted to visualize in the library Bokeh.

I didn't like other examples that require a recursive function call. (I find it difficult to reason about, and I believe python has a relative small recursion limit)

Tell me about the styling, ease of (or lack of) understanding and what you would do.

Here's my code:

def flatten_dict(dictionary, delimiter='.'):
    """
    Function to flatten a nested dictionary structure
    :param dictionary: A python dict()
    :param delimiter:  The desired delimiter between 'keys'
    :return: dict()
    """

    #
    # Assign the actual dictionary to the one that will be manipulated
    #
    dictionary_ = dictionary

    def unpack(parent_key, parent_value):
        """
        A function to unpack one level of nesting in a python dictionary
        :param parent_key: The key in the parent dictionary being flattened
        :param parent_value: The value of the parent key, value pair
        :return: list(tuple(,))
        """

        #
        # If the parent_value is a dict, unpack it
        #
        if isinstance(parent_value, dict):
            return [
                (parent_key + delimiter + key, value)
                for key, value
                in parent_value.items()
            ]
        #
        # If the If the parent_value is a not dict leave it be
        #
        else:
            return [
                (parent_key, parent_value)
            ]

    #
    # Keep unpacking the dictionary until all value's are not dictionary's
    #
    while True:
        #
        # Loop over the dictionary, unpacking one level. Then reduce the dimension one level
        #
        dictionary_ = dict(
            ii
            for i
            in [unpack(key, value) for key, value in dictionary_.items()]
            for ii
            in i
        )
        #
        # Break when there is no more unpacking to do
        #
        if all([
            not isinstance(value, dict)
            for value
            in dictionary_.values()
        ]):
            break

    return dictionary_

dictionary_ = dict(
        ii
        for i
        in [unpack(key, value) for key, value in dictionary_.items()]
        for ii
        in i
    )

This is probably the worst part in the code, yet this is pretty much the whole code. Variable names are terrible and the intent is a bit hidden. I understand that unpack returns lists so your list-comprehension generates a list of lists. So this expression is flattening a list of list of tuples and turning it into a dictionary.

For starter, flattening iterables of iterables can be achieved more easily using itertools.chain.from_iterable so you just need dictionary_ = dict(itertools.chain.from_iterable([unpack(key, value) for key, value in dictionary_.items()])) and get rid of the awful variable names.

Second, and this also apply to the all() call, you can use simpler generator expression instead of full list-comprehension to play it nicer with the memory. This only means removing the square brackets around the expression:

dictionary_ = dict(itertools.chain.from_iterable(
        unpack(key, value) for key, value in dictionary_.items()
))

Lastly, but this is highly debatable, I would use itertools.starmap instead of the whole expression:

dictionary_ = dict(itertools.chain.from_iterable(itertools.starmap(unpack, dictionary_.items()))

I would also change unpack to fit more nicely with these changes: make it a generator instead of building lists (that you are going to discard right away). And just to propose an alternative way, I’ll go with an EAFP approach:

def unpack(parent_key, parent_value):
    """A function to unpack one level of nesting in a dictionary"""

    try:
        items = parent_value.items()
    except AttributeError:
        # parent_value was not a dict, no need to flatten
        yield (parent_key, parent_value)
    else:
        for key, value in items:
            yield (parent_key + delimiter + key, value)

You may also find this function to choke on dictionaries whose keys are not strings. This is due to the parent_key + delimiter + key part that will fail to "concatenate" numbers and strings for instance. Or any custom object…

There are two solutions to this problem depending on the intended use-case:

use str.format to convert each key to a string;
use a tuple as the flattened key instead of a string.

I prefer the second solution as a key of '3.14' can ambiguously be comming from the key '3' and '14' or '3.14' in the first solution. Whole code would be:

from itertools import chain, starmap

def flatten_dict(dictionary):
    """Flatten a nested dictionary structure"""

    def unpack(parent_key, parent_value):
        """Unpack one level of nesting in a dictionary"""
        try:
            items = parent_value.items()
        except AttributeError:
            # parent_value was not a dict, no need to flatten
            yield (parent_key, parent_value)
        else:
            for key, value in items:
                yield (parent_key + (key,), value)

    # Put each key into a tuple to initiate building a tuple of subkeys
    dictionary = {(key,): value for key, value in dictionary.items()}

    while True:
        # Keep unpacking the dictionary until all value's are not dictionary's
        dictionary = dict(chain.from_iterable(starmap(unpack, dictionary.items()))
        if not any(isinstance(value, dict) for value in dictionary.values()):
            break

    return dictionary


Using default None values in Python when assigning split() to a tuple Let's say I want to parse audio track information into two variables like this:

'2/15' -> track = 2, num_tracks = 15
'7' -> track = 7, num_tracks = None

What's an elegant way to do this in Python? For example:

track, num_tracks = track_info.split('/')

This works for the first format, but not for the second, raising ValueError: need more than 1 value to unpack. So I came up with this:

try:
    track, num_tracks = track_info.split('/', 1)
except:
    track, num_tracks = track_info, None

How can this be more elegant or better?

try:
    track, num_tracks = track_info.split('/', 1)
except:
    track, num_tracks = track_info, None

Honestly, this is not a terrible solution. But you should almost never use except:; you should catch a specific exception.

Here is another way:

tracks, _, num_tracks = text.partition('/')
return int(tracks), int(num_tracks) if num_tracks else None