Converting a SecureString to a byte array Does it allocates something that hasn't freed? Something that remains in memory? Except result byte array, ofc.

private unsafe byte[] ToByteArray(SecureString secureString, Encoding encoding = null)
{
    if (secureString == null)
        throw new ArgumentNullException(nameof(secureString));
    if (encoding == null)
        encoding = Encoding.UTF8;

    int maxLength = encoding.GetMaxByteCount(secureString.Length);

    IntPtr bytes = Marshal.AllocHGlobal(maxLength);
    IntPtr str   = Marshal.SecureStringToBSTR(secureString);

    try
    {
        char* chars = (char*)str.ToPointer();
        byte* bptr  = (byte*)bytes.ToPointer();
        int len     = encoding.GetBytes(chars, secureString.Length, bptr, maxLength);

        byte[] _bytes = new byte[len];
        for (int i = 0; i < len; ++i)
        {
            _bytes[i] = *bptr;
            bptr++;
        }

        return _bytes;
    }
    finally
    {
        Marshal.FreeHGlobal(bytes);
        Marshal.ZeroFreeBSTR(str);
    }
}

Although you have done a decent job, you could do this in a better way, without even to use unsafe.

First thing to mention, you should always use braces {} although they are optional for single lined if statements. This will just make your code less error prone which is in such a security context wanted.

Instead of using if (encoding==null) { encoding = Encoding.UTF8; } you can use the null coalescing operator ??.

How I use this usually (integrated in your method)

private byte[] ToByteArray(SecureString secureString, Encoding encoding = null)
{
    if (secureString == null)
    {
        throw new ArgumentNullException(nameof(secureString));
    }

    encoding = encoding ?? Encoding.UTF8;

    IntPtr unmanagedString = IntPtr.Zero;
    try
    {
        unmanagedString = Marshal.SecureStringToGlobalAllocUnicode(secureString);

        return encoding.GetBytes(Marshal.PtrToStringUni(unmanagedString));
    }
    finally
    {
        if (unmanagedString != IntPtr.Zero)
        {
            Marshal.ZeroFreeBSTR(unmanagedString);
        }
    }
}

Based on the valid comment

  Marshal.PtrToStringUni allocates managed string so until GC clear this we will have unprotected string with sensitive data in memory.

I would like to suggest another aproach which depends on your implementation. The main problem I see is that with your implementation the returned byte[] needs to be cleaned up from the caller of the code.

How about having a class which implements IDisposable which is wrapping the shown method ?

public sealed class SecureStringWrapper : IDisposable
{
    private readonly Encoding encoding;
    private readonly SecureString secureString;
    private byte[] _bytes = null;

    public SecureStringWrapper(SecureString secureString)
          : this(secureString, Encoding.UTF8)
    {}

    public SecureStringWrapper(SecureString secureString, Encoding encoding)
    {
        if (secureString == null)
        {
            throw new ArgumentNullException(nameof(secureString));
        }

        this.encoding = encoding ?? Encoding.UTF8
        this.secureString = secureString;
    }

    public unsafe byte[] ToByteArray()
    {

        int maxLength = encoding.GetMaxByteCount(secureString.Length);

        IntPtr bytes = IntPtr.Zero;
        IntPtr str   = IntPtr.Zero;

        try
        {
            bytes = Marshal.AllocHGlobal(maxLength);
            str   = Marshal.SecureStringToBSTR(secureString);

            char* chars = (char*)str.ToPointer();
            byte* bptr  = (byte*)bytes.ToPointer();
            int len     = encoding.GetBytes(chars, secureString.Length, bptr, maxLength);

            _bytes = new byte[len];
            for (int i = 0; i < len; ++i)
            {
                _bytes[i] = *bptr;
                bptr++;
            }

            return _bytes;
        }
        finally
        {
            if (bytes != IntPtr.Zero)
            {
                Marshal.FreeHGlobal(bytes);
            }
            if (str != IntPtr.Zero)
            {
                Marshal.ZeroFreeBSTR(str);
            }
        }
    }

    private bool _disposed = false;

    public void Dispose()
    {
        if (!_disposed)
        {
            Destroy();
            _disposed = true;
        }
        GC.SuppressFinalize(this);
    }

    private void Destroy()
    {
        if (_bytes == null) { return; }

        for (int i = 0; i < _bytes.Length; i++)
        {
            _bytes[i] = 0;
        }
        _bytes = null;
    }

    ~SecureStringWrapper()
    {
        Dispose();
    }
}

now the caller could use it like so

using (SecureStringWrapper wrapper = new SecureStringWrapper(secureString))
{

    byte[] _bytes = wrapper.ToByteArray();
    // use _bytes like wanted

}

A good read about the topic of SecureString: http://web.archive.org/web/20090928112609/http://dotnet.org.za/markn/archive/2008/10/04/handling-passwords.aspx

I guess the Finally-block is not executed if the return statement makes it leave before then. Due to this the free is not called.


Parsing JSON in one go I need to parse a simple JSON string (flat JSON, no hierarchy) for keys and values, and there is a system constraint that I cannot use any built-in JSON library and can only read a string once due to latency requirements. I need to use Python 2.7.x series and cannot use a higher version.

Currently, my major concern for below code is, I still do a bit more than one pass string parsing, since I need to go backward/forward to remove unnecessary characters around a word, which is what wordBeautify does.

This is a follow-up to "Json String Parsing"

def wordBeautify(word1, beginIndex, endIndex):
    noMeaningChars=[' ', '"', '{', ',', ':','}']
    while word1[beginIndex] in noMeaningChars:
        beginIndex+=1
    while word1[endIndex] in noMeaningChars:
        endIndex-=1
    return (beginIndex,endIndex+1)

def parseElegant2(str1):

    keyStr=''
    valueStr=''
    beginWord = False
    isKey=True
    beginIndex=0
    endIndex=0

    for i in range(len(str1)):

        if str1[i]==':':
            endIndex=i-1
            (x,y) = wordBeautify(str1, beginIndex, endIndex)
            keyStr=str1[x:y]
            print "key string " + keyStr
            beginIndex=i+1
        elif str1[i]==',' or str1[i]=='}':
            endIndex=i
            (x,y) = wordBeautify(str1, beginIndex, endIndex)
            valueStr=str1[x:y]
            beginIndex=i+1
            print "value string " + valueStr
            print 'key and value, '+ keyStr, valueStr

def parseElegant(str1):

    keyStr=''
    valueStr=''
    beginWord = False
    isKey=True
    beginIndex=0
    endIndex=0

    for i in range(len(str1)):

        if str1[i] == '"' and beginWord==False:
            beginWord=True
            beginIndex = i+1
        elif str1[i] == '"' and beginWord==True:
            beginWord=False
            endIndex=i
            if beginIndex<endIndex:
                print "get word, " + str1[beginIndex:endIndex]
        elif str1[i]==':':
            keyStr=str1[beginIndex:endIndex]
        elif str1[i]==',':
            valueStr=str1[beginIndex:endIndex]
            print 'key and value, '+ keyStr, valueStr

if __name__ == "__main__":

    #JSONString = '{ "id": 1, "name": "A green door", "price": 12.50, "tags": ["home", "green"]}'
    JSONString1 = '{ "id": "1", "name": "A green door", "price": "12.50", "tags": "home green"}'
    JSONString2 = '{ "id": 1, "name": "A green door", "price": 12.50, "tags": "home green"}'

    parseElegant2(JSONString1)
    parseElegant2(JSONString2)

First of all, let me say well done on the progress made from your first question to now!

Code Review is for reviewing code

As you seem to be seeking advice "about how to implement" an idea (i.e. streams), I think you may be better off asking on Programmers.SE, since that is a more appropriate place for questions about "software architecture and design" and "algorithm and data structure concepts".
Code Review is "not for questions about broken code, hypothetical code, or non-existent code, as such questions will be closed as off-topic." (taken from this help page).

