Promise.jsm demo of Promise.all and Promise.defer I created this demo example for MDN documentation to show how to use Promise.all and Promise.defer in user created promise. I used lets because I thought if I didn't it would mess it up.

People are recommending I avoid let because the standard is not there yet and it looks like when it lands it will be a little different.  Can you please advise?

The code downloads images from an array and after all images are downloaded it completes the main promise.

Components.utils.import("resource://gre/modules/Promise.jsm");

var myPromise = myUserDefinedPromise();
myPromise.then(
  function(aSuccessReason) {
    alert('myPromise was SUCCESFUL and reason was = "' + aSuccessReason + '"');
  },
  function(aRejectReason) {
    alert('myPromise FAILED for reason = "' + uneval(aRejectReason) + '"');
  }
);

function myUserDefinedPromise() {
  try {
    var mySubPromises = [];

    var imagePaths = ['http://www.mozilla.org/media/img/firefox/favicon.png', 'https://developer.cdn.mozilla.net/media/redesign/img/favicon32.png'];
    [].forEach.call(imagePaths, function(path) {
      let myImage = new Image();
      let loadThisImagePromise = Promise.defer();
      mySubPromises.push(loadThisImagePromise.promise);
      myImage.onload = function() {
        loadThisImagePromise.resolve('Succesfully loaded image at path = "' + path + '" the width of this image is = "' + this.naturalWidth + '".');
        if (!this.naturalWidth) {
          loadThisImagePromise.reject('Image loaded but it has 0 width at path = "' + path + '" the naturalWidth was 0');
        }
      }
      myImage.onerror = function(e) {
        loadThisImagePromise.reject('An error occured while loading path = "' + path + '". The error = ' + uneval(e));
      }
      myImage.onabort = function(e) {
        loadThisImagePromise.reject('Image load was aborted loading path = "' + path + '".');
      }
      myImage.src = path;
    });

    return Promise.all(mySubPromises);
  } catch(ex) {
    return Promise.reject(ex);
  }
}

Here's my implementation:

// So we define a function that loads images, and returns a promise for us to listen to
function loadImages(images){

  // We can take advantage of `map` which iterates through an array and makes an array
  // out of the returns of each iteration. In this case, we return a promise for each
  // url, for `Promise.all` to listen to later.
  var promises = images.map(function(url){

    // Create a Deferred object and image, and handle the resolution stuff.
    var deferred = Promise.defer();
    var image = new Image();
    image.onload = function(){deferred.resolve('image success: '+ url);};
    image.onerror = function(){deferred.reject('image failed: ' + url);};
    image.onerror = function(){deferred.reject('image aborted: ' + url);};
    image.src = url;

    // Return the promise, which will be stored in the array, thanks to `map`
    return deferred;
  });

  // Listen on the array of promises (aha moment: jQuery.when == Promises.all, more or less.)
  return Promise.all(promises);
}

// Now we use `loadImages`, loading an array of urls, and listening for notices
loadImages([
  'http://www.mozilla.org/media/img/firefox/favicon.png',
  'https://developer.cdn.mozilla.net/media/redesign/img/favicon32.png'
]).then(function(value){
  console.log('loadImages successful: ' + value);
},function(error){
  console.log('loadImages failed: ' + error);
});

Additional notes:

Don't do [].forEach.call. Aside from looking very hackish, it creates an unnecessary array. Do Array.prototype.forEach.call instead.
I'd avoid using try-catch. While it's a good measure for errors, it's bad for optimization. Some optimizing compilers (I think V8) can't optimize code inside try-catch.
Name your variables and functions properly and verbosely.


Naive implementation of KMP algorithm After reading this answer to the question "High execution time to count overlapping substrings", I decided to implement the suggested Knuth-Morris-Pratt (KMP) algorithm. I used the pseudo-code listed on Wikipedia for the functions kmp_table and kmp_search.

However, when running it on some corner-cases, I have observed that it is a lot slower than the standard str.find, which apparently uses a modified Boyer-Moore-Horspool algorithm and should thus have worse worst-case performance.

The specific case I looked at is:

$ ipython -i kmp.py
In [1]: text = "A"*1000000 + "B"
In [2]: word = "A"*100 + "B"
In [3]: %timeit kmp_search(text, word)
1 loop, best of 3: 410 ms per loop
In [4}: %timeit text.find(word)
1000 loops, best of 3: 703 µs per loop

So the difference is about a factor 1000 for this input. This is probably due to the fact that the native one is written in C and this is written in Python, but I still wanted to see if I did anything stupid here or missed any obvious optimization.

def kmp_table(word):
    table = [0] * len(word)
    position, candidate = 2, 0
    table[0] = -1

    while position < len(word):
        if word[position - 1] == word[candidate]:
            table[position] = candidate + 1
            candidate += 1
            position += 1
        elif candidate > 0:
            candidate = table[candidate]
        else:
            table[position] = 0
            position += 1
    return table

def kmp_search(text, word):
    m, i = 0, 0
    table = kmp_table(word)
    while m + i < len(text):
        if word[i] == text[m + i]:
            if i == len(word) - 1:
                return m
            i += 1
        else:
            if table[i] > -1:
                m += i - table[i]
                i = table[i]
            else:
                m += 1
                i = 0
    return len(text)

Minor comments, but...

    else:
        if table[i] > -1:
            m += i - table[i]
            i = table[i]
        else:
            m += 1
            i = 0

This sort of construct, an else which contains only an if-else chain, can be simply written as an elif-else chain.

    elif table[i] > -1:
        m += i - table[i]
        i = table[i]
    else:
        m += 1
        i = 0

        table[position] = candidate + 1
        candidate += 1

These statements seem weird, why not first add one and then set?

        candidate += 1
        table[position] = candidate


Coin Flip Streaks - correct streak condition This is a practice task from Automate the Boring Stuff with Python. I imagine many others have asked for their version of the solution to be checked, so I apologise beforehand  for boring you yet again.
In brief, the task entails writing a code that carries out an experiment of checking if there is a streak of 6 'heads' or 'tails' in 100 coin tosses, then replicates it 10,000 times and gives a percentage of the success rate.
import random
numberOfStreaks = 0
listOf100 = []
streak = 0

def toss():
    flip = random.randint(0, 1)
    if flip == 0:
        return 'H'
    else:
        return 'T'

for experimentNumber in range(10000):
    # Code that creates a list of 100 'heads' or 'tails' values.
    for flipCoin in range(100):
        listOf100.append(toss())

    # Code that checks if there is a streak of 6 'heads' or 'tails' in a row.
    for listItem in range(len(listOf100) - 1):
        if listOf100[listItem] == listOf100[listItem + 1]:
            streak += 1
            if streak == 5:
                numberOfStreaks += 1
                streak = 0
                break
        else:
            streak = 0
    listOf100 = []

print('Chance of streak: %s%%' % (numberOfStreaks / 10000))

My question is, am I correct in setting the condition if streak == 5?
My reasoning is that there are 5 pairs to be checked for similarities, if the actual streak is to be 6, for example:
if listOf100[0] == listOf100[1]
if listOf100[1] == listOf100[2]
if listOf100[2] == listOf100[3]
if listOf100[3] == listOf100[4]
if listOf100[4] == listOf100[5]
So, if all 5 such pairs increase streak with 1, it means that there are 6 list items in a row that are either 'heads' or 'tails'.
Thank you!

PEP 8
The Style Guide for Python Code "gives coding conventions for the Python code ... intended to improve the readability of code and make it consistent across the wide spectrum of Python code."
Since a large majority of Python projects follow PEP-8 guideline, it behooves you to follow those conventions as much as possible (except when you shouldn't, as outlined in section 2 of the document).
These conventions include:

using snake_case for variable names, instead of mixedCase.  Eg, numberOfStreaks should be named number_of_streaks.
imports should be followed by a blank line
functions should appear after import and before main code.  Eg) number_of_streaks = 0, list_of_100 = [], and streaks = 0 should appear after def toss():
the mainline code should be inside a "main-guard" (if __name__ == '__main__':) statement.

Correctness
Task

I fear you have interpreted the practice task incorrectly, or at least, implemented it wrong.
The task is to check "if there is a streak of 6 'heads' or 'tails' in 100 coin tosses, not "how many" streaks occurred.  It asks for a percentage success rate.  If you had an unfair coin, with your code you may find several dozen streaks in each experiment, and well over 10,000 streaks in the course of the 10,000 experiments, which would lead to a "percentage success rate" that exceeds 100%, which is suspect.

