Solving a one-dimensional Euler equation for fluid dynamics

I'm going to assume your target machine is a super scalar CPU with some variation of Tomasulo architecture. I'm also going to assume that there is no significant cache miss ratio at cause for your performance (you should verify this).

When compiling optimised code, the compiler will rearrange operations to reduce stalls due to availability of input data to the instructions. Hence you cannot completely trust the line numbers in the profiler. As the CPU is super scalar some instructions may execute and complete out of order (but are committed in order).

I see that you are computing the reciprocal of cavg and den to save yourself two divisions. It is possible (but doubtful) that this could cause a delay stage as the two dependent operations could end up stalling until the result is available instead of executing in parallel on different execution units.

Contrived example:

double a_inv = 1.0/a;
double b = x*a_inv;
double c = y*a_inv;

If executed as is this would require two execution stages. The two multiplications would have to wait until the division is completed before they can start. The total delay is 1 div + 1 mul (the two mul can execute in parallel on the cpu). Compare this to:

double b = x/a;
double c = y/a;

Execution can begin immediately and the total delay is 1 div as the divs can run in parallel. This is assuming the compiler doesn't do this behind your back already (I'm not sure if it is allowed to by the IEEE rules) . It also depends on if there is enough instructions to fill the execution units while calculating the reciprocal. Any way it's worth trying.

Apart from that I can't think of anything that would make the function as it is any faster.

If you really want speed you need to use SIMD instructions in addition to using many thread. But that requires you to restructure your data from "array of structs" to "structs of arrays".

Edit: I've had an opportunity to take a closer look and there are some things that you can do to improve performance.

This calculates cavg* 1/cavg at two occasions:

  double alpha_3 = 0.5f*(d2 + (cavg - uavg)*d1 - cavg*alpha_2)*cavg_inverse;

which can be re-written as:

double alpha_3 = 0.5f*((d2 - uavg*d1)*cavg_inverse + d1 - alpha_2);

This reduces from 4 mult + 3 add to 3 mult + 3 add.

Here you need to make sure that the compiler actually emits fabs instruction and not cmp+cmov:

double lambda_1 =  fabs(uavg - cavg);
double lambda_2 =  fabs(uavg);
double lambda_3 =  fabs(uavg + cavg);

Here, the compiler will perform CSE so lambda_x*alpha_x products will already be available and we can rewrite:

double f1 = lambda_1*alpha_1 + lambda_2*alpha_2 + lambda_3*alpha_3;
double f2 = lambda_1*alpha_1*(uavg-cavg) + lambda_2*alpha_2*uavg + lambda_3*alpha_3*(uavg+cavg);
double f3 = lambda_1*alpha_1*(havg-cavg*uavg) + 0.5f*lambda_2*alpha_2*uavg*uavg + lambda_3*alpha_3*(havg+cavg*uavg);

as:

double la1 = lambda_1*alpha_1; // For readability, compiler does this anyway
double la2 = lambda_2*alpha_2;
double la3 = lambda_3*alpha_3;
double f1 = la1 + la2 + la3;
double f2 = uavg*f1 - cavg*(la1-la3);
double f3 = la1*(havg-cavg*uavg) + 0.5f*la2*uavg*uavg + la3*(havg+cavg*uavg);

which reduces the expression of f2 from 3 mult + 4 add to 2 mult + 2 add as the results from f1 can be re-used. The compiler should put the calculation of uavg*f1 towards the end of the function and calculate f3 while f1 computes.

There could be more of the same type of optimizations you can do by rearranging operations but unfortunately I don't have time to give a more thorough look.

Disclaimer: The compiler may or may not do these optimizations depending on the FP precision setting. For example -ffast-math on GCC would probably do these, but only if -ffast-math as IEEE is not associative due to round off errors. There is an equivalent option for ICC but I don't know what it's called. But be advised this option can break stuff, like NAN handling/checking.


The two classes seem useless to me.

ServerResults only contains a list, so just use a list. Edit: see comment below.

The CallThreads class is unnecessary, this:

t = CallThreads(poll_server, s, results)

can be written like that:

t = Thread(target=poll_server, args=(s, results))

Note that you could also use the partial function:

t = Thread(target=partial(poll_server, s, results))

or a lambda:

t = Thread(target=lambda: poll_server(s, results))


Haskell requires more memory than Python when I read map from file. Why?

I can't seem to download your data set. Could you re-upload a smaller dataset (say, ~5mb? and with your testing statistics for it) to Pastebin or somewhere?

The only obvious performance difference with your Python version is that Data.Map is based on a binary tree, whereas Python's dict is a hashtable. This makes fromList an O(n * log n) operation in Haskell (unless you luck into the fact that your list of (form, base) pairs happen to all be distinct and sorted in ascending order) but constructing the Python dict is O(n). This should make a significant time difference, but not necessarily space, so at this point I'm out of ideas. Make sure you're compiling with -O2 I guess.