I can see that you have code already, and are seeking to improve it (in a way that doesn't add any functionality) - so I consider your question to be on-topic.  Although your desire to learn about other ways of implementing it entirely may not be best placed on this site.

A Quick Review

Remove old code
You have included the (now unused) code for parseElegant (version 1).  While I can understand you not wanting to erase this, you have already linked to your previous question (which contains the old code).  Including version 1 here just adds ~24 lines for reviewers to have to scroll through.

Can't you just use strip?
The wordBeautify function (which you are seeking to make redundant/remove) seems to be VERY similar to the Python built-in string.strip method?

Consider the following code:

test="{:foo }"
(x,y)=wordBeautify(test,0,len(test)-1)
a=test[x:y]
b=test.strip(' "{,:}')

Result:  a and b both work out to be "foo"!

Use better variable names!
At present, your parseElegant2 function takes an argument called str1.  While it is obvious that this means something along the lines of the first string - it is not actually particularly helpful in knowing what that variable represents.

Something more like json, text_candidate, or candidate may be better - as it expresses that this is a long text containing json, or something that is a candidate to be processed into JSON.

You have a similar issue in wordBeautify, with word1.  This could just be named word, or similar.

Return is better than print
I've noticed that in your function you are not return'ing any values.  Instead you are just print'ing to the standard output (STDOUT).
Is this your goal? Or will your program eventually need to construct some kind of data structure (such as a dict) of the JSON values?

I would recommend refactoring your code to return a data structure, and have your function write the JSON values into that data structure.

A word on string stream processing

As @ferada commented, I think you should "consider using a stream, looking only at the (single) next character at a time and then build a state machine around it.", however your current program is not doing this.

Instead, your loop is going through the indices of the string.  And is (at present), also calling your wordBeautify function to do the same (with indices).

Rather than using for i in range(len(str1)):, as you currently are - I would recommend that you loop through the characters themselves (one at a time, as @ferada suggested).  Thus, your loop may look more like: for c in str1:

This way, you are essentially forced into only looking at the current character. (i.e. you are unable to increment/decrement any number value to go forwards or backwards in the string.)

Constructing a state machine

From the Wikipedia article about Finite-state machines:

  A finite-state machine (FSM) or simply state machine, is a mathematical model of computation used to design computer programs.  It is conceived as an abstract machine that can be in one of any finite number of states.  The machine is in only one state at a time, called the current state. It can change from one state to another when triggered, this is called a transition.  A particular state machine is defined by a list of its states, and the triggering condition for each transition.
   (Emphasis mine)

You can read the article for more background information, but essentially your are seeking to construct a state machine.  As you can read in the last sentence (that I've bolded), this requires:

a list of states
the trigger conditions for each state transition

What are the states?

Luckily, the official JSON website (JSON.org) actually gives us these exact things!
In the form of railroad diagrams.  Specifically, they tell us that:

  An object is an unordered set of name/value pairs. An object begins with { (left brace) and ends with } (right brace). Each name is followed by : (colon) and the name/value pairs are separated by , (comma).

  (source: json.org)

So we know that STRING is a state, for example.  They also tell us how we get into (and out of) the STRING state:

  A string is a sequence of zero or more Unicode characters, wrapped in double quotes...

  (source: json.org)

So we know that a STRING state is triggered when we encounter a " (quote) character. (And that it ends the same way.)
Note:  For now, let's ignore the backslash (\) escape style string components.

We also are told what a VALUE might be as well:

  A value can be a string in double quotes, or a number, or true or false or null...

  (source: json.org)

This gives us another state, namely NUMBER - since a value can be a number.

Some limitations (Let's not reinvent the whole wheel)

We already know that you only want to deal with "flat JSON (no hierarchy) for keys and values".
So, we'll forget about OBJECT and ARRAY in the diagram above. ;-)

Let's also (just to make it easy), ignore the fact that a value can be true, false, or null.  (i.e. it is either a string or a number).

The states so far

So far, we have been able to use the official JSON.org website diagrams to determine we have the following two states:

STRING
NUMBER

We also know that a string is triggered by the condition: c=='"' (where c is our stream's current character).

But how is a NUMBER triggered?
Well the JSON.org website has a rather complicated way of detecting numbers (which takes into account negative numbers, scientific notation, etc).  but for these purposes let's just say that a NUMBER is triggered by a sequence of 1 or more of the following characters: 0123456789. (the decimal point is included).

Other transitions

If we look back at the first railroad diagram:

We can see that the COLON (:) and COMMA (,) characters are also transitions between states..

Putting it all together

If we combine all of these states and transitions, plus the idea of a character (rather than index) loop, and the other improvements I've mentioned - we can produce a single function which is capable of processing a JSON string "all at once":

def parseElegant3(candidate):
    READY=0
    STRING=1
    NUMBER=3
    COLON=-1
    COMMA=-2

    search={'"':STRING,  ':':COLON,  ',':COMMA,  STRING:'"',  COMMA:','}
    NOP=" {\t}\r\n"
    stringIgnore='{,:}'
    numbers='0123456789.'

    result={}
    if not len(candidate):
        return result
    accumulator=""
    mostRecentKey=None

    state=READY
    for c in candidate:
        newState=state
        if state is READY:
            if c in NOP:continue
            if c in numbers:
                newState=NUMBER
            if c in search.keys():
                newState=search[c]
        if newState is COLON:
            newState=READY
            if len(accumulator):
                accumulator=accumulator.strip()
                result[accumulator]=None
                mostRecentKey=accumulator
                accumulator=""
            else:
                state=READY
                continue
        elif newState is COMMA:
            newState=READY
        elif newState is NUMBER and not state is NUMBER:
            accumulator+=c
        if state is STRING and newState is STRING:
            if c in stringIgnore:continue
            if c==search[STRING]:
                newState=READY
                if not mostRecentKey is None:
                    result[mostRecentKey]=accumulator
                    accumulator=""
                    mostRecentKey=None
            else:
                accumulator+=c
        elif state is NUMBER:
            if c in numbers:
                accumulator+=c
            else:
                if c in NOP:continue
                assert c == search[COMMA]
                try:
                    result[mostRecentKey]=int(accumulator)
                except ValueError:
                    result[mostRecentKey]=float(accumulator)
                accumulator=""
                mostRecentKey=None
                newState=READY
        state=newState
    return result

JSONString = '{ "id": 1, "name": "A green door", "price": 12.50, "tags": "home green"}'

parseElegant3(JSONString)

If it's possible for you to convert the JSON string to a dictionary data structure like so:

d = eval(JSONString)

then you could use the

string.strip()

and

string.replace()

methods on the individual keys.

You could then either work with the dict/JSON as is, or rebuild it to resemble a string if necesssary.


Toggle Function in JavaScript Here is my current toggle function:

$scope.toggleSelect = event => {
  $scope.selection = event.target.innerHTML;

  if ($scope.selection === 'Select All') {
     $scope.emailList = $scope.users.filter(user => !user.report.emailed);
  } else {
     $scope.emailList = $scope.emailList = [];
  }
};

Here are my thoughts and questions about the best practices:

I make an assignment to $scope.emailList in the if and else statement. Instead of separate assignments, is one assignment better? Refactoring to one assignment will pollute the code with unnecessary variables, but it may make the code more readable?

$scope.toggleSelect = event => {
  $scope.selection = event.target.innerHTML;
  const emailListUpdate;
  if ($scope.selection === 'Select All') {
    emailListUpdate = $scope.users.filter(user => !user.report.emailed);
  } else {
    emailListUpdate = $scope.emailList = [];
  }
   $scope.emailList = emailListUpdate;
};

The main benefit I see here is that anybody reading the code can skim to the last line and know the overall purpose. Overall, I don't see this to be a beneficial refactor since I don't think it adds additional readability and potentially makes it harder to follow. I would appreciate thoughts on this.
Ternary or if/else:

I reviewed a great post about the benefits and use cases of using ternary or if/else. Here is  what the code would look like refactored to a ternary:

$scope.toggleSelect = event => {
  $scope.selection = event.target.innerHTML;
  $scope.emailList = $scope.selection === 'Select All' ? $scope.users.filter(user => !user.report.emailed); : [];

};

Quoting from the article linked above:

  An if/else statement emphasizes the branching first and what's to be done is secondary, while a ternary operator emphasizes what's to be done over the selection of the values to do it with.

I feel the if/else feels more natural, but I'm not sure if that is just related to my lack of experience using ternary.
I have another toggleItem function used when a checkbox is clicked.

$scope.toggleItem = (event, user) => {
  if (event.target.checked) {
    $scope.emailList.push(user);
  } else {
    $scope.emailList = _.reject($scope.emailList, item => item._id === user._id);
  }

I've thought about combining both of my toggle functions but overall I think it is better to keep them separate instead of trying to make a generic toggle function with multiple use cases.

In the toggleItem function above, I think it is cleaner (and more obvious) to use an if/else instead of a ternary since I am only making an assignment in the else statement.

I'm trying to improve on writing cleaner code, If anyone has any input or thoughts on best practices to use here or how to clean up this code it would be appreciated.

Short and sweet is best.
Your statement

"The main benefit I see here is that anybody reading the code can skim to the last line and know the overall purpose"

That makes no sense at all, you say all that is needed to understand the functions is
$scope.emailList = emailListUpdate;

Nobody jumping into someone else code will just skim, the only people that skim code are those that know the code.
You can make a few assumptions.

All that read your code are competent coders.
All that read your code have read the project specs.
Every line of code will be read by a coder new to the code.

Example
The best code is brief as possible without being a code golf entrant.
Notes

Why innerHTML, should it not be textContent????
This function is not a toggle. It is based on selection value.
The ternary expression is too long, break the line so it does need to be scrolled
The ternary has a syntax error. Misplaced ;
the ; on the last line after "}" is redundant.

Code, best option.
$scope.selectEmailUsers = event => {
  $scope.selection = event.target.textContent;
  $scope.emailList = $scope.selection === "Select All" ?
       $scope.users.filter(user => !user.report.emailed); : [];
  //                                                    ^ remove syntax error
}; // << remove the ;

Some comments here are inspired by below line:

Programs must be written for people to read, and only incidentally for machines to execute. — Abelson and Sussman

When I first looked at the code, first thing I could not stop appreciating was avoiding the ternary operator. Using if makes it easier to setup correct expectations on what is happening without extra attention.
'Select All' is a CONSTANT which is deciding the method output. So, my suggestion is to move it to the top of the method and assign to a variable.
When the code contains conditional execution, having a single place that consumes the result of different blocks would be good choice. I see in your second block of code you made that change. $scope.emailList = emailListUpdate;
$scope.emailList = _.reject($scope.emailList, item => item._id === user._id); I review lot of code internally. No way I would accept code like this. It's not about how smartly we can write code, it's about how easy it is for others to understand. Separate the logic from return statements. Give it a meaningful name to convey what it is and why are you filtering.
$scope.users.filter(user => !user.report.emailed) is smart! But $scope.users.filter((user) => { return user.report.emailed === false }) is easier to understand what's happening.

Hope this helps!

Edit:
I am curious to understand why my answer deserve negative vote. Someone please take a minute to help me understand.


Project Euler #4 "Largest Palindrome product" in Python Given

  A palindromic number reads the same both ways. The largest palindrome made from the product of two 2-digit numbers is 9009 = 91 × 99.

  Find the largest palindrome made from the product of two 3-digit numbers.

Solution

import time

def reverse(num):
    dreverse = 0
    while (num > 0):
        dreverse = dreverse * 10 + num % 10
        num /= 10
    return dreverse

def is_palindrome(num):
    if (num < 0):
        raise ValueError('Non-negative integers only')
    return num == reverse(num)

def main():
    largest_palindrome = 0
    lower = 999
    while lower >= 100:
        upper = 999
        while upper >= lower:
            if upper * lower <= largest_palindrome:
                break
            if is_palindrome(upper * lower):
                largest_palindrome = upper * lower
            upper -= 1
        lower -= 1
    return largest_palindrome

start_time = time.time()
print main()
print("--- %s seconds ---" % (time.time() - start_time))
o/p: 906609
--- 0.0150721073151 seconds ---

I am including timing metric from now on. This solution seems \$O(N^2)\$. Can I improve it further?

You are recomputing the product upper * lower three times:

    while upper >= lower:
        if upper * lower <= largest_palindrome:
            break
        if is_palindrome(upper * lower):
            largest_palindrome = upper * lower

You should save it in a variable:

    while upper >= lower:
        product = upper*lower
        if product <= largest_palindrome:
            break
        if is_palindrome(product):
            largest_palindrome = product

Also, how about this version of is_palindrome:

def is_palindrome(n):
  s = str(n)
  return (s == s[::-1])

Update

Replace your loops with for and xrange:

largest = 0
for lower in xrange(999,99,-1):
    for upper in xrange(999,lower-1,-1):
        ...

It makes the intent of your code a lot clearer, and it is easier to correctly write the loop.

(The following is note quite right)

Finally, if upper*lower <= largest_palindrome, you actually return from the function:

        if upper * lower <= largest_palindrome:
            return largest_palindrome

The return will essentially break out of both while loops. Using just break will only break out of the inner loop. Of course, the remaining cases will execute very quickly since they will only last one iteration of the inner loop, but it is still a good optimization to know about.

I am going to suggest a solution that is 20x less perfomant than yours, because Python was really made to optimize programmer's time, not execution time. It should be noted that my solution is still under a second of execution time.

def is_palindrome(string):
    return string == ''.join(reversed(string))

A number is just a kind of string, so I wrote a more general (and, yes, slower) function to check for palindromes. The number should be converted to string before being passed in to the function.

The main body of this program is really close to natural langauge, it is just:

print(max(i * j for i in range(500, 999) for j in range(500, 999)
                if is_palindrome(i * j)))

it uses a generator expression, so the usage of memory is O(1) and the time complexity is O(N^2) as in your solution.

Almost all the time is spent in the is_palindrome function, using your function would make this as fast as your solution.


Simple Blackjack/21 game in console I made a simple Blackjack/21 game in C++. It does not use any fancy graphics, just the console output. It is as simple as it can get.

#include <iostream>
#include <cstdlib>
#include <ctime>
#include <algorithm>

void printData(int pTotal, int dCard) {

    std::cout << "\nYour total is " << pTotal;
    std::cout << "\nDealer has a " << dCard << " showing.\n";

}

int getCard() {

    return std::rand() % 11 + 2;

}

void turn(int& pTotal, char choice, bool& stay) {

    // If the user wants to hit
    if (choice == 'H' || choice == 'h') {

        // If the user hits 21 or above
        if ((pTotal += getCard()) >= 21) {

            std::cout << "Your total is " << pTotal;
            stay = true;

        }

        return;
    }

    // Since the only other option can be to stay, then stay
    stay = true;

}

int main() {

    bool stay;
    char choice = NULL;

    while (true) {

        std::cout << "================================";
        stay = false;

        // Initialize srand and get random card numbers
        std::srand(std::time(0));
        int dCard = getCard();
        int pTotal = getCard() + getCard();
        int dTotal = dCard + getCard();

        // Player's turn
        while (!stay) {

            printData(pTotal, dCard);
            std::cout << "[H]it or [S]tay?\n";
            std::cin >> choice;
            turn(pTotal, choice, stay);

        }

        stay = false;
        std::cout << "\n\nIt is now the dealer's turn!\n";

        // Dealer's turn
        while (dTotal <= 21 && !stay) {

            if (dTotal >= 17)
                stay = true;
            else
                dTotal += getCard();

        }

        // Display winner
        std::cout << "\n\nThe player has " << pTotal << ".\nThe dealer has " << dTotal << ".\n\n";
        if ((std::abs(21 - pTotal)) < (std::abs(21 - dTotal))) std::cout << "The player wins!\n";
        else std::cout << "The dealer wins!\n";

    }

}

Is this code efficient?
Is the code easy to read/understand and to follow?
Are variable names appropriate?

I want my money back!
Your total is 15
Dealer has a 4 showing.
[H]it or [S]tay?
s

It is now the dealer's turn!

The player has 15.
The dealer has 23.   <<=====

The dealer wins!     <<=====

Your casino is cheating! Also you're not handling aces, all face cards are supposed to have a value of 10, and there are 13 ranks in a suit but your getCard function can only return 11 different values (2 to 12).
This means that you can lose right off the bat:
$ ./a.out
================================
Your total is 24       <<======     :-(
Dealer has a 9 showing.
[H]it or [S]tay?

In turn, if the player enters h and doesn't bust, you're not setting stay - this happens to work given the code flow, but it looks odd. Set it before returning.
If the user enters gibberish, you always interpret that as stay. That's not user-friendly.
In main:
bool stay;
char choice = NULL;

That's not a valid initializer for a char - chars are not pointers. And leaving a variable (stay) uninitialized is not a good idea. You should move stay inside the loop, only declare it when you give it its first value. (I'd also use two separate variables for the dealer and the player.)
Remove choice from main altogether. Only turn needs it, so handle the user input in there completely. (And have turn return hit/stay so you don't need that out parameter.)
This shouldn't be inside the loop:
std::srand(std::time(0));

You should initialize the RNG only once, do that at the very start of main. C++11 introduced new random generation facilities, you should look into them.
These lines are too long:
// Display winner
std::cout << "\n\nThe player has " << pTotal << ".\nThe dealer has " << dTotal << ".\n\n";
if ((std::abs(21 - pTotal)) < (std::abs(21 - dTotal))) std::cout << "The player wins!\n";
else std::cout << "The dealer wins!\n";

Try something like:
// Display winner
std::cout << "\n\nThe player has " << pTotal
          << ".\nThe dealer has " << dTotal << ".\n\n";

if ((std::abs(21 - pTotal)) < (std::abs(21 - dTotal))) {
  std::cout << "The player wins!\n";
} else {
  std::cout << "The dealer wins!\n";
}

And once you fix your logic so that a bust directly ends the players turn, and a dealer bust makes the player win, you can get rid of the std::abs since both scores will be at most 21.

Handle the Aces! (That's my only problem, I got a 22 at start)


Find All Permutations of String in Scala I created the following function, permutations, to produce all permutations of a List[A].
Example:
scala> net.Permutations.permutations("ab".split("").toList)
res3: List[List[String]] = List(List(a, b), List(b, a))

Code:
object Permutations {

  def permutations[A](str: List[A]): List[List[A]] =
    str match {
      case Nil => List(Nil)
      case list @ _ :: _ =>
        val shifteds: List[List[A]] =
          shiftN(list, list.length)

        shifteds.flatMap {
          case head :: tail =>
            permutations(tail).map { lists: List[A] =>
              head :: lists
            }
          case Nil => Nil
        }
    }

  private def shiftN[A](list: List[A], n: Int): List[List[A]] = {
    if (n <= 0) Nil
    else {
      val shifted: List[A] = shift(list)
      shifted :: shiftN(shifted, n - 1)
    }
  }

  private def shift[A](arr: List[A]): List[A] = arr match {
    case head :: tail => tail ++ List(head)
    case Nil => Nil
  }
}

I think it's correct since the following property-based check succeeds:
import munit.ScalaCheckSuite
import org.scalacheck.Prop._
import org.scalacheck.Gen

class PermutationsSpec extends ScalaCheckSuite {

  private val listGen: Gen[List[Int]] =
    for {
      n <- Gen.choose(0, 7)
      list <- Gen.listOfN(n, Gen.posNum[Int])
    } yield list

  property("permutations works") {
    forAll(listGen) { list: List[Int] =>
      val mine: List[List[Int]] = Permutations.permutations(list)
      val stdLib: List[List[Int]] = list.permutations.toList
      assert(stdLib.diff(mine).isEmpty)
    }
  }

}

Please evaluate for correctness, concision and performance.

The code looks to be correct, yes, the testing looks good too.  I was
thinking whether List(List()) for the input List() makes sense, but
it seems like that's a sensible output.
For the code readability I'd rename str, especially since it's not
really a string, but a list.  The complicated match expression in
permutations can just be simplified to case _ and in the body the
original argument can be reused again.
I'd also inline shifteds value since it's just a single call and the
name doesn't really tell me anything.  On that note, docstrings for the
functions might be a nice touch, especially for the shift and shiftN
methods.
The unused case labels can also just be _ everywhere.  Depends of
course, for me this makes it clearer that really there's always just two
cases, either matching an empty list, or a non-empty one, there's no
third case.
Would look like this then:
object Permutations {
  def permutations[A](list: List[A]): List[List[A]] =
    list match {
      case Nil => List(Nil)
      case _ =>
        shiftN(list, list.length).flatMap {
          case head :: tail =>
            permutations(tail).map(head :: _)
          case _ => Nil
        }
    }

  def shiftN[A](list: List[A], n: Int): List[List[A]] = {
    if (n <= 0) Nil
    else {
      val shifted: List[A] = shift(list)
      shifted :: shiftN(shifted, n - 1)
    }
  }

  def shift[A](list: List[A]): List[A] = list match {
    case head :: tail => tail ++ List(head)
    case _ => Nil
  }
}

Lastly, performance-wise it depends what your constraints are: For
List input and List output, restricting it to single linked lists,
this is fine, though I haven't benchmarked them of
course.  Potentially converting the shifted :: shiftN(...) call into
using an accumulator might be worth a bit, instead of having a deep call
stack, but again, it'll probably only matter for longer inputs.
But there are way quicker algorithms, though you might want to copy the
input into a vector that can be accessed in constant time for each
index.  (I found the
QuickPerm algorithm, as explained here
absolutely straightforward to implement from scratch.)

Here is a another solution using fold that has the similar performance characteristics. One difference is that it doesn't have to compute the linear List Length

  def inserts[A](x: A): List[A] => List[List[A]] =
    ls =>
      ls match {
        case Nil       => List(List(x))
        case (y :: ys) => (x :: y :: ys) :: (inserts(x)(ys)).map( y :: _)
      }
  def permutations[A](ls: List[A]): List[List[A]] =
    ls.foldRight(List(List[A]()))((x, xss) => xss.flatMap(inserts(x)))


Displaying member information from a CSV file The code is about displaying membership information of all clients from database.  I was just wondering what I could do to improve/change it.

def optionA():
    global DoAnother
    while DoAnother == True:
        print("")
        member = input("Please enter the member number: ")
        if len(member) !=4:
            print ("")
            print("The member number is not valid")
            DoAnother = True
        else:
            global memberfile
            memberfile = open("members.txt","r+")
            searchMem = True
            for line in memberfile.readlines():
                listed = line.split(",")
                if member in listed:
                    print("")
                    print("Team Code: {0}".format(listed[0]))
                    print("Member number: {0}".format(listed[2]))
                    print("Date of joining: {0}".format(listed[1]))
                    print("Membership type: {0}".format(listed[3]))
                    print("Amount paid: {0}".format(listed[5]))
                    searchMem = False
                    nextchoice()
            if searchMem == True:
                optionA()

def optionB():
    print("")
    print("Team Code  Member No           Fee      Amount Paid      Amount Outstanding")
    print("----------------------------------------------------------------------------------------")
    global memberfile
    memberfile=open("members.txt","r+")
    amountOut = 0
    for line in memberfile.readlines():
        listed = line.split(",")

        if listed[3] =="F" and listed[5].strip('\n')<listed[4]:
            difference = (int(listed[4])-int(listed[5]))
            amountOut = (amountOut + difference)
            print("{0}          {1}             £{2}            £{3}          £{4}".format(listed[0], listed[2], listed[4], listed[5].strip('\n'), difference))
    print("")
    print("                                                         Total Outstanding: £{0}".format(amountOut))
    nextchoice()

There are a number of issues with this code; the first two items below are seriously bad habits.

Inappropriate use of functions.  Functions are not goto labels.  Having optionA call itself or nextchoice() makes no sense.  This must be corrected — it is a fundamental misunderstanding of how to write programs.
Use of global variables.  If you design your functions properly, there should be no need for any global variables.
Lack of abstraction.  There is nothing in common between these two functions, other than the fact that they both read from members.txt.  It would be a good idea to have a function to read, interpret, search the file.  Furthermore, since the data are in a csv format, you should use the Python csv module.
Using string comparison for numbers. This is a bug:

listed[5].strip('\n')<listed[4]

… because you are comparing numbers ASCIIbetically.
Lack of documentation. optionA and optionB are both mysterious names for functions.  It's not immediately clear what the members.txt file looks like, or why optionB ignores rows whose membership type column is not F.
File descriptor leak.  If you call open() without a corresponding call to close(), you might leak file descriptors.  In Python, you almost always want to call open() using a with block.
Member search by all attributes. You ask the user to enter a member number.  However, when you do

listed = line.split(",")
if member in listed:
    …

… you are actually searching all fields, not just the third column.
Python style guide.  PEP 8, the official Python style guide, says that variable names should be lower_case_with_underscores unless you have a good reason to deviate from the norm.

Suggested solution

import csv
from decimal import Decimal

MEMBERS_TXT_FIELDS = [
    ('Team Code', str),
    ('Date of joining', str),
    ('Member number', str),
    ('Membership type', str),
    ('Fee', Decimal),
    ('Amount paid', Decimal),
]

def members(member_number=None):
    with open('members.txt') as member_file:
        csv_file = csv.reader(member_file)
        for row in csv_file:
            member = {
                field_name: conv(val)
                for (field_name, conv), val in zip(MEMBERS_TXT_FIELDS, row)
            }
            if member_number is None or member_number == member['Member number']:
                yield member

def show_single_member(member_number):
    for member in members(member_number):
        print("""
Team Code: {Team Code}
Member number: {Member number}
Date of joining: {Date of joining}
Membership type: {Membership type}
Amount paid: {Amount paid}""".format(**member))
        return True
    return False

def show_owing_members():
    def value_fmt(field_def, member):
        field_name, width = field_def
        value = member[field_name]
        if isinstance(value, Decimal):
            return "£{0}".format(value.quantize(Decimal('0.01'))).rjust(width - 1) + ' '
        else:
            return str(member[field_name]).ljust(width)

    TABLE_FIELDS = [
        # Field name, width
        ('Team Code', 11),
        ('Member number', 20),
        ('Fee', 9),
        ('Amount paid', 17),
        ('Amount outstanding', 31),
    ]

    # Print headings
    print(''.join(field.ljust(width) for field, width in TABLE_FIELDS))
    print(''.join('-' * width for field, width in TABLE_FIELDS))

    total_outstanding = Decimal('0.00')
    for member in members():
        if member['Membership type'] == 'F' and member['Amount paid'] < member['Fee']:
            member['Amount outstanding'] = member['Fee'] - member['Amount paid']
            print(''.join(value_fmt(field_def, member) for field_def in TABLE_FIELDS))
            total_outstanding += member['Amount outstanding']
    print()
    w = sum(width for field, width in TABLE_FIELDS if field != 'Amount outstanding')
    print("Total outstanding: ".rjust(w) + "£{0}".format(total_outstanding))

def menu():
    while True:
        print("A: Member lookup")
        print("B: Outstanding fees")
        print("Q: Quit")
        choice = input("Choice: ").upper()
        if choice == 'A':
            ok = show_single_member(input("Please enter the member number: "))
            if not ok:
                print("No such member")
        elif choice == 'B':
            show_owing_members()
        elif choice == 'Q':
            break
        print()

I would implement something more functional, that way if you go on to create a larger business program you could debug, and section your needs better.

#seperating the formatting/print function from the lookup would be alot cleaner
#and allow for long-term changes without having to mess with the parts that work

def getMemberInfo():

    #you also don't need loop values, rather keep testing bad input
    #as the looping mechanism
    member = input("Please enter the member number: ")
    while len(member) != 4:
        member = input("\nPlease enter a valid member number: ")

    #cleaner way to open files
    with open("members.txt", "r+") as memberFile:
        for line in memberFile:
            listed = line.split(",")
            if member in listed:

                #append the difference value, to avoid anything other
                #than formatting in the second function
                listed.append(int(listed[5]) - int(listed[4]))
                return listed

            #else:  read next line
        return(None) # member wasn't found in file

def printMemberInfo( listed ):
    print("")
    print("")
    print("Team Code: {0}".format(listed[0]))
    print("Member number: {0}".format(listed[2]))
    print("Date of joining: {0}".format(listed[1]))
    print("Membership type: {0}".format(listed[3]))
    print("Amount paid: {0}".format(listed[4]))

    #you can play around with the formatting
    #exceeding col: 80 was irking me

    print("{0} {1} £{2} £{3} £{4}".format( \
           listed[0], listed[2], listed[4], listed[5].strip('\n'), listed[6]))

    print("")
    print(" Total Outstanding: £{0}".format(listed[6]))

With this code, you could have something that determines what the user wants to do, maybe getMemberInfo(), updateMemberInfo(), deleteMember(), and then can call those respective functions inside a main(): eventloop!


Handling data returned from fetch_assoc() I connected to the db, pull the data using fetch_assoc() (fetch_all(NUMB) is not available on the machine we are working with else this would be less of an issue). I get the returned data and load it into an array. However, it ends up in as an numeric array embedded inside another assoc array. What is the best way to have get the returned data in one, single, simple ASSOC array?

/**
* @Param $conn object [required], $tableName string [required]
* @Return $clmNames array
*/
public function getColumnName($conn = NULL, $tableName = NULL )
{
    if ( !is_object( $conn ) || !is_string( $tableName ) )
    {
        //params are not of correct type. This will eventually throw an error instead of printing to screen
        echo("Error: getColumnName returning false");
        return false;
    }

    //create SQL query
    $stmt = "SELECT column_name FROM information_schema.columns WHERE table_name =  '". $tableName ."' ORDER BY ordinal_position";
    //create query
    $query = $conn->query($stmt);

    //execute query
    $tmp = array();
    while ($row = $query->fetch_assoc())
    {
        //load all returned rows into an array
        $tmp[] = $row;
    }

    //clear memory
    $query->free();unset($query);

    //reCreate Array as a single assoc array instead of a double enbedded one.
    $clmNames = array();
    foreach( $tmp as $tmpKey=>$tmpValue )
    {
        foreach( $tmpValue as $tmpValueKey=>$tmpValueValue )
        {
            $clmNames[] = $tmpValueValue;
        }
    }

    return  $clmNames;
}

The above returns the data correctly. However, I would like to be able to remove the entire foreach dealing with $tmp and $clmNames if possible.

First off, there is no need to check if $conn is an object, simply use the proper procedure for checking if the connection was successful.

if( ! $conn ) {//MySQL
if( $conn->connect_error ) {//MySQLi
try {
    $conn = new PDO( $dsn, $user, $pass );
} catch( PDOException $e ) {
    die( $e->getMessage() );
}

And what is $tableName if not a string? The only other value I see is NULL, in which case you can use an is_null() check instead.

Anyways, now that we are done with that, lets look at the actual problem. The reason you are getting a numerical array of associative arrays is because that is the way you have it coded. You are appending a new array element on to the end of the $tmp array foreach $row in the fetch_assoc() table. Because you used an empty assignment [], this element uses the internal iterator to determine its position in the array, which is always the next available numeric index.

If you want to combine all of these arrays together into one associative array, then you will have to merge them somehow. The easiest way would be to use array_merge(), but then you risk overwriting keys from a previous row. This means if you have identical keys in each row array, then only the last row will be available. This, as you can imagine, would be rather pointless. That leaves us with merging it ourselves. Or does it? Since it appears that you only want the insides of the $row array, then you can just do it all at once, for example:

while( $row = $query->fetch_assoc() ) {
    foreach( $row  AS $value ) {
        $clmNames[] = $value;
    }
}

The above should, if I understand your code correctly, result in the same thing. I'm not sure if its the "best" solution, but it removes two loops, an unnecessary array, and makes your code a little more obvious in what it is trying to do.

A couple more things before I go, try only looping over information once. As shown above, if you find yourself looping over information just to create a new array to loop over it again, then chances are you can do whatever you needed to the first time and avoid having that second loop altogether. Finally, lets look at your foreach loops. If you compare yours to mine, you will notice I am not assigning the key pointer in mine. This is because it is unnecessary unless I am planning on using that value. Avoid assigning variables you are not going to use.

Hope this helps!

using

while( $row = $query->fetch_assoc() ) {

 statement relating $row

}

The loop Will Escape the first OR the Zeroth Index in the Query
as While loop loops till the End of the Array is reached the
$row is assigned automatically the first value and is per-incremented
so you wont help
Array(first_query);

if you want it to retrieve values from your db
use a array such that

$a2=array();
    $a2[0]=mysqli_fetch_assoc($my_sql_query);   //fetches the first missing value
    while( $row = $query->fetch_assoc() ) {
     statement relating $row
    $a2[]=$row;
}
echo json_decode($a2);


struct,template arguments, C++ I am writing color model logic in C++. I ask review in  defining limits for the color model, by saying Limits I mean this.
Consider a struct which represents RGB color model.
template<typename T>
struct RGB {
   T r,g,b;
};

So I declared a type RGB<T> where T can be any arithmetic type (the assertion is done, don't worry). So now I can create types using RGB_Float = RGB<flaot> or using RGB_888 = RGB<unsigned char> and so on.
As you already guessed I am doing image processing and sometimes there comes a time where I want a type RGB<float> where each component limits are [0-1] and sometimes I want same RGB<float> with the limits [10-20] (An example) . Also I need each type to know its limits, at compile time. Syntax is like RGB_Float::Limits::max() \\Returns max RGB_Float and RGB_Float::Limits::min() \\ Returns min RGB_Float. So I need to change my struct RGB<T> this way.
template<typename T, typename LIMITS>
struct RGB {
    using Limits = LIMITS;
    T r,g,b;

}

And before declaring any RGB<T,LIMITS> type I need to create a struct like
RGB_FloatLimits {
     static RGB<float,RGB_FloatLimits> max( return {1.0f,1.0f,1.0f};
     ... // min implementation too.
}

And then using RGB_Float = RGB<float,RGB_FloatLimits>;
I know that this is a bad solution but I think it's the only if you use C++17. I spent a week trying to pass the limits as a template argument to a color model. Please review this solution and I am 90% sure that there is a better way to do this. Thx in advance.
Note In any implementation of the limits sizeof the color model must remain the same.(So we must store the max and min statically)

C++11
Pre C++20 you have to use the type traits based approach, which is essentially what you have done. I dont think this is a bad solution, but probably the way to go if you need to.
C++20 solution (if integral min and max are ok also for C++11 )
In C++ 20 this all becomes easier as it finally allows for class and float non-type template arguments.
template<typename T, T min, T max>
struct RGB {
    T r,g,b;
}

works for T = int, T = char, T = float and so on.
This is the most straightforward solution I could think of that does not need any helper classes. If you only need min and max this is probably good.
If you could live with integer min and max then this also works pre C++20. Simply use int for the min and max template arguments.
The drawback is that in every function that accepts an RGB you need three  template parameters (T, min and max), which could create a lot of noise in the code and make it less readable.
In case you need more than two config parameters
In case min and max is not a sufficient description of the number range, you could add addtional template parameters. However I would not advise this but make the parameters now a class.
In C++20 this is very straightforward again, as you can also use class values as template parameters. This is a bit more clear than the previous type based traits approach (which is also the one you are using).
template <class T>
struct RGBconfig<T> {
   T min, max;
}

template<typename T, RGBconfig<T> config>
struct RGB {
    T r,g,b;
}

use as
constexpr RGBconfig<float> float1020 = {10, 20}

using RGBfloat1020 = RGB<float, float1020>

alternatively one could do
using RGBfloat1020 = RGB<float, RGBconfig<float> {10,20} >

Reduce to one template parameter => include type in RGBconfig
Perhaps it is even possible to reduce it down to one template argument
template <class T>
struct RGBconfig<T> {
   using type = T;
   T min, max;
}

template<auto config>
struct RGB {
    typename decltype(config)::type    r,g,b;
}

then use as
constexpr RGBconfig<float> float1020 = {10, 20};
using RGBfloat1020 = RGB<float1020>;

or
using RGBfloat1020 = RGB<RGBconfig<float> {10,20} >;


Project Euler #1 in Haskell Looking for feedback/critiques/alternatives to my solution for the 1st Project Euler question:

{-|
If we list all the natural numbers below 10 that are multiples of 3 or 5, we get 3,5,6 and 9. The sum of these multiples is 23. Find the sum of all the multiples of 3 or 5 below 1000.
-}

import Data.List

-- Finds all multiples of a factor below set limit, by computing factor*n and adding to a list, until factor*n > limit
f_multiple :: [Int] -> Int -> Int -> Int -> [Int]
f_multiple multiples current limit factor
        | (current * factor) >= limit   = multiples
        | otherwise         = f_multiple (multiples ++ [current * factor]) (current + 1) limit factor

sum_multiples :: Int -> Int -> Int -> Int
sum_multiples max first_factor second_factor = foldr (+) 0 ( (f_multiple [] 0 max first_factor) `union` (f_multiple [] 0 max second_factor))

There are several approaches to Project Euler Question 1.  The one you have chosen is not particularly efficient, as it involves creating two lists, then merging them.  I'm not going to present alternative solutions here.  If you are going with this approach, you can write it much more simply:

sum_multiples :: Int -> Int -> Int -> Int
sum_multiples max factor1 factor2 = sum $ multiples1 `union` multiples2
  where
    multiples1 = [factor1, 2 * factor1 .. max - 1]
    multiples2 = [factor2, 2 * factor2 .. max - 1]

In particular,

foldr (+) 0 is just the built-in sum function.
f_multiple can be replaced with ranges.

In terms of style, your last line is hard to read due to its length.  To shorten it,

Use shorter variable names, e.g. factor1 instead of first_factor.
Give names to complex subexpressions using a where clause.
Use the $ operator to form a "pipeline", instead of nesting parentheses.

I think your approach is more general and at least in my opinion slightly more complex than required. Here's my attempt:

sum [n | n <- [1..1000], n mod 3 == 0 || n mod 5 == 0]

I suppose I could pass in the "1000", "3" and "5" as parameters or even enable "n" number of filters to be passed. I think it would be an over-kill for this problem but a useful exercise :-)


A Java Iterable over multiple arrays GlueArrays is used for iterating over an array of arrays, and it looks like this:

Main.java

import java.util.Iterator;
import java.util.NoSuchElementException;

public class Main {

    public static class GlueArrays<T> implements Iterable<T> {

        private final T[][] arrays;

        public GlueArrays(T[]... arrays) {
            this.arrays = arrays;
        }

        @Override
        public Iterator<T> iterator() {
            return new GlueArrayIterator(arrays);
        }

        private static class GlueArrayIterator<T> implements Iterator<T> {

            private final T[][] arrays;
            private int arrayIndex = 0;
            private int localIndex = 0;

            GlueArrayIterator(T[][] arrays) {
                this.arrays = arrays;
            }

            @Override
            public boolean hasNext() {
                if (arrayIndex == arrays.length) {
                    return false;
                }

                return arrayIndex < arrays.length ||
                       localIndex < arrays[arrayIndex].length;
            }

            @Override
            public T next() {
                if (!hasNext()) {
                    throw new NoSuchElementException("Nothing to iterate.");
                }

                T returnValue = arrays[arrayIndex][localIndex];

                if (localIndex < arrays[arrayIndex].length) {
                    localIndex++;

                    if (localIndex == arrays[arrayIndex].length) {
                        localIndex = 0;
                        arrayIndex++;
                    }
                }

                return returnValue;
            }
        }
    }
    public static void main(String[] args) {
        Number[] arr1 = {1, 2, 3};
        Number[] arr2 = {4f, 5f, 6f};
        Number[] arr3 = {7.0, 8.0, 9.0};

        for (Number number : new GlueArrays<>(arr1, arr2, arr3)) {
            System.out.println(number);
        }
    }
}

As always, any critique is welcome.

First off, this is buggy. If one of the input arrays is empty, you'll get an ArrayIndexOutOfBoundsException - and even if you correct that error, the iterator will stop if it encounters an empty array.

Consider using proper tests with multiple scenarios, especially with edge cases, in order to catch errors like that.

Then I get two warnings from Eclipse (other compilers and their settings may differ, but I believe these are "common" warnings, that normally would be raised). You should never have warnings. Code should preferably be adjusted or rewritten to avoid them, or at the very least (if you have a good reason to keep the warning) annotated with @SuppressWarnings and commented on with those reasons.

1) Due to the use of generics together with variable arguments the GlueArrays constructor should have the annotation @SafeVarArgs.

2) The instantiation of the iterator in iterate() is lacking its type parameters. It should be

return new GlueArrayIterator<T>(arrays);

or, if you are at least using Java 1.7, just

return new GlueArrayIterator<>(arrays);

Do you really need an iterator? You could do the same with three lines of Streams and flat mapping:

Arrays.stream(twoDimensionalArray)
    .flatMap(array -> Arrays.stream(array))
    .forEach(element -> System.err.println(element));


Calculate class to practice encapsulation I realize this is very beginner code, but I just today grasped the basic concepts of private variables, constructors, and such. I wrote this very simple class, and a class to test it. Any and all input welcome.

Calculate.java

import java.lang.Math.*;

public class Calculate {
    private final double num1, num2;

    Calculate(double num1, double num2) {
        this.num1 = num1;
        this.num2 = num2;
    }

    public double Add() {
        return num1 + num2;
    }
    public double Subtract() {
        return num1 - num2;
    }
    public double Multiply() {
        return num1 * num2;
    }
    public double Divide() {
        return num1 / num2;
    }
    public double Exponent() {
        return Math.pow(num1, num2);
    }
}

TestCalculate.java

class TestCalculate {
    public static void main(String[] args) {
        double num1 = 5, num2 =7;
        Calculate calc = new Calculate(num1, num2);
        // show outputs from calculations
        System.out.println("Number 1: " + num1);
        System.out.println("Number 2: " + num2);
        System.out.println("Addition: " + calc.Add());
        System.out.println("Subtraction: " + calc.Subtract());
        System.out.println("Multiplication: " + calc.Multiply());
        System.out.println("Division: " + calc.Divide());
        System.out.println("Exponent: " + calc.Exponent());
    }
}

Sample output using given variables:

  Success   time: 0.1 memory: 320256 signal:0 Number 1: 5.0 Number
  2: 7.0 Addition: 12.0 Subtraction: -2.0 Multiplication:
  35.0 Division: 0.7142857142857143 Exponent: 78125.0

On Formatting

You can format your output using System.out.printf , or String.format()

For example, if you wanted your division test result to only include 2 digits after the decimal you could instead call System.out.printf("Division: %.2f%n", calc.divide());

The % is a special character; when the printf method reaches it it replaces that location with the specified formatted version of the proceeding arguments (You can have several in one line as long as they are in order). The '.2' that proceeds is what enforces only 2 digits after the decimal. The %n that immediately follows is so you can still have a new line after calling printf. You can read more about formatting and find some additional examples here.

On Exception Handling

I'll also echo the previous answer in saying that handling edge cases would do you some good, this could be done with a simple conditional within the method to ensure the divisor is not 0, which would otherwise cause an error or report NaN/infinity when it is undefined.

You could throw an exception if the divisor is 0 within the divide method itself, like so:

public double divide() {
        if (num2 == 0) {
            throw new ArithmeticException("Dividing by 0 is undefined");
        }

        return num1 / num2;
    }

The message could be  a simple "Cannot divide by 0." Which, if you catch it, would allow a 0 divisor to exist, and you don't have to constrict the possible num2 values and calculations possible.

On Java Convention & Readability

variables and methods are camelCase
Class and interface name are PascalCase
CONSTANTS are UNDERSCORE_CAPITALIZED

Also, try to have a line break between your methods, it'll aid in readability once method body increases. Though I personally omit this when the methods are closely linked, like getters and setters for the same value, as @Rhuarc13 suggest you have.

Although I disagree with adding anything not explicitly necessary(any user trying to pass a no arg constructor would encounter a compilation error) since this is an exercise on encapsulation I think a getter has some merit. Setters don't make sense in this case given your variables' final state, but if you instantiated several of these Calculate objects with different values it would be helpful to have a method to keep track of them without providing users with access to the variable itself, which would break encapsulation*.

Access to the variable directly is arguably inconsequential since it's final, but the point of encapsulation is that the external users should not concern themselves with the fields or implementation.

If you go in your calculate and added methods like so:

public double getNum1() {
    return this.num1;
}

public double getNum2() {
    return this.num2; // this keyword refers to that particular object
}

Then when you're interested in what value is actually contained within the object for example in your test case you could call

System.out.println("Number 1: " + calc.getNum1());
System.out.println("Number 2: " + calc.getNum2());

Alternative Implementation

If you were interested in setters as well, if you had private instead of final values you could use something like this:

public class Calculator {
    private double num1;
    private double num2;

    public Calculator setNum1(double num1) {
        this.num1 = num1;
        return this;
    }

    public Calculator setNum2(double num2) {
        this.num2 = num2;
        return this; // return the current state of Calculator to allow for chaining
    }

    public double divide() {
        return num1 / num2;
    }

    public double add() {
        return num1 + num2;
    }

    public static void main(String[] args) {
        // You can chain call them
        Calculator example = new Calculator().setNum1(3).setNum2(5);

        System.out.println(example.add());
        // Or use it separately.
        example.setNum1(45);
        System.out.println(example.divide());
    }
}

On User Input + Further input validation

Your revision seems concerned with user input, if you actually want to get and use input, a simple way to do that is to make use of Scanner.

Make sure to retrieve the resource at the top: import java.util.Scanner and then declare an instance: Scanner input = new Scanner(System.in)

The nice thing about that is you could ensure a non-0 divisor with a while loop.

for instance if in your main method you did:

System.out.print("Enter second value: ");
double val2 = input.nextDouble();

You could proceed with this:

while (val2 == 0) {
    System.out.print(
        "Second value cannot be 0" +
        "\nEnter a non-0 value: ");

    val2 = input.nextDouble();
}

and that would make sure that your user provided a non 0 divisor.

You should add a default constructor.

Calculate() { num1 = 0.0; num2 = 0.0; }

Default constructors help just in case some one creates a calculator object, but doesn't pass in any variables.
Another thing to add is getters and setters. For example:

public void setNum1(double newNum1) { num1 = newNum1; }
public double getNum1() { return num1; }

Getters and setters allow you to change or view variables later on. They are extremely useful.


Basic template engine I wanted to know how I can improve it. I mainly want to look at speed and performance, but if there are any improvements please give a suggestion.

<?php
defined("SECURE") or exit('Please define SECURE keyword to continue.');
class template
{
    private $templateParams;

    public function __construct()
    {
        $templateParams = array();
    }

    public function loadTemplate($configClass)
    {
        $this->organiseParams($configClass);
        $this->displayContent($configClass);
    }

    private function organiseParams($configClass)
    {
        $values = $configClass->getAll();

        while(list($title, $value) = each($values))
            $this->addParameter('[config ' . $title . ']', $value);
    }

    private function displayContent($configClass)
    {
        if (strlen($_GET['page']) >= 1)
        {
            ob_start();

            if (file_exists(ROOT . $configClass->getConfig("style.folder") . '/' . $configClass->getConfig("style.name") . '/' . $_GET['page'] . '/public_html/content.php'))
                include ROOT . $configClass->getConfig("style.folder") . '/' . $configClass->getConfig("style.name") . '/' . $_GET['page'] . '/public_html/content.php';
            else
                include ROOT . $configClass->getConfig("style.folder") . '/' . $configClass->getConfig("style.name") . '/error/public_html/content.php';

            $htmlOutput = ob_get_contents();
            ob_end_clean();

            while (list($paramTitle, $paramValue) = each($this->templateParams)) {
                $htmlOutput = str_replace($paramTitle, $paramValue, $htmlOutput);
            }

            echo $htmlOutput;
        }
        else
        {
            header("location: " . $configClass->getConfig("website.url") . "/index");
        }
    }

    private function addParameter($title, $value)
    {
        $this->templateParams[$title] = $value;
    }
}
?>

Your first concern is performance, so let's start there. The way you're including and processing your content is going to slow you down. Because you're buffering it, then putting it to a variable, then doing a search replace, you're going to end up working through the content several times before you get your end result. If the content is small then don't even worry about it, but if you have embedded base64 images or just tons of content, then you're going to want a different approach.

From a code point a view I think you're on the right track with your object design. It's always good to break things out to make them easier to maintain. One thing you want to consider though is to take a more MVC style approach by separating processing from the visuals. This makes code easier to read and much easier to maintain. The long story short of this point is to change the echo of the output to a return so that your calling (visual) code can output it. It sounds minor but as your application becomes more complex, it can be a headache saver.

Back to your first point, how to improve getting of the content. My approach would be to make the included content an object file. The file could then include a get method of sorts that returned the html in a string with your titles and parameters already in place. That would allow you to have your content file be built just like a concatenated string (saving the find and replace work). Pass your values as parameters and you're good to go.

Here's how I would do this. I didn't include code for the content class as the code in this Template class should be enough to go on. Note that based on my example, all include files should have the class ContentObj with the method getOutput. I simplified your include path code to make things much easier to read. I also added an exit after your header redirect as there's no need for further processing (usually) after a redirect.

<?php
defined("SECURE") or exit('Please define SECURE keyword to continue.');
class Template
{
    private $configClass;
    private $templateParams;

    public function Template($configClass)
    {
        $this->configClass = $configClass;
        $this->templateParams = array();
    }

    public function loadTemplate()
    {
        $this->organiseParams();
        $this->displayContent();
    }

    private function organiseParams()
    {
        $values = $this->configClass->getAll();

        while(list($title, $value) = each($values))
            $this->addParameter('[config ' . $title . ']', $value);
    }

    private function displayContent()
    {
        if (strlen($_GET['page']) >= 1)
        {
            $contentPath = ROOT . $this->configClass->getConfig("style.folder") . '/' . $this->configClass->getConfig("style.name");

            if (file_exists($contentPath . '/' . $_GET['page'] . '/public_html/content.php'))
                include $contentPath . '/' . $_GET['page'] . '/public_html/content.php';
            else
                include $contentPath . '/error/public_html/content.php';

            $contentObj = new ContentObj($paramTitle, $paramValue, $htmlOutput);
            $htmlOutput = $contentObj->getOutput($this->templateParams);

            return $htmlOutput;
        }
        else
        {
            header("location: " . $configClass->getConfig("website.url") . "/index");
            exit;
        }
    }

    private function addParameter($title, $value)
    {
        $this->templateParams[$title] = $value;
    }
}
?>

First at all, I don't think it's good oop, you can rewrite this class in one function, read some articles about SOLID principles. Try to KISS (Keep It Simple Stupid!) and good luck, you should improve your skills!

Use return instead of echo

It's better to use return, cause you can decide what to do with your output. Echoing things just force us to show it, but what if we want just save it to file??

Use ternary (shortcut that damn ifs!)

You can make that ifs more readable by using ternary operators.

$path = (file_exists($folder . '/' . $_GET['page'] . '/public_html/content.php')
              ? '/' . $_GET['page'] . '/public_html/content.php'
              : '/error/public_html/content.php';

Use objects when you need to

You can rewrite you class as function. Look at this piece of code:

<?php

function loadTemplate(ConfigClass $config)
{
    $params = array();

     foreach ($config->getAll() as $key => $value) {
         $params['[config ' . $key . ']'] = $value;
    }

    if (strlen($_GET['page']) >= 1)
    {
        ob_start();

        $folder = ROOT . $config->getConfig("style.folder") . '/' . $config->getConfig("style.name");

        $path = (file_exists($folder . '/' . $_GET['page'] . '/public_html/content.php')
              ? '/' . $_GET['page'] . '/public_html/content.php'
              : '/error/public_html/content.php';

        require $folder . $path;

        $htmlOutput = ob_get_contents();

        ob_end_clean();

        foreach ($params as $key => $value) {
            $htmlOutput = str_replace($key, $value, $htmlOutput);
        }

        return $htmlOutput;
    }
    else
    {
        header("location: " . $config->getConfig("website.url") . "/index");
    }
}

It does the same thing. Think about it. Good luck!


Factorial in JavaScript I decided to write a simple factorial() function in JavaScript and ask here for opinions. The idea is to further improve my JavaScript style and learn the best practices. Is there something bad with this code? What can be improved? Any suggestions are welcome.

/**
 * Calculates the factorial of a given non-negative finite integer.
 * @param {Number} n
 * @return {Number} factorial
 * @throws TypeError if n is not a Number
 * @throws TypeError if n is a Number but not an integer
 * @throws RangeError if n is a negative integer
 */
function factorial(n) {
    if (typeof n !== "number") {
        throw new TypeError("factorial() expects a number.");
    }
    if (Math.floor(n) !== n) {
        throw new TypeError("factorial() expects an integer.");
    }
    if (n < 0) {
        throw new RangeError("factorial() expects a non-negative number.");
    }

    // 170 is the biggest integer whose factorial does not become Infinity in JavaScript.
    // Therefore, we can improve performance by checking directly.
    // This if statement also handles the important edge case of n === Infinity, which
    // would cause the function to be stuck forever.
    if (n > 170) {
        return Infinity;
    }

    var accumulator = 1;

    for (var i = 2; i <= n; i++) {
        accumulator *= i;
    }

    return accumulator;
}

if (typeof n !== "number") {
    throw new TypeError("factorial() expects a number.");
}
if (Math.floor(n) !== n) {
    throw new TypeError("factorial() expects an integer.");
}
if (n < 0) {
    throw new RangeError("factorial() expects a non-negative number.");
}

First, I discourage manual type and value checks on runtime. They're redundant since the JS engine will emit an error if you operate on the wrong data anyways. It's also runtime overhead as well as unnecessary code.

Often times, developers forward this task to the IDE when writing code. With the right plugins and annotations, the IDE can warn you of incompatible data types and operations. This only prevents writing the API incorrectly though. There's still risk of using the function incorrectly, but that's the consumer's fault, not the API.

var accumulator = 1;

JS has let, the block-scoped version of var. It's a good habit to use let since in most cases, you'll want block-scoped vars. It also prevents ambiguity of where the variable exists in the code, which usually causes problems (like var in for or if). If I remember correctly, it's because of hoisting that developers started defining vars explicitly on the top of the function body to signal that a variable of that name exists in the function.

if (n > 170) {
    return Infinity;
}

var accumulator = 1;

for (var i = 2; i <= n; i++) {
    accumulator *= i;
}

return accumulator;

In the end, the factorial code is just the one above. A shorter version factorials can be done using recursion.

function factorial(n) {
  return n > 170 ? Infinity    // Bail out on 170
    : n == 0 || n == 1 ? 1  // When the answer is just 1
    : n * factorial(n - 1)  // Multiply current with next smallest
}

console.log(factorial(1))
console.log(factorial(2))
console.log(factorial(3))
console.log(factorial(4))
console.log(factorial(5))

One possible way to refactor the different cases you did to check if the input is valid  is by relying on Number.isInteger() function which returns true if the argument is an integer.

   // This also test if the integer is positive
   if (Number.isInteger(n) && n > 1){
        var accumulator = 1;
        for (var i = 2; i <= n; i++) {
            accumulator *= i;
        }
        return accumulator;
   } else {
        // Throws an error for ALL other wrong inputs
        throw new Error("Expected a positive integer value");
   }

So your code can be:

function factorial(n) {
   if (n === 0 || n=== 1){
        return 1;
   }
   if (n > 170) {
        return Infinity;
   }
   if (Number.isInteger(n) && n > 1){
        var accumulator = 1;
        for (var i = 2; i <= n; i++) {
            accumulator *= i;
        }
        return accumulator;
   } else {
        throw new Error("Expected a positive integer value");
   }
}

// Testing
console.log(factorial(0));
console.log(factorial(1));
console.log(factorial(9));
console.log(factorial(170));
console.log(factorial(171));
// All below test will throw an error:
//console.log(factorial("100"));
//console.log(factorial(a));
//console.log(factorial(2.2));
//console.log(factorial(-2));


Case insensitively removing a substring, efficiently I am going through the CodingBat exercises for Java. I have just completed this one:

  Given two strings, base and remove, return a version of the base string where all instances of the remove string have been removed (not case sensitive). You may assume that the remove string is length 1 or more. Remove only non-overlapping instances, so with xxx removing xx leaves x.

I wanted to try using a StringBuilder to solve this because I have not done so before. Here is my code:

public String withoutString(String base, String remove){

    String removeLo = remove.toLowerCase();
    String removeHi = remove.toUpperCase();

    int rL = remove.length();

    StringBuilder s = new StringBuilder(base);

    for (int i = 0; i < s.length(); i++) {
        int j = s.indexOf(remove, i);

        if (j < 0) {
            break;
        } else {
            s.delete(j, j + rL);
        }
    }

    for (int i = 0; i < s.length(); i++) {
        int j = s.indexOf(removeLo, i);

        if (j < 0) {
            break;
        } else {
            s.delete(j, j + rL);
        }
    }

    for (int i = 0; i < s.length(); i++) {
        int j = s.indexOf(removeHi, i);

        if (j < 0) {
            break;
        } else {
            s.delete(j, j + rL);
        }
    }
    return s.toString();
}

The code repeats the for loop 3 times in order to search for the unaltered, lower case, and upper case versions of remove. My questions are:

Is it possible to reduce this down to one for loop, and is that practical (with good readability)?
The test strings involve digits. Is it problematic to be 'converting' these to lower and/or upper cases? Should I deal with those before testing the alphabetic strings?
The following test cases are the ones that require all three for loops:

withoutString("xxx", "x")
withoutString("1111", "1")
withoutString("MkjtMkx", "Mk")
withoutString("Hi HoHo", "Ho")

The rest of the tests work without the first for loop (unaltered remove). The second two make sense, because the remove string uses upper and lower case, but the first two don't follow this. I have used the debugger in Eclipse but I still can't figure this out. Can you please explain?

Regular expressions make your life really easy when solving such exercises:

public static String withoutString(String base, String remove) {
    return Pattern.compile(Pattern.quote(remove), Pattern.CASE_INSENSITIVE).matcher(base).replaceAll("");
}

I explain this code a little:

Pattern.compile(Pattern.quote(remove), Pattern.CASE_INSENSITIVE): Creates a regular expression pattern containing the string you want to have removed. Pattern.quote(remove) takes care of special characters that may be interpreted as a regular expression (such as: *, \, +, (), [] etc.). The flag Pattern.CASE_INSENSITIVE makes sure, that the case of the characters don't matter.
.matcher(base): returns a Matcher that holds all matches of the regular expression in the string base.
.replaceAll("");: replaces all matches that have been found with an empty string, effectively removing them.

You may have noticed, that I made this method static because it does not access any fields or methods that are non-static. This is always advised unless it really needs to be non-static for some reason.

My EASY & SIMPLE Java Solution using a single for loop, O(N) run time :)
public String withoutString(String base, String remove) {

  String returner = "";

  for(int i = 0; i < base.length(); i++)
  {
    if(i+remove.length() <= base.length() && base.substring(i,i+remove.length()).toLowerCase().equals(remove.toLowerCase()))
      i+=remove.length()-1;
    else
      returner += base.charAt(i);
  }

  return returner;
}


Check if a given number N is a power of k I am trying to solve a problem to check whether a given number N is a power of k.

Example:

Input 9, 3  > True //  3**2 = 9
Input 10, 2  > False
Input 4096, 16  > True // 16**3 = 4096

I want to know if there is a better solution for this problem.

def check_Power(N,k):
    if N <= 0 or k <=0:
        print "not a valid input"
    else:
        for i in range (1,20):
            x = k**i
            if x == N :
                print " True "
                print k, "power ", i , "is", N
                break
            elif x> N:
                print "false"
                break

check_Power(244140625,5)
check_Power(4096, 16)
check_Power(0, 16)
check_Power(1,1)

I'd follow Ludisposed's answer.

Except, there's a simple way to do this. Use logarithms.
Since the equation is \$N = k^a\$ you can change it to \$a = \log_k N\$.
And so if \$a\$ is an integer, then you know that \$N\$ is a power of \$k\$.

import math

def check_power(N, k):
    return math.log(N, k).is_integer()

Since the above can error, and if there's an error it's definately not a power, then you can use:

def check_power(N, k):
    try:
        return math.log(N, k).is_integer()
    except Exception:
        return False

This has the some problems with floating point precision, so your can round the number to the closest integer, as you want to know if the \$a\$ is an integer. And so you can sub it back into the original equation to ensure that the numbers are right.

def check_power(N, k):
    return N == k**round(math.log(N, k))

Or if you're on Python 2:

def check_power(N, k):
    return N == k**int(round(math.log(N, k)))

This has a problem with some inputs, such as when \$N\$ and \$k\$ are \$0\$ or \$1\$. And so we can handle these special cases.

def check_power(N, k):
    if N == k:
        return True
    return N == k**round(math.log(N, k))

And so merging all the above, we can use the following:

def check_power(N, k):
    if N == k:
        return True
    try:
        return N == k**int(round(math.log(N, k)))
    except Exception:
        return False

first it will validate the both Base Number and Result are not equal to zero and both numbers are divisible are not.
Here Count is iterating variable it will check condition for each pass.

BaseNumber = 16
Result = 4096
count = 1
power = 0
if Result and BaseNumber != 0 and Result % BaseNumber == 0:
while count != Result:
    count = BaseNumber * count
    power += 1
print(power)
else:
print("wrong input")


Inserting a user sign-up record using mysqli My Code:
Connectivity(php)

<?php

$conn=mysqli_connect("localhost","root","","webchat_data");
//if($conn)
//{
//  echo "hi";
//}
if(!$conn){
    die("CONNECTION FAILED" .mysqli_connect_error());
}

?>

Signup

<?php
include 'connectivity.php';
$uname=($_POST['uname']);
$email=($_POST['Email']);
$pass=($_POST['Password']);

$sql = "INSERT INTO `sign-up`(USERNAME,EMAIL_ID,PASSWORD) VALUES('$uname','$email','$pass')";
//$result=$conn->query($sql);
$result = mysqli_query($conn, $sql);
//$query=mysqli_query($conn,$sql);

header("Location:index.php");

?>

Someone told me that my query is good for a learner, but not for real world applications. Please make some recommendations to make this code more suitable for real world scenarios.

Like it was said in the comments, this code needs not a review but just some basic practices. That said, good basic practices are a rare specimen in the wild, so you cannot be blamed, given the number of awful tutorials out there. Luckily I am the renowned collector of good practices and here you are

Connectivity

There ate many things that could be improved in your connectivity file. To name a few

the connection character set must be configured to avoid issues with characters
the proper error reporting mode for mysqli must be set
the connection error smust be not bluntly echoed out

All these issues are covered in my article How to connect properly using mysqli: so let's take the code from there:

$host = '127.0.0.1';
$db   = 'webchat_data';
$user = 'root';
$pass = '';
$charset = 'utf8mb4';

mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);
try {
    $conn = new mysqli($host, $user, $pass, $db);
    $conn->set_charset($charset);
} catch (\mysqli_sql_exception $e) {
     throw new \mysqli_sql_exception($e->getMessage(), $e->getCode());
}

Password hashing

In two words, never store passwords in plain text. Use password_hash() function instead. This topic is thoroughly explained in the question: https://stackoverflow.com/questions/1581610/how-can-i-store-my-users-passwords-safely

Prepared statements

Just never add a variable to SQL query directly, but mark its place with a question mark instead. Then bind the actual variable and finally call execute()

In detail this matter is explained in this question: https://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php

So here is your signup code reviewed

<?php
include 'connectivity.php';

$uname = $_POST['uname'];
$email = $_POST['Email'];
$pass  = password_hash($_POST['Password'], PASSWORD_DEFAULT);

$sql = "INSERT INTO `sign-up`(USERNAME,EMAIL_ID,PASSWORD) VALUES(?,?,?)";
$stmt = $conn->prepare($sql);
$stmt->bind_param("sss", $uname,$email,$pass);
$stmt->execute();

header("Location:index.php");

The most important points:

Never use User-Input in unsecured queries. Use prepared statements!

Never save passwords in cleartext. A common form is hashing.

Unfortunately, these topics would exceed this format.


Insert sort on a linked list I want to do insert sort in a linked list without using dummy nodes.

This is my code. How can this be improved? Any input is appreciated.

public static Node insertSort(Node node){

   Node sortedList = null;
   while(node != null){
    Node current = node;
    node = node.next;
    Node x;
    Node previous = null;
    for(x = sortedList; x != null; x = x.next){
        if(current.value < x.value){
                break;
         }
         previous = x;
    }
    if(previous == null){
          current.next = sortedList;
          sortedList = current;
    }
    else{
       current.next = previous.next;
       previous.next = current;
    }
  }
  return sortedList;
}

Algorithmically, your implementation seems fine. One improvement would be to initialize the sortedList to the first node right away. This necessitates some error checking up front, which I think is better style anyway.

I would also run a check for the current node being the new sorted head first. That way I can automatically assume that I'm inserting after the search position. That allows elimination of the previous variable.

But this is all stylistic stuff to make the logic flow easier to understand. As I said, I think your implementation functions perfectly well.

public static Node insertNode(Node node) {
    if (node == null)
        return null;

    // Make the first node the start of the sorted list.
    Node sortedList = node;
    node = node.next;
    sortedList.next = null;

    while(node != null) {
        // Advance the nodes.
        final Node current = node;
        node = node.next;

        // Find where current belongs.
        if (current.value < sortedList.value) {
            // Current is new sorted head.
            current.next = sortedList;
            sortedList = current;
        } else {
            // Search list for correct position of current.
            Node search = sortedList;
            while(search.next != null && current.value > search.next.value)
                search = search.next;

            // current goes after search.
            current.next = search.next;
            search.next = current;
        }
    }

    return sortedList;
}

See it run in JavaScript: http://jsfiddle.net/2LDGD/2/

well sry, i dont have much time to read your code but one of the best ways i used in such problems is to create a "swap function", that uses in every sort algorithm, then you can implement every algorithm you want and change the swap function with yours and of course custom traverse commands


Calling many web services asynchronously I need to process a bunch of completed orders (thousands). To make it faster, I'm using a SemaphoreSlim to schedule a number of orders to be processed in parallel. Every order goes through multiple web service calls to various APIs (SOAP, REST). Then I need to take the returned data and process it into a PDF. Here is what I am doing currently (note that this is in an ASP.NET app):

private async Task ProcessOrders(List<Orders> orders)
{
    var allTasks = new List<Task>();
    var throttler = new SemaphoreSlim(initialCount: 5);

    foreach (var order in orders.Where(x => !x.HasError))
    {
        // do an async wait until we can schedule again
        await throttler.WaitAsync();

        // using Task.Run() to run the lambda in its own parallel
        // flow on the thread pool
        allTasks.Add(Task.Run(async () =>
        {
            try
            {
                // Call external web service here
                // Build order report PDF here
            }
            finally
            {
                throttler.Release();
            }
        }));
    }

    await Task.WhenAll(allTasks);
}

However, I'm new to TPL and I don't know if this is the right way to do something like this. If it's relevant, this app is hosted on Amazon's AWS cloud.

I'm not all that familiar with async/await (I really need to get going on this), but I find your code pretty clear, except why the magic number  5 is being used for an initialCount.

I like your usage of var, but the comments are a little redundant, they say nothing much that the code doesn't already tell.

It would have been nice to see the actual code in place of the placeholder comments in the try block - I'd expect to see a method call here; if that comment stands for code that actually implements what's described, you need to factor it out.

Also I would have indented the async task's body, but that's more of a nitpick.

I would probably also have introduced a var validOrders = orders.Where(order => !order.HasError); to make the foreach loop clearer. Not sure I like x for a name in the lambda: try to use meaningful names everywhere.

You can also use the task factory for this, as shown below.

private void ProcessOrders(List<Orders> orders)
{
    var allTasks = new List<Task>();
    foreach (var order in orders.Where(x => !x.HasError))
    {
        allTasks.Add(Task.Factory.StartNew(() =>
           // process order
        ));
    }

    Task.WaitAll(allTasks.ToArray());
}


Group collections by their elements property I have a Customer class:

public class Customer
{
    public Guid Id { get; set; }
    // Some other properties...}

And three transactions classes that have a reference to Customer:

public class Order
{
    public Guid CustomerId { get; set; }
    // Some other properties...}

public class Invoice
{
    public Guid CustomerId { get; set; }
    // Some other properties...}

public class Payment
{
    public Guid CustomerId { get; set; }
    // Some other properties...}

I have a collection for each of transaction type. And I want to get this collection to be "grouped" by their elements CustomerId properties. So, as a result I'd like to get a collection of such objects:

public class CustomerInfo
{
    public CustomerInfo(Guid customerId, IEnumerable<Order> orders,
        IEnumerable<Invoice> invoices, IEnumerable<Payment> payments){...}

    public Guid CustomerId { get; set; }
    public IEnumerable<Invoice> Invoices { get; set; }
    public IEnumerable<Order> Orders { get; set; }
    public IEnumerable<Payment> Payments { get; set; }
}

Right now I am doing so by this function:

private IEnumerable<CustomerInfo> _GetCustomerInfo(
    IEnumerable<Payment> payments, IEnumerable<Invoice> invoices,
    IEnumerable<Order> orders)
{
    var invoicesGroupdByCustomers = invoices.GroupBy(x => x.CustomerId);
    var ordersGroupdByCustomers = orders.GroupBy(x => x.CustomerId);
    var paymentsGroupdByCustomers = payments.GroupBy(x => x.CustomerId);

    var result = new List<CustomerInfo>();
    foreach (var group in invoicesGroupdByCustomers)
        result.Add(new CustomerInfo(group.Key,
            ordersGroupdByCustomers.FirstOrDefault(x => x.Key == group.Key),
            group,
            paymentsGroupdByCustomers.FirstOrDefault(x => x.Key == group.Key)));

    foreach (var group in ordersGroupdByCustomers)
        if (!result.Any(x => x.CustomerId == group.Key))
            result.Add(new CustomerInfo(group.Key,
                group,
                invoicesGroupdByCustomers.FirstOrDefault(x => x.Key == group.Key),
                paymentsGroupdByCustomers.FirstOrDefault(x => x.Key == group.Key)));

    foreach (var group in paymentsGroupdByCustomers)
        if (!result.Any(x => x.CustomerId == group.Key))
            result.Add(new CustomerInfo(group.Key,
                ordersGroupdByCustomers.FirstOrDefault(x => x.Key == group.Key),
                invoicesGroupdByCustomers.FirstOrDefault(x => x.Key == group.Key),
                group));

    return result;
}

Here is sample of calculations where I use these transactions collections:

private Dictionary<Guid, decimal> _CalculateSales(
    IEnumerable<CustomerInfo> customersSalesInfo)
{
    var result = new Dictionary<Guid, decimal>();

    foreach (var customerInfo in customersSalesInfo)
    {
        var sales = customerInfo.Invoices.Sum(x => x.Header.SubTotalAmt) +
            customerInfo.Orders.Sum(x => x.Header.SubTotalAmt) -
            customerInfo.Payments.Sum(x => x.Header.SubTotalAmt);
        result.Add(customerInfo.CustomerId, sales);
    }

    return result;
}

Is there a better approach?

If they all implemented a common interface then you could simplify it.

 public interface ICustomerItem
    {
        Guid CustomerId {get;}
     }

Concat the the items together

var items = invoices.Concat(payments).Concat(orders);

Then project them into your class

 items
.GroupBy(x => x.CustomerId)
.Select(g =>
          new CustomerInfo(g.Key,
                 g.OfType<Order>(),
                 g.OfType<Payment>(),
                 g.OfType<Invoice>())

You could take your original approach and use .ToList()

var invoicesGroupdByCustomers = invoices.GroupBy(x => x.CustomerId).ToList();
var ordersGroupdByCustomers = orders.GroupBy(x => x.CustomerId).ToList();
var paymentsGroupdByCustomers = payments.GroupBy(x => x.CustomerId).ToList();

Just another option to be aware of.  I hope this helps.