(Incorrect, but left in to support Heap Overflow's answer)
Math
print('Chance of streak: %s%%' % (numberOfStreaks / 10000))
Simply dividing a count by the total possible does not yield a percentage; 95 / 100 = 0.95 ... you must multiply by 100 to compute the result as a percentage.
WET -vs- DRY and locality of reference.
Your code reads (roughly):
listOf100 = []

# ...

for experiment ...:

    for flipCoin in range(100):
        listOf100.append(toss())

    ...

    listOf100 = []

You see the listOf100 = []?  WET stands for "Write Everything Twice".  In contrast, DRY stands for "Don't Repeat Yourself".  In general, with less code, the code is easier to understand and maintain.  If variables are defined near where they are used, the code is also easier to understand and maintain.
Let's DRY this code up.
# ...

for experiment ...:

    listOf100 = []
    for flipCoin in range(100):
        listOf100.append(toss())

    ...

Now, listOf100 = [] exists only once, and it exists right before it is being used.
Now, as demonstrated in the other two answers, you can replace the initialization and repeated .append() with a more concise list comprehension.
Magic Numbers
I see several numbers in the code: 10000, 100, listOf100, 1, 5, 0.  What do these numbers mean?
If you wanted to change the number of experiments from 10000 to 20000 how many changes would you have to make?  Two?
If you wanted to changed the number of tosses per experiment from 100 to 200, how many changes do you have to make?  Change a number once, and a variable name 6 times???  That seems awkward and unmaintainable.  And wrong, because there is also the comment.
Named constants go a long way to improving maintainability.
NUM_EXPERIMENTS = 10_000

...

for experiementNumber in range(NUM_EXPERIMENTS):
   ...

print('Change of streak: %s%%' % (numberOfStreaks / NUM_EXPERIMENTS))

Finally, 5 is the length of the streak.  No, wait, 6 is the length of the streak.  Uh.  It would be nice to have a STREAK_LENGTH = 6 named constant, and then the algorithm could use if streak == STREAK_LENGTH - 1:, with perhaps a comment explaining the "why".
Unused variables
The variable created in this statement:
for experimentNumber in range(10000):

is never used anywhere.  It only serves two purposes.

to make a syntactically valid for statement.
indicate this loop is executed once per experiment.

The second reason is obsoleted by changing the magic number 10000 into the named constant NUM_EXPERIMENTS.  By convention, _ is used as the throw-away variable, used only to satisfy syntactical reasons.  So this for statement could become:
for _ in range(NUM_EXPERIMENTS):

Ditto for the for flipCoin in range(100): statement; it could become (say):
    for _ in range(COIN_TOSSES_PER_EXPERIMENT):

Formatting numbers
Using the %s format code for a number is not a good habit to get into.  It may produce ok results here; you are dividing by 10,000 so likely will get a number with only 4 decimal points.  But if you were asked to perform a different number of experiments, such as 7, you could get a lot of digits after the decimal point.
Using the format code %.4f produces four digits after the decimal point, regardless of the actual number of experiments.
Improved Code
Others have answered with advanced -- or at best, tricky, and at worst, confusing -- methods of detecting the streaks including:

string concatenation and substring searching
functional programming
converting head/tail coin values into same/different values

In the spirit of the beginner tag, let's investigate a clearer way.
You are currently testing listOf100[listItem] == listOf100[listItem + 1] to check if a coin face is the same as the next.  The [listItem + 1] is the awkward part here, necessitating stopping our loop one element before the end of the list.  Let's rethink this.  Instead of comparing two coins at a time, how about examining only one coin at a time?  Simply remember the whether the streak is currently heads or tails, and ask if the current coin matches that streak:
    for coin_face in coin_tosses:
        if coin_face == current_streak_face:
            streak_length += 1

When we find a coin that doesn't match the current streak, we have to start the streak with one instance of the new face.
        else:
            current_streak_face = coin_face
            streak_length = 1

Of course, we have to initialize our state variables.  The first coin won't match any previous value, so we should start off with some value which is neither heads nor tails.
    current_streak_face = None
    streak_length = 0

Using this, we can create a simple coin streak detector function:
def contains_a_streak(coin_tosses, minimum_length):

    current_streak_face = None
    streak_length = 0

    for coin_face in coin_tosses:
        if coin_face == current_streak_face:
            streak_length += 1
        else:
            current_streak_face = coin_face
            streak_length = 1

        if streak_length >= minimum_length:
            return True

    return False

Notice that since we are initialize the streak_length to 1 when we find a different coin face, and adding 1 when we find a matching face, our streak_length counter is actually the length of the streak, and not one less.  No more 5 -vs- 6, confusion, which is a huge win for clarity.
Actually, there is nothing about this detector that is specific to coin tosses.  We could use it for dice rolls, win-loss streaks, and so on.  Just need to change some variable names ... and change the initial value from None to a different sentinel, so it could even properly detect a streak of None values at the start of a sequence of values.
def contains_a_streak(iterable, minimum_length):

    current = object()   # a unique value that can't possibly match this first
    streak_length = 0

    for value in iterable:
        if current == value:
            streak_length += 1
        else:
            current = value
            streak_length = 1

        if streak_length >= minimum_length:
            return True

    return False

Now, our code for one experiment could become:
def coin_toss_experiment(number_of_tosses, streak_length):

    tosses = []
    for _ in range(number_of_tosses):
        tosses.append(toss())

    return contains_a_streak(tosses, streak_length)

As noted elsewhere, the list initialization and repeated appending could be replaced with list comprehension:
def coin_toss_experiment(number_of_tosses, streak_length):

    tosses = [toss() for _ in range(number_of_tosses)]

    return contains_a_streak(tosses, streak_length)

(Actually, a generator expression might be even better, but since we're focusing at the beginner level, we'll just note it in passing.  When you're a bit more comfortable with Python, look up what it is and what it would do for you, and why you might want to use one.)
We need to run multiple experiments to compute the streak success rate:
def repeated_coin_toss_experiment(num_experiments, num_tosses, streak_length):
    successes = 0
    for _ in range(num_experiments):
        if coin_toss_experiment():
            successes += 1

    print(f"Chance of streak: {successes/num_experiments*100:.2f}%")

Finally, we need to run our experiment:
if __name__ == '__main__':
    repeated_coin_toss_experiment(10_000, 100, 6)

If you want to change the number of tosses, you only have to change one number.  If you want to change the number of experiments, again, you just have to change one number.  Change the streak length?  Well, you get the idea.


Compare Collection with the information in DB using EF and LINQ I want to inform the Receivers about new contracts we have this week. After I sent the Emails, the Information about sent contracts are stored in MS SQL Database.

To avoid that someone become the same Email about contract more than one time I perform this steps:

I have the Collection with new contracts as the input
I read the information from DB using Entity Framework about sent Emails
If the Emails was already sent I add new Instance to the sent Emails Collection

After that, I remove the contracts from input collection that are already in sent collection:

public Collection<Contract> GiveContractsThatWereNotSent(Collection<Contract> newThisWeekContracts)
{
   var contractsWereSent = new Collection<Contract>();

   using (var accountingEntities = new AccountingEntities())
   {
      foreach (Contract newContract in newThisWeekContracts)
      {
         bool found = (from sent in accountingEntities .Tbl_SentProtocol
                       where
                          sent.CONTRACT_NO == newContract.ContractNo
                          && sent.CONTRACT_NO_ALT == newContract.ContractNoAlt
                        select sent ).Any();

         if (found)
         {
            contractsWereSent.Add(newContract);
         }
      }
   }

   foreach (Contract contract in contractsWereSent)
   {
      newThisWeekContracts.Remove(contract);
   }

   return newThisWeekContracts;
}

Several points:

Write your code against abstractions. Collection is a specific implementation.

You don't need newThisWeekContracts to be specifically a Collection. I'd take in any IEnumerable<Contract> instead.
By returning a Collection, the caller can add to it, or remove from it, which probably isn't the intended behavior. I'd return an IEnumerable<Contract>, or, if adding/removing is ok, I'd return an IList<Contract> or <ICollection<Contract> (i.e. an abstraction, not a specific implementation).

I like the descriptive naming, casing is correct and all, but the names read a little bit awkward still.

It looks like the number of queries can be reduced, haven't profiled this but I would think it consumes fewer resources; the main thing is combining .Where() with .Any() and !.Any() as needed:

public IEnumerable<Contract> GiveContractsThatWereNotSent(IEnumerable<Contract> newContracts)
{
    using (var entities = new AccountingEntities())
    {
        var sentContracts =
                   entities.Tbl_SentProtocol
                           .Where(sent =>
                                  newContracts.Any(contract =>
                                                   sent.CONTRACT_NO == contract.ContractNo
                                                && sent.CONTRACT_NO_ALT == contract.ContractNoAlt))
                           .ToList();

    }

    // ...
}

That gives you sent contracts (/ contractsWereSent), but that's not what you're after. Now you need to return the contracts in newContracts (/ newThisWeekContracts) that are not in sentContacts.

What you have here:

foreach (Contract contract in contractsWereSent)
{
    newThisWeekContracts.Remove(contract);
}

Involves calling EqualityComparer<T>.Default on each contract, which verifies whether Contract implements IEquatable<T>, otherwise uses overrides of .Equals and .GetHashCode if Contract overrides them, otherwise uses reference equality... which doesn't sound very neat, since if a contract is "equal" when its ContractNo and ContractNoAlt exists in sendContracts when comparing in Tbl_SentProtocol, then I'd use the same equality comparison when I want to "remove" items from the newContracts.

In fact, I wouldn't remove anything from newContracts. Keep inputs for inputs!

How about this?

IEnumerable<Contract> sentContracts;
using (var entities = new AccountingEntities())
{
    sentContracts = // trying to prevent horizontal scrolling...
        entities.Tbl_SentProtocol
                .Where(sent =>
                       newContracts.Any(contract =>
                                        sent.CONTRACT_NO == contract.ContractNo
                                     && sent.CONTRACT_NO_ALT == contract.ContractNoAlt))
                .ToList();
}

return newContracts.Where(contract => !sentContracts.Any(sent =>
                                        sent.CONTRACT_NO == contract.ContractNo
                                     && sent.CONTRACT_NO_ALT == contract.ContractNoAlt))
                   .ToList();

One more thing, the entities should be named so as to read like normal code - entity type SentContract can be mapped to table tbl_SentProtocol, and properties ContractNumber and ContractAltNumber can map to columns CONTRACT_NO and CONTRACT_NO_ALT ;)


Sort string's characters into alphabetic order Any improvements that I can make with this code?
Also, is there another way to convert a vector to a string variable without using a for ranged-based loop? From the posts I've seen for converting, I only saw printing the vector instead of returning a string.

#include <string>
#include <vector>
#include <sstream>
#include <algorithm>
#include <iterator>

using namespace std;

std::string AlphabetSoup(string str) {
std::vector<char>letters;

 istringstream iss(str);

 copy(istream_iterator<char>(iss),
        istream_iterator<char>(),
        back_inserter(letters));

 std::sort(letters.begin(),letters.end());

 //Regrouping the characters into a string
 std::string orderedString;
 for(auto element : letters)
 {
     orderedString += element;
 }

 return orderedString;
}

Don't use using namespace std

You don't need a stringstream or an istringstream or a vector for this. std::sort can be performed on a string directly. An std::string_view can be used to avoid copying (more on this later)

There seems to be 3 options how to make this work best. If you don't care about chewing up your original string you can perform the operations on it directly without ever copying anything. Here you will pass by reference:

std::string& AlphabetSoup(std::string& str) {
   std::sort(str.begin(), str.end());
   str.erase(std::remove(str.begin(), str.end(), ' '), str.end());

 return str; // or omit this and change the return value to void
}

You can also pass by value which will copy your string and leave the original intact:

std::string AlphabetSoup(std::string str) {
   std::sort(str.begin(), str.end());
   str.erase(std::remove(str.begin(), str.end(), ' '), str.end());

 return str;
}

Or you can pass in as a std::string_view which will not copy.

std::string AlphabetSoup(std::string_view str) {
   std::string s(str);
   std::sort(s.begin(), s.end());
   s.erase(std::remove(s.begin(), s.end(), ' '), s.end());
   return s;
}

The method I used to remove the spaces is known as the erase-remove idiom. The character specified in the remove() function is moved to the end of the range (provided as parameters) where they can be efficiently erased. There are faster iterators on sorted sets that can speed things up as @TobySpeight pointed out but I haven't used them. (Namely std::lower_bound and std::upper_bound)

Consider providing test cases and output. This too will help reviewers and make it more likely for you to get solid answers.


CSV to Object converter with JavaDoc comments This is a simple class but I'm looking to check I have the basics down to scratch.

Is my documentation thorough enough? (I'm very new to writing JavaDoc) Have I created too many variables, and does this sacrifice performance for readability in a bad way?

This class is called once to create an array of TestDataObjects to use with our Parsing unit tests.

    /**
 * Converts our test data csv in to an ArrayList
 * We divide the csv entries up line by line for each calendar event then create a TestData object from each line.
 * The fields we wish to parse have negated new line characters to ensure they don't interfere with the csv parsing.
 * Once the csv has been parsed we need to unnegate these so the test data is the same as the original data.
 * @param calendar The csv to convert
 * @return An Array List of TestData objects that have all the three fields we wish to parse and the correct results we wish to obtain.
 */
private ArrayList<TestData> loadTestData(String calendar) {
    ArrayList<TestData> testDataArray = new ArrayList<>();
    String[] lines = calendar.split("\r\n");

    // always skip lines[0] as it contains the headers
    for (int i = 1; i < lines.length; ++i) {
        ArrayList<String> fields = CSVReader.parseLine(lines[i]);
        String title = fields.get(0).replace("\\n", "\n");
        String location = fields.get(1).replace("\\n", "\n");
        String description = fields.get(2).replace("\\n", "\n");
        ArrayList<String> correctHostCodes = CSVReader.parseLine(fields.get(3));
        ArrayList<String> correctParticipantCodes = CSVReader.parseLine(fields.get(4));
        ArrayList<String> correctPhoneNumbers = CSVReader.parseLine(fields.get(5));

        TestData testDataObject = new TestData(title + "\n" + location + "\n" + description, correctHostCodes, correctParticipantCodes, correctPhoneNumbers);
        testDataArray.add(testDataObject);
    }

    return testDataArray;
}

Prefer interfaces to implementations

private ArrayList<TestData> loadTestData(String calendar) {
    ArrayList<TestData> testDataArray = new ArrayList<>();

and

        ArrayList<String> fields = CSVReader.parseLine(lines[i]);

and

        ArrayList<String> correctHostCodes = CSVReader.parseLine(fields.get(3));
        ArrayList<String> correctParticipantCodes = CSVReader.parseLine(fields.get(4));
        ArrayList<String> correctPhoneNumbers = CSVReader.parseLine(fields.get(5));

You would normally write these as

private List<TestData> loadTestData(String calendar) {
    List<TestData> testDataArray = new ArrayList<>();

and

        List<String> fields = CSVReader.parseLine(lines[i]);

and

        List<String> correctHostCodes = CSVReader.parseLine(fields.get(3));
        List<String> correctParticipantCodes = CSVReader.parseLine(fields.get(4));
        List<String> correctPhoneNumbers = CSVReader.parseLine(fields.get(5));

That way, even if parseLine changes its return type, you don't have to change unless it changes to something that doesn't implement the List interface.

Range-based for loops

    String[] lines = calendar.split("\r\n");

    // always skip lines[0] as it contains the headers
    for (int i = 1; i < lines.length; ++i) {
        ArrayList<String> fields = CSVReader.parseLine(lines[i]);

You could rewrite this as

    List<String> lines = Arrays.asList(calendar.split("\r\n"));

    // always skip lines[0] as it contains the headers
    for (String line : lines.sublist(1, lines.size())) {
        List<String> fields = CSVReader.parseLine(line);

Or not.  If you find the indexed form easier to follow, that works too.


Python Decimal to Binary converter (without using bin() or output formatting) There is no input validation for this, but that aside, I would appreciate it if anyone could help me improve the logic of this script.

import math
#Input
num = input("Enter a whole number: ")

#Floors input in case of a non whole number
decimal = math.floor(float(num))

#Variables
exp = 0
divTwo = 1
zeros = 0
total = ""
if decimal < 0:
    total += str('-')
    decimal = -decimal
if decimal == 0:
    total = str('0')
else:
    #Loops while remainder is > 0.
    while int(decimal) > 0:

        #Increases exponent by one if 2^exp is less than or equal to the remainder.
        if 2 ** exp <= int(decimal):
                exp += 1

        else: #Multiplies 2 by the exponent and places zeros in between 1s. Also subtracts from the exponent.
                exp -= 1

                #Determines whether or not to subtract from the remainder and add a '1' to the binary translation or just add a '0' to the binary translation.
                if 2 ** exp <= int(decimal):

                        #Counts how many zeros need to be added to the end of the binary translation.
                        if 2 ** exp == int(decimal):
                                divTwo = int(decimal)

                                #Loops until all remaining zeros have been accounted for.
                                while divTwo > 1:
                                        divTwo = divTwo / 2
                                        zeros += 1

                        decimal = int(decimal)
                        decimal -= 2 ** exp
                        total += str('1')

                else:
                        total += str('0')

#Places remaining zeros.
while zeros > 0:
        total += str('0')
        zeros -= 1

#Displays answer.
print ('Binary: ' + total)
input()

#
#
#Or 'bin(decimal)' works too, but this was for fun
#
#

Here is a working version of the script.

You can remove half of you code.
You need the variables exp, total and decimal.
The rest are just noise.
You should remove all the int(decimal)s, decimal is only ever subtracted from, and so it's not going to become a float.
And so these are not needed.

You can remove all the code in if 2 ** exp == int(decimal):, instead just use exp as you do zeros.
You should split your while loop so that it only does one thing at a time.
You want an int log and then a while loop that converts the input to binary.
You can replace while zeros > 0: with Pythons way to duplicate strings. 0 * zeros.
You should make this a function.

This leads to code like:

def binary(number): # 4
    output = ''
    if number < 0:
        output = '-'
        number = -number

    exp = 1
    while 2 ** exp <= number: # 2
        exp += 1

    while number > 0:
        exp -= 1
        if 2 ** exp <= number:
            number -= 2 ** exp
            output += '1'
        else:
            output += '0'

    return output + '0' * exp # 1, 3

number = int(input("Enter a whole number: "))
print('Binary: ' + binary(number))


Constant-time string equality check This is my attempt to answer my own equally named question on SO. In this case, I need a method comparing two strings so that the running time is input independent.

// Not private in order to prevent optimizations.
static class Blackhole {
    private static void eat(long n) {
        dummy += n;
    }

    // Not private in order to prevent optimizations.
    static volatile long dummy;
}

public boolean areEqual(String input, String secret) {
    final int length = secret.length();
    // No need to keep the length secret.
    if (input.length() != length) {
        return false;
    }
    long delta = 0;
    for (int i = 0; i < length; ++i) {
        // Any char is interpreted as a non-negative number.
        // Xoring such numbers preserves non-negativity.
        // The result depends on every input bit, so no short-circuit is possible.
        // Overflow is impossible, therefore diff is zero <=> all chars are the same.
        delta += input.charAt(i) ^ secret.charAt(i);
    }
    Blackhole.eat(delta);
    return delta == 0;
}

It's a really short snippet and I'm mainly interested in ensuring that it really works in constant time.  In particular, I'm afraid that the loop could indeed get optimized to something like

int i = 0
for (; ; ++i) {
    if (input.charAt(i) != secret.charAt(i)) break;
}
for (; i < length; ++i) {
    delta += input.charAt(i) ^ secret.charAt(i);
}

where the first loop could be faster on some architectures, as -- after unrolling -- it translates to load, xor with memory, and a conditional jump. Assuming some future architecture capable of executing 6 instructions per cycle when there are no data dependencies and no mispredictions, it could be a win. Or the JIT could believe, it makes sense...

Concerning the Blackhole, I wonder if volatile is necessary. I didn't want to depend on JMH nor copy this monster.

In the meantime I've got an idea avoiding volatile writes nearly perfectly

    private static void eat(long n) {
        if (n == dummy) {
            dummy = new SecureRandom().nextLong();
        }
    }

Firstly, we cannot rely on arguments that the "compiler isn't smart enough to do that" because that may change tomorrow. We need to base our assessment of the code on what the compiler is and isn't allowed to do.

Given two input strings and a yes/no return if the string match, any program performing the function can be transformed into the trivial short-circuited for-loop. Provided that there isn't anything prohibiting the compiler from doing the transform.

Will writing the sum of xor differences to a volatile prevent the transform? Maybe, it depends on the execution model and guarantees of volatile in Java. In C++ under the as-if rule all writes and reads to/from volatile memory regions must occur in the same order as-if the program was executed according to the wording in the standard (somewhat simplified but that's the gist of it). So for C++ this would work; However I'm unsure if Java has the same guarantee. For C++ this limitation is natural because it must be able to interface with device drivers and bus addresses where writes and reads cannot be re-ordered or omitted. But Java does none of the kind and as such I wouldn't be terribly surprised if volatile has a more lax meaning in Java.

What about SecureRandom? Well it is really just volatile reads from /dev/urand in disguise to get the seed, the rest is deterministic which the compiler that compiled the JVM isn't allowed to reorder or remove, but what the JIT is allowed to do in this context is beyond me. It is conceivable that the JIT can deduce that the read from /dev/urand will not affect the program in an observable way and it will remove the code all together.

If the volatile keyword doesn't have the same meaning in Java as in C++ I believe that doing this kind of processing in Java in a reliable and guaranteed way is difficult. I would consider a hand written loop in assembler in a JNI library, this is guaranteed by your C/C++/assembler to not be fudged in any way.

I'm sorry this isn't an authoritative yes/no answer on correctness, but I hope that it is of some help any way.


First garbage collector Today I tried to learn how to make a simple garbage collector for a future project, it didn't need to be fast nor complex nor optimal, it just needed to work. After lots of searching and reading, I was able to make something work.

So, how can I improve it from here? At some point I guess I'll have to ditch the std::array (the small size is just for testing purposes). Do I actually need to use new and delete, or can I just keep using STL containers?

For what I've read, I have a couple options: mark and compact, copying garbage collection, generational garbage collector. Which of them would be the more natural evolution from what I've done?

#include <array>
#include <iostream>
#include <string>

using std::cout;
using std::endl;

template <typename T>
struct Handle {
    bool marked {};
    T value {};
    Handle * next {};
};

template <typename T>
class Heap {
    std::array<Handle<T>, 10> heap;

    Handle<T> * root {};
    Handle<T> * free_list {};

    void push(Handle<T> * handle) {
        handle->next = free_list;
        free_list = handle;
    }

    void reset() {
        for (size_t i = 0; i < heap.size(); ++i) {
            heap[i].marked = false;
        }
    }

    void mark(Handle<T> * handle) {
        if (handle && !handle->marked) {
            handle->marked = true;
            mark(handle->next);
        }
    }

    void sweep() {
        free_list = nullptr;
        for (size_t i = 0; i < heap.size(); ++i) {
            if (!heap[i].marked) {
                heap[i].value = {};
                push(&heap[i]);
            }
        }
    }

public:
    Heap() {
        for (size_t i = 0; i < heap.size(); ++i) {
            push(&heap[i]);
        }
    }

    template <typename U>
    Handle<T> * allocate(U && u) {
        if (!free_list) {
            reset();
            mark(root);
            sweep();
            if (!free_list) {
                throw std::bad_alloc();
            }
        }
        Handle<T> * handle = free_list;
        free_list = free_list->next;
        handle->value = std::forward<T>(u);
        handle->next = nullptr;
        return handle;
    }

    Handle<T> * keep_alive(Handle<T> * handle) {
        handle->next = root;
        root = handle;
        return handle;
    }

    void signal() {
        root = nullptr;
    }
};

int main() {

    Heap<std::string> heap;

    auto a = heap.allocate("A");
    auto b = heap.keep_alive(a);

    a = heap.allocate("B");
    auto c = heap.keep_alive(a);

    a = heap.allocate("C");
    a = heap.allocate("D");
    a = heap.allocate("E");

    cout << a->value << endl;
    cout << b->value << endl;
    cout << c->value << endl;

    return 0;
}

From a design and interface point of view, I find the use of:

auto a = heap.allocate("A");
cout << a->value << endl;

unnatural. I would expect a to act like a pointer to std::string since it's the value returned from a function named allocate using an object of type Heap<std::string>.

It would be more natural if:

*a evaluated to std::string& (std::string const& if a were const)
a-> evaluated to std::string* (std::string const* if a were const)

Then, you could replace a->value by *a.

auto a = heap.allocate("A");
cout << *a << endl;

P.S. I haven't spent enough time delving into the implementation of Heap to suggest how it can be done.


More concise and/or idiomatic max subarray in Clojure? I've implemented the following two versions of the classic "Max Sub-Array" problem in Clojure, using the Kadane algorithm.

First with loop / recur

(defn max-sub-array [A]
  (loop [x (first A)
         a (rest A)
         max-ending-here 0
         max-so-far 0]
    (if (seq a)
      (recur (first a) (rest a) (max x, (+ max-ending-here x)) (max max-so-far, max-ending-here))
      max-so-far)))

Then with reduce

(defn max-sub-array-reduction [A]
  (letfn [(find-max-sub-array [[max-ending-here max-so-far] x]
             [(max x (+ max-ending-here x)) (max max-so-far max-ending-here)])]
    (second (reduce find-max-sub-array [0 0] A))))

Is there a more concise implementation, perhaps using filter or merely by making the reduce version more "idiomatic" somehow?

Great answer from Jean Niklas L'Orange on the Clojure Google Group:

(defn max-subarray [A]
   (let [pos+ (fn [sum x] (if (neg? sum) x (+ sum x)))
         ending-heres (reductions pos+ 0 A)]
     (reduce max ending-heres)))


Usage of state machine to model lifetime of an IP address I need to model an IP address and thought using a state machine (the state_machine gem) would be a good idea.

An IP has the following characteristics:

It is assigned to a server.
It can be in state unbound (implies unprotected), bound (implies protected), reserved.
It has several firewall rules which should be automatically activated/deactivated when binding/unbinding.
It can be migrated from/to another server.
All operations, such as (de)activating firewall rules and (un)binding, may fail or externally change their state (consider a machine reboot).

Here is what I came up with so far:

require 'rubygems'
require 'state_machine'
require 'awesome_print'

class Ip
  state_machine :state, :initial => :unbound do
    # idle states, i.e. only an event (either external or internal) will trigger a transition.
    state :unbound, :bound, :reserved

    # transitionary states, we expect a state change after a certain period of time.
    state :protected, :unprotected

    event :cycle do
      transition :unbound => :protected
      transition :protected => :bound
      transition :bound => :unprotected
      transition :unprotected => :unbound
    end

    event :reserve do
      transition :unbound => :reserved
    end

    event :release do
      transition :reserved => :unbound
    end

    before_transition any => any, :if => :refreshing? do |ip, transition|
      puts "refreshing, before_transition -> state is: #{transition.from}, state will be: #{transition.to}"
      throw :halt unless ip.send("is_#{transition.to}?")
    end

    before_transition any => any do |ip, transition|
      puts "before_transition -> state is: #{transition.from}, state will be: #{transition.to}"
    end

    before_transition any => :protected, :do => :activate_firewall_rules
    before_transition any => :unprotected, :do => :deactivate_firewall_rules

    before_transition any => :bound, :do => :bind_to_server
    before_transition any => :unbound, :do => :unbind_from_server

    after_transition any => any do |ip, transition|
      puts "after_transition -> state was: #{transition.from}, state is: #{transition.to}"
    end
  end

  def bind!
    wait!(:bound)
  end

  def unbind!
    wait!(:unbound)
  end

  def migrate!(new_server)
    restore_state do
      unbind!
      self.server = new_server
    end
  end

  def refresh!
    return if reserved?
    self.state = self.class.new.state # set to initial state
    @refreshing = true
    nil while cycle
    @refreshing = false
  end

  def restore_state
    old_state = state
    yield
    wait!(old_state)
  end

  def wait!(desired_state)
    cycle! while !state?(desired_state)
  end

  def refreshing?
    @refreshing
  end

  def activate_firewall_rules
    puts "activate_firewall_rules..."
    @is_protected = true
  end

  def deactivate_firewall_rules
    puts "deactivate_firewall_rules..."
    @is_protected = false
  end

  def bind_to_server
    puts "bind_to_server..."
    @is_bound = true
  end

  def unbind_from_server
    puts "unbind_from_server..."
    @is_bound = false
  end

  def is_protected?
    puts "is_protected: #{@is_protected}"
    @is_protected
  end

  def is_unprotected?
    !is_protected?
  end

  def is_bound?
    puts "is_bound: #{@is_bound}"
    @is_bound
  end

  def is_unbound?
    !is_bound?
  end
end

The public API should be something like this:

bind! (bring up firewall and bind address to server)
unbind! (unbind address to server and bring down firewall
refresh! (get real/external state and update internal state)
migrate! (assign to new server, retain state)

Since this is my very first state machine I thought I'd like to ask what you think about it. Do you think using a state machine makes sense here at all? Am I using it correctly or am I doing something wrong? What and why?

Better late than never.

Yes, a state machine makes sense here.  You've chosen good names and your coding style is good.

A few comments:

The "puts" lines appear to be for debugging.  If that is so, and they are going to remain in the code, I would consider writing to an IO object rather than $stdout.  This will make tests much nicer to run, since when passing, you won't be bothered by all the debugging information.  For example:

attr_accessor :debug_io
...
after_transition any => any do |ip, transition|
  @debug_io.puts "after_transition -> state was: #{transition.from}, state is: #{transition.to}"
end

And because you want it to still work even if the caller has not set debug_io, give it a default.  "No output" is a good default:

class NullIO
  def puts ; end
end

def initialize
  super
  @debug_io = NullIO.new
end

The return value of is_protected? depends only upon the state, being true if in the bound or protected state.  Remove:

before_transition any => :protected, :do => :activate_firewall_rules
before_transition any => :unprotected, :do => :deactivate_firewall_rules

and the methods they call, and change is_protected? to:

def is_protected?
  [:protected, :bound].include?(state)
end

Similarly, remove:

before_transition any => :bound, :do => :bind_to_server
before_transition any => :unbound, :do => :unbind_from_server

and the methods they call, and change is_bound? to:

def is_bound?
  [:bound, :unprotected].include?(state)
end

Ruby predicates usually leave off the "is_" prefix.  Consider changing is_bound? to bound? and is_protected? to protected?.  The is_ prefix comes from languages which do not allow question marks in identifiers.

Mark as "private" methods which are for the use of the class only, and should not be called by the class's user.  This will help prevent accidental misuse of the class, but more importantly, document the class's public signature, which helps in understanding and is a big help when refactoring.  Methods such as bind_to_server and unbind_from_server are private (or were, before they were removed).

It's not clear what this line is for:

throw :halt unless ip.send("is_#{transition.to}?")

If it's not for debugging, it seems like an odd bit of hidden coupling between the class and the rest of the program; I would try to find another way to do this that makes it clearer why the :halt is being thrown.


FizzBuzz from a casual rubyist As a casual Rubyist I am mainly interested, how ideomatic my solution is.

class Integer
  def divisible_by?(n)
    (self % n).zero?
  end
end

def fizzbuzz(upper_bound)
  1.upto(upper_bound).map do |number|
    next "fizzbuzz" if number.divisible_by? 3 and number.divisible_by? 5
    next "fizz" if number.divisible_by? 3
    next "buzz" if number.divisible_by? 5
    next number
  end
end

puts fizzbuzz 100

Remarks: I borrowed the idea for monkeypatching Integer from @tokland here

If you're aiming for readability rather than efficiency or maintainability — and that is a reasonable tradeoff for FizzBuzz — then I would say "Well done." (You could test for divisibility by 15 for efficiency and avoid monkey-patching for maintainability.)

Conventionally, the output should be one entry per line, rather than an array.

If you're going to take an upper bound as a parameter, you may as well take a range, and the parameter should default to 1..100.

Therefore, a better way to call the code would be

fizzbuzz(1..100).each { |output| puts output }

or, using the default,

fizzbuzz.each { |output| puts output }


Replicate / Sync files from source to destination with powershell only I have lots of live and disaster recovery servers, which should have the same files and folders. I searched on the web but could not find a Powershell script which has all of the functionality on the same script. So I have created one in the below function.
Please do not use it in Production without fully testing it!!!!!
This script replicates / synchronizes files and folders recursively from source location to destination location with the sync switch.
If you use -Sync $True then the script will remove all of the files and folders on destination location which do not exist in the source location.
Usage:

Sync-Files -SourceRoot "C:\Test" -DestinationRoot "\\Server\Test"
Sync-Files "C:\Test" "\\Server\Test"
Sync-Files -SourceRoot "C:\Test" -DestinationRoot "\\Server\Test" -Sync $true

Source:
    <#
    .Synopsis
    synchronies files between two location
    .DESCRIPTION
    This function is synchronising all files from source location to destination location.
    Script has ability to fully sync files by deleting the destination files if not exists in the source directory.

    if -Sync is set $true script will delete files which exists in the destination directory only.
    Please do not run this script before test in test/dev environment.

    .EXAMPLE
    Sync-Files C:\Test \\SERVER\Test
    .EXAMPLE
    Sync-Files -SourceRoot "C:\Source" -Destination "\\SERVER\DestinationDirectory\

    .EXAMPLE
    Sync-Files -SourceRoot "C:\Source" -Destination "\\SERVER\DestinationDirectory\ -Sync $True
    #>
    function Sync-Files
    {
    [CmdletBinding()]
    Param
    (

    [Parameter(Mandatory=$true,
               ValueFromPipelineByPropertyName=$true,
               Position=0)]
    $SourceRoot,

    [Parameter(Mandatory=$true,
               ValueFromPipelineByPropertyName=$true,
               Position=1)]
    $DestinationRoot,

    [Parameter(
               ValueFromPipelineByPropertyName=$true,
               Position=3)]
    [bool]$Sync

    )
    Begin
    {
    $SourceRootFiles = Get-ChildItem $SourceRoot -Recurse
    $DestinationRootFiles = Get-ChildItem $DestinationRoot -Recurse
    $CompareObjects = Compare-Object -ReferenceObject $SourceRootFiles -DifferenceObject $DestinationRootFiles -ErrorAction SilentlyContinue -IncludeEqual

    }
    Process
    {
    ForEach($CompareObject in $CompareObjects){

        $FinalPath = Join-Path $DestinationRoot $CompareObject.InputObject.FullName.Substring($SourceRoot.length)

        if($CompareObject.SideIndicator -eq "<="){

            if($CompareObject.InputObject.PSIsContainer){
                Copy-Item -Path $CompareObject.InputObject.FullName -Destination $FinalPath -Force
            }

            Elseif(!(Test-Path $FinalPath)){

                Copy-Item -Path $CompareObject.InputObject.FullName -Destination $FinalPath -Force

            }

        }

        if($CompareObject.SideIndicator -eq "=="){

            if((Get-FileHash $CompareObject.InputObject.FullName).Hash -ne (Get-FileHash $FinalPath).Hash){

                Copy-Item $CompareObject.InputObject.FullName $FinalPath -Force
            }

        }

        if($CompareObject.SideIndicator -eq "=>"){
            if($Sync -eq $true){
                if(Test-Path $CompareObject.InputObject.FullName){
                    Remove-Item $CompareObject.InputObject.FullName -Force -Recurse -Verbose
                }
            }

        }

    }
    }
    End
    {

    }
    }

    Sync-Files -SourceRoot "C:\Test" -DestinationRoot "\\Server\Test" -Sync $true

Frankly, robocopy.exe is such a good tool for this job and has been a part of the base Windows install since Vista/2008 that I would just use that instead of reinventing the wheel (especially when you're replacing a top fuel dragster wrinkle slick with an iron-shod wooden wagon wheel).  Yes, robocopy only uses datetime (LastWriteTime) and file size (Length) to detect changes, but if that doesn't work you're doing something rather strange.  That and the fact that Compare-Object is an awful command to work with because extremely unclear exactly what it's doing.

However, from a quick look:

$SourceRootFiles = Get-ChildItem $SourceRoot -Recurse
$DestinationRootFiles = Get-ChildItem $DestinationRoot -Recurse

These will both ignore Hidden and System files without the -Force parameter.

$CompareObjects = Compare-Object -ReferenceObject $SourceRootFiles -DifferenceObject $DestinationRootFiles -ErrorAction SilentlyContinue -IncludeEqual

As far as I'm aware the above only compares file name.  At the very least, you'll want to specify the parameters you're comparing with Compare-Object.  It does not compare all properties like you might expect.  You've got to specify them:

$CompareObjects = Compare-Object -ReferenceObject $SourceRootFiles -DifferenceObject $DestinationRootFiles -ErrorAction SilentlyContinue -IncludeEqual -Property Name, LastWriteTime, Length

However, even then, Compare-Object isn't going to work here when files move within the directory structure.

Example:

# Create two directory trees
$SourceDir = mkdir DirectoryA
$DestDir = mkdir DirectoryB
$DestDirSub = mkdir "$DestDir\DirectoryC"

# Create some files in the SourceDir
Push-Location $SourceDir
0..9 | ForEach-Object {
    Set-Content -Path $_ -Value $null
}
Pop-Location

# Copy the files
Get-ChildItem -Path $SourceDir -File | Copy-Item -Destination $DestDir

# Move some files to a subdirectory
Get-ChildItem -Path $DestDir -File | Select-Object -Last 5 |  Move-Item -Destination $DestDirSub

OK, now lets get the list of files.  0-9 should be in DirectoryA, 0-4 in DirectoryB, and 5-9 in DirectoryB\DirectoryC.  Now do the comparison:

# Fetch file lists
$Source = Get-ChildItem -Path $SourceDir -Recurse
$Dest = Get-ChildItem -Path $DestDir -Recurse

# Do comparison
Compare-Object -ReferenceObject $Source -DifferenceObject $Dest -IncludeEqual -Property Name, LastWriteTime, Length

I get:

InputObject SideIndicator
----------- -------------
0.txt       ==
1.txt       ==
2.txt       ==
3.txt       ==
4.txt       ==
5.txt       ==
6.txt       ==
7.txt       ==
8.txt       ==
9.txt       ==
DirectoryC  =>

And that's why I hate Compare-Object.

You can't just specify -Property FullName, LastWriteTime, Length or Compare-Object [...] -Property * either, since it's got the root part of the path in the properties.  You'll need to do something like this:

# Normalize the source and destination paths
$SourcePath = $SourceDir.FullName.TrimEnd('\');
$DestPath = $DestDir.FullName.TrimEnd('\');

# Fetch file lists
$Source = Get-ChildItem -Path $SourcePath -Recurse | Select-Object -Property *, @{n = 'RelativeName';e ={$_.FullName.Substring($SourcePath.Length)}}
$Dest = Get-ChildItem -Path $DestPath -Recurse | Select-Object -Property *, @{n = 'RelativeName';e ={$_.FullName.Substring($DestPath.Length)}}

# Do comparison
Compare-Object -ReferenceObject $Source -DifferenceObject $Dest -IncludeEqual -Property Name, LastWriteTime, Length, RelativeName

Alternately, you could do something like this using Tuples as the key in a Dictionary:

$SourceTable = @{}
$Source = Get-ChildItem -Path $SourceDir -Recurse | ForEach-Object {
    $t = [System.Tuple]::Create($_.FullName.Substring($SourceDir.FullName.TrimEnd('\').Length), $_.LastWriteTime, $_.Length)
    $SourceTable.Add($t, $_.FullName)
}

$DestTable = @{}
$Dest = Get-ChildItem -Path $DestDir -Recurse | ForEach-Object {
    $t = [System.Tuple]::Create($_.FullName.Substring($DestDir.FullName.TrimEnd('\').Length), $_.LastWriteTime, $_.Length)
    $DestTable.Add($t, $_.FullName)
}

# Files in source and dest and could be filehashed
$SourceTable.GetEnumerator() | Where-Object { $DestTable.ContainsKey($_.Key) }

# Files in source and not in dest
$SourceTable.GetEnumerator() | Where-Object { !$DestTable.ContainsKey($_.Key) }

# Files in dest and not in source
$DestTable.GetEnumerator() | Where-Object { !$SourceTable.ContainsKey($_.Key) }

That will be really fast once it's loaded all the tables, but it's really pretty gross to set up.


Code behaviour: Working object orientated with a database I'm working on my own PHP framework for a long while now.
Right now I'm in a refactoring process.

I'm came up with the question what a programmer would excpect what happens when he see following code:

// working object orientated with a database.

$UserGroup = new UserGroupModel();
$UserGroup->Title = 'Guest';
$UserGroup->Insert(); // execute sql

$User = new UserModel();
$User->Name = 'Peter';
$User->UserGroupID = $UserGroup->UserGroupID;

// print all users
print_r($UserGroup->Users);

$User->Insert();  // execute sql

// print all users
print_r($UserGroup->Users);

Would you expect that $UserGroup->Users imeditaley contains the reference or after the  statement $User->Insert();.

(Currently it is the second one.)

The same with an update statement:

// lets pretend we got the $UserGroup1 and $UserGroup2 from the database

$User = $UserGroup1->Users[0];
$User->UserGroupID = $UserGroup2->UserGroupID;

print_r($UserGroup1->Users);
print_r($UserGroup2->Users);

$User->Update();

print_r($UserGroup1->Users);
print_r($UserGroup2->Users);

Would you expect that $UserGroup->Users imeditaley contains the changed reference or after the  statement $User->Update().
(Currently it is the first one.)

Because working with new Objects and already existing ones behave differently I thought it might could be confusing.
My suggestion would be that the reference is updated imeditaley.

At the moment references for new Objects aren't updated imeditaley because the reference is based on the primary key that doesn't exists yet. It exists after we run Insert().

Maybe I could solve this problem by working internal ids and not database ids.

But the real question is, what would you expect what happens (for the references) without reading any documentation?

EDIT

@showerhead

If I got you right, you would consider this as an better way:

$UserGroup = new UserGroupModel();
$UserGroup->Name = "...";
$UserGroup->Save(); // runs an insert or update sql statement

$User = new UserModel();
$User->Name = "...";
$User->UserGroupID = $UserGroup->UserGroupID;
$User->Save(); // runs an insert or update sql statement

print_r($UserGroup->Users); // would be still empty here

$UserGroup->LoadChanges(); // load the changes

// now we have the new reference in $UserGroup->Users
print_r($UserGroup->Users);

Without reading any documentation, huh? Well let me read you your code as I see it then.

Create new Author
Give it a name(instead of title? or is that really the book too?)
Add it to the database.
Create a new Book
Give it a title
Assign it an author using initiated author class
Print Author's books, which is empty
Add book to database
Now, maybe with an update, I would expect Author to have the new book

Your second bit of code is just confusing. Why would you allow your properties to be reassigned? The author isn't going to change. If there are multiple authors, then I would expect you to have a setAuthors() method that accepted an array, or multiple arguments.

Otherwise this looks fine.

Edit

Still not sure if I understand, and changing the example didn't help. If anything it confused matters. Don't delete contents or drastically change it. Just add an Update or Edit section like I've done in my post. Let's see if I can clear up some of these misunderstandings.

Your main question seems to be how I would expect the contents of $UserGroup to be updated. You are right, I originally misunderstood you. So long as I am not misunderstanding you now, this update should help. Anything that caches information, even just temporarily in local scope, I would expect to need to check periodically for updates. Because the content of $UserGroup is "cached" (or should be), I would not expect any change to be immediately available to it. That being said, I also would not expect $User to be responsible for this. $User is a completely separate class and should not be concerned with $UserGroup at all. Instead $UserGroup should explicitly request new data from the database when it anticipates that the data may have changed.

Your comment leads me to believe you also would like to know how this update method would look. I think I just explained it, but essentially it will do what you are already doing when you first initialize the object, it should fetch the new data from the database to replace the old. To be efficient, you should have some sort of check to see if any changes were actually made before requesting the new data. In other words, you would reinitialize the object.

Next you ask the same question again, but say you are using a different method from when you first asked the question. I'm assuming this is a typo from when you updated your question, so I'll skip over this.

The problem with updating the $UserGroup immediately, through the $User object, is that the $UserGroup has no control over it. This means that information could change in some other part of the application, and you would never know because you did not expect it. An example of how this might be a problem: Say you have initialized this object and call another method, and, unbeknownst to you, it changes the information in your object. You didn't expect it, so you continue using it and now indices could be messed up and, using the new example you gave, you could be giving out someone's password to a completely different user. This is why I said earlier that $UserGroup should be concerned with its own data, nothing else should touch it. The best way to do this is to just not load the data until after you have already made all the changes you need to make.

The last two statements about referencing the data is just confusing. Of course you should be using internal indices. When you fetch data from a database, it is no longer associated with that database. It will now have its own reference pointers you should use, usually array indices. When its time to update the database with the new data, then you will use references common to both. For instance, in the $UserGroup database, you might use a user's username or ID to reference the proper cell in the database. These are pretty typical primary keys because they are unique.

I hope this better answers your question. I'm sorry I can't help more, but I"m having difficulties understanding what you are trying to say. I'm assuming because of some language barrier.


Clearing the text when `text select` release I required a trigger, whenever the window text selection cleared.
Finally I found this approach.

Using the same id, I have added multiple event handlers, but I am not sure my work is the best.

Is there a better way to do this?

 $('#tenderContent').on('mouseup', '.tenderdocument', function (e) {
    e.stopPropagation();
    if ($.trim(window.getSelection().toString()).length) {
        $('#text').text(window.getSelection().toString());
    }

});

$('#tenderContent').on('mousemove', '.tenderdocument', function (e) {
    e.stopPropagation();
    if (!$.trim(window.getSelection().toString()).length) {
         $('#text').empty();
    }
});

LiveDemo

The jQuery $.on(); event listener can use 2+ events in its event listener parameter and then let you specify which one through e.type. This allows us to shorten a lot of code. With that in mind, we can also shorten your $.trim(window.getSelection().toString()).length to be $.trim('' + window.getSelection()) and assign it to a variable for use in our if's. We can then further simplify it by testing for e.type being 'mouseup' and if it's evaluates to false, it has to be 'mousemove'. And so we don't have nested if's, we can just use logical AND and OR operators to do something if it even has a value, and something else if it doesn't. That brings your code down to this:

$('#tenderContent').on('mouseup mousemove', '.tenderdocument', function(e) {
  e.stopPropagation();
  var selection = $.trim('' + window.getSelection());
  if ('mouseup' == e.type) {
    selection && $('#text').text(selection);
  } else {
    selection || $('#text').empty();
  }
});

That is a major cut off the amount that you had before. It also simplifies a lot of stuff to be fast and efficient.


Decimal to Binary Conversion and vice-versa in Python Given a decimal number I need to convert it into binary
Given a binary number I need to convert it into decimal

After the conversion I need to perform some operation on it (for example, addition). I also need to print the result with a specified width.

Binary-Decimal:

n1 = int(input("Enter a binary number: "))
n2 = int(input("Enter a binary number: "))

# type cast to 'int' is done to provide width
decimalN1 = int("{0:d}".format(n1), 2)
decimalN2 = int("{0:d}".format(n2), 2)
decimalSum = int("{0:d}".format(n1 + n2), 2)

width = len(str(decimalSum))
print("max width = {}".format(width))

print ("{0:0{3}} + {1:0{3}} = {2:0{3}}".format(decimalN1, decimalN2, decimalSum, width))
print ("{0} + {1} = {2}".format(type(decimalN1), type(decimalN2), type(decimalSum)))

Decimal-Binary:

n1 = int(input("Enter a decimal number: "))
n2 = int(input("Enter a decimal number: "))

# type cast to 'int' is done to provide width
binaryN1 = int("{0:b}".format(n1))
binaryN2 = int("{0:b}".format(n2))
binarySum = int("{0:b}".format(n1 + n2))

width = (n1 + n2).bit_length()
print("max width = {}".format(width))

print ("{0:0{3}} + {1:0{3}} = {2:0{3}}".format(binaryN1, binaryN2, binarySum, width))
print ("{0} + {1} = {2}".format(type(binaryN1), type(binaryN2), type(binarySum)))

I want to know if there is any other (better) way of doing this. I know the bin() function could be used, but then it returns a string, so I cannot perform (integer) operation on it after the conversion.

Rather than repeating the same logic you could make a couple of functions binary_to_decimal and decimal_to_binary.

def binary_to_decimal(num):
    return int("{0:d}".format(num), 2)

def decimal_to_binary(num):
    return int("{0:b}".format(num))

Whilst it doesn't have too big an impact on the readability of your code it abstracts the code enough for anyone passing by to understand what you mean when you're doing the string formatting. You could also change binary_to_decimal to use str rather than string formatting, as I don't think we're going to change to the binary form any time soon. I also thing your string formatting for changing to binary is better than using bin as you'd have to cut the first 0b off the output of bin, which would make the code more cryptic.


Unit test with Moq callback anonymous function I have the following unit test that is testing predicate logic in a repository (Note: mockOpportunityRepository.Setup):

[Test]
public void GetFollowedByUserIdShouldGetTheUsersFollowedOpportunitiesFromOpportunityFollowRepositoryAndThenGetThoseFollowedOpportunities()
{
    const int ExpectedOpportunityId = 9;

    var testOpportunities = new List<Domain.EF.Opportunity>()
    {
        OpportunityBuilder.Default().WithId(1).Build(),
        OpportunityBuilder.Default().WithId(2).Build(),
        OpportunityBuilder.Default().WithId(ExpectedOpportunityId).Build(),
    };

    var actualOpportunities = new List<Opportunity>();

    var expectedOpportunityFollows = new List<OpportunityFollow> { new OpportunityFollow { OpportunityFollowId = 1, OpportunityId = ExpectedOpportunityId, UserId = UserId } };
    mockOpportunityFollowRepository.Setup(x => x.GetByUserId(UserId)).ReturnsAsync(expectedOpportunityFollows);

    mockOpportunityRepository.Setup(
        s =>
            s.GetAsync(
                It.IsAny<Expression<Func<Domain.EF.Opportunity, bool>>>(),
                It.IsAny<Func<IQueryable<Domain.EF.Opportunity>, IOrderedQueryable<Domain.EF.Opportunity>>>(),
                It.IsAny<int?>(),
                It.IsAny<int>()))
        .Callback<
            Expression<Func<Domain.EF.Opportunity, bool>>,
            Func<IQueryable<Domain.EF.Opportunity>,
            IOrderedQueryable<Domain.EF.Opportunity>>,
            int?,
            int>((queryParam, sortParam, takeParam, skipParam) =>
            {
                var func = queryParam.Compile();
                actualOpportunities = testOpportunities.Where(func).ToList();
            });

    var results = sut.GetFollowedByUserId(UserId);

    mockOpportunityFollowRepository.Verify(x => x.GetByUserId(UserId), Times.Once);
    actualOpportunities.Count.Should().Equals(1);
    actualOpportunities[0].OpportunityId.Should().Equals(ExpectedOpportunityId);
}

I have several tests like this all utilising the same bulky mockOpportunityRepository.Setup and wanted to refactor it into one re-useable place. Essentially the setup has a callback which runs an anonymous function. The anon function compiles the query parameters and executes them against a test list of opportunities. The outer test function needs the result of this filter to Assert the predicate logic.

If I try to move the mockOpportunityRepository.Setup into a separate function and try to output the actualOpportunities by reference I get a compiler error that ref and out parameters are not allowed in anonymous method body. I'm not very knowledgeable on functions and am sure it can be done.

Can anyone refactor this and suggest a way of moving the mockOpportunityRepository.Setup out of the test to make it reusable yet somehow get the results of the anonymous function (actualOpportunities) back to the calling test?

I think, for what it's worth, you have mostly only succeeded in testing your mocks here.  Even if you moved these mocks out of the test, I think you'd still only be testing your mock implementation and not much production code. So these tests can pass all day long without reflecting the condition of your production implementations.  (If I'm understanding every properly :)

[Test]
public void GetFollowedByUserIdShouldGetTheUsersFollowedOpportunitiesFromOpportunityFollowRepositoryAndThenGetThoseFollowedOpportunities()
{
    const int ExpectedOpportunityId = 9;

    var testOpportunities = new List<Domain.EF.Opportunity>()
    {
        OpportunityBuilder.Default().WithId(1).Build(),
        OpportunityBuilder.Default().WithId(2).Build(),
        OpportunityBuilder.Default().WithId(ExpectedOpportunityId).Build(),
    };

    var actualOpportunities = new List<Opportunity>();

    var expectedOpportunityFollows = new List<OpportunityFollow> { new OpportunityFollow { OpportunityFollowId = 1, OpportunityId = ExpectedOpportunityId, UserId = UserId } };
    mockOpportunityFollowRepository.Setup(x => x.GetByUserId(UserId)).ReturnsAsync(expectedOpportunityFollows);

    mockOpportunityRepository.Setup(
        s =>
            s.GetAsync(
                It.IsAny<Expression<Func<Domain.EF.Opportunity, bool>>>(),
                It.IsAny<Func<IQueryable<Domain.EF.Opportunity>, IOrderedQueryable<Domain.EF.Opportunity>>>(),
                It.IsAny<int?>(),
                It.IsAny<int>()))
        .Callback<
            Expression<Func<Domain.EF.Opportunity, bool>>,
            Func<IQueryable<Domain.EF.Opportunity>,
            IOrderedQueryable<Domain.EF.Opportunity>>,
            int?,
            int>((queryParam, sortParam, takeParam, skipParam) =>
            {
                var func = queryParam.Compile();
                actualOpportunities = testOpportunities.Where(func).ToList();
            });

    // NOTE: not sure which guy your sut variable is here.
    // NOTE: no assertions on the results is suspect.
    var results = sut.GetFollowedByUserId(UserId);

    // NOTE: this is testing the plumbing of a function.  that my thing called another thing.  in general,
    //       i have found this kind of testing to be less valuable and brittle.  if the function
    //       can do the job without calling the other thing in the future, this test will break,
    //       even though the function works properly. ymmv.
    mockOpportunityFollowRepository.Verify(x => x.GetByUserId(UserId), Times.Once);

    // NOTE: unfortunately, all thats tested here is that the mock implementation returns
    //       the right result.  no production code is exercised for these.
    actualOpportunities.Count.Should().Equals(1);
    actualOpportunities[0].OpportunityId.Should().Equals(ExpectedOpportunityId);
}

For all the mocking etc... I believe the above code is functionally equivalent as what I have below (or close to it).  And almost all of it is still mock implementation.  What production code we are testing is hard to know.

[Test]
public void GetFollowedByUserIdShouldGetTheUsersFollowedOpportunitiesFromOpportunityFollowRepositoryAndThenGetThoseFollowedOpportunities()
{
    const int ExpectedOpportunityId = 9;

    var testOpportunities = new List<Domain.EF.Opportunity>()
    {
        OpportunityBuilder.Default().WithId(1).Build(),
        OpportunityBuilder.Default().WithId(2).Build(),
        OpportunityBuilder.Default().WithId(ExpectedOpportunityId).Build(),
    };

    Expression<Func<Domain.EF.Opportunity, bool>> queryParam; // = ???

    var actualOpportunities = new List<Opportunity>();
    var func = queryParam.Compile();

    actualOpportunities = testOpportunities.Where(func).ToList();

    actualOpportunities.Count.Should().Equals(1);
    actualOpportunities[0].OpportunityId.Should().Equals(ExpectedOpportunityId);
}

You could consider this to be more of an integration test.  But then what's being tested?  That entity framework works?  Or SQL?  If so, these fall significantly outside of the boundaries of what a unit test can do well for you.

For my money it's an integration test and needs to exercise production code.  I think you can safely remove this test as a unit test.


Answer checking script I got this basic script which I'll be using to check if the answer is right. The only problem is that I'll be using with many forms. That's why I need this code to be more like DRY code.

            function checkAnswer()
        {
            var answer = document.getElementById("answer").value;
            if (answer.toLowerCase() == "correct answer")
            {
                alert("Well done!");
            }
            else
            {
                alert("Too bad!");
            }

            var answer2 = document.getElementById("answer2").value;
                     if (answer2.toLowerCase() == "correct answer")
            {
                alert("Well done!");
            }
            else
            {
                alert("Too bad!");
            }

            var answer3 = document.getElementById("answer3").value;
            if (answer3.toLowerCase() == "correct answer")
            {
                alert("Well done!");
            }
            else
            {
                alert("Too bad!");
            }
        }

Checking against a string like this is error prone. What happens if you accidently misspell "correct" one of the times? You're better off using a number constant as an enum to signify success/failure (After rereading your post, I realized I may have misinterpreted your intent).
I'd just extract that repeated bit into a function:

function answerIsCorrect(answer, correctAnswer) {
    return answer.toLowerCase() === correctAnswer;
}

Then, since you're also repeating the use of this code, you might as well use a loop to prevent duplication:

function checkAnswers(correctAnswers) {

    //You could also just use a standard for-loop for this
    correctAnswers.forEach(function(correctAnswer, i) {
        //Note unless you check prevent i from being appended when i == 1,
        // your first answer ID will be "answer1" instead of "answer"
        var answer = document.getElementById("answer" + (i + 1)).value;

        if (answerIsCorrect(answer, correctAnswer)) {
            alert("Well done!");

        } else {
            alert("Too bad!");
        }
    });
}

correctAnswers could also be a list of numbers, representing each answer's position in the list of answers. This would mean you would only need to check the index of the answer, instead of the value of the answer, which would make manipulations like toLowerCase unneccessary.
Your indentation is a little off at the top, but that could be a pasting error.


Time delay indication bar for a text game For a game I am writing in Python 3.6.5, I made a text-based animation to indicate the passage of time (when chopping a tree or mining a rock, etc). I did a lot of trial and error to get the output how I wanted it, however it seems like I went about it in an overly-complicated way. I was wondering if there is a more efficient way to write this.

import os
import time

def progress_bar(text,secs):
    bar = '['
    c = 0 #used in sum method to determine number of Xs in bar

    for i in range(0,11):
        print(text+'\n')
        bar = bar.replace(' ','') #get rid of previous spaces added to bar
        num_of_X = sum(c == 'X' for c in bar) #get number of Xs (progress)
        for x in range(0,(10 - num_of_X)): #Fill remaining space after Xs
            bar += ' '
        print(bar + ']')
        bar += 'X'
        time.sleep(secs)
        os.system('cls' if os.name == 'nt' else 'clear') #clears terminal

    input('Loading complete!')

progress_bar('Enter Your Text Here!',0.15)

You can use bar.count rather than your sum
You can use ' ' * n, rather than your for loop.
You don't need [ in your bar.
You don't need to add whitespace, str.format can do that for you.

>>> '[{:<10}]'.format('XX')
'[XX        ]'

You don't need the bar variable.
I'd move the input out of the function, as it doesn't have much to do with the bar.
You could change the size of the bar with another argument.

def progress_bar(text, secs, size=10):
    for i in range(size + 1):
        print('{}\n\n[{:<{}}]'.format(text, 'X' * i, size))
        time.sleep(secs)
        os.system('cls' if os.name == 'nt' else 'clear')

You can also remove the need for os if your terminal support \r. This will mean that you can remove text from the function too, as IMO it shouldn't really be there either.

def progress_bar(secs, size=10):
    for i in range(size + 1):
        print('\r[{:<{}}]'.format('X' * i, size), end='', flush=True)
        time.sleep(secs)
    print()

print('Your text here!\n')
progress_bar(0.15)
input('>')

Alternately you can use \b, this is better if you want to allow text like bar: [].

def progress_bar(secs, size=10):
    for i in range(size + 1):
        print('[{:<{}}]{}'.format('X' * i, size, '\b' * (size + 2)), end='', flush=True)
        time.sleep(secs)
    print()

print('bar: ', end='', flush=True)
progress_bar(0.15)
input('>')


Socket client in C using threads I'm working on socket programming in C. I have no problem with usage the threads. This all works fine but I'm new in this area. I wrote this code in client.c but is there any misused code or something may cause problems in the future?
My client sends a message to the server and client can receive it. There are 2 major operations: recv and send.
CLIENT
#include<stdio.h>
#include<sys/socket.h>
#include<arpa/inet.h> // for inet_addr
#include <string.h>
#include <zconf.h>
#include <stdlib.h>
#include <pthread.h>

void *sendMessage(void *sock_desc) {
    //Send some data
    while (1) {
        char message[2000];
        printf("%s","> ");
        scanf("%[^\n]%*c", message);
        fflush(stdin);

        if (send(*((int *) sock_desc), message, strlen(message) + 1, 0) < 0) {
            puts("Send failed");
        }
    }
}

void *receiveMessage(void *sock_desc) {
    while (1) {
        char server_reply[2000];
        if (recv(*((int *) sock_desc), server_reply, 2000, 0) < 0) {
            puts("recv failed");

        }
        //Receive a reply from the server
        printf("\033[32;1m %s \033[0m\n", server_reply);
    }
}

int main(int argc, char *argv[]) {
    int                sock;
    struct sockaddr_in server;
    //char               message[2000], server_reply[2000];

    //Create socket
    sock = socket(AF_INET, SOCK_STREAM, 0);
    if (sock == -1) {
        printf("Could not create socket");
    }

    server.sin_addr.s_addr = inet_addr("127.0.0.1");
    server.sin_family      = AF_INET;
    server.sin_port        = htons(8888);

    //Connect to remote server
    if (connect(sock, (struct sockaddr *) &server, sizeof(server)) < 0) {
        perror("Connect failed. Error");
        return 1;
    }

    puts("Connected to server\n");
    int *new_sock;
    new_sock = malloc(1);
    *new_sock = sock;
    //keep communicating with server

    pthread_t send_thread, receive_thread;
    pthread_create(&send_thread, NULL, sendMessage, (void *) new_sock);
    pthread_create(&receive_thread, NULL, receiveMessage, (void *) new_sock);

    pthread_join(receive_thread, NULL);
    pthread_join(send_thread, NULL);

    close(sock);
    return 0;
}

You have to allocate sizeof(int) bytes for socket file descriptor, not 1
fflush(stdin) has undefined behavior
its not good idea to access a file descriptor in two threads simultaneously, be careful
joins are never happens, because none of two threads have exit conditions, so the socket will never close gracefully


React/Redux/Node/Express crud application with a React class component https://try.gitea.io/nadal/repository

Used this in a coding challenge after a job interview. I didn't get chosen, but I was wondering if it was because of the code quality in the project.

import React, { Component } from 'react';
import * as ExpenseActions from '../actions/ExpenseActions';
import { bindActionCreators } from 'redux';
import { connect } from 'react-redux';
import { PropTypes } from 'prop-types';
import ExpenseTable from '../components/ExpenseTable';

export class ExpenseContainer extends Component {

    constructor(props) {
        super(props)
    }

    /**
     * Triggers the create action
     * @method
     *
     */
    createExpense = (expense) => {
        this.props.actions.CreateExpense(expense)
    }

    /**
     * Triggers the start edit action
     * @method
     *
     */
    startEditing = (id) => {
        this.props.actions.StartEditing(id);
    }

    /**
     * Triggers the cancel action
     * @method
     *
     */
    cancelEditing = (id) => {
        this.props.actions.CancelEditing(id);
    }

    /**
     * Triggers the edit action
     * @method
     *
     */
    editExpense = (expense) => {
        this.props.actions.UpdateExpense(expense);
    }

    /**
     * Triggers the delete action
     * @method
     *
     */
    deleteExpense = (expense) => {
        this.props.actions.DeleteExpense(expense);
    }

    render() {
        return (
            <div className="expense-container">
                <ExpenseTable
                    subTotal={this.props.expenses.subTotal}
                    total={this.props.expenses.total}
                    expenses={this.props.expenses.expenses}
                    createExpense={this.createExpense}
                    startEditing={this.startEditing}
                    cancelEditing={this.cancelEditing}
                    editExpense={this.editExpense}
                    deleteExpense = {this.deleteExpense}
                />
            </div>
        );
    }
}

ExpenseContainer.propTypes = {
    actions: PropTypes.object.isRequired,
    expenses: PropTypes.object.isRequired
}

function mapStateToProps(state, ownProps) {
    return {
        expenses: state.expenses
    }
}

function mapDispatchToProps(dispatch) {
    return {
        actions: bindActionCreators(ExpenseActions, dispatch)
    }
}

export default connect(mapStateToProps, mapDispatchToProps)(ExpenseContainer);

Aside the fact, I could have used a hook called useSelector and useDispatch. I am not sure how I could have made it better. The backend side, I am not too sure if there's anything wrong with it, because I am not too familiar with Node/Express. Try to be as nitpicky as possible.

Sorry you didn't get the position, but perhaps the interviewers/code reviewers were looking for something else or some very specific things.
You cannot, in fact, use useSelector or useDispatch, nor any other react hook as they are only compatible with functional components. Other than that this appears to be clean and readable code, although it does have some redundancy.
Suggestions

The constructor doesn't actually do anything, so it can be removed
All the class functions simply proxy props that are passed, so they, too, can be removed
This is debatable, but IMO you should explicitly map the actions to props that you will use, it makes it clearer what the component can do
The connect HOC's second parameter mapDispatchToProps automagically wraps each actionCreator with a call to dispatch as well
It appears this.props.expenses object shape is identical to the props ExpenseTable takes, so you can spread those in, more DRY

Nit-picky Suggestions

Try to be as precise/explicit when defining proptypes
Consistently use semi-colons
In react only Components are PascalCased, most other variables and objects should be normal camelCased
Destructure props to be more DRY, i.e. much less this.props.X, this.props.Y, this.props.Z. This one is debatable however as the argument against is that not destructuring them it is easy to see what is a prop versus state versus neither. I say you can just look to the top of the function where variables should normally be defined anyway to glean these details.

Component
import React, { Component } from 'react';
import { connect } from 'react-redux';
import { PropTypes } from 'prop-types';
import ExpenseTable from '../components/ExpenseTable';
import {
    CancelEditing,
    CreateExpense,
    DeleteExpense,
    StartEditing,
    UpdateExpense,
} from '../actions/ExpenseActions';

export class ExpenseContainer extends Component {
    render() {
        const {
            CancelEditing,
            CreateExpense,
            DeleteExpense,
            expenses,
            StartEditing,
            UpdateExpense,
        } = this.props;

        return (

                <ExpenseTable
                    {...expenses}
                    createExpense={CreateExpense}
                    startEditing={StartEditing}
                    cancelEditing={CancelEditing}
                    editExpense={UpdateExpense}
                    deleteExpense={DeleteExpense}
                />

        );
    }
}

ExpenseContainer.propTypes = {
    CancelEditing: PropTypes.func.isRequired,
    CreateExpense: PropTypes.func.isRequired,
    DeleteExpense: PropTypes.func.isRequired,
    StartEditing: PropTypes.func.isRequired,
    UpdateExpense: PropTypes.func.isRequired,
    expenses: PropTypes.shape({
      expenses: PropTypes.string.isRequired, // or number or whatever the specific type is
      subTotal: PropTypes.string.isRequired,
      total: PropTypes.string.isRequired,
    }).isRequired
};

function mapStateToProps(state) {
    return {
        expenses: state.expenses
    }
};

const mapDispatchToProps = {
    CancelEditing,
    CreateExpense,
    DeleteExpense,
    StartEditing,
    UpdateExpense,
};

export default connect(mapStateToProps, mapDispatchToProps)(ExpenseContainer);

A final suggestion would be to just convert ExpenseContainer to a functional component since it doesn't use any of the class-based lifecycle functions.
export const ExpenseContainer = ({
    CancelEditing,
    CreateExpense,
    DeleteExpense,
    expenses,
    StartEditing,
    UpdateExpense,
}) => (

        <ExpenseTable
            {...expenses}
            createExpense={CreateExpense}
            startEditing={StartEditing}
            cancelEditing={CancelEditing}
            editExpense={UpdateExpense}
            deleteExpense = {DeleteExpense}
        />

);

ExpenseContainer.propTypes = { ... };

function mapStateToProps(state) { ... };

const mapDispatchToProps = { ... };

export default connect(mapStateToProps, mapDispatchToProps)(ExpenseContainer);


Query SteamIDs for presence of an avatar I have a table vanrust_corev2.tblPlayerLog that has approx 6.7k records,
I also have a table in a different database vanrust_forum.tblPlayerAvatars which has approx 6.7k records.

The purpose of this script is to pull the SteamID of players who don't have an avatar stored for them yet. However the issue here is, the execution time of this script is often 29-31 seconds which seemed a little strange to me as I'm sure there are much more sophisticated and larger databases that would not have this issue.

SELECT
*
FROM
vanrust_corev2.tblPlayerLog log
WHERE
not exists(
SELECT
a.SteamID
FROM vanrust_forum.tblPlayerAvatars a
WHERE a.Steamid = log.SteamID)

This is for the same user, just to a different database.

This script works perfectly the only issue here is execution time so I'm hoping for some pointers on where I could be going wrong.

Here is the execution plan:

I normally wouldn't recommend this, but...
According to your query execution plan, it is doing 22719171 table scans in order to fetch the next row. What I think the problem stems from is querying across databases, which may have to open a connection for each Handler_read_rnd_next, then close the connection each time. One way or the other, if your columns are properly indexed, something like this shouldn't take near that long. I think something about the setup is causing the optimizer to go into a loop across two databases, and SQL is not good at looping. To make this more set-based, I'd suggest pulling your data sets into one of the two databases with a construct like a temp table:
START TRANSACTION;
  DROP TEMPORARY TABLE IF EXISTS vanrust_corev2.users_with_avatars;
  CREATE TEMPORARY TABLE vanrust_corev2.users_with_avatars (SteamID INT NOT NULL);
  INSERT INTO vanrust_corev2.users_with_avatars (SteamId) VALUES
    SELECT
      a.SteamID
    FROM vanrust_forum.tblPlayerAvatars a
COMMIT;

Then, you could simply join the record sets within the vanrust_corev2 database. This would increase I/O for a bit, but likely remove the need to connect between 2 databases millions of times while it does table scans:
SELECT
*
FROM
vanrust_corev2.tblPlayerLog log
WHERE
NOT EXISTS (
  SELECT
  a.SteamID
  FROM vanrust_corev2.users_with_avatars a
  WHERE a.Steamid = log.SteamID
);

And of course it's a good idea to DROP TEMPORARY TABLE vanrust_corev2.users_with_avatars; when you are done, although it will otherwise automatically drop when the session is ended with the database.

On another note, I strongly recommend you start naming aliases in a way that describes what they represent. log is OK, but a doesn't seem to mean anything.


Update Trigger (Comparing Descriptive string), as well as checking for valid updates I have an audit table EpisodesHistory. One of the fields that gets copied is eDescription. I expect eDescription to be regularly be a couple kilobytes in length.

For database space and transmission to client applications, I wanted to do something where if, upon recording the change, eDescription is identical to a previous iteration, it records that value.

Let's say I have this EpisodesHistory table:

HistoryID(PK) | EpisodeID | eDescription | ... other fields
1               10          Watch my dog do a trick
2               10          Watch my cat sleep
3               11          Watch my dog do a trick

In the case of row 3, it was inserted because the episodeID is different.

However, if I then added:

HistoryID(PK) | EpisodeID | eDescription | ... other fields`
4               10          Watch my dog do a trick

I wanted to auto-update an additional field EpisodesHistory.eDescRef to the ID of the row containing the same description. 1 in this case because 1 is the first row to have this field.

It took me a few attempts to get something working so I want to make sure that this is the optimum method as this will be one of the busier triggers in my database.

USE [Shows]
GO
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
ALTER TRIGGER [dbo].[tr-EpisodesHistory]
on [dbo].[Episodes]
FOR UPDATE
AS

IF TRIGGER_NESTLEVEL(OBJECT_ID('dbo.EpisodesHistory')) > 1
    BEGIN
        RETURN;
    END;

DECLARE @CMP INTEGER
DECLARE @NEWID INTEGER = 0

IF NOT UPDATE(eActor) OR NOT EXISTS((SELECT *, eActor = 0 FROM Inserted except SELECT *, eActor = 0 FROM Deleted))
BEGIN
    -- Weed out updates to episode scoring, or where nothing that matters in the row was changed (eActor doesn't matter)
    -- eVersion is incremented code-side, so this has to be undone.
    -- Could be incremented here instead, but thats yet another query for successful updates
    UPDATE Episodes
      SET eVersion = eVersion - 1
    WHERE EpisodeID = (SELECT EpisodeID From Inserted)
    return;
END

SET @CMP = (Select Top 1 Past.HISTORYID
         FROM DELETED Curr, EpisodesHistory Past
         where Curr.EpisodeID = Past.EpisodeID
           and Curr.eOwner = Past.eOwner
           and Curr.eDescription = Past.eDescription)

SET @CMP = ISNULL(@CMP, 0)
INSERT into EpisodesHistory
SELECT *, eDescRef = @CMP, hWhen = getDate() from deleted;
SET @NEWID = SCOPE_IDENTITY()

IF @CMP > 0
BEGIN
    UPDATE EpisodesHistory
    SET eDescRef = @CMP,
        eDescription = ''
    WHERE HistoryID = @NEWID
END

It will only work as intended for single-row updates

The fact that you use TOP 1 and SCOPE_IDENTITY() tells me this was designed with one-row updates/inserts in mind. But what happens when you update multiple rows? (In case you don't know, the trigger only fires once for the whole changed set, rather than once for each row. The changed rows are added to the inserted and deleted tables).

To solve this, I'd recommend using a temp table or variable table instead of @CMP, and the output clause instead of @NEWID. For example:

-- Example table with identity
create table Things (KeyId int identity(1, 1), Value varchar(31))`

-- Table to store identity ints which get created
declare @NewIds table (NewKeyId int)

insert into Things (Value)
output inserted.KeyId into @Keys (NewKeyId) -- this line gets the new identities
select 'Value text'
from OtherTable

Now you can access the created IDs using @Keys. Of course you'd want to store the EpisodeId or something in the same table, to relate them.

EDIT: Untested replacement

So I've just roughly written some SQL I think should work instead of everything including and below the SELECT Top 1 statement. The main issue I have with that part of your trigger is that it will only work for one-row updates, and it performs an INSERT then an UPDATE on that inserted data, which can be optimised.

INSERT into EpisodeHistory
SELECT
    EpisodeID = d.EpisodeID,
    eOwner = d.eOwner,
    eDescription = iif(h.HistoryID is null, d.eDescription, ''),
    eDescRef = isnull(h.HistoryID, 0),
    hWhen = getDate(),
from deleted d
    left join (
        select
            HistoryID, EpisodeID, eOwner, eDescription,
            row_number() over (partition by EpisodeID, eOwner, eDescription ORDER BY HistoryID) as RowNumber
        from EpisodeHistory
        where eDescription <> ''
    ) h
    on d.EpisodeID = h.EpisodeID
        and d.eOwner = h.eOwner
        and d.eDescription = h.eDescription
        and h.RowNumber = 1

Rather than inserting the EpisodeHistory, then checking and updating based on the data we've just inserted, this code just does the operation all in one.

First I'll explain the left join. The point of the join is to find the HistoryID of the earliest matching EpisodeHistory row (since we want to put it in eDescRef, right?). So we do a normal select of the EpisodeHistory table, except we also add a column for row_number(), which, when combined with the fact that we've put RowNumber = 1 in the join condition, will means it will only join to the first HistoryID that matches. This makes sure that if there are two rows with the same EpisodeID, eOwner and eDescription, it will only join to the one with the lowest HistoryID.

Now the actual columns that are inserted are fairly simple. If the left join fails (if no rows in EpisodeHistory match), selecting h.HistoryID will return NULL. Therefore, eDescription = iif(h.HistoryID is null, d.eDescription, '') means that if a HistoryID was not found in the join, it will just do a normal insert of eDescription. If one was found, it will blank that field. The next line is fairly simple in that it inserts HistoryID if the join happened, otherwise it puts in 0.

Since the entire operation can be done in one statement like above, so you can ignore that bit I said above about temporary tables and an output clause.

This code above also has the added bonus of working for multiple-row updates.

It's a trigger...

You may already know, but the SQL Server community is fairly divided when it comes to whether triggers should be avoided or not. They can be easy to forget, hard to debug, and can hinder performance as they become part of the write operation.

Since you mention performance I'll elaborate on that aspect, and I'll make some assumptions I hope you can forgive. I'm going to assume there's an application that sits in front of this database, and the application prompts the insert/update. Say the user is on a form and makes changes to an episode, and submits the change. Usually the application will wait for confirmation of the update/insert operation before proceeding (before loading the next page, for instance). With the trigger, the application now has to wait for the initial insert/update, as well as the trigger to complete its operation too.

Therefore, if you haven't already, consider the possibility of separating out this process. In this example I gave above, you could have it so that the application still waits for the initial insert/update operations, loading the next page normally, but then in the background (asynchronously) it tells the database to perform audit operations. Boom, you've reduced page load time, better UX. This also has the benefit of you not having to worry so much about performance tuning the auditing operation.

Checksum?

I'm going to assume since performance is an issue that you've considered indexes, and have an index set up to optimise this Episode and EpisodeHistory comparison. Have you considered the use of CHECKSUM()? I don't have any experience with it myself, however I'd imagine you could use it in combination with persistent computed columns to improve the load when write and maybe reading to the indexes.

In this instance you'd have an extra column on both tables called something like eDescriptionChecksum, which would be a checksum of eDescription, and you'd put that column in the index instead of eDescription. Then when you perform the comparison to see if they're the same, you'd be comparing the relatively small and light int checksum column instead of the potentially-huge (n)varchar column.

Minor issue

I see when you set the @CMP, you select from multiple tables. It's recommended to use joins instead – use of multiple tables in the from clause is being depreciated by all DBMSs.

If anything's unclear, let me know. Hope you found this helpful.