(There also seems to be a functional difference between your two versions if my understanding of Python hasn't rusted off after years of neglect. {j:words[0] for j in words} will pair words[0] with itself, so maybe you want parseLine to be defined as let forms@(base:_) = ....)

On issues of style, see the below rewrite. Note particularly the reordering of definitions so as to eliminate the where-clause. In your version it was easy to miss due to being inline with the do-block, you can do as I did, or try decreasing the indent.

{-# LANGUAGE OverloadedStrings #-}

import qualified Data.Map             as Map
import qualified Data.ByteString.Lazy as ByteString
import qualified Data.Text.Lazy       as Text
import           Data.Text.Lazy.Encoding (decodeUtf8)

main :: IO ()
main = do
    text <- fmap decodeUtf8 $ ByteString.readFile "baseforms.txt"
    let parseLine line = let (base:forms) = Text.words line
                         in  zip forms (repeat base)
        m = Map.fromList . concatMap parseLine . Text.lines $ text
    print $ Map.lookup "sometext" m
    print $ Map.size m


Your clear function uses while (--sp > 0); when sp = 0; does the same thing in faster and clearer way. also consider not throwing an error when clearing an empty stack, because it still behaves as expected.
The condition sp > 0 && (sp - 1) > 0 is esentially just sp > 1 or sp >= 2, which I would prefer, but that is just a matter of personal style.
Instead of using val[sp-1] to get the top element of the array (in swap and duplicate) use the top() function, as it is a good idea to express your stack operations in terms of more primitive stack operations.
Follwing the concept of the defining operation in terms of primitive operations, duplicate can be written as:

void duplicate(void) {
    push(top());
}

and swap can be written as:

void swap(void) {
    int elem1 = pop();
    int elem2 = pop();
    push(elem1);
    push(elem2);
}

Notice how there is no error checking in these functions,
because push(), pop(), and top() handle the full or empty
stack for us.


The OO part seems to be OK when it comes to the type inheritance and the choice of an abstract class.

Code review.

For the abstract class

public DiscountCard(int turnOver, int purchaseValue){

This is a public constructor for a class that cannot be instantiated, consider protected instead. It also contains a turnOver, but that's specific to an owner it seems to me. So that doesn't make sense without the owner string. Similarly, I don't think the card is specific to one particular purchase. The card is applied on a particular purchase.

    setTurnover(turnOver);
    setPurchaseValue(purchaseValue);

Generally you'd set the fields directly, instead of calling the public methods. Calling methods can be dangerous, especially if they can be overridden.

abstract void calculateDiscountRate();

For what? That depends on purchaseValue but that should not depend on any class fields, as already established.

public String getOwner() {
    return owner;
}

Here you make a classic mistake: never leave your objects in an invalid state. As the owner may not be set, this method could return null.

public void setOwner(String owner) {
    this.owner = owner;
}

No. If the holder of the card can change then this would be in the model, and I don't see that. Generally cards are personalized and are therefore not transferable (at least from the payment desk's point of view).

public void setTurnover(int turnOver) {
    if(turnOver>=0) {
        this.turnover = turnOver;
    }else{
        System.out.println("Turnover can't be less than 0.");
    }
}

The turnover may actually be negative if the customer decided to return. In this kind of situations it is important to perform a reality check: what could happen. Furthermore, I guess that this should be called after each period of time (it's for the "previous month" according to the instructions. So call it, say, setTurnoverOfLastMonth() or something similar.

public void setDiscountRate(double discountRate) {
    this.discountRate = discountRate;
}

Is that really a public call, you override the discount rate? Seems to me that you'd have to switch cards for that.

For the gold card:

calculateDiscountRate();

By now you should have noticed that there are more rates possible, you are still trying to get along with one.

setDiscountRate(2);

setDiscountRate(getTurnover()/100 + getDiscountRate());

And you aren't just overwriting the single discount rate in the second call?

if(getDiscountRate()>10){
    setDiscountRate(10);
}

Leukoplast patching ain't gonna help, son.

For the store calculator

Where are the purchases? In the card?

I'd expect something like:

public Payment calculateResult(double purchaseValue);

Where the Payment contains all the details (card used, discount percentage, discount value and of course the final price).

Note too that the examples have a purchase value with .00 behind it. Somehow that indicates to me that every cent is appreciated (and you store as an int - which is actually fine if it means cents).

For the PayDesk

Just prints out stuff. Maybe correct, maybe it isn't, but just printing the bill is not what I was expecting. Probably can be merged with the StoreCalculator although I like the fact that you've created a separate calculator. That would certainly be a good idea for a more complicated PayDesk class.

Beware: it says in your requirements: "create instances with sample data as shown in Example outputs section;"

Now I don't personally think that the purchase should ever be part of the card data, but if it is stated this way I'd almost include it myself as a field.

Very non-nerdy of me, but I'd include a welcoming message for the card holder (Thank you for your purchase, ). Your GOLD CARD discount is: ... Etc... This would show your employer that you can act outside what is basic development.

And probably I'd also welcome non-card holders to make a purchase.


Function to validate parameter and throw exception when invalid

I'm generally strongly against unthrown exceptions. An exception without its stack trace is simply a named container for an error message. You're using this kind of exceptions for innerException parameter of the exceptions your really throw:

throw new IdentityNotMappedException("", new ArgumentException("", nameof(userId)));

There is no need to have an inner exception if outer one has not been originated by that. When you feel that needing you are, probably, using wrong exception.

There is no need for ArgumentException and userId cannot be null because it is a value type. IdentityNotMappedException alone itself is sufficient to "describe" your error condition (but note that, according to your true code, I'd use a more descriptive error message).

if (userId == Guid.Empty)
    throw new IdentityNotMappedException("User identity is unknown.");

"Action unrecognized" is not a very useful error message (same as "Must follow C_UD!"). Your error messages must quickly describe what's wrong (hints to solve the problem are welcome but optional).

if (state == EntityState.Detached)
    throw new ArgumentException("Entity can not be in Detached state.", nameof(state));

If state is a function argument then you should throw ArgumentException, if it's object state then you may throw InvalidOperationException:

if (state == EntityState.Detached)
    throw new InvalidOperationException("Entity can not be in Detached state.");

Let me stress again to use a better error description. You're checking for a parameter vs null; there may be multiple reasons for this (caller programming error, invalid object state), unless you're sure about what's going on then limit yourself to say what you actually know (argument is null).

if (data == null)
    throw new ArgumentNullException(nameof(data));

To answer your questions directly:

  Choice of exception names. Are those informative enough?.

Last exception is not right one (IMO) because you can be more specific. First one can be right if (and only if) you're actually working with System.Security.Principal stuff. If not then you'd better to do not use it because it carries extra context that it's not applicable in your case.

  Introduction of tiered exceptions. Are more levels of inner exceptions needed?).

No, you do not even need these inner exceptions, for sure you don't need more. As I already said an exception without stack trace is merely a named container for an error message and moreover you give a false information: IdentityNotMappedException has been originated because of a catched ArgumentException. If you want to give more information don't forget that each exception has a Data dictionary for this purpose:

if (state == EntityState.Detached) {
    var e = new ArgumentException("Cannot use detached entity state.", nameof(state));
    e.Data.Add("Hint", "Must follow C_UD");

    throw e;
}

  Omission of the values. Should I provide the faulty ones?

When value is implicit you don't need to provide it (for example if you're throwing because state is EntityState.Detached then you don't need to repeat, same when you throw ArgumentNullException). You have this doubt just because your error messages are not actually describing what is wrong.

<edit>
As you already noted by yourself these error messages may be as much cryptic as original ones. Context will help to clarify them when required, if - for example - entity state is an unknown implementation detail (from caller point of view) then it has no use to expose it and a more abstract error message is preferable.
</edit>

In your example there are no need to specify any faulty value but, for example, in this situation you should do it:

if (entity.State != EntityState.Unchanged)
    throw ArgumentException($"Invalid entity state {entity.State} (state must be Unchanged).", nameof(entity));


The flow control seems excessively verbose.  This would do the same:

public override bool PropertySearch(string value, bool searchPartial = false)
{
    return searchPartial ? (dataValue.IndexOf(value, StringComparison.OrdinalIgnoreCase) >= 0)
                         :  dataValue.Equals(value, StringComparison.OrdinalIgnoreCase);
}

I would also question your decision to lump these two operations together into the same method.  The code would likely be clearer if you wrote two separate methods.  But you haven't shown the superclass or rest of this class, so I can't say for sure.


There are three magic numbers in here:

AES.KeySize = 256;
//AES.BlockSize = 128;

var key = new Rfc2898DeriveBytes(passwordBytes, saltBytes, 100);
AES.Key = key.GetBytes(AES.KeySize / 8);

Also, that comment should go. Dead code is dead.

As stated in the comments:

byte[] saltBytes = SHA512.Create().ComputeHash(passwordBytes);

Generating the saltBytes based on the password is a bad idea. The salt should be random and if two users with the exact same password have the same salt, well now either of them can attack the other.
The salt does not need to be a secret. You can keep it in plain-text. Many time, in the case of a database, it will have two (or three) columns: EncryptedPassword and Salt (maybe IV if it's not based on the Salt). (Or in the case of ASP.NET Entity Framework / Simple Membership this is all stored in one column with separators.)
The attacker can know the Salt and IV, you can even use the Salt to generate the IV (or initialize the RNG for it), but you cannot use the password to generate either. That defeats the purpose of the password. If I know Salt is generated from the password, then I don't need the password at all, I can just brute force everything to find that Salt, or I can use a rainbow table lookup, et. al.


How could I do DateTime.Now.Date (from C#) in JavaScript?

It is not ideal, but you can do it this way...

var today = new Date();
var dateWithoutTime = new Date(today.getFullYear() , today.getMonth(), today.getDate());

There are multiple ways to make Dates in javascript.  The format above will create a new Date based on "today"'s year, month, and date (think day of the month).

Technically you could also do this with a DateString, however "new Date(dateString)" is implementation dependent, and may have inconsistent behavior across different browsers.

And if making 2 Date objects makes you uncomfortable, you could always do the following:

var today = new Date();
today.setHours(0); // or today.toUTCString(0) due to timezone differences
today.setMinutes(0);
today.setSeconds(0);


I don't believe your current code is thread safe.  Looking at your gate_Enter function:

The first thing you do is skip the wait loop, this is fine:

"jmp gate_Enter_check\n"
"gate_Enter_wait:\n"
"pause\n"

Then you check your reentry count.  If it's > 0 then you assume you are the the one holding the lock and skip the actual lock part.

"gate_Enter_check:\n"
"cmp %[lock], %[checkin]\n"
"jge gate_Enter_skip\n"
"mov %[lock], %%eax\n"
"lock xchg %%eax, %[gate]\n"
"test %%eax, %%eax\n"        // Hope for the best.
"jnz gate_Enter_wait\n"      // There is no hope, go back in line.

And increment the nesting level.

"gate_Enter_skip:\n"         // We are VIP!
"add %[lock], %[checkin]\n"  // Checkin pass like pro!

The issue is you don't actually know that the thread calling gate_Enter is the same one that originally entered the gate.  If another thread calls exampleUse function it's using the same pass. You could get around this by making use of thread local storage for your pass/checkin variable.  Or you could create exampleUsePass on the stack, then pass it in to recursive calls instead of using global vars.

You would have a similar issue with your gate_Leave method, where you unlock the gate, then decrement the nesting count.  It would be better to decrement the value, whilst you are within the locked section, something like this:

"add %[checkout], %[pass]\n"
"jnz gate_Leave_skip\n"       // If not zero, this is a nested call
"mov %[unlock], %[gate]\n"   // Close the gate because I am the last one to leave.
"gate_Leave_skip:\n"

As an aside, I'm not a huge fan of the way you've essentially renamed the pass parameter to your gate_Enter function to checkin.

[checkin] "=m" (*pass)

A slightly more robust version of your code, which will abort if there is an  unexpected exit from gate when you don't own the gate is shown below (along with a simple test harness).

#include <stdio.h>
#include <unistd.h>
#include <unistd.h>
#include <string.h>
#include <pthread.h>

#define gate_Gate volatile int
#define gate_Pass volatile int

/*extern*/ inline void
gate_Enter
(gate_Gate *gate, gate_Pass *pass)
{
    asm volatile (
        "jmp gate_Enter_check\n"     // Skip the line.
        "gate_Enter_wait:\n"         // Wait in line.
        "pause\n"                    // Wait a long time.
        "gate_Enter_check:\n"        // Now we are at the gate.
        "cmp %[lock], %[pass]\n"  // See if our pass is any good.
        "jge gate_Enter_skip\n"      // Skip if pass >= lock.
        "mov %[lock], %%eax\n"       // eax = 1.
        "lock xchg %%eax, %[gate]\n" // Attempt to validate our pass and connect it to the gate.
        "test %%eax, %%eax\n"        // Hope for the best.
        "jnz gate_Enter_wait\n"      // There is no hope, go back in line.
        "gate_Enter_skip:\n"         // We are VIP!
        "add %[lock], %[pass]\n"  // Checkin pass like pro!
        : [gate] "=m" (*gate), [pass] "=m" (*pass)
        : [lock] "r" (1)
        : "eax"
    );
}

/*extern*/ inline void
gate_Leave
(gate_Gate *gate, gate_Pass *pass)
{
    asm volatile (
       "add %[checkout], %[pass]\n"
       "js error_abort\n"            // Abort, underflow on *pass
       "jnz gate_Leave_skip\n"       // Skip next step if I have checked in more then once.
        "mov %[unlock], %%eax\n"
        "lock xchg %%eax, %[gate]\n"
        "test %%eax, %%eax\n"
        "jnz gate_Leave_skip\n"      // If zero, abort because we unlocked an unlocked gate!
        "error_abort:\n"
        "call abort\n"
        "gate_Leave_skip:\n"
        : [gate] "=m" (*gate), [pass] "=m" (*pass)
        : [unlock] "r" (0), [checkout] "r" (-1), [isLast] "r" (1)
        : "eax"
    );
}

// Example use
gate_Gate gate = 0;
__thread gate_Pass exampleUsePass = 0;  // notice this uses thread
                                        // local storage
void exampleUse(int count)
{
    printf("pass=%d\n", exampleUsePass);
    gate_Enter(&gate, &exampleUsePass);
    if (count == 3) {
        // exampleUsePass = 0; // force deadlock.
    }
    if (count < 5) {
        printf("Count = %d\n", count);
        exampleUse(++count);
    }
// uncomment this line to simulate Leaving twice when we owned the gate
//gate_Leave(&gate, &exampleUsePass);
    gate_Leave(&gate, &exampleUsePass);
    printf("pass=%d\n", exampleUsePass);
}

void *doSomeThing(void *arg) {
 // Uncomment lines in this method to force alternate thread to
 // try to leave gate, when it doesn't own it.
 //  if(arg == NULL)
        exampleUse(0);
 //   gate_Leave(&gate, &exampleUsePass);
    return 0;
}

pthread_t tid[2];

int main(void)
{
    int i = 0;
    int err;

    while(i < 2)
    {
        err = pthread_create(&(tid[i]), NULL, &doSomeThing, i?&err:0);
        if (err != 0)
            printf("\ncan't create thread :[%s]", strerror(err));
        else
            printf("\n Thread created successfully\n");

        i++;
    }

    sleep(5);
    return 0;
}


Coding style

You should follow PEP8, the official coding style guide of Python.
The most notable violations:

Don't put all those imports on a single line, use multiple lines
Use CamelCase for class names
Use snake_case for function and variable names

Simplify and improve __init__

Don't create class member attributes that you don't need.
For example self.fileStem is completely unused,
and self.ext could be replaced with a local variable.
Don't pollute your class with unnecessary elements.

Exception handling is best for dealing with anomalies.
If the script is used with a file with an unsupported extension,
that's not really an anomaly.
It would be better to handle this with an if statement instead.

It's not a good idea to exit from the constructor of a class:
the main method of the program should be in charge of shutting down or not.
It would be better to define a custom exception,
make the constructor raise it, and let the main method handle it.

The custom exception can be as simple as this:

class UnsupportedCompressionException(Exception):
    pass

The constructor with the above suggestions applied:

def __init__(self, filepath):
    ext = os.path.splitext(filepath)[1].lower()
    if ext not in self.fileOpenerDict:
        raise UnsupportedCompressionException("Only gz, zip, and bz2 files allowed.")
    self.filepath = filepath
    self.fileOpener = self.fileOpenerDict[ext]

The main method handling the exception:

if __name__ == "__main__":
    try:
        c = CompressedFile(sys.argv[1])
        n = c.count_lines()
        print("N lines: %d" % n)
    except UnsupportedCompressionException as e:
        print(e)
        exit(1)

Improve count_lines

It would be better to do the counting and return the result from inside the with.
This is because in general it's best to limit all code to the smallest possible scope.

The line loop variable is unused.
It's customary to use _ as the loop variable name in such case.

Instead of count = count + 1, it's more common and recommended to write count += 1.

As a result, the method becomes:

def count_lines(self):
    with self.fileOpener(self.filepath) as fh:
        count = 0
        for _ in fh:
            count += 1
        return count


Casting an Enumeration with Descriptions into a Dictionary

I have this class in my code which is very similar to what you are looking for:

public static class EnumHelper {
    public static string GetRealName(this Enum value) {
        if (value == null) {
            throw new ArgumentException("value");
        }

        string name = value.ToString();
        var fieldInfo = value.GetType().GetField(name);
        if (fieldInfo == null) return "";
        var attributes = (RealNameAttribute[])fieldInfo.GetCustomAttributes(typeof(RealNameAttribute), false);

        if (attributes.Length > 0) {
            name = attributes[0].Name;
        }

        return name;
    }
    public static bool IsIgnored(this Enum value) {
        if (value == null) {
            throw new ArgumentException("value");
        }

        string name = value.ToString();
        var fieldInfo = value.GetType().GetField(name);

        return fieldInfo == null || fieldInfo.IsDefined(typeof(IgnoreAttribute), true);
    }

    public static Dictionary<int, string> ToDictionary(this Type enumType) {
        return Enum.GetValues(enumType)
            .Cast<object>()
            .Where(f => !((Enum)f).IsIgnored())
            .ToDictionary(k => (int)k, v => ((Enum)v).GetRealName());
    }
}


One instance per method call or multiple method call per instance

Option 1 is surprising.
I find it funny that you have return Enum.Parse(_propertyType, _parameter); in a parameterless Parse() method. Enum.Parse is just an example (int.Parse would be another); these methods are commonly used by C# programmers and your single-use parser seems to break POLS in my opinion.
Option 2 is [more] consistent with the framework.
I don't mean to repeat what I just said, but it feels natural for a Parse method to take in all the parameters it needs. Now there is another problem here: your API is exposing object, which means the parsed value will need to be cast from the calling code, into the correct type. This is bad. Very bad. Nothing should ever be returning an object. If you're parsing a value type, you're boxing it right there. Is that not likely?
How about keeping it type-safe and taking a generic type argument instead of a Type parameter?
public T Parse<out T>(string value)
{
    ...
    return Convert.ChangeType(value, typeof(T));
}

This way if T is a value type, you're not boxing it. And you're not returning an object, and it's still the calling code that decides what type to parse the string with.
Option 3 feels wrong.
Whenever I feel the need for a class to be called xxxxxHelper, something itches. The class should be static as well, if it's only going to expose static methods. That said, as @Simon has noted (oh, well, Simon says...) this brings DI to its knees. Best stay clear from that.


Deal three Poker hands of five cards each in Python

You've got a reasonable implementation for steps 1-3.  I see no glaring issues with the code.  Perhaps, we can over-engineer some parts of it.

General

Add docstrings.

class Card

It is mutable.  I can be dealt the '6 of Spades' and by tweaking it card.rank = "Ace" all of a sudden have a better card for my hand than I was dealt.  Trick decks aside, cards should be immutable, so we could change this into a tuple to keep in from changing.  Since we want named fields, we need a namedtuple, and since we want to customize the str(card), we'll derive our own class from the namedtuple.

from collections import namedtuple

class Card(namedtuple("Card", ['rank', 'suit'])):
    __slots__ = ()   # Allows no additional fields, keeps memory footprint small

    def __str__(self):
        return f"{self.rank} of {self.suit}"

# Use like:
ace_of_spades = Card('Ace', 'Spades')

Note: I'm using the Python3.6 f-strings.  If you're not using 3.6+, you'll have to adapt it to .format() ... or upgrade ;-)

Rank & Suit

We only want "Clubs", "Diamonds", "Hearts", and "Spades" as suits.
And we only want "Ace", "2", "3" ... "10", "Jack", "Queen", and "King" as ranks.

Perhaps we might want an Enum for suit, and (maybe) another Enum for rank.

from enum import Enum

Suit = Enum('Suit', 'Clubs Diamonds Hearts Spades')
Rank = Enum('Rank', 'Ace Two Three Four Five Six Seven Eight Nine Ten Jack Queen King')

card = Card(Rank.Ace, Suit.Spades)
print(card)

Ew.  This prints "Rank.Ace of Suit.Spades".  We can fix this by changing the __str__(self) function of Card to include the .name of the rank and suit fields:

    def __str__(self):
        return f"{self.rank.name} of {self.suit.name}"

Much better.

But do we want to do this?  It restricts the possible ranks and suits of our cards.
If you wanted a Tarot deck, we might want the Cups, Wands, Pentacles and Swords for our suits.  There are other specialty decks too, so maybe this is too restrictive.  In which case, feel free to ignore this whole section.  But if so, then add something like:

SUITS = ['Clubs', 'Hearts', 'Diamonds', 'Spades']

so you can iterate over a named constant, instead of needing to spell out the valid values each time you need them.

class Deck

What is a deck of cards?  Is it always 52 standard playing cards?  You can shuffle a deck and deal from a deck.  But you can also shuffle two decks together and get 104 cards to deal from (perhaps you're playing Kanasta!  No, wait, you'd need the Jokers too).  What would you call two decks of cards mixed together?  Is it still a deck?  Maybe you want to pass some creation function into the Deck initializer, to control how it is created, instead of always building a deck of standard playing cards.  Again, just a thought.

With the Enums created above, Deck.build() can be simplified:

def build(self):
    self.cards = [ Card(rank, suit) for rank in Rank for suit in Suit ]

Shuffling the cards: def shuffle(self, num=1).  What is the num for?  It isn't used.

Dealing the cards.  Ok.  It deal can deal 3 player a hand of 5 cards each.  Good.  Now, player #1 wants to draw a card ... and they get ... uhm ... one of their first cards again???  The deal() method needs to keep track of which cards you've already distributed, and which remain in the deck.

Is a hand of cards just a list of 5 cards?  If so, why isn't a Deck just a list of 52 cards?  Perhaps you need to deal the cards into a Hand class.  The Hand class would be a good container for evaluating the hand, discarding cards and adding additional drawn cards to, etc.


Are my forms a danger towards code injection?

The Short answer is yes you are vulnerable to injection. XSS to be precise which you can read more about here https://www.owasp.org/index.php/XSS_(Cross_Site_Scripting)_Prevention_Cheat_Sheet

Explaination:

All user input should be sanatized for example:

if you input <script>alert("This will alert");</script> into your form you will notice an alert message will appear on your page

however if you sanatize the code i.e.

print "Hello, " . htmlentities($_POST['user']);

you will no longer see the alert message

using htmlentities() will help protect you from the script injection.

You would also be better validating the data that will be expected from the user

Other points which you can see here https://stackoverflow.com/questions/11554432/php-post-dynamic-variable-names-security-concerns which are based more on dynamically creating variables


I created a "enum string" container in c++

Design Review

I am not sure why you would want to do this.

enum Colours {Red, Green, Blue};

Is much better and type safe.

This code does not have any type safety and thus does not give me any confidence in its usage. Also the comparison is a string compare (not very efficient). Also completely different types are comparable and do not generate a compiler error.

class Color :       public EnumStringContainer {/* STUFF */}
class TypesOfCoral: public EnumStringContainer {/* STUFF */}

if (Color::red == TypesOfCoral::redCoral) {
    std::cout << "It worked\n";
}

Also there is a slight chance that different types could get a positive match!!!!!

The only reason I can see to use this code is for streaming an enum string, but there is no mechanism to convert that stream back into the correct type so you can't read it back in. So that is a fail (and there are better ways to do that).

Code Review

protected:

Protected! Not sure that is ever useful. But I see you are trying to say this class never stands alone and always has to be inherited from. Think I would prefer the curiously recurring template pattern to achieve that result. But Tomato/Tomato (you have to hear it with me changing accent).

Sure the standard constructor. But in most situations are you not going to construct this with a string literal. Which means you are creating several string objects and copying around a lot of data.

  EnumStringContainer(const std::string &_str);

I move use the new move semantics to prevent the copying.

Sure:

  bool operator==(const EnumStringContainer&rhs) const;

But with == usually comes !=.

Sure:

  std::string ToString() const;      // But why return by value.

By returning by value you are forcing a copy.

Can really do this wrong.

  const char* ToCharArray() const;

But not sure why you would need it.

The only reason to use the two above functions is so that you can serialize your enum's easily. So why not have conversion operators or stream operators rather than explicit get the underlying structure.

Also this allows you to stream your object out but not in.

Don't use this->

   this->str = _str;

This is considered good in Java but in C++ it is considered bad practice.

The only reason you need to use this-> is to distinguish between member and local variables. Which means you have local variables with the same name as members, shadowing the members.

Shadowing variables is a real no-no. Because the compiler can't tell when you accidentally do it wrong (using the local and not the member). But you can get the compiler to tell you when you shadow a member. Thus by never shadowing (and getting the compiler to error when you do shadow) your code is actually better as it will have less potential errors in it from using the wrong variable.

In constructors use the initializer list:

EnumStringContainer::EnumStringContainer(const std::string &_str)
{
   this->str = _str;
}

By doing it this way. You force the code to initialize the member with the default constructor. Then you reinitialize it with the assignment operator. Best to just initialize it once.

EnumStringContainer::EnumStringContainer(const std::string& str_)
    str(str_)
{}

Put the & next to the type

In C the * usually goes next to the variable. But in C++ the * and & go next to the type. This is because type information is much more important than in C and this is information about the type.

// So rather than this
EnumStringContainer::EnumStringContainer(const std::string &_str)

// I expect to see:
EnumStringContainer::EnumStringContainer(const std::string& _str)

Most counter arguments against this go along the lines.

// Here p and x are different types.
int*  p,x;

And I will go yes. But show me a coding standard that allows you to define two variables on the same line. Nobody allows this in modern standards. Its one line per variable;

int*  p;
int   x;

Your operator==() is inside your API. I see little use in call ToString().

   return this->ToString() == rhs.ToString();

Member naming

Generally the standard is user defined types have an initial capitol letter. While variables and members have an initial lower case letter. Thus allows us to quickly determine what is a type and what is a function or variable.

Prefer not to use _ as a prefix

The rules for using _ as a prefix are quite complicated (ok only a tiny bit complicated). But the problem is most people don't know them. So best to stay away from using _ as a prefix.

In your code you don't do it wrong so there is nothing wrong with your code. But I bet you don't know why.

Now there is no restrictions on using _ as a suffix.

// I would avoid the underscore here.
EnumStringContainer::EnumStringContainer(const std::string &_str)

// prefer the suffix version as there are no restrictions.
EnumStringContainer::EnumStringContainer(std::string const& str_)

I mention this last as it is just one of those quirks and it does not actually break your code. see: What are the rules about using an underscore in a C++ identifier?


Congrats on building this game, it's pretty cool!
I just have a few suggestions for you:

Don't use jQuery. You really don't need it for anything. The native versions of finding elements in your DOM are much faster than jQuery's methods, and you don't have to load a library.
Don't append HTML as strings. Instead, create dom elements. Then you can add id's, classes and event handlers without querying for them in the DOM.
Put CSS rules in a CSS file, and use javascript only to add and remove CSS classes from elements.

Let me try to rewrite this function following my own rules:

function generateFields() {
  var y = 1;
  var top = 50;
  for (var i = 0; i < 9; i++) {
    var x = $("#board").append(
      "

"
    );
    var fieldI = $("#nr" + i);

    if (i == 0 || i == 3 || i == 6) {
      fieldI.css("margin-left", "5px");
    }
    if (i >= 3 && i <= 8) {
      fieldI.css("margin-top", "15px");
    }

    fieldI.click(chooseField);
  }
}

Could become:

function generateFields() {
  let y = 1;
  let top = 50;
  let board = document.querySelector("board");
  for (var i = 0; i < 9; i++) {
    let field = document.createElement("div");
    board.appendChild(field);
    field.classList.add("field hover-field");
    field.id = "nr"+i;
    if (i == 0 || i == 3 || i == 6) {
      field.addClass("marginLeft");
    }
    if (i >= 3 && i <= 8) {
      field.addClass("marginTop");
    }
    field.addEventListener("click", chooseField);
  }
}

CSS

   .marginLeft {
      margin-left: 5px;
   }
   .marginTop {
      margin-top: 15px;
   }

There's more to improve, but hopefully these tips are helpful!


Code downloads a text file from a website, saves it to local disk, and then loads it into a list for further processing

Your code is nice and concise however there are some changes you can make:

You can just return f.readlines() in make_wordlist.
If you've done this to show that the result is a list then it'd be better to use the typing module.

from typing import List

def make_wordlist(filename: str) -> List[str]:
    ...

get_mylist can be replaced with wordlist[:numlines]. This is because if len(wordlist) is smaller or equal to numlines, then it will return the entire thing anyway.
Performance wise it's best to use print('\n'.join(list)) rather than for item in list: print(item).
I would prefer to be able to change chunk_size in save_webpagecontent and so you can make it a default argument.
IIRC multi-line docstrings shouldn't start on the same line as the """, nor should they end on the same line either.

import requests
from typing import List

Response = requests.Response

def get_webpage(uri) -> Response:
    return requests.get(uri)

def save_webpagecontent(r: Response, filename: str,
                        chunk_size: int=8388608) -> None:
    """
    This function saves the page retrieved by get_webpage. r is the
    response from the call to requests.get and
    filename is where we want to save the file to in the filesystem.
    """
    with open(filename, 'wb') as fd:
        for chunk in r.iter_content(chunk_size):
            fd.write(chunk)

def read_wordlist(filename: str) -> List[str]:
    with open(filename) as fd:
        return fd.readlines()

def print_mylist(word_list: List[str]) -> None:
    print('\n'.join(word.strip() for word in word_list))

"""
List of words collected and contributed to the public domain by
Grady Ward as part of the Moby lexicon project. See https://en.wikipedia.org/wiki/Moby_Project
"""
uri = 'https://ia802308.us.archive.org/7/items/mobywordlists03201gut/CROSSWD.TXT'
filename = 'wordlist.txt'

r = get_webpage(uri)
save_webpagecontent(r, filename)
print_mylist(read_wordlist(filename)[:10])


Phone Number Extracting using RegEx And HtmlAgilityPack

This code has a major design flaw.

Always keep your user interface (UI) separated from your data access layer (DAL).

ScrapLinks

Should be named ScrapeLinks.
If a Function doesn't return anything, then it should be a Sub. In this case, we still want it to be a function.
No need to have a shared HtmlDocument as this is created by the HtmlWeb.
In every iteration where the conditions are met  you add a new value to the ListBox item collection. This will result in a bunch of windows messages being sent. I guess the most costly is WM_PAINT. Scrape the links first, then compare the result with current links. Use the AddRange method to insert new items.
A Try-Catch block, whose only purpose is to display an error, should never be used outside the UI layer.

Public Shared Function ScrapeLinks(url As String) As IEnumerable(Of String)

    If (url Is Nothing) Then Throw New ArgumentNullException("url")

    Dim web As New HtmlWeb()
    Dim document As HtmlDocument = web.Load(url)

    If (Not document.DocumentNode Is Nothing) Then

        Dim rootNode As HtmlNode = document.DocumentNode.SelectSingleNode("//*[@id='ad_list']")

        If (Not rootNode Is Nothing) Then

            document.LoadHtml(rootNode.InnerHtml())

            If (Not document.DocumentNode Is Nothing) Then

                Dim childNodes As HtmlNodeCollection = document.DocumentNode.SelectNodes("//a[@href]")

                If (Not childNodes Is Nothing) Then

                    Return (
                        From childNode As HtmlNode
                        In childNodes
                        Let href As String = childNode.GetAttributeValue("href", String.Empty)
                        Where href.Contains("/detail/")
                        Select href
                    )

                End If

            End If

        End If

    End If

    Return Nothing

End Function

Scrapnums

(Same as above)
Should be named ScrapeNumber as it only returns one number.

Public Shared Function ScrapeNumber(url As String) As String

    If (url Is Nothing) Then Throw New ArgumentNullException("url")

    Dim web As New HtmlWeb()
    Dim document As HtmlDocument = web.Load(url)
    Dim root As HtmlNode = document.DocumentNode.SelectSingleNode("//*[@class='det_ad f_left']")

    If (Not root Is Nothing) Then

        Dim regexMatch As Match = Regex.Match(root.InnerText, "(\+92|0092)-?\d{3}-?\d{7}|\d{11}|\d{4}-\d{7}")

        If (Not regexMatch Is Nothing) Then
            Return regexMatch.Value
        End If

    End If

    Return Nothing

End Function


I'm no cryptography guru, so my comments will aim at the code in itself, not the algorithms.

Message.h / Message.cpp:

There doesn't seem to be a need to include <iostream> nor <fstream> in the header file. Avoid importing dependencies that you don't use. The cost of an include is in compile-time. One too many big projects suffer with long compile times due to poor management of include files.
Keep your parameter names in function/method prototypes. This will self document your code. Take this constructor for instance:

Message(const std::string&, const int&);

Impossible to tell which parameter is doing what without consulting the implementation.
cryptoContainer is a "type alias", nevertheless, it names a type. Your naming convention for types is PascalCase, so CryptoContainer would be more self-consistent.
writeOriginalMessage is currently returning a meaningless integer value (always zero). It could very well return nothing (void). If there is a possibility of failure, then return a boolean true or false.
Still talking about writeOriginalMessage and writeCryptedMessage, both have a quite similar loop inside:

for (auto it = m_originalContent.begin(); it != m_originalContent.end(); it++)

You can significantly shorten them by using a range based foreach loop:

for (const auto & item : container)
{
    stream << item;
}

Notice the reference &. Care should be taken to avoid copying things in this style of iteration since the default in C++ is always a copy.
Writing void in the parameter list of a C++ function that takes no parameters (as in getOriginalMessage(void)) is a C-ish style. C++ does not require that; () is just as good. So avoid the unnecessary verbosity.
virtual destructors are only necessary when your class is meant to be inherited from. In this particular code, that doesn't seem to be the case with Message. If you did that to allow users of your code to inherit from the class, I would advise against, since Composition is usually much more elegant and with less coupling. You should remove virtual from the destructor declaration and perhaps even mark your class final. While your are at it, them make the protected section private and throw the destructor away, since it is empty.
I see that you have a couple places with names not in English, while the bigger whole is all in that language. It would be better to keep things uniform, so replace things like texte and clef with the proper English terms.
In functions/methods that return void, no need to explicitly return at the end. E.g.: setOriginalMessage and setCryptedMessage.
Careful with noisy/meaningless comments. // constructors and // destructor are self evident.

Chiffre.h / Chiffre.cpp:

Again, unnecessary includes: you don't need <vector> in the header file.
Also, the reasons for Chiffre having a virtual destructor are not clear. If you don't have inheritance, you shouldn't be making it virtual.
The empty constructor is also unnecessary. Leave it out if you have no manual initialization to perform.


Caching lookup values in an e-commerce website

Global static state makes your code untestable...

I would define your car make lookup as:

public class CarMakeLookup
{
    public CarMakeLookup(IEnumerable<(short Id, string Make)> data)
    {
        Makes = data.ToDictionary(d => d.Id, d => d.Make);
        Ids = data.ToDictionary(d => d.Make, d => d.Id);
    }

    public IReadOnlyDictionary<short, string> Makes { get; }
    public IReadOnlyDictionary<string, short> Ids { get; }
}

And read it with:

public class CarMakeReader : IReader<CarMakeLookup>
{
    public CarMakeReader(Func<ApplicationDbContext> context) =>
        Context = context;

    Func<DbContext> Context { get; }

    public Task<CarMakeLookup> ReadAsync()
    {
        using (var context = Context())
            return new CarMakeLookup(
                from cm in context.CarMakes
                select (cm.CarMakeId, cm.CarMakeName));
    }
}

Where IReader<> is simply:

public interface IReader<TSet>
{
    Task<TSet> ReadAsync();
}

There should be also an extension method Cache defined nearby:

public static class Reader
{
    public static IReader<TSet> Cache<TSet>(this IReader<TSet> source) =>
        new CachingReader<TSet>(source);
}

Where:

class CachingReader<TSet> : IReader<TSet>
{
    public CachingReader(IReader<TSet> source) =>
        Lookup = new Lazy<Task<TSet>>(() => source.ReadAsync());

    Lazy<Task<TSet>> Lookup { get; }
    public Task<TSet> ReadAsync() => Lookup.Value;
}

Now register you reader in the IoC container as a decorated singleton:

containerBuilder.RegisterInstance(ctx => new CarMakeLookup(
    ctx.Resolve<Func<ApplicationDbContext>>())
    .Cache()).AsImplementedInterfaces();


Using Java stream to filter a list and create a map

This transformation is not so obvious, but still feasible without instantiating a dedicated Map directly and without using forEach.

First of all, I suggest to implement hashCode and equals in Hotel and OTAProperty classes: this is often useful, especially when storing objects in collections.

Here is how the functional sequence can look like:

public static Map<Long, Hotel> createHotelMapByOtaName2(List<Hotel> hotels, String otaName) {
  return hotels.stream()
             .map(hotel ->
                hotel.getOtaProperties().stream()
                     .filter(op -> otaName.equals(op.getOtaName())) // 1
                     .collect(Collectors.toMap(OTAProperty::getId,
                                               (h) -> hotel))) // 2
             .map(Map::entrySet) // 3
             .flatMap(Collection::stream) // 4
             .collect(Collectors.toMap(Map.Entry::getKey,
                                       Map.Entry::getValue)); // 5
}

Equivalent of the original if condition: we take only OTAProperties that match the required name.
Each hotel item is wrapped to a Map<Long, Hotel>, which is a subset of the resulting map.
Transformation of the stream of Maps into a stream of Set<Entry<Long, Hotel>>.
Concatenation of streams of Sets into a single stream of Entry<Long, Hotel>. Unfortunately, we cannot join Maps so easily, that is why there is this intermediate transformation.
Finally, the stream of entries is collected into a Map representing the result of the sequence.


C++ Dijkstra's shortest path implementation

OK this does not implement dijkstra exactly (its close).

Dijkstra looks more like this:

PriorityQueue<{Node,cost}>   frontierList;
List<Node>                   alreadyVisited;

frontierList.push({start,0});

while(!frontierList.empty())
{
    node, cost = frontierList.pop();
    if (node == end) {
        return cost;
    }
    if (alreadyVisited.find(node) != alreadyVisited.end()) {
        continue;
    }
    alreadyVisited.push(node);
    for(connect: node.connections) {
        if (alreadyVisited.find(connect.node) == alreadyVisited.end()) {
            frontierList.push({connect.node, cost + connect.cost});;
        }
    }
}
return -1; // No  path found.

Notice we keep the cost in the priority list (not in the node). Otherwise you may have nodes in the list with a higher cost that need to be expunged.

You get more efficiency by using an already visited list because you don't have to expand all its connections (if you already found it then you already have the shortest path to that node).

Note: You can't put the test for alreadyUsed inside the connections loop because by the time you get to them the priority loop may have found a better way to the node.

For more details see:

https://stackoverflow.com/a/3448361/14065
https://stackoverflow.com/a/3765889/14065
https://codereview.stackexchange.com/a/126688/507
A generic implementation of Dijkstra
https://codereview.stackexchange.com/a/103896/507
https://codereview.stackexchange.com/a/69024/507
https://codereview.stackexchange.com/a/77256/507

Code Review

Passing by pointer should be avoided.

bool dijkstra::GetShortestPath(std::deque<CNode> &vGraph, CNode *mStart, CNode *mEnd, std::deque<CNode*> *vPath)
{
    if(mStart == 0 || mEnd == 0 || vPath == 0)
        return false;

If you pass these value by reference then you don't need to make this check.

This is doggy:

    //Set distance to start node to 0
    mStart->DistanceTo = 0;

It assumes that in all your nodes. You have set Completed and DistanceTo and PreviousNode to their default values. That means the algorithm is intrusive into the graph. This means the algorithm only works with this specific type of graph and can not be applied to a generic graph type.

It also assume that you go through a manual reset of the graph before you can call this function.

Don't particularly like your do {} while(); loop I think this falls more into the while() {} category.

OK this is gets a bit hard. I was sure this was broken. But as it turns out that it works fine (just inefficient).

            if(it.To->DistanceTo > (mTopNode->DistanceTo + it.Distance))        //If a shorter path to a node is found, overwrite DistanceTo & PreviousNode
            {
                it.To->DistanceTo = (mTopNode->DistanceTo + it.Distance);
                it.To->PreviousNode = mTopNode;
                std::make_heap(vUnvisited.begin(), vUnvisited.end(), mCompare); //Completely sort the queue
            }

Every time you encounter a path to a destination node you update the destination node with the shortest distance. But this also means you have to re-sort your whole priority queue each time you change the distance.

This is not very efficient. This is why the destination node and distance are supposed to be stored together in the priority queue (sorted by distance). Then you will always find the shortest distance to a node first. Then duplicates will be quickly removed by the Already Found List.

Checking the ->Completed flag here is an optimization that I don't have in my example (that can be added). But putting it here is not enough. You also need to check the ->completed flag before you start processing the current node mTopNode Otherwise each copy on the frontier list will push all its children on the frontier list thus growing it exponentially.

            if(it.To->Completed == false)       //If an unvisited node has been found, add it to the queue and sort it in
            {
                vUnvisited.push_back(it.To->Addr());
                std::push_heap(vUnvisited.begin(), vUnvisited.end(), mCompare);
            }

This line does not need to be in the loop.

            mTopNode->Completed = true;


Import "izip" for different versions of Python

Not sure this is really an answer, or I should elaborate on my comment, and in hindsight probably not even a very good comment anyway, but:

Firstly, you can just simplify it to:

try:
    from itertools import izip as zip
except ImportError: # will be 3.x series
    pass

What I was thinking about was:

From 2.6 you can use as per the docs:

from future_builtins import map # or zip or filter

You do however then have the same problem of ImportError - so:

try:
    from future_builtins import zip
except ImportError: # not 2.6+ or is 3.x
    try:
        from itertools import izip as zip # < 2.5 or 3.x
    except ImportError:
        pass

The advantage of using future_builtin is that it's in effect a bit more "explicit" as to intended behaviour of the module, supported by the language syntax, and possibly recognised by tools. For instance, I'm not 100% sure, but believe that the 2to3 tool will re-write zip correctly as list(zip(... in this case, while a plain zip = izip may not be... But that's something that needs looking in to.

Updated - also in the docs:

  The 2to3 tool that ports Python 2 code to Python 3 will recognize this usage and leave the new builtins alone.


Simplifying minimum search of two dimensional vector with complex struct

Simplifying isn't enough. I mean, your function is already quite simple, and complexity isn't what's questionable about it. To improve it, I'd suggest to:

handle empty input better

The way you handle empty input is questionable.

First, if you want to initialize min with the highest value for a double, use std::numeric_limits<double>::max() in <limits> instead of a macro.

Second, what if entryContact is empty, or if all its members are empty? You'll return DBL_MAX. And what if the smallest element inside is equal to DBL_MAX? You'll return DBL_MAX. Your return value is ambiguous.

At that point, you need to decide if an empty input is meaningless or meaningful. If it's meaningless, you might handle it by throwing an exception; if not, your return type should represent the fact that there are no smallest element if your input vector is empty. std::optional is a good candidate if c++17 is available to you.

Use the standard library more

Beyond std::optional, I would recommend to use algorithms from the standard library over raw loops. std::min_element will fit the bill for the inner vectors, but not for the outer one. You could use std::accumulate (std::reduce would be better but is only available in clang as for now).

Improve your code readability

I would advocate that using the keyword auto makes for a more readable code sometimes, and gives more flexibility, especially for lambdas because they are defined locally.

So here's how I would write it:

#include <algorithm>
#include <numeric>
#include <optional>
#include <limits>

const auto miniFunc = [](const auto& entry_vectors, auto member_var) {
    auto value  = [=] (auto&& entry)            { return entry.*member_var; };
    auto cmp    = [=] (auto&& lhs, auto&& rhs)  { return value(lhs) < value(rhs); };
    auto min    = [=] (auto&& init, auto&& vec)   {
        auto vec_min = std::min_element(vec.begin(), vec.end(), cmp);
        if (vec_min == vec.end()) return init;
        auto candidate = std::make_optional(value(*vec_min));
        return init ? std::min(init, candidate) : candidate;
    };
    return std::reduce(entry_vectors.begin(), entry_vectors.end(), std::optional<double>{}, min);
};

Edit: cf @TobySpeight's comment


Get the total counted stock take lines for each hour between two dates

It's probably easier to switch to a Left Join instead of Union/Except:

with tempCountedHours as
(
    select
        count(*) as counted,
        b.date
    from stocktakebinline l
    join
    (
        --Gets minutes and how many where counted in that hour for all lines.
        select
            lineid,
            date_trunc('hour', counteddate) as date
        from stocktakebinlinebatch
        where
            --Where the stock level is over 0 or if it is 0,
            --then only zero stocks that have been counted.
            counteddate is not null
            and (instock > 0 or (instock = 0 and countedstocklevel > 0))
        group by
            lineid,
            date_trunc('hour', counteddate)
    ) b on l.id = b.lineid
    group by
        b.date
    order by
        b.date asc
)
select
    COALESCE(tch.counted, 0), -- set missing data to zero
    d.date
from
 (
    --Get all of the hours between the created date of the stock take
    --and right now
    select
        date_trunc('hour', d) as date
    from generate_series(
        '2017-02-15 09:00',
        now(),
        '1 hour'::interval
 ) d
left join tempCountedHours as tch
on d.date = tch.date
order by
    d.date asc;


Separate responsibilities
Your functions should, where possible, limit themselves to doing one thing only. This is the  single-responsibility principle. In case of fibonacci(), I expect this function to just calculate the desired Fibonacci number, and not deal with user input and output. This shortens the function and makes it easier to reason about. So:
def fibonacci(terms):
    count = 0
    x = 0
    y = 1
    z = x + y

    while count < terms:
        yield x
        z = x + y
        x = y
        y = z
        count += 1

Instead of printing the values, it yields them. To print the values, use the following:
terms = int(input("How many terms? "))
for number in fibonacci(terms):
    print(number)

You can even go futher and make fibonacci() an infinite generator, and leave it up to the caller to decide when to stop:
import itertools

def fibonacci():
    x = 0
    y = 1

    while True:
        yield x
        z = x + y
        x = y
        y = z

terms = int(input("How many terms? "))
for number in itertools.islice(fibonacci(), terms):
    print(number)

Here I used itertools.islice() to take only the first terms items from the generator.
If you want to handle invalid input gracefully, I suggest you write another function whose responsibility is asking the user for a postivive integer:
def get_positive_integer(prompt):
    while True:
        user_input = int(input(prompt))
        if ...:
            return user_input
        print("Enter a positive integer.")

Avoid using os.system()
Using os.system() to clear the screen is very inefficient and not portable. Of course it is prettier to clear the screen, but if it is not really important for your program, I would just not do it; portability is more important.


When you feel yourself going to use a cast, especially when combined with an if on the save variable, then you most likely want to use a match.

def insert(node: Tree[Char], value: Char): Tree[Char] = {
  node match {
    case End => // hand end case here
    case n: Node[Char] => // handle the node case here
    case _ => // throw an exception, or do nothing if this case can't happen
  }
}

Now the node case is complicated enough that you might extract a method for it. Otherwise, it looked fine to me.


Turning a data class with too much functionality into a POD - SysLat Cleanup #2

Suggestions:

Make export data a static method, pass in the object to be exported.
Do not have a member variable just to store JSON data; generate the JSON only as part of the export method.


Handle multiple menu click events in one Sub

See this Stack Overflow answer, where Cody Gray explains the syntax for the two ways of registering event handlers in vb.net:

  The first involves the use of the Handles keyword, which you append to the end of the event handler method's definition. [...] The second involves the explicit use of the AddHandler statement, just like += in C#.

This means instead of statically declaring all the handled events with a Handles keyword, you write code to add the handlers yourself:

Public Sub New()

    InitializeComponents()

    AddHandler mnuEnglish.Click, AdressOf mnuEnglish_Click
    AddHandler mnuFrench.Click, AdressOf mnuEnglish_Click
    '...

End Sub

Or even better, loop through the menu items in that menu object, and assing all Click events the AdressOf mnuEnglish_Click, so you won't have the opportunity to forget updating that piece of code when you add a new language / menu item in the designer:

Public Sub New()

    InitializeComponents()

    ForEach item As ToolStripMenuItem In mnu.DropDownItems.OfType(Of ToolStripMenuItem)()
        AddHandler item.Click, AdressOf mnuEnglish_Click
    Next

End Sub


Returning postal codes between the ones that are given

Reusability
As it stand your code compute zip codes between '79-900' and '80-155'. In case you want to change these bounds, you have to modify the code itself. Which means you can even call this function twice with different bounds within the same program.
You should instead make x and y parameters of the function:
def codes(x, y):
    # Rest of the code

And then you can call it like:
codes('79-900', '80-155')
codes('80-998', '81-003')

You can also benefit from using more descriptive variable names here; like begin and end or start and stop. Same applies for the function name, something like zip_codes_range is more meaningful.
Range
The name zip_codes_range is not chosen arbitrarily. It is because it expand over the name of the range function. And this is because, in the end, zip codes can be viewed as integers and the range function is meant to generate "integers between the ones that are given" (note how this is close to your title).
So the idea is to just parse the zip codes into integers and then feed them to range so it can generate intermediate values:
def int_from_zip(zip_code):
    parts = zip_code.split('-')
    value = ''.join(parts)
    return int(value)

def zip_codes_range(start, stop, step=1):  # Mimic `range` signature
    start = int_from_zip(start)
    stop = int_from_zip(stop)
    for zip_code in range(start, stop+1, step):
        begin = zip_code // 1000
        end = zip_code % 1000
        print('{:02}-{:03}'.format(begin, end))

You could also use begin, end = divmod(zip_code, 1000) instead of the 2 lines process.
Print vs return
To make your function more useful, you should separate concerns. This function should only serves to compute the needed zip codes and return them somehow; the printing of the values being done in another part of the code.
This allow any program using this function to use the value as they intend to, and not necessarily print them.
The first thing you can do is return a list of all the requested values:
def int_from_zip(zip_code):
    return int(''.join(zip_code.split('-')))

def zip_codes_range(start, stop, step=1):
    start = int_from_zip(start)
    stop = int_from_zip(stop)
    return [
        '{:02}-{:03}'.format(*divmod(zip_code, 1000))
        for zip_code in range(start, stop+1, step)
    ]

if __name__ == '__main__':
    for code in zip_codes_range('79-900', '80-155'):
        print(code)

But you could consider using generators, the same way range changed from returning a list in Python 2 to being a generator in Python 3:
def zip_codes_range(start, stop, step=1):
    start = int_from_zip(start)
    stop = int_from_zip(stop)
    for zip_code in range(start, stop+1, step):
        yield '{:02}-{:03}'.format(*divmod(zip_code, 1000))


Python, given a word, generate a list with one letter changing for each letter in alphabet

Just use regular expressions!

import re
# creating regular expression with "any" letter at 3 position
r = re.compile("co[a-z]e")
# and taking all the matches:
matches = r.findall("some of my code, nope to come, cofe and cole, or cope and dome, rope and hope!")
count = len(matches)
print(count)


Random positioning and movement of a rabbit on a table

Sampling

This is your approach to placing the rabbit randomly on a circular table:

startingpoint = RandPoint()

while not(startingpoint.check()):
    startingpoint = RandPoint()

How many tries does it take to place a rabbit?  Each attempt succeeds with a probability \$p = \dfrac{\pi}{4}\$, so the expected number of tries is

$$\frac{\pi}{4}\left(1 + 2\left(1 - \frac{\pi}{4}\right) + 3\left(1 - \frac{\pi}{4}\right)^2 + 4\left(1 - \frac{\pi}{4}\right)^3 + \ldots\right) \approx 1.273$$

Can it be done without retrying?  Yes.  You can use

$$\begin{array}{rl}
x =& \sqrt{r}\ \cos \theta \\
y =& \sqrt{r}\ \sin \theta
\end{array}$$

… where \$r\$ is sampled uniformly from the interval [0, 1] and \$\theta\$ is sampled uniformly on [0, 2π].

Which is better for performance: taking a chance with looping (and consuming 27% more random numbers), or performing a square root and trigonometric calculations?  It's hard to say in advance, but it turns out that the performance of my solution below is about the same.  I think that the \$\langle \sqrt{r}\ \cos \theta, \sqrt{r}\ \sin \theta\rangle\$ method is more elegant.

Critique

In Python 2, classes should explicitly derive from object.

In RandPoint, x = 0 and y = 0 are unnecessary, and in fact misleading.  Those variables are not the same as self.x and self.y.

In this case, math.pow() should be written more succinctly using the exponentiation operator, since you don't need the validation that math.pow() performs.  You also don't need math.sqrt() in check().

move() and check() are specific to the exercise at hand and feel out of place in a RandPoint class.  Furthermore, based on just the method name, it's not obvious what check() would check.  Perhaps is_in_unit_disc() would be clearer, but I would choose to reorganize that code altogether.

passed is a count, so it should be an int rather than a float.  A better way to count a bunch of boolean values is to use the built-in sum() function with a generator expression.

The idiom to repeat something num times is for _ in xrange(num): ….  By starting a 1, you skipped one iteration.

To print the result, don't do so many string concatenations.  You can use str.format() instead.

Suggested solution

import math
import random

class Point(object):
    @staticmethod
    def random_polar(r=None):
        """Pick a point in a random direction with some magnitude.
           If r=1, this samples a unit circle uniformly.
           If r=None, this samples a unit disc uniformly."""
        if r is None:
            r = random.uniform(0, 1)**0.5
        theta = random.uniform(0, 2 * math.pi)
        return Point(r * math.cos(theta), r * math.sin(theta))

    def __init__(self, x, y):
        self.x = x
        self.y = y

    def add(self, other):
        self.x += other.x
        self.y += other.y
        return self

def rabbit_trial():
    """Rabbit at a random point in the unit disc moves one unit in
       a random direction.  Return True if the rabbit is still in
       the unit disc, or False if the rabbit is outside the disc."""
    p = Point.random_polar().add(Point.random_polar(r=1))
    return p.x**2 + p.y**2 < 1**2

trials = int(raw_input("How many trials would you like? "))
inside = sum(rabbit_trial() for _ in xrange(trials))
print '{} tries, {} passes, {}%'.format(trials, inside, 100.0 * inside / trials)


Base converting with strings from bases 1 to 36 to bases 1 to 36

I don't see anything particularly wrong with your code, except that you could benefit from using a StringBuilder to create the String to return, instead of appending to new Strings each time in

for (; multi >= 1; sum %= multi, multi /= toBase) {
    converted += baseinfochar[sum / multi];
}

But, in your case, you need to support only 32-bit signed integer. So it would be way easier to use built-in methods to make that conversion:

public static String convBase(String toConvert, int fromBase, int toBase) {
    return Integer.toString(Integer.parseInt(toConvert, fromBase), toBase);
}

This parses the given String from the given base and converts it back into a String with the target base. Integer.parseInt(s, radix) works for all bases between 2 and 36, like your requirement. It doesn't throw an exception on an invalid base; it defaults to 10, but you could add a simple check for that. Similarly Integer.toString(i, radix) works for base 2 to 36, defaulting to base 10 in case of invalid base.

I ran a JMH benchmark of your current code versus that simple solution on random integers between 0 and 100,000, 1,000,000 and 10,000,000. The benchmark simply converts the number from base 10 to 8; you could make it accross all bases but the result would be pretty much the same. Here are the results:

Benchmark               (length)  Mode  Cnt    Score    Error  Units
StreamTest.convBase       100000  avgt   30  139,514 ± 11,276  ns/op
StreamTest.convBase      1000000  avgt   30  151,537 ±  6,123  ns/op
StreamTest.convBase     10000000  avgt   30  176,489 ±  8,768  ns/op
StreamTest.convBaseInt    100000  avgt   30   51,728 ±  3,473  ns/op
StreamTest.convBaseInt   1000000  avgt   30   54,398 ±  2,329  ns/op
StreamTest.convBaseInt  10000000  avgt   30   64,694 ±  6,240  ns/op

They show that the built-in solution is actually faster than your current code, about 3 times faster. I also ran it with making the StringBuilder change mentioned above but the results were practically the same.

Here's the benchmark code:

import java.util.concurrent.ThreadLocalRandom;
import java.util.concurrent.TimeUnit;

import org.openjdk.jmh.annotations.Benchmark;
import org.openjdk.jmh.annotations.BenchmarkMode;
import org.openjdk.jmh.annotations.Fork;
import org.openjdk.jmh.annotations.Level;
import org.openjdk.jmh.annotations.Measurement;
import org.openjdk.jmh.annotations.Mode;
import org.openjdk.jmh.annotations.OutputTimeUnit;
import org.openjdk.jmh.annotations.Param;
import org.openjdk.jmh.annotations.Scope;
import org.openjdk.jmh.annotations.Setup;
import org.openjdk.jmh.annotations.State;
import org.openjdk.jmh.annotations.Warmup;

@Warmup(iterations = 10, time = 700, timeUnit = TimeUnit.MILLISECONDS)
@Measurement(iterations = 10, time = 700, timeUnit = TimeUnit.MILLISECONDS)
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
@Fork(3)
public class StreamTest {

    @State(Scope.Benchmark)
    public static class StringContainer {

        @Param({ "100000", "1000000", "10000000" })
        private int length;

        private String value;

        @Setup(Level.Iteration)
        public void setUp() {
            value = String.valueOf(ThreadLocalRandom.current().nextInt(length));
        }

    }

    private static final String BASEINFO = "0123456789abcdefghijklmnopqrstuvwxyz";

    public static String convBase(String toConvert, int fromBase, int toBase) throws NumberFormatException {
        if (toBase > 36 | fromBase > 36 | toBase < 2 | fromBase < 2) {
            throw new NumberFormatException("Bases specified must be less than 36 and greater than 2.");
        }
        String converted = "", thisBase = BASEINFO.substring(0, fromBase);
        char[] argument = toConvert.toCharArray(), baseinfochar = BASEINFO.toCharArray();
        int sum = 0, i = argument.length, multi = 1, baseind;
        for (; i > 0; multi *= fromBase) {
            baseind = thisBase.indexOf(argument[--i]);
                    if (baseind == -1)
                        throw new NumberFormatException("Character " + argument[i] + " is not a valid number in base " + toBase);
                    else
                        sum += multi * baseind;
        }
        multi = (int) (Math.log(sum) / Math.log(toBase));
        for (i = multi, multi = 1; i > 0; i--)
            multi *= toBase;
        for (; multi >= 1; sum %= multi, multi /= toBase) {
            converted += baseinfochar[sum / multi];
        }
        return converted;
    }

    public static String convBaseInt(String toConvert, int fromBase, int toBase) {
        return Integer.toString(Integer.parseInt(toConvert, fromBase), toBase);
    }

    @Benchmark
    public String convBase(StringContainer container) {
        return convBase(container.value, 10, 8);
    }

    @Benchmark
    public String convBaseInt(StringContainer container) {
        return convBaseInt(container.value, 10, 8);
    }

}


Interesting questions.

Is setting 'style' still considered bad practice when creating elements through templating?

Only if you cannot reasonably use css classes (which is your case)
Should I be setting the style using jQuery after I generate the HTML from the template?

Since you know the height and size prior to generating, you should make this part of the template

Also, this :

Hello World

Only accesses data under data.get('racks').at(0), so that is what you should provide to the template ( as say, data ), then your template becomes

Hello World


Any way to make this recursive function better/faster?

Just to round out the other answers: I would be inclined to write your solution like this:

static IEnumerable<T> DepthFirstTreeTraversal<T>(T root, Func<T, IEnumerable<T>> children)
{
    var stack = new Stack<T>();
    stack.Push(root);
    while(stack.Count != 0)
    {
        var current = stack.Pop();
        // If you don't care about maintaining child order then remove the Reverse.
        foreach(var child in children(current).Reverse())
            stack.Push(child);
        yield return current;
    }
}

And now to achieve your aim, you just say:

    static List<Channel> AllChildren(Channel start)
    {
        return DepthFirstTreeTraversal(start, c=>c.Children).ToList();
    }

Now you have a more general-purpose tool that you can use to get a depth-first traversal of any tree structure, not just your particular structure.

Another nice feature of my solution is that it uses a fixed amount of call stack space. Even if your hierarchy is twenty thousand deep, you never run out of stack space because the method is not recursive to begin with. All the information that would be needed for recursion is stored on the "stack" data structure instead of in activation records on the real call stack.


There is an illegal comma before FROM.

I would put bank.isCurrent = 1 in the WHERE-clause instead of as a join condition.

For consistency, I would reverse the equalities in the join conditions:

JOIN loan ON loan.personId = bank.personId
JOIN offer on offer.loanId = loan.loanId

… to match …

JOIN person ON person.personId = bank.personId
JOIN bankInstitution ON bankInstitution.bankInstitutionId = bank.bankInstitutionId

None of the above affects performance.  What you have is a very ordinary joining of tables, and your query expresses it in the usual way.  There's not much more that can be done for performance by tweaking the query.

What you should do, though, is ensure that all of the joins are being performed using indexes, not full-table scans.  You can verify that by prepending ANALYZE to your SELECT, which will give you the query execution plan.  If any of the joins is being performed with a full-table scan, create an index on the relevant column.  If your schema is properly defined, with the PRIMARY KEYs and FOREIGN KEYs declared, then you should already be fine, I think.


This code was very well organized and was extremely easy to follow along.

Looks to me that board is just a 1-based array.

It would be easier (and it makes more sense) to use an array, instead of an object.

But don't change how you are asking the user for input. You want to translate the user input one number back, so if the user enters "1" for box number 1, just subtract 1 from the integer version of the input and then pass it to markBoard.

if (validateMove(result.position) === true) {
    markBoard(result.position - 1, player);
    printBoard();

In your markBoard function, there is no point in calling .toUpperCase as the player variable passed in from playTurn is already a capital letter.

You are making validateMove too complicated. Just do:

return isInt(position) && board[position] === ' ';

If the expression evaluates to true, it will return true. If it evaluates to false, it will return false.

Note: I omitted the === true because JavaScript automatically checks for something to equal true.

It doesn't look like you have anything to check if the user enters an invalid number.

I would add something to the end of validateMove to check to make sure that the option is in bounds.

Here is what I came up with:

return isInt(position) && board[position] === ' ' && (position >= 0 && position <= 8);

It would be >= 1 && <= 9 if you continued to use the object instead of the array for storing squares of the board

This might help your efficiency:

Try moving this conditional from checkWin

if (markCount === 3) {
    return true;
}

Outside of the inner for loop so you aren't doing a conditional check with each iteration.

In checkWin, you do a lot of looping. In each loop, you are destroying and re-making variables as they exit and leave the scope (the loop).

This is very optimal. To help this, move the declarations of markCount, i, and j to the very top of the function (before the loops). You can set them to 0 when they are needed.

Here is what I am saying:

function checkWin(player) {
    var i, j, markCount
    for (i = 0; i < winCombinations.length; i++) {
        markCount = 0;
        for (j = 0; j < winCombinations[i].length; j++) {
            if (board[winCombinations[i][j]] === player) {
                markCount++;
            }
            if (markCount === 3) {
                return true;
            }
        }
    }
    return false;
}

Notice how I removed the vars?

I'm a little confused by what your isInt function is trying to accomplish.

To make it simpler, just run isNaN on the position. It will return NaN (which acts like false in a conditional) if the user entered a string, rather than a number.


private void create (List<? extends T> items) {
    root = new TreeNode<T>(null, items.get(0), null);

This crashes if an empty list is passed in. And since you have this function...

public MorrisInOrder(List<? extends T> items) {
    create(items);
}

It's entirely possible for such a thing to happen.

final int left = 2 * i + 1;
final int right = 2 * i + 2;

right can be set to left + 1 instead.

current  = current.right;

and

TreeNode<T> current  = root;

There's an extra space in here.

            /*
             * found the in-order predecessor.
             */
            while (predecessor.right != null && predecessor.right != current) {
                predecessor = predecessor.right;
            }

You're using a block-style comment for single line comment. Why?


Divide a 3D matrix into multiple smaller 3D matrix

Just a quick shot regarding performance.....
The code is calculating
xIndex * xSize xSize*ySize*zSize times
yIndex * ySize xSize*ySize*zSize times
zIndex * zSize xSize*ySize*zSize times
while it only should be calculated once for each iteration of the while loop.
In addition, x only changes in the first for loop and y only changes in the second for loop.


Resize renderer on browser window size change

I think your code style is fine. If you are happy with this method, I think you've written it pretty well.

As potential upgrades, here are two alternative methods that better obey the Rules of Clarity and Simplicity.

First, is there a reason you're using so much machinery to get the window size? This would be clearer with fewer moving parts. Anyone coming to your codebase knowing only JavaScript/CoffeeScript will understand what is happening here:

# setup canvas
canvas = document.createElement 'canvas'
document.body.appendChild canvas

# update ui every frame
tick = ->
  resize()
  requestAnimationFrame tick
requestAnimationFrame tick

resize = ->
  # only resize canvas if necessary
  canvas.width = window.innerWidth if canvas.width isnt window.innerWidth
  canvas.height = window.innerHeight if canvas.height isnt window.innerHeight

There's a fiddle of this here.

You could also employ modules that throttle redraws and polyfill requestAnimationFrame
like this example. I would argue that this method would also be clearer for a newcomer to your codebase.

raf = require 'raf' # requestAnimationFrame polyfill
throttle = require 'lodash.throttle'

# only redraw every 200 milliseconds
raf().on 'data', throttle(resize, 200)

See it in action (as JavaScript) on RequireBin here.


welcome to code review.

Your organization is clean, I mostly just see a few holes in your logic:

Combining groups - A group applies the same operator (and, or) to all rules in it.  e.g. rule1 AND rule2 AND rule3.  However, since groups cannot contain other groups, you cannot combine operations, e.g. rule1 AND (rule2 OR rule3)
Missing Operator/condition - you should add NOT to your enum(s) of operators and/or conditions
Rule Type - I'm not sure what the purpose of this enum is.  If you think that you need it, you could leave it is as, or alternatively make an IRuleParent interface, then make ConditionRuleParent and GroupRuleParent subclasses (this would be the more C#-ish way to model it).  By doing it the swcond way, you can declare the type (condition/group) in the class name when you instantiate it, rather than tacking it on as a mandatory argument.
Clunky syntax - it's object oriented, but very verbose to use and mentally parse, which leads us to...
Greenspun's tenth rule - Any sufficiently complicated C or Fortran program contains an ad-hoc, informally-specified, bug-ridden, slow implementation of half of Common Lisp. (or LINQ in this case).  While there are benefits to rolling your own implementation, you are basically recreating a subset of LINQ (the fluent syntax style).  If you want something like LINQ, just use LINQ.  If you are rolling your own, you might want to spend a little bit of time learning how it works (from a user point of view, at a minimum) and then steal ideas from it as you write your own version.


To check your code against coding standard and some error detection, I'd suggest you pylint, if you don't know it already.
Running pylint on your original code gives a list of things to check and a score 7.29/10 which is a good start.

What you should fix:

add docstrings at module, class and function level.
remove temporary variables when used just once.

r = requests.get(url, params=params)
return r

return requests.get(url, params=params)

prefer string formatting over concatenation.

url = API_URL + '/products/' + product_id + '/candles'
url = '%s/products/%s/candles' % (API_URL, product_id)

no need to initialize each dictionary's key separately.

order = {}
order['side'] = side
order['price'] = price
order['size'] = size
order['product_id'] = product_id
order['type'] = order_type

order = {
    'side': side,
    'price': price,
    'size': size,
    'product_id': product_id,
    'type': order_type
}

consider parameter injection for the GDAXExchangeAuth instance in all your functions, it will simplify testing or reusing of the code with a different Authentication method.

Pylint complains about GDAXExchangeAuth class having too few public methods, but in this case I like the usage of a function class and there is no real need of other methods, so for me it's fine.

I assume this is probably just an example, but in any case I suggest also getting sensitive information from environment variables, something like

    import os

    API_PASSPHRASE = os.environ['API_PASSPHRASE']


Library loaded twice

If library.txt exists, it will get loaded twice:

if os.path.isfile('library.txt'):
    library = load_library()
else:
    # download the file ...

library = load_library()

I guess this was just an oversight, with a simple fix:

if not os.path.isfile('library.txt'):
    # download the file ...

library = load_library()

Filter while loading

Instead of loading the library first and then filtering out the commented lines, it would be better to filter as you read:

with open('library.txt', 'r') as library:
    return [line for line in library if not line.startswith(';;;')]

Use generator expressions for better performance

It's a pity to loop over the entire list and filtering when you will only use the first match:

phonemes = [y for y in library if y.startswith(string)][0][len(string):]

You would do better with a generator expression,
which is more or less as simple as replacing the surrounding [...] with (...).
However, generators are not subscriptable, so to get the first element you must use the next(...) builtin:

phonemes = next(y for y in library if y.startswith(string))[len(string):]


Code for showing installed python packages/modules

#!/usr/bin/env python
#
# This script will run on Python 2.7.x and later
#
# By default, iter_modules looks in sys.path which is initialized from
# the environment variable PYTHONPATH, plus an installation-dependent default.
from pkgutil import iter_modules
import sys

def parse_command_line():
    import argparse # added to Python 2.7 to replace optparse

Its typically considered best to import all modules at the beginning. Avoid importing inside functions. Also, the python history lesson was pointless.

    python_version = sys.version

Why are you doing this? Why not just use sys.version when you need this?

    parser = argparse.ArgumentParser(description='Use this script to view the installed packages and/or modules for Python version: '+python_version)
    parser.add_argument('--type',dest='list_type',type=str,default='packages',help='Can be \'packages\' (default), \'modules\', or \'both\'')
    parser.add_argument('--ncols',type=int,default=2,help='Number of columns in output (default: 2)')
    args = parser.parse_args()
    return args

I'd combine these last two lines.

def select_packages():
    return [ module[1:] for module in iter_modules() if module[2] ],'Packages'

def select_modules():
    return [ module[1:] for module in iter_modules() if not module[2] ],'Modules'

def select_both():
    return [ module[1:] for module in iter_modules() ],'Packages and Modules'

These three functions are awkward. For one, when looping over tuples, its generally best to unpack the tuples. This will make the code easier to follow. They are also largely duplicate of each other.

options = { 'packages' : select_packages,
            'modules' : select_modules,
            'both' : select_both,
}

The python standard is to use ALL_CAPS for global constants.

def get_printed_names(pack_tuple):

pack_tuple is a bad name, I don't know what its in it.

    p_name = []

avoid abbreviations like p, it just makes it harder to see what's going on.

    for code in pack_tuple:
        if ( code[1] ):

You don't need the parens. It'd also be easier to follow if you used for name, ispkg in pack_tuple:

            p_name.append(code[0]+' (Package)')
        else:
            p_name.append(code[0]+' (Module)')
    return p_name

Why does this function operate on a list of packages?

def fmt_cols(my_list, cols):

my_list is a bad name. I have no idea what's in the list.

    import math
    max_width = max(map(lambda x: len(x), my_list))

lamba x: len(x) is just len(x). So you can do max(map(len, my_list))

    col_length = int(math.ceil(len(my_list)/cols+1))
    justify_list = list(map(lambda x: x.ljust(max_width), my_list))

In python 2.x, map produces a list already. (Perhaps you're aiming to work on Python 3 as well?) It's also probably better to use a list comprehension.

    lines = [ ' '.join(justify_list[i::col_length]) for i in range(col_length) ]
    print("\n".join(lines))

There's not much point in putting everything in a line just to print it. I'd just print it directly in a for loop.

def sort_func(a_str):
    tmp_str = a_str.lower() # fine with ASCII subset of characters
    for index,letter in enumerate(a_str):
        if letter == '_':
            continue
        else:
            return tmp_str[index:]

It seems to me that you are just stripping off the _ so instead, you can use return a_str.lower().lstrip('_')

# main()

Normal style is to put this in a main function.

arguments = parse_command_line()
detail_level = arguments.list_type
try:
    installed_code,tag = options[detail_level]()
except KeyError:
    print('Error: detail_level argument must be \'packages\', \'modules\', or \'both\'')
    sys.exit()

Why don't you have argument parser check this?

ncols = arguments.ncols

Why are you copying this in a local?

print('\nFor output options type \'python_pkginfo.py -h\'')
print('\nInstalled '+tag+' for Python version '+sys.version+':\n')
print('To see package versions try: pip freeze | sort')
print('after issuing the appropriate \"setpkgs -a python\" command\n')
print_list = get_printed_names(installed_code)
print_list.sort(key=sort_func)
fmt_cols(print_list,ncols)

My rewrite of your script:

#!/usr/bin/env python
#
# This script will run on Python 2.7.x and later
#
# By default, iter_modules looks in sys.path which is initialized from
# the environment variable PYTHONPATH, plus an installation-dependent default.
from pkgutil import iter_modules
import sys
import argparse
import math

def parse_command_line():
    parser = argparse.ArgumentParser(description='Use this script to view the installed packages and/or modules for Python version: ' + sys.version)
    parser.add_argument('--type',dest='list_type',type=str,default='packages',help='What to list', choices = ['packages', 'modules', 'both'])
    parser.add_argument('--ncols',type=int,default=2,help='Number of columns in output (default: 2)')
    args = parser.parse_args()
    return args

LIST_TYPE = {
    'packages': (True, False, 'Packages'),
    'modules': (False, True, 'Modules'),
    'both': (True, True, 'Packages and Modules')
}

def find_modules(include_packages, include_modules):
    for module_loader, module_name, is_package in iter_modules():
        if is_package and include_packages:
            yield module_name, is_package
        elif not is_package and include_modules:
            yield module_name, is_package

def formatted_name(module):
    name, is_package = module
    if is_package:
        return name + ' (Package)'
    else:
        return name + ' (Module)'

def print_cols(cells, cols):
    max_width = max(map(len, cells))
    rows = int(math.ceil(len(cells)/cols+1))
    for row in range(rows):
        print(' '.join(x.ljust(max_width) for x in cells[row::rows]))

def main():
    arguments = parse_command_line()
    include_packages, include_modules, tag = LIST_TYPE[arguments.list_type]
    installed_code = sorted(
        find_modules(include_packages, include_modules),
        key = lambda module: module[0].lower().lstrip('_'))

    print('\nFor output options type \'python_pkginfo.py -h\'')
    print('\nInstalled '+tag+' for Python version '+sys.version+':\n')
    print('To see package versions try: pip freeze | sort')
    print('after issuing the appropriate \"setpkgs -a python\" command\n')
    print_cols(list(map(formatted_name, installed_code)), arguments.ncols)

if __name__ == '__main__':
    main()


Creating an interactive tour for a website

I would go for an approach as follows:

clickme

clickme

then some javascript fun:

var GrumblingSteps = (function(){
    var grumbleMagic = function($grumbleTarget) {
        //create grumble
        $grumbleTarget.grumble({
            text : $grumbleTarget.data('grumble-text'),
            angle : $grumbleTarget.data('grumble-angle'),
            distance : $grumbleTarget.data('grumble-distance')
        });
        //add click handler to the next in line if any
        if ( $grumbleTarget.data('grumble-next') ) {
            $nextGrumble = $('#'+$grumbleTarget.data('grumble-next'));
            $nextGrumble.on('click', function() {
                $grumbleTarget.grumble('hide');
                grumbleMagic($(this));
            });
        }
    }

    return function(firstGrumble) {
        var $firstGrumble = $(firstGrumble);
        grumbleMagic($firstGrumble);
    }
})();

Someone might be able to change this to a jQuery plugin or so. But for now you use it as follows:

GrumblingSteps('#grumble-1');

  It might have some errors in it since I coded this directly inside this comment box ;)


You can use an Extract Method refactoring to get rid of the duplicate code:

private static string GetExportFilePath(string csvFilePath, DateTime date, string fileSuffix)
{
    return string.Format("{0}{1:yyyy-MM-dd}-{2}.csv", csvFilePath, date, fileSuffix);
}

I would add those selectedExportType and path variables as parameters to the Export() method. I hope they are not used anywhere. It is usually pretty subjective whether to add something as method parameters or leave it as class member, but here I would definitely pass them as parameters. I assume this class is a “View” since you have button click event handlers, and I also tend to move such methods out from the views.

If we use these two ideas, your event handlers would be simplified to:

private void btnBuildingList_Click(object sender, EventArgs e)
{
    var path = GetExportFilePath(csvFilePath, datDate.DateTime, "BuildingList.csv");
    Export((int)ExportType.Building, path);
}

private void btnOwnerList_Click(object sender, EventArgs e)
{
    var path = GetExportFilePath(csvFilePath, datDate.DateTime, "OwnerList.csv");
    Export((int)ExportType.Persons, path);
}

private void btnFacts_Click(object sender, EventArgs e)
{
    var path = GetExportFilePath(csvFilePath, datDate.DateTime, "FactsData.csv");
    Export((int)ExportType.Facts, path);
}


What is a good way to structure mark-up generating code and avoid the example mess?

A good practice is Observe which part of the code does not change Extract that keep it in a separate method.  Pass the variable Which changes as an input to that method.

    public string CommonMethod(string formattedString)
    {
        string title = GetTitle();
        string mediaType = GetMediaType();
        string description = GetDescription();
        /***string releaseYear = GetReleaseYear();***/ this is not required.
        string contributors = GetContributors();
        string image = GetImage();
        string genres = GetGenres();
        string ratings = GetRatings();

        string top = **formattedString**;

        StringBuilder metadata = new StringBuilder();
        metadata.AppendLine(contributors + "
");
        metadata.AppendLine(genres + "

");
        metadata.AppendLine(ratings + "

");
        metadata.AppendLine(description);

        return string.Format("{0}

{1}	{2}

", top, metadata.ToString(), image);
    }

each time the difference might be on a different line or couple of lines.

This part is difficult to answer. Even in that scenario, omit the one or two lines which may change, pass the things which may change as input.