Converting Java int-based amount to Coin metrics

Highlighting what a few helper classes can do :

Imagine this enum

private enum Coin {
    QUARTER(25, "quarter"), DIME(10, "dime"), NICKEL(5, "nickel"), PENNY(1, "penny", "pennies");
    ...

and a tuple of quotient and remainder :

private static class QuotientAndRemainder {
    private final int quotient;
    private final int remainder;

so the enum can sport this method :

public QuotientAndRemainder divide(int pennies) {
    return QuotientAndRemainder.quotient(pennies / pennyValue).withRemainder(pennies % pennyValue);
}

and this one :

public String format(int amount) {
    return Integer.toString(amount) + ' ' + (amount == 1 ? singular : multiple);
}

then the convert method simply becomes :

public static String convert(int amount) {
    return print(minimalChange(amount));
}

private static String print(Map<Coin, Integer> amounts) {
    StringBuilder builder = new StringBuilder();
    for (Coin coin : amounts.keySet()) {
        builder.append(coin.format(amounts.get(coin))).append('\n');
    }
    return builder.toString();
}

private static Map<Coin, Integer> minimalChange(int amount) {
    int penniesLeft = amount;
    Map<Coin, Integer> amounts = new EnumMap<Coin, Integer>(Coin.class);
    for (Coin coin : Coin.values()) {
        QuotientAndRemainder quotientAndRemainder = coin.divide(penniesLeft);
        amounts.put(coin, quotientAndRemainder.quotient());
        penniesLeft = quotientAndRemainder.remainder();
    }
    return amounts;
}

Adding larger units of currency now simply comes down to adding them to the enum.


Generating a maximal geometric progression of integers

No threading, please!!

First of all, get rid of the threads all-together. That should speed things up: Python threads are (sort-of) fine when things are blocking on IO, but in this case everything is CPU-based, and that means the GIL (global interpreter lock) makes sure that only one thread is running.

Also, the following code is a bit curious:

for s in xrange(int(n/4),int(n*0.8)+1):
    t=Thread(target=calc,args=(s,n,g))
    t.start()
    t.join()

I know that threading is a bit of a problem anyway here, but I would just like to comment on this specifically: by writing t.join() you are waiting on the thread to finish before starting the next iteration. This gets read of the threading all together, as there will only be two threads alive at a given time: 1 worker thread, and the main thread waiting on the worker to die.

I'd suggest writing it as:

threads = []
for s in iterable:
    t = Thread(...)
    threads.append(t)
    t.start()

for t in threads:
    t.join()

Naming and describing

Ok, with threading out of the way, I'm having a difficult time finding what the application is supposed to do.

It does something with an n, and each worker with an s and an n. What are those? The code could be a lot clearer in this regard.

Anyway, let us clean things up a bit first, and maybe I can find a pattern to help you improve the algorithm.

Performance

Checking for integer-valued lists.

Let's start looking at int_check.

def int_check(l):
    for i in l:
        if i-int(i)!=0: return False
    return True

You're checking if a number is integral by doing a lengthy calculation. Why not just

if i.is_integer(): return False

? Available since Python 2.6. https://docs.python.org/2/library/stdtypes.html#float.is_integer

Also, looping is expensive! Suggestion, use a comprehension together with the all.

def int_check(l):
    return all(i.is_integer() for i in l)

You should also name it better:

def is_integer_list(l):
    return all(i.is_integer() for i in l)

Useless duplication of lists

Let's look at the following code:

def check(obj):
    cpy0,cpy1=tee(obj)
    iter0,iter1=tee(float(x)/y for x,y in izip(cpy0,islice(cpy1,1,None)))
    return all(x==y for x,y in izip(iter0,islice(iter1,1,None))) and len(obj)>2 and int_check(obj)

First of all, let us do some short-circuiting. You're doing a lot of logic to see if it is a nice geometric progression, and after all the hard work, you check some very easy conditions. Turn that around!

def check(obj):
    if len(obj) <= 2 or not int_check(obj):
        return False
    cpy0,cpy1=tee(obj)
    iter0,iter1=tee(float(x)/y for x,y in izip(cpy0,islice(cpy1,1,None)))
    return all(x==y for x,y in izip(iter0,islice(iter1,1,None)))

I'm doubtful of you using tee for this. But I don't see a better builtin for it either. But I see a lot of logic here which I'd like to separate a bit more:

def steps(iterable):  # Needs better name, please help me!
    iterator = iter(iterable)  # Cast iterable to iterator.
    cur = next(iterator)
    for val in iterator:
        yield cur, val
        cur = val

A quick test to see that it does what we want.

>>> list(steps(range(10)))
[(0, 1), (1, 2), (2, 3), (3, 4), (4, 5), (5, 6), (6, 7), (7, 8), (8, 9)]

Using that, we get

def check(obj):
    if len(obj) <= 2 or not int_check(obj):
        return False
    ratios = (float(x)/y for x, y in steps(obj))
    return all(x == y for x, y in steps(ratios))

Don't know if it's faster, but at least it is more readable! And I expect faster. But the main gain might be in the next section

The main loop

Your code was written as

def calc(s,n,q):
    for r in xrange(125,n*100+1,5):
        t=[s]
        while t[-1]<=n:
            t.extend([t[-1]*((0.0+r)/100)])

        if check(t):
            q.put(takewhile(lambda i: i<=n,t))

Since I want to get rid of the threading, I'll assume q is a list instead of a queue.

def calc(s,n,q):
    for r in xrange(125,n*100+1,5):
        t=[s]
        while t[-1]<=n:
            t.extend([t[-1]*((0.0+r)/100)])

        if check(t):
            q.append(takewhile(lambda i: i<=n,t))

(In reality I'd rather yield the results instead of adding to a queue. Anyhow, I'm shelving that for now)

takewhile vs slicing.

In the q.put you write takewhile. By the while loop above, you already know that will be all but the last value.

So, write

q.append(t[:-1])

Or, even

t.pop()
q.append(t)

That should be faster, because now the lambda i: i <= n doesn't have to be executed anymore.

Useless recalculation

Having written the above loop now as

def calc(s,n,q):
    for r in xrange(125,n*100+1,5):
        t=[s]
        while t[-1]<=n:
            t.extend([t[-1]*((0.0+r)/100)])

        if check(t):
            t.pop()
            q.append(t)

We can turn our focus to the expression t.extend(...) above:

    for r in xrange(125,n*100+1,5):
        t=[s]
        while t[-1]<=n:
            t.extend([t[-1]*((0.0+r)/100)])

Now in each iteration of the while loop, (0.0+r)/100 gets recalculated. Why not

    for r in xrange(125, n*100+1,5):
        r_ = float(r) / 100
        t=[s]
        while t[-1]<=n:
            t.extend([t[-1]*r_)

Incorrect algorithm

The worst part, I think, is that your code works by trying different ratios, and will miss some geometric sequences.

For instance, the sequence

81, 90, 100  # r == 100.0 / 90.0 == 90.0 / 81.0 (approx 1.1111111)

While your code will only try 1.25, 1.30, 1.35, ... .

Now, maybe there is some high-level mathematical proof that the solution will always involve a ratio which can be expressed as (125 + 5*k)/100, but I don't think so.

I would suggest turning the problem up-side-down. The following is an O(n^2 * log(n)) suggestion (estimated, but I think correct. Maybe even O(n^2)).

for x1 in xrange(1, n):
    for x2 in xrange(x1, n):
        # Using that
        #     x1 == a
        #     x2 == a * r
        # we get that
        #     r == x2 / x1
        ratio = float(x2) / x1
        # Continue as before, starting from t = [x1, x2]

Another point of incorrectness

In check, you check that the length is > 2, but then you filter on <=n. This should be the other way around.

A tiny suggestion.

Floating point arithmetic is challenging for problems like these. Maybe look into the fractions module?

from fractions import Fraction

...
ratio = Fraction(x2, x1)
...

Now, Fraction does not have an is_integer, but you can do the following:

>>> Fraction(5, 1).denominator == 1
True

Hope that all helps.

Corner cases

As an addendum, here are some cases your code flagged incorrectly:

>>> geom_prog(16)
28.0  # 4 + 8 + 16

but it should be at least 37: 9 + 12 + 16.

>>> geom_prog(100)
244.0  # 64 + 80 + 100

but it should be at least 271: 81 + 90 + 100 (maybe even more, but doubt it).


There is a serious flaw in your handling of the empty list. A user would expect that these definitions are equivalent, but they ain't:

Nlist n1 = new NList("x");
Nlist n2 = new NList("x", new NList());

I strongly recommend to have an explicit subclass for the empty list (possibly a singleton) and to prohibit null for _cdr.


Don't use an alibi-class. It doesn't make the code any more OO, even in the cases it would be desirable, it just adds to the clutter.
Solution and convert are such generic names. Naming is hard, but good names really help.
You know an int need not be positive? An unsigned type may be more appropriate for a count, but even then you should handle zero.
Only if you need a null-terminated string should you consider a parameter of type std::string const&. In all other cases, using std::string_view is more efficient.
While creating a temporary string of length 1 probably doesn't cause dynamic allocation due to small-string-optimisation (or allocation ellision), doing so just to add a character to the result-string is cumbersome and potentially quite inefficient (in theory, the compiler could optimise it all out). Just append it directly.
If you assign directly, diff is gone.
useDiff doesn't make a difference, so remove it.
Consider adding additional test-cases. For that, you should rewrite main().

See the modified code live on coliru:

#include <string>
#include <string_view>
#include <stdexcept>

auto zigzag(std::string_view s, std::size_t cRows) {
    if (!cRows)
        throw std::domain_error("Must output at least one row.");
    if (!cRows || cRows >= s.size())
        return std::string(s);
    std::string r;
    r.reserve(s.size());
    const auto period = 2 * cRows - 2;
    for (auto row = 0 * cRows; row < cRows; ++row) {
        auto delta = 2 * row;
        auto delta2 = period - delta;
        if (!delta) delta = period;
        if (!delta2) delta2 = period;
        for (auto i = row; i < s.size(); i += delta) {
            r += s[i];
            std::swap(delta, delta2);
        }
    }
    return r;
}

#include <iostream>

struct test {
    std::string_view in;
    std::size_t n;
    std::string_view out;
};

int main() {
    constexpr test cases[]{
        "0123456789", 3, "0481357926",
        "", 1, "",
        "0123456789", 10, "0123456789",
        "0123456789", 2, "0246813579",
    };
    for (auto&& x : cases) {
        std::cout << "zigzag(\"" << x.in << "\", " << x.n << ") == \"" << x.out;
        auto r = zigzag(x.in, x.n);
        if (r == x.out)
            std::cout << "\": PASSED\n";
        else
            std::cout << "\": FAILED, got \"" << r << "\"\n";
    }
}


for (int w = 0; w < passwordView.length; w++) {
            passwordView[w] = '_';
        }

There's a built-in for this

        Arrays.fill(passwordView, '_');

You don't have to do it manually.

        char letter = 'x';
        String guess;
        boolean success;

You could declare all these inside the loop instead by changing

            success = false;
            System.out.println("Type a char.");
            while (!success) {
                guess = input.nextLine();
                if (guess.length() == 1 && !(answers.contains(guess))) {
                    letter = guess.charAt(0);
                    success = true;
                    answers = answers + letter + ", ";

to

            char letter;
            boolean success = false;
            System.out.println("Type a char.");
            while (!success) {
                String guess = input.nextLine();
                if (guess.length() == 1 && !(answers.contains(guess))) {
                    letter = guess.charAt(0);
                    success = true;
                    answers = answers + letter + ", ";

or even better, get rid of success entirely.

            char letter;
            System.out.println("Type a char.");
            while (true) {
                String guess = input.nextLine();
                if (guess.length() == 1 && !(answers.contains(guess))) {
                    letter = guess.charAt(0);
                    answers = answers + letter + ", ";
                    break;

or consider something like

            char letter = inputLetter(input, answers);

with a properly defined method.

                new PrintingArray(passwordView);
                System.out.print("\n");

You don't need PrintingArray, but you could just say

                System.out.println(passwordView);

and get the same effect.

            CheckingIfPasswordContainsLetter checker = new CheckingIfPasswordContainsLetter();
            if (checker.cointans(password, letter)) {

You don't need this either.  You could just say

            int firstOccurrence = word.indexOf(letter);
            if (firstOccurrence >= 0) {
                for (int i = firstOccurrence; i < password.length; i++) {

This also has the side effect of not duplicating the scan.  Note that the original version started over at the beginning.


I am not sure how far you intend to expand Migrated, but something like this could be an optimisation:

char* Migrated[]={"026","109"};

int isMigrated(char* codeB)
{
    int i;
    // since we know that all strings in Migrated are 4 bytes long
    int32_t _codeB_as_int32 = *(int32_t *)codeB;
    int32_t * Migrated_as_int32 = Migrated;

    for(i=0;
        i<sizeof(Migrated)/sizeof(*Migrated); // sizeof is a compile time constant, better to put it here
        i++)
    {
        if(codeB == Migrated_as_int32[i])
            return 0;
    }
    return -1;
}

it all really depends on what you intend to have in Migrated.


When cracking argv, you have an opportunity to use argparse if you wish.

Please bury all this code under def main():, rather than creating lots of top-level globals like f.

Rather than f = open() ... f.close(), you might phrase it this way:

with open('ros.fa', 'w') as f:
    ...

There are some comments you might elide. The identifier string is accurate, but a bit vague. Might be better to elide the temp variables:

f.write(req.data.decode('utf-8'))

Overall, this code looks like it gets the job done.


A CLI-based calculator is often a good way to prepare one for bigger programming tasks, as there are just so many ways of doing it, catering for beginner to advanced implementations. I happen to have one myself about two months ago, for reference. :)

Indentation

There's no other way of putting this, but your indentation is extremely poor. Nested code blocks inside { } braces should be indented accordingly, instead of left-aligning everything.

Naming style

Please use camelCase instead of PascalCase for your variable and method names. As you can tell from the syntax highlighting on your present code, PascalCase is the naming convention for type/class names. This makes reading your code slightly harder as other developers have to 're-parse' your variable/method names from what looks like class names.

try-with-resources

Since Java 7, try-with-resources is the recommended approach to read from an I/O source, such as wrapping a Scanner over System.in:

try (Scanner scanner = new Scanner(System.in)) {
    // ...
    String line = scanner.nextLine();
    // ...
}

This lets the JVM close I/O resources automatically for you.

Less reliance on 'global' static variables

You should slowly adopt better object-oriented (OO) programming practices, such as relying less on static variables. It's better to make more effective use of method arguments so that other developers can understand what the method requires. More on this will be illustrated below.

return early

Inside your Answer() method, you should return early so that you can do away with temporary variables. In my opinion, temporary variables are often a crutch to writing longer methods, which makes code harder to read and understand in the long run. A possible improvement can be:

private static float getAnswer(String action, int first, int second) {
    switch (action) {
    case "+":
        return (float) first + second;
    case "-":
        return (float) first - second;
    // ...
    }
}


Also, I sense that running time should be logarithmic but how to derive that exactly seems quite tough.

Nope, running time is \$ O(\sqrt N) \$ worst-case. Consider the case of a prime number (or particularly bad cases, like the product of twin primes). You have to check \$ \sqrt N \$ possible values to find the answer. No way around that.

Code-wise, you have a bug, only minor/trivial comments otherwise. First the bug. The issue is here:

return factor

What is factor at the end? It's just the first number whose square is larger than whatever num has become. It's not necessarily a factor of the original value. It's just an index. As an example, max_factor(8) == 3, max_factor(9) == 4, etc. You need to keep track of which of the attempted factors actually are factors. Something like:

def max_factor(num):
    """Find the maximum prime factor."""
    best = None
    factor = 2
    while factor * factor <= num:
        while num % factor == 0:
            best = factor
            num /= factor
        factor += 1
    if (num > 1):
        return num
    return best

As others have pointed out, you don't do input validation. I don't really consider that hugely important here and it's perfectly fine to just require that the user passes reasonable numbers in. But it couldn't hurt to just make that explicit:

def max_factor(num):
    """Find the maximum prime factor."""
    assert num >= 2
    ...

Otherwise, you have a counting loop with a non-trivial condition. This is one of those things that's always annoying to express in Python. In C or C++, that'd be:

for (factor=2; factor*factor <= num; ++factor) { ... }

and we have everything on one line. In Python, we have three options, none of which I'm thrilled about. Yours:

factor = 2
while factor * factor <= num:
    ...
    factor += 1

Using itertools.count:

for factor in itertools.count(start=2):
    if factor * factor > num: break
    ...

Using itertools.takewhile and count():

for factor in itertools.takewhile(lambda f: f*f <= num, itertools.count(start=2)):
    ...

Yeah, even if we put everything on one line, I'm not sure that helps any. Meh.

Lastly, factor-checking. The factors you are checking, in order, are:

2, 3, 4, 5, 6, 7, 8, ...

That is pretty inefficient. First, once you check 2, you don't need to check any of the even numbers. Similarly for 3 and multiples of 3. A more efficient check would be:

2, 3 then 5, 7, 11, 13, 17, 19, 23, ...

Basically alternating adding 2 and 4 from then on out. We end up with just odd numbers that aren't multiples of 3. So we only have to check 2 numbers out of every 6. We could write a generator for that:

def potential_factors(num):
    yield 2
    yield 3

    fact = 5
    incr = 2
    while fact * fact <= num:
        yield fact
        fact += incr
        incr ^= 6

Which we can use:

def max_factor_mine(num):
    assert num >= 2

    def potential_factors():
        yield 2
        yield 3

        fact = 5
        incr = 2
        while fact * fact <= num:
            yield fact
            fact += incr
            incr ^= 6

    best = None
    for factor in potential_factors():
        while num % factor == 0:
            best = factor
            num /= factor

    return num if num > 1 else best

That's about as good as you're going to get with this approach. If you want better performance, you'd have to get a different algorithm. In this answer, I show an approach with Pollard's rho, which would give a dramatic performance improvement just by doing something completely different:

+---------------------+----------+--------------------+---------+
|                     | OP       | OP w/fewer factors | Pollard |
+---------------------+----------+--------------------+---------+
| 600851475143        |  0.003s  |  0.002s            |  0.092s |
| 145721 * 145723     |  0.298s  |  0.174s            |  0.018s |
| 1117811 * 1117813   |  2.286s  |  1.331s            |  0.262s |
| 18363797 * 18363799 | 40.379s  | 21.895s            |  0.825s |
+---------------------+----------+--------------------+---------+


A better way is by using a template library like Handlebars.

The reason why it is better, stand on the fact you could separate logic code from your visualization code.

And more, using templates, you could have a closer look on what the page will became by looking to html code with the placeholders, that is easy to debug, instead of following complicated sequences of DOM calls.

I kindly suggest you to use such type of approach.

UPDATE

In response of your comment I'm extending my answer.

About your code, the issue I see is that you define an array of DOM element inside your function, but those elements are defined outside the scoop of your function.

This is like a nightmare to maintenance, and could easily be avoided.

Just pass the DOM object as the function arguments, and you have solved the issue:

var addEl = function(rootNode) {
    var nodes = Array.prototype.slice.call(arguments, 1);

    for(var i=0; i<nodes.length; i++) {
        rootNode.appendChild(nodes[i]);

        for(var j=0; j<3; j++){
            nodes[j].appendChild(nodes[j+1]);
        }
    }
};


Measuring the length of binary numbers in English words in C

Optimization, step by step
I ran your program and it took 38.5 seconds to run on my computer.  After playing around with it for a bit, I was able to speed it up by 100x.  I went through three different iterations to get to that point.
1) Use bit manipulation instead of division
The first thing I noticed was that your letter_sum() function was doing a loop across the bits, but it was using a division function to do so.  If you switched to using bitwise operators, it would speed up the program by a lot.  The remainder when dividing n by 2 is simply n & 1.  The quotient after dividing n by 2 is n >> 1.  In this rewrite, I have switched to using uint32_t to ensure that the shifting is an unsigned shift.
static uint32_t letter_sum(uint32_t n)
{
    uint32_t sum = 0;
    while (n != 0) {
        sum += (n & 1) ? 3 : 4;
        n >>= 1;
    }
    return sum;
}

Switching to this function made the program run in 4.4 seconds, or 8.75 times faster than the original.
2) Use more advanced bit tricks
The function can be improved even more by using more advanced bit counting techniques.  Essentially, you are adding 4 for each 0 bit and 3 for each 1 bit, up to the leading 1 bit in the number.  You can count these bits without using a loop by using two types of bit counting:

CLZ or Count Leading Zeros, gives you a count of how many zeros appear before the leading 1 bit.
POPCOUNT or Population Count, gives you a count of how many 1 bits are in the number.

Both CLZ and POPCOUNT can often be done in single machine instructions, so they are much more efficient than looping through each bit.
If you have the above two counts, then the letter sum of a 32-bit number will be:
128 - CLZ(n)*4 - POPCOUNT(n)

The reasoning for this equation is that you start with the maximum letter sum, which is 32 zeros = 128 letters.  From that, you subtract 4 for each leading zero, because leading zeros do not count towards the letter sum.  After that, you subtract 1 for each 1 bit, because "one" has one less letter than "zero".  So the function now looks like this (assuming a gcc compiler):
static inline uint32_t letter_sum(uint32_t n)
{
    return 128 - __builtin_clz(n)*4 - __builtin_popcount(n);
}

I added the inline keyword since this function is so simple it should be inlined.  After switching to this function, the program now ran in 2.4 seconds, or 16 times faster than the original.
3) Use memoization limited to a small range
The third step is to add memoization to the program.  But first, we should realize that the result of letter_sum() can only be in the range 3..127.  The lowest letter sum is for 1, with sum 3.  The highest letter sum is for 100000...0000, with sum 127.  So you can use a boolean array of size 128 to remember whether the sum ends in 18 or 13.
Here is the final rewrite using memoization.  I kept your main() function pretty much the same, although I wasn't sure why you were printing 100000000 < INT_MAX.
#include <stdlib.h>
#include <assert.h>
#include <stdio.h>
#include <limits.h>
#include <stdint.h>
#include <stdbool.h>

// Maximum index will be 127 because a 32-bit number with 1 "one" and 31
// "zero" has 127 letters.
#define CACHE_MAX        128

// cache[i] is true if letter_sum(i) eventually leads to 18.
static bool cache[CACHE_MAX];

// Maximum is 32 "zero" = 128
// Remove 4 letters for each leading zero, since leading zeros don't count
// Remove 1 letters for each "one", because "one" has 1 less letter than "zero"
static inline uint32_t letter_sum(uint32_t n)
{
    return 128 - __builtin_clz(n)*4 - __builtin_popcount(n);
}

static void fillCache(void)
{
    for (uint32_t i=1; i<CACHE_MAX; i++) {
        uint32_t n = i;
        while (n != 13 && n != 18) {
            n = letter_sum(n);
        }
        if (n == 18)
            cache[i] = true;
    }
}

static inline bool ends_in_18(uint32_t n)
{
    return cache[letter_sum(n)];
}

int main(void)
{
    assert(letter_sum(18) == 18);
    fillCache();

    uint32_t eighteen_count = 0;
    for (uint32_t i = 1; i < 100000000; i++) {
        if (ends_in_18(i)) {
            eighteen_count++;
        }
        else { printf("%d ", i); }
    }
    printf("%d %d\n", 100000000 < INT_MAX, eighteen_count);
}

This version ran in 0.39 seconds, or 100x faster than the original.


Get a "train or test" slicing elegantly after splitting

Inspired by Gulzar at Stackoverflow.
I do not realize it works until after a night's sleep.
So, below is enough:
offset_orig = range(m)
offset_train, offset_test = train_test_split(offset_orig, train_size=0.7, random_state=666)

x_train = x.iloc[offset_train]  # later
x_test = x.iloc[offset_test]

The offset_train list ([0, 2, ...]) has the same feature in later programming against the [True, False, True, False ...] list for slicing I pursued at the beginning.
For example, I can use offset_train later as below:

import matplotlib.pyplot as plt
import seaborn as sns
fig, axes = plt.subplots(nrows=2, ncols=2)
fig.set_size_inches(12, 10)
sns.boxplot(data=df.iloc[offset_train], y='count', ax=axes[0][0])
sns.boxplot(data=df.iloc[offset_train], y='count', x='season', ax=axes[0][1])
sns.boxplot(data=df.iloc[offset_train], y='count', x='hour', ax=axes[1][0])
sns.boxplot(data=df.iloc[offset_train], y='count', x='workingday', ax=axes[1][1])

And even as below:

# get noise index
mu_count = df.loc[offset_train, 'count'].mean(axis=0)
sigma_count = df.loc[offset_train, 'count'].std(axis=0)
idx_bad_np = abs(df.loc[offset_train, 'count'] - mu_count) > 3 * sigma_count
idx_good_np = np.invert(idx_bad_np)
# remove noise index
print(f'Before removal: sum(idx good) = {sum(idx_good_np)}, sum(idx bad) = {sum(idx_bad_np)}, len(df[train]) = {len(df.iloc[offset_train])}')
offset_train = list(set(offset_train) - set(np.array(offset_train)[idx_bad_np]))
print(f'After removal: sum(idx good) = {sum(idx_good_np)}, sum(idx bad) = {sum(idx_bad_np)}, len(df[train]) = {len(df.iloc[offset_train])}')
# check again
idx_bad_np = abs(df.loc[offset_train, 'count'] - mu_count) > 3 * sigma_count
print(f'Check sum(idx bad) after removal: {sum(idx_bad_np)}')

Thanks to the community. I feel I have so far to go on the path of Python.


Error-testing be with cucumber/capybaraweb

Personally, I only write feature tests for critical and very common cases, and outsource more specific tests (i.e is a certain field on a model validated) to model specs.

I wouldn't consider it a feature that a certain validation error is shown to the user, but I consider it a feature that the user gets informed that something went wrong and his post couldn't be created.

TL;DR Test the user experience in a feature spec and data integrity in model / controller specs.

Use different thinking hats for different tests:

Feature tests: Think in the user's perspective, how does the user interact with your app? what is the user expecting from your app?
Model tests: Think compulsive orderliness, how can you ensure data integrity?
Controller tests: Think paranoid, who can access what? does your controller comply with your API specs?

I would test in a feature spec for a "post":

That the user is able to create a post with an expected set of information (i.e the minimum that is required to create a post) and is redirected to the new post which shows the entered information.
That the user is unable to create a post by just hitting the "Submit" button without filling anything in and is shown a validation error. After correcting the user's mistake he should be able to create the post and be redirecred to the new post which shows the entered information.

This ensures the following "features":

Posts can be created
The user is informed about mistakes
Mistakes can be fixed
The user is assured that his post has been created.

I would test in a model spec:

Spec.describe Post, type: :model do
  subject { create(:post) }

  context 'validations' do
    it { should validate_presence_of(:title) }
    # for my taste, it's good enough to test that we validate the length
    it { should validate_length_of(:title) }

    it { should validate_presence_of(:body) }
    # if you want to be super-strict, you can check the length
    it { should validate_length_of(:body).is_at_least(100) }
  end
end


Some actions I go through are asynchronous, so I have to wait for them to completely finish before passing to a new one.

That's right. However, your interpretation of that statement is wrong.

The actions need to be serialized, but only on a per file basis.

const buffer = await file.buffer();
return await this.bufferIsImageFile(buffer);

Concurrency

You can further proceed with Promise.all as you did, but it has a few caveats.

Since any file that is not image means you should return false, not caring about the others. And so you should be able to cancel all the other file scans once you find the first file that is not image. Native promises are not cancelable but I am sure there are user land implementations.
You have no control about the number of files consumers will ask for. If they ask for a lot of files, you start a lot of async tasks and this may consume a lot of memory. You may want to only start a limited amount of tasks at a time and add more once some finishes.

The latter can be solved using Promise.race, but the actual implementation is not that trivial and so I will not write this for you :)

Maybe this SO question might be of help here:
https://stackoverflow.com/questions/42896456/get-which-promise-completed-in-promise-race

Or you may want to use some promises wrapper like bluebird which also provides cancellation mechanism (http://bluebirdjs.com)

Filtering

The filteredFiles list seems to me too hard coded (but it all seems like you pulled it together just for this review, in which case you may consider this irrelevant).

It should be injected to the method somehow, your options include:

method argument (thats awkward tho)
class property injected through constructor
decorator pattern (https://lmgtfy.com/?q=decorator+pattern+nodejs)


Sort Facebook Messenger contacts by idle time

Normally I would suggest pulling up the functions. Though they get hoisted by JS, it's nice from a readability point of view that you know they exist first before code tries to use them.

// if chat sidebar found in any mutations
if (mutations.some(function(mutation) {

This is a huuuge if statement. You might want to consider pulling out that callback function. It doesn't use mutations anyways.

if (mutation.type === 'childList') {
    // return if any added element is the chat sidebar
    return Array.from(mutation.addedNodes).some(initFbChatSidebar);
  // check if class is changed in existing element
  } else {
    // return if changed element is the chat sidebar
    return initFbChatSidebar(mutation.target);
  }

Can be simplified into:

var isChildListMutation = mutation.type === 'childList';
return isChildListMutation  ? Array.from(mutation.addedNodes).some(initFbChatSidebar);
                            : initFbChatSidebar(mutation.target);

// start locating sidebar
exists.observe(document.body, { attributes: true, childList: true, subtree: true, attributeFilter: ['class'] });

Consider breaking the config into newlines. As a single line, it isn't really readable.

  Array.from(li).sort(function(la, lb) {
    // get offline times
    var a = getOfflineTime(la);
    var b = getOfflineTime(lb);

    // compare offline times
    return a - b;
  }).forEach(function(li) {
    // add list items to list in sorted order
    ul.appendChild(li);
  });

Consider appending to a document fragment instead of directly to the DOM. Only after you append every li to the fragment, append the fragment to the ul.

var num, unit;

num = +str.slice(0, -1);
unit = str.slice(-1);

Consider putting var on the assignment operation instead. Save an extra line. Additionally, your num is not guarded against NaN.


You should not use XHTML 1.0 anymore ( which is where you would use CDATA ). What is even more important, you should no have javascript inside your html files. Instead you should include external files via <script src="/my/js/file.js"></script>.


Creating indexes from a theoretical decimal/int, splitting into before and after arrays

The code in the question seems to have adequate complexity for the getOutput function. Some small hints may be given, like:

do not use comma-expressions and decrement,
initialize middle with null and check for it with !== instead of typeof,
also, I do not like comparing undefined middle with 0.

The current code is more or less readable.

As for optimizations, one observation is that apart from [-1, 1] range, all other inputs always use the same alternative, making unnecessary to check left, right and middle individually. (eg, if right < 0, then so are middle and lift). How much optimization it really brings is hard to tell. If input numbers are almost always large, then making separate branch and constructing an array directly [left, middle, right] may make the code more efficient.

The near-zero case may need more conditions, of course (or just some kind of lookup for ready arrays - as the number of cases is small).

If you want a more compact code, maybe something like this can be done:

 tmparray = ((right == left) ? [left-1, left, left+1] : [left, left+1]);

And after that push/unshift in a loop for each tmparray element. Not sure this will be faster though, but at least may be more readable. (left may be renamed to "lower", and right calculated inline only in the condition).


Overcomplicating

To start with, if what we're looking for is just the smallest number to the left of the largest number, we can make that algorithm more explicit:

def max_difference(xs):
    max_idx, max_elem = max(enumerate(xs), key=operator.itemgetter(1))

    if max_idx == 0:
        return -1
    return max_elem - min(xs[:max_idx])

Rather than making 4 passes to find the max and min elements and their respective indices, we can find the max element and its index in a single pass (taking advantage of enumerate and the fact that max takes a key function), and then simply searching the left side of that for the min element.

We don't need a findNewIndex-type method since once we know where the max element is and its index, that defines the domain for where the min element could be. We don't have to pop elements to find it.

A single pass

We can do better though. For each element, we can keep track of the smallest element we've seen so far and the largest element we've seen so far, and simply update as appropriate: the min element can get smaller, the max diff can only get bigger we see the new biggest:

def max_difference(xs):
    min_elem = xs[0]
    max_elem = xs[0]
    max_diff = -1

    for elem in xs[1:]:
        min_elem = min(elem, min_elem)
        if elem > max_elem:
            max_diff = max(max_diff, elem - min_elem)
            max_elem = elem

    return max_diff

Naming

The python style guide prefers snake_case to camelCase.


There is a lot of unnecessary noise in the function.

If the base path is not expected to change it can be moved out into a constant.

System.IO.Path can be used to construct the path to the desired image.

To avoid null reference errors when consuming this function, try to avoid returning null. An empty array should be returned.

const string BASEIMAGEPATH = "..\\..\\Imagens";
const string DEFAULTIMAGENAME = "\\logotipos-prototipo_logo_prototipo_print.jpg";

public static byte[] GetBytesFromImage(string imageName) {
    byte[] bytes = null;
    imageName = imageName == "" ? DEFAULTIMAGENAME : imageName;
    string path = Path.Combine(BASEIMAGEPATH, imageName);
    if (File.Exists(path) && !imageName.EndsWith("png", StringComparison.InvariantCultureIgnoreCase)) {
        using (MemoryStream image = new MemoryStream())
        using (System.Drawing.Image bmpImageToConvert = System.Drawing.Image.FromFile(path))
        using (System.Drawing.Image bmpNewImage = new Bitmap(bmpImageToConvert.Width, bmpImageToConvert.Height))
        using (Graphics gfxNewImage = Graphics.FromImage(bmpNewImage)) {
            gfxNewImage.DrawImage(bmpImageToConvert, new Rectangle(0, 0, bmpNewImage.Width, bmpNewImage.Height), 0, 0, bmpImageToConvert.Width, bmpImageToConvert.Height, GraphicsUnit.Pixel);
            bmpNewImage.Save(image, ImageFormat.Png);
            bytes = image.ToArray();
        }
    }
    return bytes ?? Array.Empty<byte>();
}

The logic should also be reviewed as it was stated

  The objective of the code is to receive an image path, check if the image exists or if it isn't png, if the image isn't png convert it to png and return a byte[] of the stream.

but currently, if the image exists and is already a PNG, it wont return the contents of the file.

const string BASEIMAGEPATH = "..\\..\\Imagens";
const string DEFAULTIMAGENAME = "\\logotipos-prototipo_logo_prototipo_print.jpg";

public static byte[] GetBytesFromImage(string imageName) {
    byte[] bytes = null;
    imageName = imageName == "" ? DEFAULTIMAGENAME : imageName;
    string path = Path.Combine(BASEIMAGEPATH, imageName);
    if (File.Exists(path)) {
        using (MemoryStream image = new MemoryStream()) {
            if (imageName.EndsWith("png", StringComparison.InvariantCultureIgnoreCase)) {
                using (FileStream imageStream = new FileStream(path, FileMode.Open, FileAccess.Read)) {
                    imageStream.CopyTo(image);
                    bytes = image.ToArray();
                }
            } else {
                using (System.Drawing.Image bmpImageToConvert = System.Drawing.Image.FromFile(path))
                using (System.Drawing.Image bmpNewImage = new Bitmap(bmpImageToConvert.Width, bmpImageToConvert.Height))
                using (Graphics gfxNewImage = Graphics.FromImage(bmpNewImage)) {
                    gfxNewImage.DrawImage(bmpImageToConvert, new Rectangle(0, 0, bmpNewImage.Width, bmpNewImage.Height), 0, 0, bmpImageToConvert.Width, bmpImageToConvert.Height, GraphicsUnit.Pixel);
                    bmpNewImage.Save(image, ImageFormat.Png);
                    bytes = image.ToArray();
                }
            }
        }
    }
    return bytes ?? Array.Empty<byte>();
}


Since you are using f strings I will presume you are using python 3.6 or later.

def __init__(self, value):
    self._cents = int(round(value * 100, 0))

Good job prefixing cents with an underscore. I think it is a good choice to make the variable "private".

By leaving out the second parameter from round, or by passing None, the returned value will be rounded to an integer. You could instead write

def __init__(self, value):
    self._cents = round(value * 100)

Note that round may not behave as you expect. Depending on the value given it may round the opposite way to what you'd expect.

def __add__(self, other):
    return self.__class__((self._cents + other._cents) / 100)

It is good that you've accounted for subclassing. Since you've had to do some work to make a new class to return, maybe that indicates you could use an alternative constructor that takes cents directly?

Unfortunately you don't do any type checking here, so the error message a user receives is not very helpful.

a = Currency(2.345)
print(a)  # 2.35
a + 2.03  # AttributeError: 'float' object has no attribute '_cents'

I would suggest type checking as you've done in mul, making sure only currency can be added to currency

def __add__(self, other):
    if not isinstance(other, Currency):
        raise TypeError(f"Unsupported type, cannot add {self.__class__} and {other.__class__}")
    ...

def __mul__(self, factor):
    if type(factor) not in [int, float]:
        raise ValueError('factor must be a scalar')

From this stackoverflow answer, isinstance is preferred over type. Also from the same answer, duck typing is preferred over isinstance. Since we don't want to have to update this list every time we find a new type that is acceptable to multiply by, lets just try it and see if it works. Multiplying a currency will then just work unless there is a reason it shouldn't. One exception might be multiplying currency by currency. Should that be allowed?

def __mul__(self, factor):
    try:
        return self.__class__(factor * self._cents / 100)
    except TypeError:
        raise TypeError(f"...")

This isn't perfect, as we may lose useful information about what doesn't work (Did the constructor fail? Did the multiplication fail? etc) but it is good enough.

# Example of currency just working with fractions
a = Currency(8.08)
a * fraction.Fraction(1, 4)   # 2.02

def __truediv__(self, divisor):
    if type(divisor) not in [int, float]:
        raise ValueError('divisor must be a scalar')

    if divisor != 0:
        return self.__class__(self._cents / divisor / 100)
    else:
        raise ValueError('Cannot divide by zero')

Since an exception is raised if you try to divide a currency by 0, I don't see an advantage of making it a ValueError over a ZeroDivisionError. It wold actually remove the error handling since it doesn't add much, and let the user catch it if they want to.

If there is a usecase for it, it may be worth defining div too. It could be as simple as

__div__ = __truediv__

def __repr__(self):
    return str(f'{self._cents/100:,.2f}')

def __str__(self):
    return str(f'{self._cents/100:,.2f}')

This is repeated code. You can deduplicate with either

def __repr__(self):
    return str(self)

or

__repr__ = __str__

@property
def dollars(self):
    return self._cents / 100

@property
def cents(self):
    return self._cents

These do not make sense to me. I would expect dollars to return the number of dollars present, and cents to just return the number of cents

c = Currency(7.89)
c.dollars  # 7.89, I expected 7
c.cents  # 789, I expected 89

You also could implement dollars.setter and cents.setter so you can change just one of them.

Here I will list things I would do differently. That doesn't mean I'm right and you are wrong, it means food for thought, but reject ideas away if they don't suit.

Use decimal. It was made for arithmetic on numbers where precision matters. It also has the work on dealing with other python numbers pretty much done for you. It also lets you choose how to round which may be important if you need banker's rounding.
Add a constructor for making an amount from dollars and cents. Something like Currency(dollars=7, cents=89) would be nice to use. The implementation is a little trickier (what if cents >= 100, what if dollars is negative).
Due to the rounding, working with this class may look unfair to whoever owns the money. Currency(5.05) / 2 * 2 == Currency(5.04) != Currency(5.05). By doing what should be nothing, they've lost a cent! To exaggerate the problem, lets say you have Currency(74.49) to divide amongst 12 people. Currency(74.49) / 12 outputs Currency(6.21). You give each person that amount Currency(6.21) * 12 == Currency(74.52), and you find you are 3 cent out of pocket. This is when auditors come in and start asking questions.


There are a few small fixes, but just to point out, pandas.read_excel is notoriously slow..

  Code.py

import pandas as pd
import os

# # NOTE: If there are multiple worksheets this will take awhile,
# # and you can speed it up by specifying just the worksheet you want
# create data frame of fiscal calendar
fiscal_calendar = pd.read_excel('Purchasing/Fiscal Calendar 15-18.xlsx')
fiscal_calendar = fiscal_calendar[["Date", "Week.1"]].rename(columns={"Date": "InvoiceDate", "Week": "Week.1"})

# set working directories for files
starting_folder = ('Purchasing/unformatted/')
save_folder = ('Purchasing/')

#create list of excel file names from folder
for file in os.listdir(starting_folder):
    input_name = starting_folder + os.fsdecode(file)
    df = pd.read_excel(input_name)

    # cleanup
    df["InvoiceDate"] = pd.to_datetime(df["InvoiceDate"])
    df = df.drop(df[df.Brand == 'NONPROD'].index)

    # create the output file and save it to .csv
    output_name = save_folder + filename + ".csv"
    Sygma = pd.merge(df, fiscal_calendar, how='left', on='InvoiceDate')
    Sygma.to_csv(output_name, index=False)


Finding the first non-repeated character in a string

Don't reinvent the wheel, there is a function (ie. singleton) in the package MoreUtils that does the job:

#!/usr/bin/perl
use Modern::Perl;
use List::MoreUtils qw(singleton);

while (<DATA>) {
    chomp;    # don't forget it, it removes the linebreak.
    # split explodes the string in character
    # singleton keeps characters that appear only once
    # ($first) contains the first character that appears only once.
    my ($first) = singleton split//, $_;
    say $first;
}

__DATA__
yellow
tooth

Output:

y
h


Calculating a payment price based on a selected phone

You have seemed to start off ok, by acknowledging that you need an object to hold the price and name of the phone.
You can separate this object's class, as others have mentioned, into another file and give it a more meaningful name (e.g. MobilePhone, Device, etc.)
Going back to the original problem, you need to calculate the final purchase price of a phone. According to your requirements, this final value might be influenced by the type of payment the user chooses (e.g Cash or Credit)
There's several ways you can approach your problem afterwards.
Example:
You can have an object that is responsible for calculating the final price of a phone based on the payment options. This might require you to create objects for the payment options as well.
(code below is missing some parts, like constructors, etc)
class PhonePriceCalculator {
    public double calcPrice(Phone phone, Cash cash) { }
    public double calcPrice(Phone phone, Credit cash) { }
}

// some classes to hold data
class Phone { }
class Cash { }
class Credit { }

Then, you just need to plug those objects in with your main() method. There are some improvements that you can make to code above.
For example, Cash is really just a type placeholder for calculating price. It never changes. Maybe you can model the Cash and Credit options through an enumeration. This will work just fine if there's only two credit types.
Another example:
Maybe modelling the price calculation through an object isn't so bad. But you didn't like the fact that we also have objects for the payment types (or an Enum if you decided to go for that instead). Well, you can simplify the price calculator by giving it a discount value instead:
class PhonePriceCalculator {
    public double calcPrice(Phone phone, double discount) { }
}

This way, this class doesn't need to know the payment type. Just the discount.
Final Considerations
I didn't touch private/public encapsulation because other users like Pimgd and Ethan Bierlien had already mentioned it. But again, it is wise to make fields inside an object private, and expose them through methods. Remember, the object is the owner of those fields. So it must choose to expose them (or not) consciously.
I know this is a toy example, but when you are dealing with money calculations in java, you're better of using the BigDecimal class instead of double. You can play with decimal scales, amongst other things.


I would agree with your concern over naming, as this doesn't really seems to be a "service", but rather a simple data store.  So perhaps FavoritesStore or similar is more appropriate.  You have some portions of code that are omitted that might suggest a more permanent data store being used in conjunction with this class, so perhaps this is really acting as a cache (FavoritesCache)?

So favorites seem to be their own entity in this context that should probably have their own class representation(s).

You might consider a structure like:

abstract class Favorite {
    const TYPE;
    public $id;
    public $ownerId;

    public function __construct($id, $ownerId)
    {
        $this->validatePositiveInteger($id);
        $this->validatePositiveInteger($ownerId);
        $this->id = $id;
        $this->ownerId = $ownerId;
    }

    protected function validatePositiveInteger($int)
    {
        if(!is_int($int) || $int < 1) {
            throw new InvalidArgumentException(
                'Positive integer value expected'
            );
        }
    }
}

class FavoriteHotel extends Favorite {
    const TYPE = 'Hotel';
}

class FavoriteRegion extends Favorite {
    const TYPE = 'Region';
}

// etc.

This also allows you to potentially tie different properties/behaviors to different types of favorites.  If you don't have the need to customize different favorites and are concerned over creating/managing a large number of favorite classes, you could alternately have just a single concrete class where upon instantiation you check against a valid list of favorite types.  In this case, perhaps consider extending this class from SplEnum so you can get enum-like behavior for your favorite types.

This would take the responsibility out of the store to enforce favorite types.

Also if you are treating this as a cache and your Favorite as a model for some record somewhere in a permanent store (i.e. database).  Then perhaps this class has methods on it to persist to permanent store or instantiate favorites objects from the permanent store.

So perhaps something like:

public function persist()
{
    // your logic to save to permanent store
}

public static function getInstanceById($id)
{
    // access your permanent store to get single record
    $class = 'Favorite' . $row->type;
    return new $class($row->id, $row->ownerId);
}

public static function getInstancesForOwnerId($ownerId)
{
    $instances = [];
    // access your permanent store to get records
    // for each record
    $class = 'Favorite' . $row->type;
    $instances[] = new $class($row->id, $row->ownerId);
    return $instances;
}

public static function deleteByOwnerId($ownerId)
{
    // your logic to delete records in permanent store
}

The last three methods might also be considered for separation into their own factory class (i.e. FavoriteFactory) if you need to expand on the means by which you instantiate favorite objects.

The takeaway here is that favorite class(es) represent the data model and own the means for how they are persisted into and recovered from the database.  Your simple in memory store/cache does not and should not need to know or understand this.

Your cache would just need to utilize this methods like:

// in save function after setting to cache
$favorite->persist();

// in get function when you have cache miss
$favorites = Favorite::getInstancesForOwnerId($ownerId);

// in reset function
$result = Favorite::deleteByOwnerId($ownerId);

I don't understand why you have buildResult method or have it exposed statically.  Do you really need a method to build an array? Why is this not just done in save().

i.e.

$this->data[$ownerId][] = ['id': $id, 'type': $type];

Also, if you consider making favorite into class as suggested, you can clean up your save() code like this:

public function save(Favorite $favorite) {
    $this->data[$favorite->ownerId][] = $favorite;
    // other stuff
    return true;
}

I don't see what API key is doing in this class, as it is not used anywhere.  I would also question why an API key would migrate as far down the call stack as where this store probably lives.  Typically, if you are exposing a service for GET'ing, POST'ing, PUT'ing, DELETE'ing data in a store, end user authentication via a mechanism such as an API key would be one of the first things done in validating the request. If the key is not valid, you should fail out the request with a 403 or whatever long before you get to the point of trying to set up data storage objects such as this.

Consider changing save method name to set and reset to unset as I think you would find these more typical method names for setting and unsetting items in a store or cache.

You are inconsistent in your approach to validating data passed to your public methods.  You do throw InvalidArgumentException in one case, but you do nothing to validate integer values for example.  Of course, that can get taken care of quite easily in some cases if you take the suggestion to put favorite into class that can be type-hinted against.

  /**
   * string
   */
  private $key;

Glad to see you using doc blocks, however these property blocks should probably be changed to @var string, @var array, etc.

You code indentation is inconsistent.

Consider making your code compliant against PSR-1 and PSR-2 since you are so close to being compliant already (i.e. you have generally good coding style).

That would require you to:

add namespace declaration
move opening brace for methods/functions onto line after declaration

The professional PHP development community has really gravitated towards the PSR standards in recent years, so probably best to be aware of them (and hopefully adopt them).  If you can configure you IDE and/or use style checker tools to enforce the coding standards, that is always a good approach.


The first thing I would consider is saving the resulting $markersArray into a transient - I'm not sure how much data you can fit into one transient, so you might need to split the data into couple of transients. More about transients, https://developer.wordpress.org/apis/handbook/transients/
The basic idea is to run the costly query once, store the result into one value in the wp_options table, and then use that value on any subsequent requests, skipping the query alltogether.
But with transient you need to decide when and how you invalidate and rebuild them. Do you give them some lifespan (minutes, hours, days...) and let them recreate themselves whenever the lifespan expires. Or is the transient only recreated whenever you add a new post or update an existing post. Or you could conjure a scheduled action (WP Cron, replaced with system cron for accuracy) that runs the query and updates the transient at a low-traffic time of the day, which can cause delay between post publish/update and the data actually showing on the public view.
Another option could be to skip the query on the initial php load and move it to an ajax request. So you would first show an empty map, which would get populated by a looping ajax request either to admin-ajax.php or to a custom WP REST Api endpoint. For example first fetch the first 1000 posts, when they're ready the next 1000 (i.e. page 2), and so on until all of the posts / map pins have been fetched. The pin json could then be stored to the user's browser, if needed.
I'm not a database expert, but I'd imagine you could also add a custom db table to serve as a aggregate table, which would contain appropriate columns for the pins. This table would get populated / updated whenever you would add or update a post - i.e. the save_post_{$post->post_type} action, https://developer.wordpress.org/reference/hooks/save_post_post-post_type/. Then you'd query this aggregate table with $wpdb and get all of the pin data on one go.
Or instead of an aggragate table or a transient, turn the query result into json and store it into a file, which you could then include and have the map script consume.
And if not all map pins are needed at the very beginning, get only the pins for the currently visible area with an ajax request when you know the map boundaries. Then fetch more pins when the boundaries change.


A few  comments:

You should move this code from the AddButton_Click to some business logic class.
Maybe you should add some validation of LevelTextBox.Text, before using its value.
You should split this code into different methods.

  You have three responsibilities here:

  Find out  if the level exists at all.
  Find out if the specified level already exists.
  Create a new level.

  The fact that all three actions use the same data doesn't mean you need to put them in one method.

Maybe I get it wrong somewhere, but I can't understand how this code checks if the level already exists. I can't see how the code will ever get into the MessageBox.Show("Level already exists"); in both places.


Finding integer with the most natural dividers

It's not clear what you mean with "natural dividers". Better specify that, show examples, and document allnums to say what it does. A better name would also help. For example allnums(6) returns only 1, despite having divisors 1, 2, 3 and 6. At the very least I'd expect answer 2 (for the divisors 2 and 3). So probably it's incorrect, but we can't even tell because you didn't say what exactly it's supposed to do.
Anyway, as usual it suffices to try divisors until the square root, not until half. If you find a divisor i of k, then don't just count that divisor but also count its partner divisor k/i. Unless that's the same.
And max can take an iterable and key.
So:
from math import isqrt

def divisors(k):
    """Compute the number of k's divisors 1 < divisor < k."""
    cnt = 0
    for i in range(2, isqrt(k) + 1):
        if k % i == 0:
            cnt += 2 - (k // i == i)
    return cnt

print(max(range(568023, 569231), key=divisors))

Takes about 0.1 seconds.


How can I make my solution to "Climbing the Leaderboard" not time out?

\$\DeclareMathOperator{\Oh}{O}\$Frankly, the code in the HackerRank editor is a large mess.  It is promoting crap like #include <bits/stdc++.h> and using namespace std;, and the code contains many problems: int for traversing a std::vector, copying containers around, etc.  Not to mention bad practices like i++, explicitly calling fout.close(), not using standard algorithm, etc.  It really shouldn't be like that.

In your code, the whole getPosition function is redundant.  Use std::lower_bound instead.  And since std::vector::erase is \$\Oh(n)\$, the code is \$\Oh(n^2)\$, which is unnecessary inefficient.

The problem is very simple and can be finished in several lines without sacrificing readability:

using score_t = long;
using rank_t = long;

// scores is passed by value to take advantage of possible optimization.
rank_t get_rank(std::vector<score_t> scores, score_t alice_score)
{
    scores.erase(std::unique(scores.begin(), scores.end()), scores.end());

    auto it = std::lower_bound(scores.begin(), scores.end(), alice_score, std::greater<>{});
    return it - scores.begin() + 1;
}

The code is \$\Oh(n)\$ and I don't think you can do better.  Note that each score of Alice's is independent, so it makes no sense to process them together in a function.  I used long because the problem seems to require numbers as large as \$10^9\$.  scores will be modified in the function, so instead of making a copy manually, we let the compiler do so for us in the parameter list.  This enables possible optimization opportunities.

Here, we used two standard algorithms:

std::unique, which "removes" adjacent equal elements.  Standard algorithms cannot change the size of scores via iterators, so std::unique makes sure that the first \$N\$ elements are the result, where \$N\$ is the number of elements in the result.  The rest of the elements are placed in a valid but otherwise unspecified state.  Then, we call erase to erase these garbage elements.  This is also known as the remove-erase idiom.
std::lower_bound, which performs a binary search and returns the first element that compares not "less" than the provided value.  By default, "less" is defined by <, thus operating on an ascending sequence.  In this case, we use std::greater<> to define "less" by >, so that std::lower_bound is adapted to work on a descending sequence.

Now let's go through your code: (regardless of whether the code is what hackerrank gives you)

int getPosition(vector<int> scores,int value,int start,int end){

As I said, this function is provided by the standard library and you shouldn't be reinventing your own without a good reason.  If you want to see how the standard version looks like, there is an example implementation at cppreference.

vector<int> climbingLeaderboard(vector<int> scores, vector<int> alice) {

As I said before, this function is illogical.  It should handle one Alice-score at a time.  Also, climbingLeaderboard isn't really a good function name.

Passing scores by value is justified because you modify it in the functions, but passing alice by value introduces an unnecessary copy.  It should be passed by const reference instead.  And int is not only a magic type, but also a type that is not sufficiently large here.

int i=1;
while(1){
    if(i == scores.size()){
        break;
    }
    if(scores[i]==scores[i-1]){
        scores.erase(scores.begin() + i);
        continue;
    }
    i++;
}

Space.  Also, I am not sure what you are trying to achieve with the while (1) loop here: you are obfuscating the code by refusing to place the condition in the proper place.  And int isn't the correct type to use here.  Use size_type (or at least a named type).  ++i should be used instead of i++ in a discarded value expression.

And you are essentially reimplementing the \$\Oh(n)\$ std::unique in a \$\Oh(n^2)\$ way.  Just use the standard algorithm and avoid reinventing the wheel.

vector<int> results;
int endValue = scores.size()-1;

In this particular case, we know that !scores.empty(), but storing the result in a int is still inadvisable.  Use auto or std::vector<int>::size_type.

for(int j=0;j<alice.size();j++){
    endValue = getPosition(scores,alice[j],0,endValue);
    results.insert(results.end(),endValue+1);
}

Again, space.  And ++j not j++.  And the third line is just

results.push_back(endValue + 1);

There is no reason to use insert + iterator here.

    return results;

    }

Is the weird indentation a copy-paste problem?


The folks using it claim "it is going down evey now and again".

Well, yes. If they don't send you a message in 20 seconds, you're disconnecting them! Why? Simply removing that block is probably what you want.

Separation of Concerns

Your server_socket function both creates a server and handles the busy loop. You should split those into two functions:

def make_server(ip, port):
    ...

def run_echo(server):
    ...

server = make_server(ip, port)
run_echo(server)

Echoing

You wrap your echo block in a try/except. Why? What part of that code can raise an exception? What exception is raised? You should prefer to only catch precisely the exceptions that are raised (to avoid, for instance, catching SyntaxError and hiding whatever typo you wrote in your code), but in this case I don't know what exception you would be catching.

Also, you don't need to print str(len(data)), you can just print len(data).


BTC price checker with web scraping, regex and Tkinter GUI

Group your imports

You should put all of your imports at the top of the file instead of sprinkling them throughout the code. This is both a best practice and a recommendation from PEP8

Don't use wildcard imports

You import Tkinter with from tkinter import *, but the PEP8 guidelines (and best practices) say you should not. For me, the best way to import Tkinter is with import tkinter as tk because it avoids polluting the global namespace and gives you a short prefix for using Tkinter classes and objects.  The prefix helps to make your code a bit more self-documenting.

Use more functions and classes

Most of your code exists in the global namespace. While that's fine for short programs, it becomes difficult to manage as the program grows. Its good to get in the habit of always using functions or classes for nearly all of your code.

For example, I recommend putting all of the GUI code into a class. That will make the code a bit easier to read, and gives you the flexibility of moving the code to a separate file.

clearBox should restore the state of the text widget

Your clearBox function resets the state of the text widget to be editable so that it can clear the contents, but it doesn't set it back. That means that after clicking the clear button, the user could be able to type anything they want in that window. It's somewhat harmless, but is a bit sloppy.


First things first, you are not using the right tool for the job. Regular expressions can't reliably parse markup. Full stop. But more on that later, let's first take a look at your patterns, as this may help you in the future:

$bodypattern = ".*";//for example
//later
preg_replace('/'.$bodypattern.'/',...);

Now, whenever you quote a string into a regex pattern, you have to stop and think: what if the string happens to contain special chars?. I mean: half of the characters can mean a variety of things depending on the context. Thankfully, there's a handy function called preg_quote to help you with that. The safe option here would be:

preg_replace('/'.preg_quote($bodypattern, '/').'/', ...);

Now, you seem to be under the impression that a dot matches anything. And it does, except line-feeds. If I were to apply preg_match('/.+/', $thisAnswer, $matches); to this answer, the pattern would only match the text until a \n is encountered.
You need a PCRE modifier: s:

preg_match('/.+/s', $thisAnswer, $match);

That's more like it. However, this is just as an aside, because what you really should be looking into are ways to actually treat markup as markup: a DOM needs to be parsed, if you want to get the most out of it. Here's my actual (and as-originally-posted) answer:

Ouch, The first thing I noticed is that you're processing markup, and have chosen to do so using regular expressions. That's a bad idea from the off.
It's especially bad considering you have a tool at your disposal that is specifically built to parse markup, the DOMDocument object.

Basically, what you're trying to do can be written as simple as this:

$DOM = new DOMDocument;//create DOM
$DOM->loadHTMLFile($theUrl);//parse HTML
$container = $DOM->getElementById('container');//get the id=container element
if ($container)
{//make sure it was found
    $container->removeAttribute('id');//remove the id attribute
}
$body = $contents = $dom->getElementsByTagName('body')->item(0);//get body
//remove attributes
while ($body->hasAttributes())
{//while tag has attributes
    //remove attribute
    $body->removeAttribute($body->attributes->item(0)->nodeName);
}
//Get the body as string, use saveXML
//as saveHTML adds DOCTYPE, html, head and title tag again
$body = substr($DOM->saveXML($body), 6, -7);

The reason why you need to remove the attributes for the body tag, is because DOMDocument::saveXML will return the markup string with the outer <body> and </body> tags. Their length is a given, but only if the opening <body> tag hasn't got any attributes. Thankfully, we can remove them quite easily, and slice them off with substr.

If you want, you could easily use:

$body = str_replace(
    array('<body>', '</body>'),
    '',
    $DOM->saveXML($body)
);

Perhaps this is a bit easier to understand. Anyway: read the DOMDocument docs, there's a lot you can do there. Just click on the methods I used, and click the objects they return, too, so you know what instances you're working with.

Now wrap all this in a function, and we can add some checks to avoid exceptions or errors from showing up:

function getBody($file, DOMDocument $dom = null)
{//optionally pass existing DOMDocument instance
 //to avoid creating a new one each time we call this function
    if (!file_exists($file))
    {//check if the file exists, if not:
        throw new InvalidArgumentException($file. ' does not exist!');
    }
    $dom = $dom instanceof DOMDocument ? $dom : new DOMDocument;
    $dom->loadHTMLFile($file);
    $container = $dom->getElementById('container');
    if ($container)
        $container->removeAttribute('id');
    $body = $dom->getElementsByTagName('body')->item(0);
    while($body->hasAttributes())
    {
        $body->removeAttribute(
            $body->attributes->item(0)->nodeName
        );
    }
    $body = substr($dom->saveXML($body), 6, -7);
    //optionally remove trailing whitespace:
    return trim($body);
}

Call this function either like this:

$body1 = getBody('path/to/file1.html');

or, since you're going to process more than 1 file, and we can avoid creating a new DOMDocument instance for every function call:

$dom = new DOMDocument;
$files = array('file1.html', 'file2.html');
$bodyStrings = array();
foreach ($files as $file)
    $bodyStrings[] = getBody($file, $dom);

That's it.

As a guide-to-the-DOMDocument-docs:

Every class extends from the DOMNode class, except for the DOMNodeList class, which is a Traversable containing only DOMNode instances, accessible through the item method. Anyway, check the DOMNode methods and properties. Learn that class by heart, as almost every object you work with inherits its properties and methods.
The other class to check is the DOMElement class. Instances of this class represent a node (a tag if you will). It has some methods that I've used here (like removeAttribute).
Finally, everything, of course starts with the DOMDocument class. I had already linked to the docs, but an extra link never hurt anyone.


Remove duplicates in list of lists where an item in each list is ignored when determining duplicity

Maybe there's an elegant way for me to instead not delete the e column and still remove duplicates efficiently without considering the e values?

Your set-based approach almost works, and should be more efficient than the nested loops. Try storing only the first 4 items in the set, as opposed to the entire row:

def unique_by_first_n(n, coll):
    seen = set()
    for item in coll:
        compare = tuple(item[:n])    # Keep only the first `n` elements in the set
        if compare not in seen:
            seen.add(compare)
            yield item

filtered_list = list(unique_by_first_n(4, lines_list_copy))

Here I've adapted your set approach to consider only the first n items when comparing rows, but then to yield the entire row if it hasn't been seen yet. yield could also be replaced by first defining an empty output list and appending to it, like in your second solution.

We get around the del e problem by not deleting anything, just returning all of the rows where the first n columns have not been seen yet.


I assume your implementation is correct then I go directly to (2).

Even assuming you do not worry too much about speed you should replace foreach with for in PrazenWindowDensity(), relative speed-up is consistent:

for (int m=0; m < xn.Length; ++m)
{
    for (int n=0; n < xn[m].Length; ++n)
    {
        gauss += GausianFunction(xn[m][n], x, sigma2);
    }
}

Few notes: I renamed Xn to xn because parameters should be camelCase. I dropped local variable xx because it's not helpful here.

In the same function you're using LINQ to sum lengths of each array, it's deadly slow and it may be replaced with a simple ad-hoc function:

static int SumLengths(double[][] data)
{
    int sum = 0;
    for (int i=0; i < data.Length; ++i)
        sum += data[i].Length;
}

To have this:

return gauss / SumLengths(xn);

Optimization of GausianFunction() might be more controversial. I observed that often (but not always) C# compiler + JIT compiler will emit better assembly code when an expression is computed without intermediate results (if possible all values stay in 80 bit FPU registers, better accuracy and no need to convert back/to 64 bit double values). I can't say if this is your case but you may give it a chance (let me also use using static Math; I think it helps clarity when I have to work with math formulas):

private static double GausianFunction(double x1, double x, double sigma2)
    => (1 / Sqrt(2 * PI * sigma2)) * Exp(Pow(x1 - x, 2) / (-2 * sigma2));

Variance() function is actually unused then I'll go quickly through it but: if possible use double[] instead of IEnumerable<double> and avoid LINQ when you can write an ad-hoc function, relative speed-up is huge in this case (of course unless actual performance are enough for your task.)


A linked list with push pop append and destructor

I see a few things that could improve your code quite a bit:

Linked List Structure Design

You declare your Node structure outside of your LinkedList class in the header file; this means that anyone including the header file can access your Node structure and use it in their code. This is bad OO design since the Node structure is really an implementation detail of the LinkedList. Therefore, the user should be able to use the linked list without ever having to use (or the ability to use) the Node data structure. To fix this issue, I would declare the Node data structure inside of the private space of your LinkedList class. Moreover, I would provide a constructor for the Node data structure that will populate the values of the structure for me:

class LinkedList {
    struct Node {
        int data;
        Node *next;
        Node(int d, Node *n = nullptr) :
            data(d), next(n) {}
    }* head;
};

In this way, I can use the more C++ way of initializing objects on the heap over the more C way of doing things.

C++ way (recommended):

Node *node = new Node(data);

More C-style way (deprecated / not recommended):

Node *node = new Node();
node->data = data;
node->next = nullptr;

Use C++ Style

Speaking of C and C++ ways, do NOT mix new and malloc in C++ code - especially when you're using delete to free objects allocated by both. new and delete call constructors/destructors whereas malloc and free don't. Use delete to free new'd objects and free to free malloc'd objects. This code is invalid and can cause issues:

void LinkedList::push(int data)
{
    Node* cur = (Node*)malloc(sizeof(Node));
    cur->value=data;
    cur->next=head;
    count++;
    head=cur;
}

Why not use the more C++ way of doing things:

void LinkedList::push(int data)
{
    Node* cur = new Node(data, head); // use constructor I defined for you above.
    ++count;
    head = cur;
}

In addition, keep your style consistent between NULL and nullptr. In C++11 and beyond, always prefer to use nullptr.

Linked List API

I would rename some of your functions to be a bit more descriptive. For example, I would make wraparound() be called make_cyclic(). I would also provide a counter function called make_acyclic() to break the cycle. To check for a cycle, I would rename your function to is_cyclic(). I would also add another function is_empty() to signal when the list is empty and a size() function to return the size of your list so far; otherwise, what's the point of the count member variable? There is also the added benefit that the empty() member function could clear up a bunch of bugs in your code:

Bugs

In your wraparound() and isCyclicDwrap() functions, you don't check for the case when the list is empty (when the head is pointing to nullptr). If the list is empty, derefrencing nullptr like this:

Node* cur = head;
while(cur->next)

is a segmentation fault error waiting to happen. The fix is to check if the list is empty before doing such operations.

Finally, I find your implementation of making the list acyclic (if it is cyclic) a little too heavy and breaks the principle that functions should only have one purpose. We could do something like this to split the two ideas of checking for a cycle and breaking one:

bool LinkedList::is_cyclic() const
{
    if (is_empty()) {
        return false;
    }

    Node *n1 = head, *n2 = head->next;
    while (n1 != n2) {
        if (!n2 || !n2->next) {
            return false;
        }
        n1 = n1->next;
        n2 = n2->next->next;
    }
    return true;
}

void LinkedList::make_acyclic()
{
    if (is_empty() || !is_cyclic()) {
        return;
    }

    Node *curr = head;
    while (curr->next != head) { // move to the end
        curr = curr->next;
    }
    curr->next = nullptr; // break the cycle
}

Simplify Some Functions

With the newly defined Node constructor, append() can become:

void LinkedList::append(int data){

    if(!head){
        head = new Node(data);
    }
    else{
        Node *cur = head;
        while (cur->next){
            cur = cur->next;
        }
        cur->next = new Node(data);
    }
    ++count;
}


Should I set a license in the constructor or a static constructor?

This is what made me want to answer:

// Set the license... (Moderately expensive - accesses the file system)

You do not want any constructor, static or not, to do work like this. A constructor that accesses the file system is screaming "I CAN BLOW UP ANYTIME!", and can throw file system -related exceptions. You don't want that in a constructor, even less a static one:

  A static constructor is used to initialize any static data, or to perform a particular action that needs to be performed once only. It is called automatically before the first instance is created or any static members are referenced.

  http://msdn.microsoft.com/en-us/library/k9x6w0hc.aspx

Making your field static ensures that the type (not the instance) has the field value, and your logic ensures it's set only once in the lifetime of your application. If your application isn't starting any process (requirement is once per process, right?), it can probably work.

I don't like that you're assuming that your application will only ever call this method in the same process though - it's a fair assumption to make, but since the requirements say once per process, I think your code should account for System.Diagnostics.Process.GetCurrentProcess().Id, and call the SetAsposePdfLicense() method if the method wasn't called in the process your code is running in.

I'm thinking of a factory living in a Singleton scope (or just as a plain Singleton), that would maintain an ICollection<int> (a HashSet<int> would be ideal for this) where each item is a process ID for which SetAsposePdfLicense was called.

public class PdfInspectorFactory
{
    private readonly ICollection<int> _processes = new HashSet<int>();

    private void SetLicenseForCurrentProcess()
    {
        var currentProcessId = System.Diagnostics.Process.GetCurrentProcess().Id;
        if (_processes.Contains(currentProcessId))
        {
            return;
        }

        // set the license...

        _processes.Add(currentProcessId);
    }

    public PdfInspector Create()
    {
        SetLicenseForCurrentProcess();
        var inspector = new PdfInspector();

        // do whatever you need to do with your inspector before returning it

        return inspector;
    }
}

Then your client code can use this PdfInspectorFactory and call its Create method to get a PdfInspector without having to care about whether or not the license is set.

The client code will not call the factory method in its constructor, rather only when it needs it.

public class SomeClass
{
    private readonly PdfInspectorFactory _factory;

    public SomeClass(PdfInspectorFactory factory)
    {
        _factory = factory;
    }

    public void DoSomething()
    {
        try
        {
            var inspector = _factory.Create();
            // here you go, now do what you need to do with your inspector...

        }
        catch(Exception exception)
        {
            // todo: log or otherwise report exception
            throw;
        }
    }
}

The factory instance should only exist as a Singleton instance - if you're using an IoC container that is very easy to accomplish. Otherwise you need to implement the factory class itself, as a Singleton.


Turn-based game setting properties for playcards

Slight improvements of your current code

Get rid of the comments and read three lines below which symbol you are currently declaring the conditions for. Your code is very self-documenting, no need to add comments.
You should know that there is a more than or equal to operator, which can be used to make your code a lot more readable (here I have also removed the braces since your statement is only one line)

if (id >= 1 && id <= 13) this.symbol = Card.HEARTS;
else if (id >= 14 && id <= 28) this.symbol = Card.KARO;
else if (id >= 28 && id <= 41) this.symbol = Card.CLUBS;
else if (id >= 42 && id <= 52) this.symbol = Card.SPADES;

This can in turn also be simplified to:

    if (id <= 0) { /* mega-super-error-should-probably-not-happen-and-if-it-does-then-it's-no-symbol */ }
    else if (id <= 13) this.symbol = Card.HEARTS;
    else if (id <= 28) this.symbol = Card.KARO;
    else if (id <= 41) this.symbol = Card.CLUBS;
    else if (id <= 52) this.symbol = Card.SPADES;

Weird King

Are you aware that in your current code, you will have the values 1 2 3 4 5 6 7 8 9 10 11 12 0? This feels weird. If Ace is 1 then 0 is king I assume? But using 0 as king does not make sense.

Adding a parameter to the function

Even though @rolfl provides a short way to do this, I just want to object to one thing: The use of the id parameter.

It seems like you are creating a card from an ID. Now what if you some day totally forget about how to calculate these IDs from a specified suite and rank? (or vice versa). I would instead use a function with two parameters: Suite and rank. I'm not sure how you are calling this method today, but to me it feels cleaner to create cards by using a nested for-loop to loop first over suite and then over rank, rather than looping from 1 to 52.

Also, I wouldn't use an uppercase property VALUE when the others, symbol and card is lowercase. It's better to use lowercase value also. Or, you could call it rank instead. And you might want to rename the card property to picture.

As for how to do it in Java, I would say that you should try to make it the same in both Java and JavaScript. Tidy up the JavaScript and then make it the same in Java.


Using ADO to run sql script (with GO syntax) in inno setup

1. Function block

Standalone action like loading a script from file and splitting it into SQL commands by the GO command, I would move into a separate function. That function might have the following prototype:

function LoadScriptFromFile(const FileName: string; out CommandList: TStrings): Integer;
begin

end;

Having such function will isolate parsing and execution parts of the script. You would first load the script from a file and then in the other part of the script iterate the parsed commands and execute each one.

2. Object finalization

Always try to encose object finalization into a separate try..finally block. In your code it applies to the tString object. This part:

ADOConnection.Open;
try
  tString := TStringList.Create;
  ...
finally
  ADOConnection.Close;
  tString.Free;
end;

Should become:

ADOConnection.Open;
try
  tString := TStringList.Create;
  try
    ...
  finally
    tString.Free;
  end;
finally
  ADOConnection.Close;
end;

3. Useless variable

The len variable is used for only one purpose and its value is used only once, so it's actually useless there. I would remove it and write directly:

for i := 0 to tString.Count - 1 do
begin
  ...
end;

4. Summary

Except the above (not so major) things I would say your code is fine. The string concatenation with line breaks you have used should be fine for ADO connected to the SQL Server, I think.

For the sake of completness, here is what I'd write (untested!). Please note, that every SQL command is expected to be "terminated" by the GO command in the input file (that requires also your original code):

function LoadScriptFromFile(const FileName: string; out CommandList: TStrings): Integer;
var
  I: Integer;
  SQLCommand: string;
  ScriptFile: TStringList;
begin
  Result := 0;
  ScriptFile := TStringList.Create;
  try
    SQLCommand := '';
    ScriptFile.LoadFromFile(FileName);

    for I := 0 to ScriptFile.Count - 1 do
    begin
      if Pos('go', LowerCase(Trim(ScriptFile[I]))) = 1 then
      begin
        Result := Result + 1;
        CommandList.Add(SQLCommand);
        SQLCommand := '';
      end
      else
        SQLCommand := SQLCommand + ScriptFile[I] + #13#10;
    end;

    //EDIT: If there is no GO syntax present int the file AND
    //To add the script after the last GO - Govs
    CommandList.Add(SQLCommand);
    Result:= Result + 1;
  finally
    ScriptFile.Free;
  end;
end;

procedure RunScript(const FileName: string);
var
  I: Integer;
  CommandList: TStrings;
  ...
begin
  ...
  // here is already established connection
  CommandList := TStringList.Create;
  try
    if LoadScriptFromFile(FileName, CommandList) > 0 then
      for I := 0 to CommandList.Count - 1 do
      begin
        // execute each command
        ADOCommand.CommandText := CommandList[I];
        ADOCommand.Execute(NULL, NULL, adCmdText or adExecuteNoRecords);
      end;
  finally
    CommandList.Free;
  end;
  ...
end;


I don't understand why you want to pass a list of files. Simply check the existence of your filename candidates. Start with the base filename. If it does not exist, start counting. In my eyes this a straightforward approach and therefore easy to understand.

public string GetNextAvailableFilename(string filename)
{
    if (!System.IO.File.Exists(filename)) return filename;

    string alternateFilename;
    int fileNameIndex = 1;
    do
    {
        fileNameIndex += 1;
        alternateFilename = CreateNumberedFilename(filename, fileNameIndex);
    } while (System.IO.File.Exists(alternateFilename));

    return alternateFilename;
}

private string CreateNumberedFilename(string filename, int number)
{
    string plainName = System.IO.Path.GetFileNameWithoutExtension(filename);
    string extension = System.IO.Path.GetExtension(filename);
    return string.Format("{0}{1}{2}", plainName, number, extension);
}


Converting a string into an array and returning the values spelled in reverse

Good code, few suggestions.

Instead of having to check each and every time to make sure you can add a "," or not, add it for all the cases, use String.TrimEnd to remove the extra leading ","
            var names = s.Split(",");
            string temp = "";

            for (int i = 0; i < names.Length; i++)
            {
                for (int j = names[i].Length - 1; j >= 0; j--)
                {
                    temp += names[i].ElementAt(j);
                }
                temp += ",";
            }
            return temp.TrimEnd(',');

With that change, we have the ability to foreach loop to further improve the readability of the code.
            string temp = "";
            foreach (var name in s.Split(','))
            {
                for (int j = name.Length - 1; j >= 0; j--)
                {
                    temp += name.ElementAt(j);
                }
                temp += ",";
            }
            return temp.TrimEnd(',');

That's good. Although we can improve this further by using a better (faster and cleaner) method to reverse the individual names, and that is to use String.ToCharArray and Array.Reverse
The idea is to convert the string into a character array, use the inbuilt Reverse method to reverse it, add it to temp.
            string temp = "";
            foreach (var name in s.Split(','))
            {
                char[] NameArray = name.ToCharArray();
                Array.Reverse(NameArray);

                temp = temp + new string(NameArray) + ",";

            }
            return temp.TrimEnd(',');

You can also use StringBuilder which is slightly faster
            StringBuilder reversed = new StringBuilder();

            foreach (var word in str.Split(','))
            {
                char[] singlesentence = word.ToCharArray();
                Array.Reverse(singlesentence);
                reversed.Append(singlesentence);
                reversed.Append(",");
            }
            return reversed.ToString().TrimEnd(',');

Nitpick:

Prefer using a better name like reversed compared to temp, it tells more to the reader.

Use string.Empty and not ""

Benchmarks
Your original code
        static public string Reverse(string s)
        {
            var names = s.Split(",");
            string temp = "";

            for (int i = 0; i < names.Length; i++)
            {
                for (int j = names[i].Length - 1; j >= 0; j--)
                {
                    temp += names[i].ElementAt(j);
                }
                temp += i == names.Length - 1 ? "" : ",";
            }
            return temp;
        }

New
        static public string Reverse2(string s)
        {

            string reversed = string.Empty;
            foreach (var name in s.Split(','))
            {
                char[] NameArray = name.ToCharArray();
                Array.Reverse(NameArray);

                reversed += new string(NameArray) + ",";

            }
            return reversed.TrimEnd(',');
        }

New2
        static public string Reverse3(string str)
        {
            StringBuilder reversed = new StringBuilder();

            foreach (var word in str.Split(','))
            {
                char[] singlesentence = word.ToCharArray();
                Array.Reverse(singlesentence);
                reversed.Append(singlesentence);
                reversed.Append(",");
            }
            return reversed.ToString().TrimEnd(',');
        }

Note that I have static to use the function without an object reference.
I will use the following method to call the two functions multiple times, and benchmark it.
static void Perform(Func<string, string> Function, string names, int iterations, string type)
{

    var watch = System.Diagnostics.Stopwatch.StartNew();
    for (int i = 0;i < iterations;i++)
        Function(names);

    watch.Stop();

    var elapsed = watch.ElapsedMilliseconds;
    Console.WriteLine($"Time taken for {type}: {elapsed} ms");
}

        static void Main()
        {
            string names = "Jack,Dylan,Bobby,Peter";
            const int iterations = 1000000;

            Perform(Reverse,  names, iterations, "Original");
            Perform(Reverse2, names, iterations, "New");
            Perform(Reverse3,  names, iterations, "New2");

        }

Here are the results
/*
" Release " Configuration

 Iterations  |   Original  |   New     |   New2
-------------+-------------+-----------+--------
 10 000      |    29   ms  |    5   ms |   5   ms
-------------+-------------+-----------+--------
 100 000     |    226  ms  |    54  ms |   43  ms
-------------+-------------+-----------+--------
 1 000 000   |    1763 ms  |    442 ms |   405 ms

*/

Try it here


Improve performance by excluding multiple conditions and List

I think the assignment of your entities (bad name) is uber-abusing ternary expressions.

That said, my first thought was why on Earth would you want to return an Exception? So I googled up a bit and found this:

  Is it valid for a function to return exception objects?

  Absolutely. Here is a short list of examples where this may be appropriate:

  An exception factory.
  A context object that reports if there was a previous error as a ready to use exception.
  A function that keeps a previously caught exception.
  A third-party API that creates an exception of an inner type.

Fine then. But there's a flaw in your API - you're taking in an Object for what seems to be an Entity.

If I understand what you're trying to do here, I believe you'd be better off with an overloaded method:

protected override Exception GetValidationException(int value, string name)
protected override Exception GetValidationException(Guid value, string name)
protected override Exception GetValidationException(int[] value, string name)
protected override Exception GetValidationException(Entity value, string name)
protected override Exception GetValidationException(Entity[] value, string name)

...Or I'd find a way to make something like this work:

protected override Exception GetValidationException<TValue>(TValue value, string name)


Running async tasks and cancelling after a timeout if necessary - follow up (1)

Looks good. One potential improvement is to make WaitAll method async. Maybe:

//return false on timeout
public static async Task<bool> RunAsync(this IEnumerable<Action<CancellationToken>> actions, TimeSpan timeout)

This way you can support two additional scenarios:

actions.RunAsync().Wait(); //sync wait (what you currently have)
await actions.RunAsync(); //await
actions.RunAsync().ContinueWith(...); //async call, no waiting

You will probably have to use Task.WhenAll instead of Task.WaitAll to make this happen.

Also  1s timeout is not something I would expect when I see actions.WaitAll() in code. Default timeout in all BCL classes I can think of is Infinite, you should probably follow this practice for consistancy.


Java function to format seconds as minutes, hours, days

This is a case for an Enum and a NavigableMap.

TimeUnit.java

public enum TimeUnit {

    SECOND(1, "1 second", " seconds"),
    MINUTE(60, "1 minute", " minutes"),
    HOUR(3600, "1 hour", " hours"),
    DAY(86400, "1 day", " days"),
    MONTH(2629743, "1 month", " months");

    private final int SECONDS_COUNT;
    private final String ONE_UNIT;
    private final String MULTIPLE_SUFFIX;

    public TimeUnit(int secondsCount, String oneUnit, String multipleSuffix) {
        SECONDS_COUNT = secondsCount;
        ONE_UNIT = oneUnit;
        MULTIPLE_SUFFIX = multipleSuffix;
    }

    public getSecondsCount() {
        return SECONDS_COUNT;
    }

    public getOneUnit() {
        return ONE_UNIT;
    }

    public getMultipleSuffix() {
        return MULTIPLE_SUFFIX;
    }

}

TimeProcessor.java

public class TimeProcessor {

    private static final NavigableMap<Integer, TimeUnit> timePeriods = new TreeMap<>();
    static {
        for (TimeUnit unit : TimeUnit.values()) {
            timePeriods.put(unit.getSecondsCount(), unit);
        }
    }

    public static String toString(int secondsOfTime) {
        Map.Entry<Integer, TimeUnit> entry = timePeriods.floorEntry(secondsOfTime);
        if (entry == null || entry.getKey() == null) {
            return "";
        }

        int unitCount = secondsOfTime / entry.getKey();
        return toString(unitCount, entry.getValue());
    }

    public static String toString(int unitCount, TimeUnit unit) {
        return (unitCount == 1) ? unit.getOneUnit() : unitCount + unit.getMultipleSuffix();
    }

}

I find it easier to have all the data localized in one place, separate from the handling logic.  This makes it easier and more robust to add additional units like weeks, fortnights, seasons, years, decades, centuries, and millennia.

This is more verbose and self-documenting as well as easier to extend and maintain.  Each new unit is only a line of code.

When using if/else structures or ternary operators, I prefer to test positive conditions rather than negatives.  This avoids the inherent if not not logic.

When using either this version or your original, there is a bit of a problem in terms of how it handles months.  Not every month has the same length.  A month can be anywhere from twenty-eight to thirty-one days.  It might be better to make use of the existing date handling logic rather than spin your own.  Because months and larger units are not fixed length.  Even days can be problematic with daylight savings time.  I regard this more as a programming exercise than a solution to representing time intervals.

I haven't tested this, so beware of syntax or logical errors.