Better understanding of unit testing in VBA from non-returning functions

You say this is a class, but you don't have anything exposed from it. What I mean is if I create an instance of this class, there's nothing I can do with it. I can't Get or Let any properties and I can't use any of the Private methods.

Call

You don't need to Call Sub anymore. You also don't need to enclose your arguments. E.g.

goThroughProducts somecolumn + a, somecolumn + a - 1

Worksheets

Worksheets have a CodeName property - View Properties window (F4) and the (Name) field (the one at the top) can be used as the worksheet name. This way you can avoid Sheets("mySheet") and instead just use mySheet.

Naming

Maybe some of this isn't in English and so I don't understand the meanings, but your naming could be improved. I have no idea what a laailys worksheet could be. Also you use product and prod both. Try to stay consistent and clear with your naming. What is checker checking? It's boolean so normally you would name it something like isMatch.

Also a Boolean is initialized as False so no need to set it as false.

Class

It seems strange to have a class refer to ThisWorkbook. What is your goal with this class? I don't see anything that can't be done with already existing objects.

ByVal

All your arguments are being passed ByRef implicitly. You want to pass ByVal whenever possible. e.g.

Private Sub InsertProducts(ByVal productCode As Long, ByVal productAmount As Long, ByVal targetColumn As Long

Typing

You'll notice I gave your variables a type above. Otherwise they are implicitly variants, which are not needed.

Loop

In InsertShopNamesAndProd you have a call to a procedure within a loop and a with:

For a = 1 To numberOfShops
        With laailys
            .Cells(truckRow, firstTruckColm + a - 1).Value = bloem.Cells(bloemShopNamesStartRow, bloemShopNamesStartColm + a).Value
        End With
        Call goThroughProducts(bloemShopNamesStartColm + a, firstTruckColm + a - 1)
    Next a

And in that procedure you have another call within a loop -

For a = bloemShopNamesStartRow + 1 To numberOfProducts + bloemShopNamesStartRow
        If bloem.Cells(a, colmToUseBloem).Value <> vbNullString Then
            Call insertProducts(bloem.Cells(a, 1).Value, bloem.Cells(a, colmToUseBloem).Value, colmToUseLaailys)
        End If
    Next a

That seems pretty repetitive. Instead think about refactoring these procedures so that you take all the products into an array, go through your shops and insert products as you work.

If

Here

If bloem.Cells(a, colmToUseBloem).Value <> vbNullString Then
            Call insertProducts(bloem.Cells(a, 1).Value, bloem.Cells(a, colmToUseBloem).Value, colmToUseLaailys)
        End If

Can be put on one line. And here

Dim checker As Boolean
    checker = False
    For a = 1 To 200
        If laailys.Cells(a, 4).Value = productCode Then
            laailys.Cells(a, colmToUseLaailys).Value = prodAmount
            checker = True
            Exit Sub
        End If
    Next a
    If checker = False Then
        MsgBox productCode & " did not read in correctly, make sure the product code in 'Laailys' is the same as in 'Bloem'"
    End If

I'm not sure how you can get to that second If without checker being false.

You can also use a boolean in an if as the actual test:

If Not checker Then

Settings module

You use your "settings" module to get objects and rows? That doesn't sound right. Settings should be loaded and stored so they can be reset. What you're doing is populating so naming it settings isn't entirely accurate.


Two (or more) LRU caches for a Node.js app

As mentioned in the comments, remove the functions. Don't even go into OOP. As to why, they're simply unnecessary wrappers to your cache objects which you can just access directly. detailThreadCache.set is just as verbose as detailThreadSetCache.

Improving code doesn't necessarily mean reuse or encapsulation or a paradigm shift. Often times, the best code... is the one that's never written.


Inserting data in the database through POST

Here are some things that I would recommend:

PHP tags. There's no point in closing and reopening PHP tags if there is no non-PHP code in between. For this file, a single opening PHP tag is all you need for the entire thing. Closing PHP tags (anywhere in the file) can bring potential issues such as unnoticed white space, which will throw errors if you try to use header() or similar later on.
header() - When using header('Location: xxxxx');, you should use a full URL, not a relative URL, since it is making an HTTP request. Sure, it probably works most of the time, but let's follow the spec and not take chances.
isset($_POST['submit']) - Some versions of Internet Explorer (not sure when this was fixed) will ignore the name/value of the submit button if it is being used as an image (example: <input type="image" src="login.png" name="submit" value="Login" />). I would check for a different form element (or even the $_POST array itself) to eliminate that potential bug.
Required files - Since I don't know how the rest of your application is structured, this one is more speculation. But if you find yourself repeating these required lines often in different files, you should consider consolidating the require_once() lines to a single appropriate file. For instance, all of your database connections can be done in once place. All of the form validation files can be consolidated, also. This leaves you with only needing to do a single require_once() in your app's files, and only needing to maintain and update a single file elsewhere if you want to change or add functionality.
registerAccount() failure? - You perform a redirect if the registerAccount() method is successful, but don't do anything if it fails. Always have an action for when things fail, because they probably will at some point. Logging error messages, even sending yourself an email when it happens is a good way to notice a major error quickly in the event that it happens.

This one is more personal preference, but might be worth it to you:

I would rename the check() method in your user and password classes to isValid(), since that's what you're specifically looking for (even says so in the comments). I always try to name methods based on context. You could even use the methods directly in your if statement - no real point in assigning those variables, since you only use them once and they don't really save much typing later on.

I hope that helps. :)


Calling a method outside class statically, passing other method's output as parameter

You are calling load() every time you call either sidebarGuide or wrapSidebars. Are you doing anything other than returning an instance of the class? If not, then just instantiate the class and making load() a construct would be more ideal. Doing so, your class would look like:

class adminTemplate {
    public function __construct(){}

    public function sidebarGuide(){
        return "content"; # Very long html markup
    }

    public function wrapSidebars($content){
        ob_start();
        echo "
";
        echo $content;
        echo "
";
        $output = ob_get_contents();
        ob_end_clean();
        return $output;
    }
}

You would then use it as follows:

$adminTemplate = new adminTemplate();
$adminTemplate->sidebarGuide();
$adminTemplate->wrapSidebars($adminTemplate->sidebarGuide());

If you wanted, you could even store the return value of sidebarGuide() in a property of the class so that you could call wrapSidebars() without injecting sidebarGuide(), as follows:

class adminTemplate {
    private $sidebarGuideReturnValue

    public function __construct(){}

    public function sidebarGuide(){
        $this->sidebarGuideReturnValue = "content"; # Very long html markup

        return $this->sidebarGuideReturnValue;
    }

    public function wrapSidebars($content = ""){
        # Make $content an optional parameter. If its not set, grab the property
        $content = ($content ? $content : $this->sidebarGuideReturnValue);

        ob_start();
        echo "
";
        echo $content;
        echo "
";
        $output = ob_get_contents();
        ob_end_clean();
        return $output;
    }
}

If you know that would only be calling sidebarGuide() exclusively as a param to wrapSidebars(), you could even relegate sidebarGuide()'s to a private called on instantiation in the construct, or move its body to the construct itself.

One question that bothers me though is, is there any specific reason you want to use static load()? Or just because it looks nice?


Find amounts that cannot be composed using the given coins

Your code looks frighteningly slow because of the 5 nested for loops, and indeed it is.

To find an efficient algorithm, you should switch off the computer, take out a sheet of paper and solve the following task on paper:

  Given an infinite amount of coins labelled 5, 7, 16, which amounts between 1 and 100 can be composed?

While you solve that task, ask yourself in every step:

what am I doing?
why do I do this?
what do I want to achieve?
can I formulate a general rule for this that sounds reasonable, appropriate for the task?

Your current code counts up to 2000 several times. In this manual task there is no point counting up to 2000. This alone makes your algorithm more specialized than it needs to.

Essentially your code should be defined in a function like this:

/**
 * @return the amounts between min and max that are <i>not</i> composable
 * from an infinite supply of coins of the given values.
 */
public static BitSet uncomposable(int min, int max, List<Integer> coinValues) {
    …
}

In that code, neither the number 100 nor 10000 nor 2000 must appear.
There should be at most 2 nested loops.
How exactly the code looks like depends on how you solved the task on paper.
I'm sure you won't choose a time-consuming algorithm when having to do this task manually.

Bonus question: is there a limit beyond which all amounts are composable? That could be used to stop the computation early, which is a nice optimization. Try with these test cases:

{1}: the limit is 0, obviously
{2}: there's no limit since only even numbers are covered
{2, 4, 6}: …
{3, 5}: …
{1, 7, 14}: …


Be consistent with your formatting. Sometimes you have a space between keywords like if and else and the opening parenthesis and the closing parenthesis and the opening brace, sometimes not. Pick one (I prefer a space) and stick with it. I also put spaces around all binary operators: data.length / 2 to match i < res.length. Again, consistency helps readability.
The data passed to the constructor is ignored; remove it.
Can you come up with more readable names for arr1 and arr2? Perhaps left and right (even though that may be misleading and is still arbitrary) will be easier to read than spotting 1 vs. 2 in a name.
In merge I think using while (j < arr1.length && k < arr2.length) instead of for is more readable. It also allows you to move the "copy the rest of the non-empty array onto the end" logic out of the loop and avoid using break.
Don't create a result array in main before calling sort--it's thrown away immediately.


stream->list

Avoid stream->list on large lists.
In your prime? function it will create a (large)
list before passing it on to ormap.

There is a stream version of ormap - namedly stream-ormap, and
this will process the stream lazily - i.e. it will only generate
as many elements of the stream which are needed.

next-prime

Another way to eliminate the recursion here is to combine
stream-first with stream-filter and in-naturals with
your prime? predicate, e.g.:

(stream-first (stream-filter prime? (in-naturals ...) ))

Not that recursion is bad, but using streams results in
a more declarative definition.

append

I would avoid append in a Lisp or a Scheme.
I'm sure it's not efficient for use on lists.
cons, however, is always efficient, so in prime-factors you should
use:

(cons num factors)

instead of:

(append factors (list num))

You build the list in reverse order, but it won't matter
in this case.

remove-duplicates

You are also using append here which should be avoided.
The standard definition again uses cons:

(define (remove-duplicates xs)
   -- if xs is empty, return xs
   -- else return x followed by remove-duplicates on the tail elements not equal to x
     (cons (head x)
           (remove-duplicates (filter (lambda (y) (not (eq? x y))) (tail xs))))
)

For short lists this is fine - but note that its running time is O(n^2).
If you need a better one, there is one available in the standard library:
remove-duplicates in the standard library. On longer lists it uses a hash - you can view the source here: (link)

divisor-count

Honestly I had trouble understanding your prime-factors and divisor-count routines.

The conventional way of computing the number of divisors of a number n is:

find a prime divisor p of n
find the exponent e of the prime p in the factorization of n
repeat this process with n / p^e until n = 1

Then you have a list of primes and their exponents: (p1, e1), (p2, e2), etc.
and the number of divisors is:

(1+e1) * (1 + e2) * ...

How about this approach:

(define (divisor-count n)
  (apply * (map add1 (prime-exponents n))))

(define (prime-exponents n)
   -- if n == 1 return the empty list
   -- otherwise, find the first prime p dividing n
   -- find largest e s.t. p^e divides n
   -- return e and recurse on n / p^e   )

first-triangle-number

This is another place where using streams can help:

Create a stream of triangle numbers
Filter the stream keeping only those with large divisor counts
Take the first element of the stream

i.e.:

(stream-first (stream-filter ... (stream-map ... (in-naturals 1))))
                                                 \___ 1, 2, ... __/
                                 \__ 1, 3, 6, 10, ...          __/
              \___ triangular numbers having #divisors >= 50  __/
\___ answer to the problem                                   __/

Note that (in-naturals 1) is an infinite stream.

for, for/list

When you get comfortable using the stream- functions, look into
Racket's sequence comprehensions: for, for/list, for/vector, etc.

See http://docs.racket-lang.org/guide/for.html for more info.


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

Comments everywhere repeating the code violate the DRY principle:

    # increasing the Bob count
    numBob += 1

You just doubled reading / maintaining time here.

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

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

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

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


Yes, you should be opening/closing the printwriters each time.

Yes, conceptually it's a decent way to convert the stack trace..... but... there are concerns, and is there a better way?

The most significant concern is that, if there's a problem writing to the PrintWriter, the call throwable.printStackTrace(printWriter); will fail, and the method will return a default value. This is not ideal.

The reality, though, is that those methods can never fail because the IO is to a StringWriter, and there's no actual IO component. A failure there would be..... inconceivable. None of your exception handling can really happen... it's just not going to fail (the "Titanic Logger").

The issue is that your code is checking for impossible conditions in a way that's overkill.

Still, using some Java 7 semantics, you can use try-with-resource functionality, and get a much more concise method:

private static String convertStackTrace(Throwable throwable) {
    try (StringWriter sw = new StringWriter();
             PrintWriter pw = new PrintWriter(sw)) {

        throwable.printStackTrace(pw);

        return sw.toString();

    } catch (IOException ioe) {
        // can never really happen..... convert to unchecked exception
        throw new IllegalStateException(ioe);
    }
}

Note that the method is now static, and it does not need the finally section to clean up.

You can see this running in ideone: https://ideone.com/rKj9mT


Syllabification function for Turkish words

First of all you should know that the script doesn't syllabize properly for every word. For example if you give the word authenticated the function returns ['aut', 'hen', 'ti', 'ca', 'ted'] which is incorrect. The correct case would be ['au','then','ti','cat','ed']. However I do not know how to fix that.

Edit: The point made above doesn't stand since this script isn't for the english language.

Second you have a lot of ifs where you check if a character is a vowel like this:

if word[let_ind] in self.vowels:
        last_was_vowel = True
    else:
        last_was_vowel = False

They could be written better like this:

last_was_vowel = word[let_ind] in self.vowels

Or even better I would have written a function that checks if a letter is a vowel.

def is_vowel(self, char):
    return char in self.vowels

The first if in your for loop

if let_ind != len(word) - 1:
    if word[let_ind + 1] in self.vowels:
        next_is_vowel = True
    else:
        next_is_vowel = False
else:
    syllab = word[keep_index:]
    syllabs.append(syllab)
    break

checks if the next one is a vowel except the last char.
It would be more good looking like this:

for pos, char in enumerate(word[:-1]):
    next_is_vowel = self.is_vowel(word[pos + 1])
    ...

syllab = word[keep_index:]
syllabs.append(syllab)

We enumerate through the word to get the position of the letter and
the letter itself. The word[:-1] means we are going to iterate the whole word except its last letter.

Lastly you got 2 ifs here that do the same thing. You also don't need the keep_index.

if next_is_vowel and not last_was_vowel and syllab:
    syllabs.append(syllab)
    syllab = ""

elif next_is_vowel and word[let_ind] not in self.vowels and syllab:
    syllabs.append(syllab)
    syllab = ""

The could be easily written as one if like this:

if next_is_vowel and syllab and not (last_was_vowel and self.is_vowel(char)):
    syllabs.append(syllab)
    syllab = ""

The place where the keep index was used can be replaced like this
from syllab = word[keep_index:]  to syllab += word[-1]

With these changes in mind the code looks now like this.

def is_vowel(self, char):
    """

    :param char: the character to check
    :return: bool depending if the char is a vowel
    """
    return char in self.vowels

def syllabicate(self, word):
    """
    :param word: The word to be syllabicated
    :return: The syllabicated list that contains syllabs
    """
    word = word.lower()
    syllabs = []
    syllab = ""
    last_was_vowel = False
    # we don't want the last character

    for pos, char in enumerate(word[:-1]):
        next_is_vowel = self.is_vowel(word[pos + 1])

        if next_is_vowel and syllab and not (last_was_vowel and self.is_vowel(char)):

            syllabs.append(syllab)
            syllab = ""

        syllab += char
        last_was_vowel = self.is_vowel(char)

    syllab += word[-1]
    syllabs.append(syllab)

    return syllabs


Random password cracker using brute force

I'll answer your first question separately. However, since your second and third questions are closely related, I'll give the same answers to both questions.

  Am I following coding standards for Python 2 (like PEP8)

For the most part, your code complies with PEP8,

Spaces between operators
Two newlines between function definitions
Variables are lowercase with underscores as needed
etc...

The only thing I'd suggest is to break some of your longer statements up - such as your print statement at the end of your script - onto separate lines, with each separate line being indented.

However, even if sometimes you choose not to comply with a certain coding standard, make sure you are consistent with your naming conventions. I've seen Python code which is written Java style, but is still easy to read because the author was consistent in their style.

  Is there any way to make my code more "Pythonic" (like a native Python coder)? and Is there anyway to improve performance, readability, etc.

Instead of having the list_of_chars variable, make use of the string module which already defines all alphanumeric characters:

>>> import string
>>> string.digits + string.ascii_letters
'0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ'
>>>

Don't use global variables. There is rarely ever a good reason to use them. Instead of making attempted_password global, define it local to the solve_password() function. This makes much clearer where attempted_password is used.
Inside of your first for-loop, you never use the variable letter. Instead, simply use the variable _ which conveys to the reader that your only using this loop for code repetition:

for _ in range(0, random.randint(100000, 250000)):
    password += list_of_chars[random.randint(0, 61)]

The builtin range() function will already start from zero if no other start value is specified. So there is no need to explicitly do it yourself:

range(random.randint(100000, 250000)) # no need for setting 0


Portably generate uniformly random floats from mt19937 output

At least in my opinion, the efficiency question you've raised (repeated division by 2 vs. division by 232 vs. multiplication by 2-32) is best answered by profiling.

I, however, would take issue with making a blind assumption that the generator's maximum is 232-1 at all. Instead, I'd rather use the generator's own specification of the maximum value it can return, so the code would look something like this:

return (float)rng() / (float)RNG::max();

Here again, pre-inverting and multiplying might (possibly) improve speed:

static const float factor = 1.0f / RNG::max();

return rng() * factor;

I also tend to wonder whether it might make sense to make the result type generic. With this, we're no longer restricted to a generator that produces a 32-bit result, but we probably do want to assure that the result type is floating point.

template <typename Result, typename RNG>
Result uniform_dist(RNG & rng) {
    static_assert(std::is_floating_point<Result>::value, "Result must be a floating point type");

    static const Result factor = Result(1) / static_cast<Result>(RNG::max());

    return rng() * factor;
}

Then we'd specify the result type as a template parameter:

std::mt19937 rng { std::random_device()() };

std::cout << uniform_dist<float>(rng) << "\n";

std::cout << uniform_dist<double>(rng) << "\n";


Unnecessary Function

Your swap function is unnecessary. Simply replace the function call with this line:

nums[j], nums[j + 1] = nums[j + 1], nums[j]

This does the swapping for you.

Spacing

There should be spaces between values in lists

[1, 2, 3, 4, 5]

between numbers/strings and operators

if nums[j] > nums[j + 1]

and between parameters in a function call

for j in range(0, len(nums) - 1 - i, 1):

Type Hints

Your function header can look like this:

from typing import List, Union

def bubble_sort(nums: List[Union[int, float]]) -> List[Union[int, float]]:

What this says is that the function accepts a list of integers/floats, and returns a list of integers/floats. It adds another layer of descriptiveness to your code.

Docstrings

You should include a docstring at the beginning of every class/method/module you write. This allows you to describe in words what your code is doing.

def bubble_sort(nums: List[Union[int, float]]) -> List[Union[int, float]]:
    """
    A bubble sort algorithm, etc etc etc

    :param nums -> List: A list of integers/floats to sort

    :return List: The sorted list of integers/floats, from smallest -> biggest
    """


I've tried searching and googling for variations on how to solve this kind of problem, and all the searches turn up either posts made by you or references to the original documentation and the Effects library. In other words, there is not much online documentation available for the Idris language, and I'm a little unsure as to whether it is worthwhile pursuing this language. However I can comment a little on general issues in your code and state machines:

Naming is slightly inconsistent – For the types I would use Pounds and ChocolateBars (or Bars), with the corresponding pounds and bars when used as parameters
Be consistent using either (S pounds) or (pounds + 1) - Try sticking to one specific notation to indicate the incremented version. The same goes for the use of Z or 0. Be consistent.
What is the price of one chocolate bar? If it´s "1 pound" which the code related to vend it doesn't make sense to have a getChange variant. This is more of a removePounds method.
States are not interlinked – I would define more states, with actions clearly linked to the states. As it stands there is no connection between insertCoin, vend and possibly the getChange actions. All of these can be executed in an arbitrarily sequence depending on availability of pounds and bars in the vending machine.
Alternative version in pseudo code:

Empty – An empty vending machine, with no money or no bars

action: fill → state: Idle: Increases the quantity of bars

Idle - A vending machine with no currently inserted money, but has bars

action: insertCoin → state: ReadyForVending: changes the coinInSlot variable
action: removePounds → state: Idle: Clears the pounds variable

ReadyForVending – With inserted money, and bars

action: vend → state: Idle: Removes one bars, and clears coinInSlot
action: returnCoin →  state: Idle: Clear the coinInSlot

To implement this you would need another state variable like coinInSlot in addition to the bars and pounds.
Machine vs Mach is unclear – I'm not sure if you've gotten it quite correct with these, and at least the names are kind of confusing. I think rather than the strange extra notation of Mach I would've used Tuples or Records to describe the internal state, and this would simplify some of the following code as well.
Use effects for the state machine – In the online compiler I found, there was not support for effects, but the documentation (and some slides/post/papers) I found indicates that to properly implement a state machine you should use the Effects library (hangman), which aren't supported in the online compilator.

I tried compiling a new version using the online compilator at http://www.tryidris.org, but there was loads of errors and it seemed a little unstable. In addition, when trying to get information from OP regarding how you actually are running your code, and expected output, I didn't get much response. As such, I've failed getting you alternate refactored code better implementing a type dependent state machine in Idris.


Allow native iteration

It is reasonable to expect to be able to iterate over a RandomGen[erator], but the built-in for loop fails right now.

All you need to allow looping with a standard for loop is renaming next_num to __iter__

Use native iteration

    num = 0
    while num < i:
        # ... snip ...
        num += 1

Is not nice, you are manually incrementing a counter in a high level language. Just use (x)range (depending on the Python version):

for num in range(i):
    # ... snip ...


Ruby command-line app that outputs from JSON file

The way to deal with nested if's is generally to move code into methods. Another solutions is to use an 'early return'. (if !valid b; return instead of if valid then a else b)

I would write this something like:

#!/usr/bin/ruby

require 'thor'
require 'json'

class ListTrends < Thor

  VALID_API_KEY_RE = /^(?=.*[a-zA-Z])(?=.*[0-9]).{8,}$/

  desc   'list_trends json', 'list out trends from JSON file'
  option :api_key, aliases: '--api-key', type: :string, required: true
  option :format, type: :string, desc: 'one line format'
  option :no_country_code, aliases: '--no-country-code', desc: 'remove country code', type :boolean

  def list_trends(keyword=nil)

    # Check your options before reading the file
    if !VALID_API_KEY_RE.match(options[:api_key])
      puts "Invalid API Key, operation abort..."
      exit(255)
    end

    json        = File.read('trends_available.json')
    trends_hash = JSON.parse(json)
    keyword     = keyword.to_s.downcase

    trends_hash.each do |trend|
      process_trend(trend)
    end
  end

  private

  def process_trend(trend)
    return unless trend["country"].downcase.include?(keyword)

    if options[:format]
      output_formatted_trend(trend)
    else
      output_complete_trend(trend)
    end
  end

  def output_formatted_trend(trend)
    output = trend.values[0..2]
    output.delete_at(1) if options[:no_country_code]
    puts output.join(' ')
  end

  def output_complete_trend(trend)
    trend.each do |k, v|
      if v.is_a?(Hash)
        v.each do |i, j| puts "Trend location type #{i}: #{j}" end
      else
        puts "Trend #{k}: #{v}"
      end
      puts ''
    end
  end

end

ListTrends.start(ARGV)

Another advantage being that most people can only understand a few lines of code at a time, short methods also make it easier to keep track of variable scope and giving them meaningful names makes it easier to understand code without needing comments.


Count to target with javascript extended

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN "http://www.w3.org/TR/html4/strict.dtd">

  <title>Count to a particular target</title>

Starting


Well, according to the ANSI book:

  The identifiers in an enumerator list are declared as constants of type int

So the cast is redundant:

typedef enum {
    true = ~0,
    false = 0
} bool;

Also, in C, a "true" value is anything other than zero, so ~0 is no more "true" than 1 - might as well keep it simple:

typedef enum {
    false = 0,
    true = 1
} bool;

In fact, 0 and 1 are the standard choice for representing boolean values where there's no native boolean type (for example, the bit type in SQL uses 0/1).

Now you might ask yourself, "so - did we just redefine zero and one?" - well, we certainly did. As an experiment in using typedef/enum it's okay. But don't use it in actual C code, because it'll make if clauses unnecessarily complex.

For example, let's say we want to compare str and str2 and to do something if they're not equal. We'll use strcmp which compares two strings and returns 0 if they're equal.

// without bool
if(strcmp(str1,str2))
{
    ...
}

// with bool
if(strcmp(str1,str2) == 0 ? false : true)
{
    ...
}

In conclusion, as an experiment your code is OK and can be slightly improved. Generally, the lack of bool type in C seems scary at first, but it's actually quite convenient when you get used to it.


Determine whether a word appears in a sentence

The String class provides the method .equalsIgnoreCase(). It seems to me, calling .toLowerCase() and then .equals() has to process the string twice, but could be done in a single run.

It is unclear how well the sentence is processed before it enters the method as array. I will be a little optimistic in my assumptions.

You can give it a cleaner structure using streams [if you are using Java 1.8 at least]. It provides a method .noneMatch(predicate) which is short-circuiting just as your loop is now. Using streams makes it easy to add a cascade of string preprocessing steps before the final comparison. Removing punctuation as you have it now, it could be written as follows:

import java.util.Arrays;

public static boolean sentenceContainsWord(final String[] aSentence, final String aWord)
{
    return !Arrays.stream(aSentence)
                  .map((w) -> w.replaceAll("\\.|,|:|;", ""))
                  .noneMatch((w) -> w.equalsIgnoreCase(aWord));
}

My optimistic assumption here is that there is no punctuation in between a word which might result from a too simplistic tokenisation (e.g. .split(" ")). Using .replaceAll() removes a lot of the clutter around finding trailing punctuation and extracting substrings.

Resources:

Java Doc:
Stream::noneMatch
Java Doc:
String::equalsIgnoreCase
Java Doc:
String::replaceAll


You need to check for overflow before adding.

for (int i=2; i<=upLimit; i++) {
    if (isPrime[i]) {
        // Check for overflow
        if (std::numeric_limits<decltype(sum)>::max() - sum <= i) {
            std::cerr << "Overflow" << std::endl;
            return 1;
        }

        // add to sum
        sum += i;
        …

As int is only guaranteed to hold values up to 32767, you'll find that you should use a long long for sum.


Same concept as yours, but slimmed down a bit, and avoiding an explicit loop.

You might also consider making this a utility function, or a mixin with which you can dynamically extend Enumerable objects.  Making it a class which creates a special object wrapper over a list, for the purpose of adding a single method, has a clunky java-esque feel.

Also, keep in the mind the below is optimized for readability and brevity, not for speed.  In particular, left_of and right_of are making unnecessary copies of the array halves.

class BinarySearch

  attr_accessor :list

  def initialize(orig)
    raise ArgumentError unless sorted? orig
    @list = orig
  end

  def search_for(n, s_list = list, offset = 0)

    raise RuntimeError if s_list.empty?

    mid = s_list.length / 2
    mid_val = s_list[mid]

    return mid + offset if mid_val == n
    return search_for(n, left_of(s_list, mid), offset) if mid_val > n
    search_for(n, right_of(s_list, mid), offset+mid+1)

  end

  private

  def left_of(s_list, i)
    s_list[0..i-1]
  end

  def right_of(s_list, i)
    s_list[i+1..-1]
  end

  def sorted?(orig)
    orig.each_cons(2).all? { |a,b| (a <=> b) <= 0 }
  end

end


They are ok, but I would still do some things differently.

I would instead throw exceptions.
Instead of if else within your action, I would use a validator with rules.
Instead of using $_POST/$_GET directly, I would sanitize them and put it inside a Request class.
Error messages belongs to the view, so I would set $view->errors[]='message' or something like that.
Your routing class is not flexible. As a good example, you could take a look at the Silex framework.
Login_Controller should not exist. There might be Account_Controller with login_action or Authenticate_Controller with login_action.

Usually a Controller is a group of actions with to execute specific stuffs. For example, Profile_Controller has view/edit/delete action. Within the controller you check the user roles, if someone can view/edit/delete the profile. In other frameworks, they use "Access Control List (ACL)" for this purpose.

Your linked tutorial is just for the basic understanding purpose, so please don't use it. Use a readily existing framework, since you wish to build the app and not the framework itself. You might end with modifying and changing your basic structures and implementing new functions instead of developing the features of your web application.


Network helper class with retry logic on failure

Just wanted to add that you may want to consider using Polly instead of rolling your own retry mechanism - this is a library built specifically for this kind of retry mechanism (as well as many more complicated scenarios). Using Polly would look something like the following.

Note that I have renamed the attempts parameter to retryAttempts to make it clear that this does not define the total number of attempts but rather the total number of retry attempts after the initial one, to keep the behaviour the same as your sample code.

public class NetworkHelper
{
    public static Task<string> LoadAndWaitForSelectorAsync(
        Uri url, string selector, int retryAttempts = 5)
    {
        // Create a policy that will...
        var policy = Policy
            // Retry for any `WaitTaskTimeoutException` raised during execution.
            .Handle<WaitTaskTimeoutException>()
            .WaitAndRetryAsync(
                // Retry the specified number of times after the initial attempt.
                retryCount: retryAttempts,
                // Wait for the given duration between each retry attempt. Note that
                // this can depend on the retry number if required.
                sleepDurationProvider: retryNumber => TimeSpan.FromSeconds(10));

        // Execute the operation to get the selector within the retry policy. If the
        // final retry attempt does not succeed, the resulting `WaitTaskTimeoutException`
        // will not be handled by the policy and will be thrown.
        return policy.ExecuteAsync(() => TryGetSelectorAsync(url, selector));
    }

    private static async Task<string> TryGetSelectorAsync(Uri url, string selector)
    {
        using (Page page = await Handler.Browser.NewPageAsync())
        {
            await page.GoToAsync(url.ToString(), timeout: 0);
            await page.WaitForSelectorAsync(selector);
            return await page.GetContentAsync();
        }
    }
}


Lets start with the simple one:

void quote_container::stringToUpper(string &s)
{
   for(unsigned int l = 0; l < s.length(); l++)
    s[l] = toupper(s[l]);
}

How about:

std::transform(s.begin(), e.end(), s.begin(), ::toupper);

Knowledge of the standard library is very useful.

Never pass anything as a pointer (this is C++ not C).

void quote_container::allocate(string *i_string, string *i_value){

I see no testing of i_string or i_value for NULL so they can't be NULL so there is no need to pass them as pointers. So you should be passing them as references. Since neither value is modified by the function you should also probably be passing them as const to prevent accidental modification of the original value.

void quote_container::allocate(string const& iString, string const& iValue){

Don't explicitly cast values.

open = (float)temp_value;
 //    ^^^^^^^

This is code smell. If you must cast (and usually you should not) then use one of the C++ cast operators (there should never be a need to use a C cast operator (as above)). But in this case it looks like it is not needed. If you don't cast the same conversion will happen anyway.

In this case

avg_buy_price  = avg_buy_price + (float)temp_value ;

You are getting worse results. Let the compiler to the cast for you. If you had let the compiler do the cast for you with:

avg_buy_price  = avg_buy_price + temp_value;

// Then the compiler would have generated this code
avg_buy_price  = static_cast<float>(static_cast<double>(avg_buy_price) + temp_value);

This way you would have retained a tiny bit more accuracy during the addition (which may have mattered). Before you cast it back to float.

As mentioned by "The Dr" (MrSmith) above factor out any common code. But I would do it differently. Put all your conditions in a map to make the code more readable. You actually have two types of assignment. The first is simple assignment while the second is accumulation. So we break these two types of action into their own maps

std::map<std::string, float*> assign= {
    {"OPEN",             &open},
    {"DAYHIGH",          &high},
    {"DAYLOW"            &low},
    {"LOW52",            &year_low},
    {"HIGH52",            &year_high},
    {"PRICEBANDLOWER",    &lower_price_band},
    {"PRICEBANDUPPER",    &upper_price_band},
    {"TOTALBUYQUANTITY",  &tbq},
    {"TOTALSELLQUANTITY", &tsq},
    {"FACEVALUE",         &face_value},
    {"PREVIOUSCLOSE",     &prev_close},
    {"LASTPRICE",         &last_price},
    {"TOTALTRADEDVOLUME", &ttv}
  };
std::map<std::string, float*> accum = {
  {"BUYQUANTITY5",   &tbq2},
  {"BUYQUANTITY4",   &tbq2},
  {"BUYQUANTITY3",   &tbq2},
  {"BUYQUANTITY2",   &tbq2},
  {"BUYQUANTITY1",   &tbq2},
  {"SELLQUANTITY5",  &tsq2},
  {"SELLQUANTITY4",  &tsq2},
  {"SELLQUANTITY3",  &tsq2},
  {"SELLQUANTITY2",  &tsq2},
  {"SELLQUANTITY1",  &tsq2},
  {"BUYPRICE1",      &avg_buy_price},
  {"BUYPRICE2",      &avg_buy_price},
  {"BUYPRICE3",      &avg_buy_price},
  {"BUYPRICE4",      &avg_buy_price},
  {"BUYPRICE5"       &avg_buy_price},
  {"SELLPRICE5",     avg_sell_price}, /*avg_sell_price  =  tsq2 + (float)temp_value;*/
  {"SELLPRICE4",     avg_sell_price}, /* I assumed this was a copy paste mistake you */
  {"SELLPRICE3",     avg_sell_price}, /* made. If it is not then this array is slightly */
  {"SELLPRICE2",     avg_sell_price}, /* More complex for SELL (but not much) */
  {"SELLPRICE1",     avg_sell_price}
 };

Now that we have all the data stored in maps the code is highly simplified.

 std::map<std::string,float*>::const_iterator find;
 find = assign.find(iString);
 if (find != assign.end())
 {
     (*find->second) = atof( iValue.c_str() );
 }
 else
 {
     find = accum.find(iString);
     if (find != accum.end())
     {
         (*find->second) += atof( iValue.c_str() );
     }
     else
     {
         std::cout << "\n\t Ignoring --" << iString << "---" << iValue;
     }
 }

You can compreses the two maps into a single map (and thus reduce the code here) by using a functor or class abstraction as described by (@utnapistim) in his answer. Personally I would go with using std::function rather than building my own class hierarchy (but that's a you say potato I say tomato argument and down to personal preference).


I'm the lead engineer for the MongoDB Perl driver with a couple of thoughts for you:

you're using 'authenticate', which is for a very old version of the driver which is not recommended for use.  In the v1.x series, you should provide username/password in the URI or in MongoClient parameters.
Most of the value of this module seems to be setting parameters and calling connect.  I would suggest doing that with a function, not a module and then just using the resulting MongoDB::MongoClient object directly.


reformatting string to two string values in javascript

Yes, there are few built-in array functions you may use to simplify your code. I don't think you will ever notice any performance difference but code will also be easier to read:

var id = "HEL-CAS|MAD-STO|XXX-YYY"

// Format is fixed then it's easy to use a regex
var splittedId = id.split(/[\|-]/);

// Now we can pick "every other element" and then join strings
var from = splittedId.filter(function (v, i) {
    return i & 1 == 0;
}).join('|');

var to = splittedId.filter(function (v, i) {
    return i & 1 == 1;
}).join('|');


Logging Entity Framework Changes - improve performance

Not a big deal, but this:

var currentVal = property.CurrentValue == null ? "" : property.CurrentValue.ToString();
var originalVal = property.OriginalValue == null ? "" : property.OriginalValue.ToString();

Can be shortened to this:

var currentVal = (property.CurrentValue ?? string.Empty).ToString();
var originalVal = (property.OriginalValue ?? string.Empty).ToString();

  I'm not sure of the best way to extract the table name.

You're not doing that. You're looking at the name of the type of the entity - being an object/relational mapper, EF maps that entity to a table; if you need the table name then you need to look at the entity's table mappings.

  I intend to expand it to cope with any table

How about having an IEnumerable<Type> where you have all the entity types you want to log changes for, and then instead of getting the type's name and comparing with some magic string, you can just do _monitoredTypes.Contains(typeof(entry.Entity)) (not tested, just a thought).

In terms of performance, you're looping too much:

if (entry.Entity.GetType().Name.Contains("PropetyPair"))

This condition doesn't need string o, as it's working off entry - thus, you can move that condition up two levels and only enter the 2nd loop when the entity type is interesting.


Currently some of your code is very specialized to the exact use case you have. For example, the get_factors function takes the unparsed equation and needs to extract the constant itself. It would be a lot better if it just took a number and the parsing happens elsewhere. This way if you improve the parsing (to include multi-digit numbers, etc), you don't need to change the get_factors function.

At the same time, that function is borderline unreadable. It is also not the most efficient. Consider this alternative, which is shorter in lines of code, IMO way easier to read, and about two times faster:

def get_factors_of_constant(equation):
    c = int(equation[-1])
    factors = set()
    for i in range(1, int(c**0.5) + 1):
        if c % i == 0:
            factors.update([i, c // i, -i, -c // i])
    return list(factors)

For a better parsing, consider the output of the following regex:

import re

equation = 'x^3 -4x^2 +1x +6'
print(re.findall(r'\s?([\+\-]?\s?\d*)?(x)?\^?(\d*)?', equation))
# [('', 'x', '3'), ('-4', 'x', '2'), ('+1', 'x', ''), ('+6', '', ''), ('', '', '')]

It produces tuples of coefficients, whether or not there is an x and exponents. If the coefficient is empty, a 1 is assumed, if the exponent is empty, it is either a 1 or a 0, depending on if there is an x, and if all three are empty, you can skip it.

def parse(equation):
    coefficients, exponents = [], []
    matches = re.findall(r'\s?([\+\-]?\s?\d*)?(x)?\^?(\d*)?', equation)
    for coefficient, x, exponent in matches:
        if coefficient == x == exponent == "":
            continue
        coefficients.append(coefficient.replace(" ", "") or "1")
        exponents.append(exponent or "1" if x else "0")
    coefficients = list(map(int, coefficients))
    exponents = list(map(int, exponents))
    return coefficients, exponents

print(parse(equation))
# ([1, -4, 1, 6], [3, 2, 1, 0])

This can also parse multi-digits and deals both with missing parts (since you have the info on the exponent) and additional whitespace between number and sign:

parse("x^3 - 10x + 2")
# ([1, -10, 2], [3, 1, 0])
parse("x")
# ([1], [1])

It is also not perfect, for example it fails in this case:

parse("-x^2")
# ValueError: invalid literal for int() with base 10: '-'

You would need to add some special cases to the function for it to cover all edge cases, but it should be a good starting point.


Using includes() vs. hash function to check if an unordered_set is a subset of another unordered_set

There are several things that are problematic:

Broken code

The version using std::includes is actually broken. When you read up on std::includes you will find that it requires the ranges it is operating on to be ordered by some comparison function (defaults to operator<).

Now compare this with the source of the iterators which is a template call unordered_set.

You should not use std::includes on unordered_set because the element sequence of the latter does not satisfy the preconditions of the former, thus resulting in an invalid algorithm.

Meaningless benchmark

There are several problems with your benchmarking code (some of which have already been addressed):

executed only once: to reduce the impact of noise in the measurements the benchmark is usually done several times and then the average is calculated
too short measurement period: This is somewhat connected the previous one. If the runtime of an algorithm is very short (close to the time measurement granularity limit) there will be big steps for very little change so one would try to increase the runtime (again my doing multiple runs)
too little problem size: the effect of hashing is the advantage of \$O(1)\$ element access compared to \$O(log n)\$ (for trees) or even \$O(n)\$ for linear structures. However, these are only asymptotic complexities which become meaningful as n grows towards infinity. For the little problem sizes you are presenting here they hardly matter and are dominated by surrounding (constant complexity) code
you print results during measuring which will most likely account for most of the time you are measuring because printing is very slow.

C++ 11

If you've got C++11 then why not use the whole package?

Use range-based for to clean up messy iterator loops:

for (std::unordered_map <int, std::unordered_set<int>>::const_iterator SetTwo = S2.begin(); SetTwo != S2.end(); ++SetTwo)

can become

for (auto const &element: S2)

However notice that element is already the element pointed to by the dereferenced iterator.

Function Interface

Now that we got away from the iterator we should clean up the interface of IsIn. This function is far too specialized by taking an iterator. Instead take the type you want directly:

bool IsIn(const std::unordered_set<int>& haystack, std::unordered_set<int>& needles) {
    for (auto const& needle: needles)
        if(haystack.find(needle) == haystack.end())
            return false;
    return true;
}

I renamed the variables to make their intention more clearly (though they are not fitting for the current task). The function should probably be renamed too.

Unused Variable

What is the intention of variable S1? You initialize it and then just copy some of its values into SetOne. You could have just initialized SetOne and be done.

Wrong container type

Using an unordered_map for S2 is totally overkill. You have a continuous range of indices starting at 0 and it seems like you want to iterate this in exactly this order. You can use a vector for this and initialize it with the desired values:

std::vector<std::unordered_set<int>> S2{
    { { 1 }, { 2 }, { 3} }, //  S2 is a subset of S1
    { { 3}, { 4 }, { 5} }, // S2 is a subset of S1
    { { 2 }, { 4 } }, // S2 is  a subset of S1
    { { 3 }, { 4 }}, // S2 is  a subset of S1
    { { 3 },  { 5 } }, // S2 is a subset of S1
    { { 1 }, { 2 }, { 8 } }, //  S2 is a subset of S1
    { { 3 }, { 4 }, { 6 } }, // S2 is not a subset of S1
    { 2 }, {7 } }, // S2 is  a subset of S1
    { 7 }, { 8 } }, // S2 is  a subset of S1
    { 5 }, { 6 } }, // S2 is not a subset of S1
};

I would propose renaming S2 to testSets or some other name that indicates that there are multiple sets that should be tested. It may even be advisable to create two vectors, one with subsets and one that has no subsets.

Inadequate loop

In your second method you are using a while loop instead of a for loop. Benchmarkwise this might skew your results because you don't want to measure differences in the loops, so you should be using the same code there.

The for loop is clearly the better alternative here because you intend to loop over the whole container anyways.


I'm pretty sure there's no better way to do this but I thought a consensus on here might be nice. Let me know what you think.

At first glance it seems to be ok but one could assume that he/she is always getting either the value or the default value. If one would get the default value for e.g if the dictionary is empty, why shouldn't he/she get the default value if the dictionary is null or the key is null ?

If this isn't the way the method should work, then you need some documentation which cleary states the purpose and any expected exceptions.

Now, what happens if the method is called either like so

Dictionary<string, int> dict = null;
dict.GetValueOrDefault("someKey");

or (assuming the method lives in the "DictionaryExtensions" class)

Dictionary<string, int> dict = null;
DictionaryExtensions.GetValueOrDefault(dict, "someKey");

sure you get a NullReferenceException but is this the type of exception one would expect ? If I call a method with an parameter being null I would expect to get either no exception or to get an ArgumentNullException.

What about if the key is null ? Sure you get an ArgumentNullException but it takes some time because it is thrown at the FindEntry() method of the Dictionary and this is also shown in the StackTrace.

IMO it would be better to check this cases inside the GetValueOrDefault() method and throw the exceptions there.

public static TValue GetValueOrDefault<TKey, TValue>(this Dictionary<TKey, TValue> dictionary, TKey key, TValue defaultValue = default(TValue))
{
    if (dictionary == null) { throw new ArgumentNullException(nameof(dictionary)); } // using C# 6
    if (key == null) { throw new ArgumentNullException(nameof(key)); } //  using C# 6

    TValue value;
    return dictionary.TryGetValue(key, out value) ? value : defaultValue;
}


Best way to create a daemon in a JavaWeb application

Using a ServletContextListener is a good solution, I would use the same.
I would consider using a ScheduledExecutorService here (Executors.newScheduledThreadPool(1)). It has some advantages:

It has a scheduleWithFixedDelay method as well as scheduleAtFixedRate. Using one of these describes what's the developer's purpose. Currently its not completely obvious or explicit.
It supports shutdown and awaitTermination. awaitTermination returns immediately when your task finishes, no need to wait complete seconds.
awaitTermination returns false on timeout when you can call shutdownNow which interrupts the thread of the task.
It does not need manual and error-prone synchronization (see the next point).
newScheduledThreadPool can get a ThreadFactory. You can set a UncaughtExceptionHandler to handle uncatched exceptions with that factory. (See also this answer too.)

Usage of isDaemonWorking is not thread-safe. It is used by the timer's thread (inside the TimerTask.run method) as well as the ServletContextListener. Without any synchronization you risk that threads will not see each other's modification.

  [...] synchronization has no effect unless both read and write operations are synchronized.

From Effective Java, 2nd Edition, Item 66: Synchronize access to shared mutable data.
You could invert the if condition here:

for (; i<10; i++) {
    if (isDaemonWorking) {
        try {
            TimeUnit.SECONDS.sleep(1);
        } catch(InterruptedException ex) {
            Thread.currentThread().interrupt();
        }
    } else {
        break;
    }
}

It would keep the code flatten:

for (; i < 10; i++) {
    if (!isDaemonWorking) {
        break;
    }
    try {
        TimeUnit.SECONDS.sleep(1);
    } catch (InterruptedException ex) {
        Thread.currentThread().interrupt();
    }
}

I guess if you put the isDaemonWorking = false into a finally block:

try {
    isDaemonWorking = true;
    System.out.println("I RUN");
} catch (Throwable e) {
    e.printStackTrace();
} finally {
    isDaemonWorking = false;
}

then you could replace

if (i == 10)
    timerTask.cancel();

with

if (isDaemonWorking) {
    timerTask.cancel();
}

and use an usual for (int i = 0; i < 10; i++) loop too.
I would not think that initialization of TimerTask should be in a static block. The timer and isDaemonWorking also could be instance fields. I think (and I would expect that) the servlet container calls contextDestroyed on the same instance than contextInitialized.
Variable names in Java usually camelCase, so a_event should be aEvent or just event here:

public void contextInitialized(ServletContextEvent a_event) {

I guess you have a typo here: BiuDeamon (it should be BuiDaemon).


make_grid() isn't quite the right name for this function, as it is printing a grid rather than returning a grid.  I think you would be better off returning a long string to be printed.

In your list comprehension, _ is a bad choice for the iteration variable name, since _ indicates a throwaway value, by convention.  If you then call str(_), you should pick a different name.

Besides the excessively wide spacing as you have observed, using tabs is problematic because the output would be better with the entries right-aligned.  You also have a superfluous tab at the end of each line (except the header row).

Suggested solution

Use '{0:>{width}}'.format(…) to print the entries right-aligned with a fixed width.

def multiplication_grid(max):
    max += 1    # Inclusive upper bound for ranges
    grid = [['X'] + [j for j in range(max)]] + \
             [[i] + [i * j for j in range(max)] for i in range(max)]

    # Use fixed-width right alignment for every entry in the 2D grid
    width = 2 + len(str(max * max))
    fmt = lambda n: '{0:>{width}}'.format(n, width=width)
    return '\n\n'.join(''.join(fmt(entry) for entry in row) for row in grid)

[['X'] + [j for j in range(max)]] creates the header row.

[[i] + [i * j for j in range(max)] for i in range(max)] is a nested list comprehension to create the rest of the rows, where i is the row number and j is the column number.

lambda is a way to define very simple functions that do nothing more than returning an expression.  The lambda could also have been written as:

    def fmt(n):
        return '{0:>{width}}'.format(n, width=width)

The last line applies fmt() to every entry in the two-dimensional grid and joins everything into a single string.


Some Tips

Avoid using constructor.
Avoid using .bind(this)
Destructuring will help you to make cleaner code.

Questions

  Avoid initializing the state empty state in the constructor. However, I had to initialize shoppingCart: [] in componentDidMount hook to avoid type error. What is the better and cleaner approach?

You can have 2 aproaches to this

There is a way to avoid this, what you do is destruct the state in the variables you want and set then a default value of an array.

state = {}

render() {
    const {
        shoppingCart = []
    } = this.state
    ...
}

This way you don't need to check if shoppingCart is an array, and you only need to declare this.state as an object ( it can be just an empty object ).

You can also do

render() {
        const {
        shoppingCart = []
    } = this.state || {}
    ...
}

And now you don't even need state = {}
Because you are using arrow functions, you don't need .bind(this) and also you can avoid de constructor and just use

state = {
    ...
}

This way you can declare shoppingCart and items and don't need to declare it in componentDidMount (wich is much better to understand) and now don't need to destruct the state.

state = {
    shoppingCart = [],
    items = []
}

This way is good if the data you will get is always an array (or the type you specify) and another good reason to use this is that when other programmer look at the code, they will immediately know what is stored in that component state, but when you have too many things in the state, it can get confused.

This aproaches will solve some problems like using ternary to check if you have an state and then return the mapping or if you don't have, return an empty array.

render() {

    const {
        items = [],
        shoppingCart = []
    } = this.state

    let aisleList = items.map((item, index) => {
        return (

                {item}
                <input
                    type="checkbox"
                    id={index}
                    value={item}
                    onClick={this.handleAileClick}
                />

        );
    });

    let cartList = shoppingCart.map((item, index) => {
        return (

                {item}
                <input
                    type="checkbox"
                    autoComplete="off"
                    value={item}
                    onClick={this.handleCartClick}
                />

        );
    });
    ...
}

Looking at this, is much clean and easy to understand.

  Is it good practice to keep the logic simple in handleXXXClicks, separation of concern one for handling items and one for shoppingCart?

You can do the way you want, but you need to see what you will do in the future, if both will always do the same, you can create one function for both and just send a different parameter, but if they will do some extra stuff, it's good to have then separated.

*(I'm not english native speaker and maybe I didn't understand exaclty what is this part about).

  I feel aisleList and cartList bloated and I could have written itemComponent which is avoidable duplicate code.

Yes, they look so similar that you should create a component for both and pass the diference in props.

  Is there any better way to handle the item in setState? I couldn't think of using prevState in my scenario.

Here is simple setState using a function with prevState. Note that the way you are doing it, you need to have event.persist(), for more info look here

handleCartClick = event => {
    event.persist();

    this.setState(prevState => {
        return {
            items: [...prevState.items, event.target.value],
            shoppingCart: prevState.shoppingCart.filter(item => {
                return item !== event.target.value;
            })
        };
    });
};

Overall

Your code looks ok, there is only one strange thing. You are using checkboxes and when you click, some get marked and other don't. I don't know if that was a test or you also wanted to solve the checkbox problem. Let me know and I can see what I can do.

I recommend you to start using some new things like I mentioned in the tips part, that help you alot and make your code cleaner.

Also have a look at hooks.
For me, using hooks makes alot of things easier, but also, the code looks cleaner.

I made a demo for you to test and see the new code. I changed some things.

I changed handleCartClick but left handleAileClick the way it was before, so you can try useing prevState yourself.
And also left the part of itemComponent for you.

The best way to learn, is doing it!

Hope I helped you!


TCP retarder for network delay simulation

I see a number of things that may help you improve your code.

Consider using existing tools

First, let me note that I've had a need for a similar thing before, but what I eventually used was tc which was tremendously useful for my testing and required essentially no additional code on my part.  In my case, although neither of the devices I was testing ran Linux, I put a Linux box between the devices and did all of the data logging from there.  Not only did this allow me to arbitrarily delay some number of packets, but also allowed the deliberate injection of bit errors, etc. which very much expanded the possible test repertoire.

Make sure you have all required #includes

The code uses a number of things from both the standard library and from boost but doesn't show the required #includes.  Carefully consider which #includes are part of the interface (and belong in the .h file) and which are part of the implementation and provide them to users (and reviewers!) of the code.

Lock shared resources before use

The send_loop() routine prints any error message to std::cerr but does so without a lock.  This is a problem because std::cerr is a shared resource.  If this thread is in the middle of printing a message when another thread writes, you may get interleaved messages.  While this is not fatal, it's easily avoided by controlling access to std::cerr via a mutex.

Don't launch a thread without considering object lifetime

The SendRetarder constructor simply launches a detached thread with this line:

std::thread(&SendRetarder::send_loop, this).detach();

However, this has a major problem.  What if the object is destroyed after this thread is launched?  Everything within the send_loop() routine that relies on object variables is going to be potentially using de-allocated memory and non-existent objects!  Prevent this by using shared_from_this and a std::shared_ptr instead of just passing this.

Provide a destructor

What happens to the detached thread if the object is destroyed?  Nothing, at the moment, and that's a problem.  If the object is destroyed, it seems to me that either there should be a way to signal the detached thread that it's time to quit.  One simple way to do that is to close the underlying socket which is one thing that should probably be done in a destructor.

Consider using a better random number generator

If you are using a compiler that supports at least C++11, consider using a better random number generator.  In particular, instead of rand, you might want to look at std::uniform_int_distribution and friends in the <random> header.

Consider future users

The line that writes the message  to the socket looks like this:

boost::asio::write(args.first->socket_,
                   boost::asio::buffer(&args.second, sizeof(message_t)), ec);

That works right now, but what happens if you decide to have a variable size message?  For example, consider this:

using message_t = std::string;

Now the sizeof() operator will probably return the size of a pointer rather than the size of the actual object which will lead to maddenly difficult-to-find bugs.  I'd recommend adding a size() method to your existing message_t object and then changing the write to look like this:

boost::asio::write(args.first->socket_,
                   boost::asio::buffer(&args.second, args.second.size())), ec);

Then if the message_t is ever changed to something else, it will be required to implement a size() also.  One way to enforce that, if needed, would be to have a message_t base class and have a pure virtual size() but that may be overkill for your purposes.


Armond, the main thing you should do is to learn about functions.  Putting everything in main is not practicable beyond very small programs.   And nested loops are generally a bad idea too.  So for example you might extract the content of the first loop into a function and extract the second loop into another function.

So you would get something like:

int main(int argc, char **argv)
{
    do {
        user_interaction();
    } while (user_says_exit() == false);
    return 0;
}

Also note that while (true) {...} is more commonly used than do {... } while (true); although do{} while() loops clearly have their uses.

EDIT

To be honest, I like the edited version less than the original.  You are now testing the y_n_decision as the condition in both loops. And you are relying on the variable having been default-initialized. Moreover the loops are now while instead of do-while and are arguably more wrong now than they were before.  A while (true) loop is preferable to your original do ... while (true)  because the condition (true) is immediately visible at the top of the loop.  But the new loops using while (condition) rely upon the initial value of the condition - and in your case you have to force that to be false by using an initializer that is not y/q. This is at best inelegant.

Here is how I might have coded what you are doing:

#include <iostream>
#include <string>
#include <ctype.h>

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

static bool user_says_exit(void)
{
    std::string decision;
    do {
        cout << "Please press y to restart, or q to quit" << endl;
        getline(cin, decision);
    } while (decision != "y" && decision != "q");
    return (decision != "y");
}

static void user_interaction(void)
{
    std::string age;
    std::string favfood;
    std::string name;
    cout << "Welcome to my program in c++" << endl;
    cout << "Please enter your name" << endl;
    getline (cin, name);
    cout << "Enter your favourite food" << endl;
    getline (cin, favfood);
    cout << "Awesome! " << favfood << " is my favourite food too" << endl;
    cout << "Enter your age" << endl;
    getline (cin, age);
    cout << "So your name is " << name << ", your fav food is "
         << favfood << " and your age is " << age << endl;
}

int main(int argc, char **argv)
{
    do {
        user_interaction();
    } while (user_says_exit() == false);
    cout << "Quitting now" << endl;
    return 0;
}


First of all, you have weird behaviour if pending_coloring and pending_processing are both empty arrays.  The output would then be

You have . Go to widget dashboard

I assume that in such a case, it should just say "Go to widget dashboard".

I think that pluralization ought to be treated as an i18n issue.  Even if your application happens to be English-only for now, it's actually easier to generalize the solution, since pluralization is considered a solved problem in internationalization.  In particular, you can use the i18n gem (which is included in Rails, and is a simple installation with no dependencies even if you aren't using Rails).

require 'i18n'

pending_coloring = …
pending_processing = …

I18n.backend.store_translations :en, 'widget' => {
    :zero => nil,
    :one => '1 widget to %{verb}',
    :other => '%{count} widgets to %{verb}'
}
coloring_text = I18n.t 'widget', :count => pending_coloring.size, :verb => 'color'
processing_text = I18n.t 'widget', :count => pending_processing.size, :verb => 'process'

notification_text = if (!coloring_text && !processing_text) then
  ''
else
  "You have #{[coloring_text, processing_text].compact.join(' and ')}.  "
end + "Go to widget dashboard"


Requirements
Number of words
Are you guaranteed to have at least 2 words?  If so, you could simplify your loop, by removing the first read; the loop will take care of the second & subsequent words:
    String word2 = StdIn.readString();
    boolean truth = true;
    while (!StdIn.isEmpty())
    {
        String word1 = StdIn.readString();
        truth = truth && checkWordCouple(word1,word2);
        word2 = word1;
    }

Although now if given a single word, it will produce true.  This may be an improvement, but you’d need to check the exact requirements to be sure.
Word Length
Is HEAD, HEAL, TEA, TO a word chain?  Your program says it is!
What about HEAD, HEAL, HEALTHY?  That input causes your program to crash!
Your program should handle mismatched word length, unless the problem statement guarantees all inputs will be the same length.
Number of Differences
The problem description says:

In a word chain, adjacent words must differ in exactly one letter

but HEAD, HEAD, HEAD, HEAD is reported as "true".  Clearly, counter > 1 is not the correct "false" condition.  Borrowing Doi9t's suggestion, and further improving it, the correct line would read:
return counter == 1;

Self Documenting Code
You read word1, then word2, and compare the first to the second (let’s call that “forward”), then read a third into word1, and compare the third to the second (comparison in the “reverse” direction).  You are testing for one difference, so it works, but what if you were checking for one additional character?  By reversing the word order, you’d actually be testing for character removal!
Instead of word1 & word2, maybe previous_word and next_word would help keep things clearer.
    String previous_word = StdIn.readString();
    boolean truth = true;
    while (!StdIn.isEmpty())
    {
        String next_word = StdIn.readString();
        truth = truth && checkWordCouple(previous_word, next_word);
        previous_word = next_word;
    }


Find minimum and maximum coordinates for a set of points

As @200_success mentions, these classes seem a bit sparse, and you shouldn't bother with classes for them in the first place.

For instance, Point could be nicely replaced with a namedtuple.

>>> from collections import namedtuple
>>> Point = namedtuple('Point', ['x', 'y', 'z'], defaults=[0,0,0])
>>> p1 = Point(1,4,3)
>>> p2 = Point(5,2,3)

The components of Point can now easily be referred to by name (eg, p1.x or p2.z) and can still be referred to by index (eg, p1[0] or p2[2]).

Similarly, you don't need a whole class just for a minmax_coords function:

>>> def minmax_coords(*pnts):
...    mn = Point( *(min(p[i] for p in pnts) for i in range(3)) )
...    mx = Point( *(max(p[i] for p in pnts) for i in range(3)) )
...    return mn, mx
...
>>> minmax_coords(p1, p2)
(Point(x=1, y=2, z=3), Point(x=5, y=4, z=3))
>>>

Note: As opposed to a dictionary of dictionaries for the result, I've changed the return into two points: a point with the minimum coordinate on each axis, and a point with the maximum coordinate on each axis.

Demystifying the minmax_coords implementation:

min(p[i] for p in pnts) computes the minimum value of the i'th axis over all of the points.
_____ for i in range(3) is a generator expression which executes that minimum computation for each axis i=0, i=1 and i=2.
Point( *(____) ) immediately evaluates the generator expression and uses the values as the 0th, 1st, and 2nd arguments of the Point() constructor.


Bash script that checks if font is installed and installs it if necessary

If you are using bash then rather than mixing [ and [[, you should stick with one or the other, preferrably [[. See What is the difference between test, [ and [[ ?

You might also want to consider preferring printf over echo for a more robust approach. See this amazingly detailed analysis of why printf is better than echo.


Looks fine to me, I would add some small changes:

If you create a directive/service/controller/.. in Angular it's a best practice to create them with the [ ] notation to make sure minifying your code won't break:

.directive('name', [ '$timeout', function ($timeout) {

} ] );

In your example a minifier would change the $timeout variable to something as 'a' which will result in an error because 'a' is not registered and thereby not known by Angular's dependency injection system.

In my example if a minifier would change $timeout it doesn't matter at all since a minifier doesn't change string values. A rule of thumb, always ngminify your code before jsminify.

And as last I would add a default value for the timeout function:

 $timeout(function () {
     $element.removeClass("ng-hide");
 }, attrs.showWithDelay || 5000);

This way if someone does not provide an attribute it will take the default value of 5000.


JavaScript Tic Tac Toe - functional but could be DRYer

The most un-DRY code is checking if someone has won. You could add the following code to help you

// Row class

var Row = function{
    this.cells = [];
}

Row.prototype.allSame = function(){
    return this.cells[0] == this.cells[1] && this.cells[0] == this.cells[1];
}

// Expand the Board class
Board.prototype.getRow = function(num) {
    var row = new Row();

    // Horizontal rows
    if (num => 0 && num < 3) {
        row.cells.push(this.cells[3*num], this.cells[(3*num)+1], this.cells[(3*num)+2]);
        return row;
    }

    // Vertical rows
    if (num => 3 && num < 6) {
        row.cells.push(this.cells[num-3], this.cells[(num], this.cells[num+3]);
        return row;
    }

    // Diagonal rows
    if (num == 6) {
        row.cells.push(this.cells[0], this.cells[(5], this.cells[8]);
        return row;
    }

    if (num == 7) {
        row.cells.push(this.cells[2], this.cells[(5], this.cells[6]);
        return row;
    }

    return false;

};

Running on the assumtions that the first 3 horizontal rows are numbered from 0 to 2, the 3 columns from 3 to 5 and the 2 diagonals 6 and 7...

But I am not sure the amount of code you would save would be enough to make the change worth it, unless you are predicting adding bigger boards, beyond the classic 3x3.


Optimizing lighting algorithm for a voxel engine

Building upon Vadym's answer - the vector instantiation could probably be eliminated entirely.  It appears they are only created to provide nice groupings of x,y,z coordinates, as they are only ever referenced by pulling the x,y,z values back out.

If performance is really an issue, I would suggest keeping individual values instead for xincreasing, xdecreasing, etc. and using those.  It saves on the object creation and the property getter calls. (edit: and, to me at least, it is a little easier to read)

public void DoLight(int x, int y, int z, float light)
{
    int xDecreasing = x - 1;
    int xIncreasing = x + 1;
    int yDecreasing = y - 1;
    int yIncreasing = y + 1;
    int zDecreasing = z - 1;
    int zIncreasing = z + 1;

    if (light > 0)
    {
        light--;

        world.SetLight(x, y, z, (int)light);
        Blocks.Add(new Vector3(x, y, z));

        if (world.GetLight(x, yDecreasing, z) < light &&
            !world.GetBlock(x, yDecreasing, z).IsSolid &&
            world.InBounds(x, yDecreasing, z))
            DoLight(x, yDecreasing, z, light);
        if (world.GetLight(x, yIncreasing, z) < light &&
            !world.GetBlock(x, yIncreasing, z).IsSolid &&
            world.InBounds(x, yIncreasing, z))
            DoLight(x, yIncreasing, z, light);
        if (world.GetLight(xDecreasing, y, z) < light &&
            !world.GetBlock(xDecreasing, y, z).IsSolid &&
            world.InBounds(xDecreasing, y, z))
            DoLight(xDecreasing, y, z, light);
        if (world.GetLight(xIncreasing, y, z) < light &&
            !world.GetBlock(xIncreasing, y, z).IsSolid &&
            world.InBounds(xIncreasing, y, z))
            DoLight(xIncreasing, y, z, light);
        if (world.GetLight(x, y, zDecreasing) < light &&
            !world.GetBlock(x, y, zDecreasing).IsSolid &&
            world.InBounds(x, y, zDecreasing))
            DoLight(x, y, zDecreasing, light);
        if (world.GetLight(x, y, zIncreasing) < light &&
            !world.GetBlock(x, y, zIncreasing).IsSolid &&
            world.InBounds(x, y, zIncreasing))
            DoLight(x, y, zIncreasing, light);
    }
}


Besides what has already been mentioned:

public class HashMapImpl {
class HashMap {

What's going on here? HashMapImpl is never used. This should be just simply

public class HashMap {

Don't have your unit tests inside of the class implementation, unit tests should be in a separate folder structure that mirrors your source structure. That way you don't have to ship the unit tests with your program when you release it.

Also this:

public int hashCodeNew(int h) {

should be private.

You're implementing the hash chaining as a linked list which has poor cache locality. You might just be better of having:

List<List<HashEntry>> table = new ArrayList<>(SIZE_OF_TABLE);

and let HashEntry be like this:

private class HashEntry {
    int key;
    int value;

    public HashEntry(int key, int value) {
        this.key = key;
        this.value = value;
    }
}

As HashEntry is a class only used inside of HashMap it should be private or protected. Because it is protected or private and it is pretty much just a POD structure, you can just drop the accessors (get/set methods) which you weren't using anyway. Rule of thumb, if you're not using some parts of the code, remove them. The less code you have the better (generally). But don't skimp on the white space :)

Your put method allows the use of -1 as a key, but your get method uses -1 to signal a 'no such element' condition. Either you need to throw a IllegalArgumentException from put or you need to use another way of signaling that there is no such entry. I would suggest throwing NoSuchElementException like this:

public int get(int key) {
    int index = hashCodeNew(key);
    if (table[index] == null || table[index].size() < 1) {
        throw new NoSuchElementException("The key: \"" + key + "\" was not found in the map!");
    } else {
        ...
        throw new NoSuchElementException("The key: \"" + key + "\" was not found in the map!");
    }
}

While we're at it, notice that we have two code paths that result in the same action if an error occurs, we can rearrange this as:

public int get(int key) {
    int index = hashCodeNew(key);
    if (table[index] != null) {
        for(HashEntry entry : table[index]){
            if(entry.key == key)
                return entry.value;
        }
    }
    throw new NoSuchElementException("The key: \"" + key + "\" was not found in the map!");
}

Lets take a look at the put() function too, adapting it to use a list of lists for chaining and cleaning it up a bit:

public void put(int key, int value) {
    int index = hashCodeNew(key);
    HashEntry hashEntry = new HashEntry(key, value);

    List<HashEntry> chain = table[index];
    if (chain == null) {
        chain = new ArrayList<>();
    }

    Iterator<HashEntry> it = chain.iterator();
    while(it.hasNext()){
        if(it.next().key == key){
            it.remove();
            break;
        }
    }
    chain.add(hashEntry);
    table[index] = chain;
}

Your hash function is terrible:

public int hashCodeNew(int h) {
    h ^= (h >>> 20) ^ (h >>> 12);
    int hashCode = h ^ (h >>> 7) ^ (h >>> 4);
    return hashCode % SIZE_OF_TABLE;
}

It doesn't handle negative h. You're basically just changing the lower bits, the high bits are mostly unaffected. A better (and faster) way is to simply multiply by a sizeable prime number:

private final static long BIG_PRIME = 175365371; // The 9,786,124th prime number
public int hashCodeNew(int h) {
    if( h >= 0) // 0 always at index 0.
        return (int)(h*BIG_PRIME) % SIZE_OF_TABLE;
    else
        return (int)((Integer.MAX_VALUE-h)*BIG_PRIME) % SIZE_OF_TABLE;
}

Why big? So that more of the available bits in the hash code are used.

Edit I just looked up Integer.hashCode() and it returns the value of the integer. Which implies that that is good enough as a hash and why wouldn't it be? All hash values are unique and evenly distributed. So:

public int hashCodeNew(int h) {
    return (h & Integer.MAX_VALUE) % hash.size();
}

First use Integer.MAX_VALUE which is \$2^{31} -1\$ to mask out the sign bit and then just take modulo. Handles negative values without a branch and is as fast as it gets without assuming the hash table is a power of two size (in which case simply use h & (hash.size() - 1)).


type=str is already the default way argparse interprets things. However, for all the options where the only further processing is calling os.path.abspath, you can use that as type. This affects args.psexec and args.testExecute.

Python has an official style-guide, PEP8, which you should follow. It recommends using lower_case_with_underscore for variables and functions.

Comments should explain why the code is the way it is, not what it does. The latter should be obvious from the code itself, the name of the function you encapsulated it in and/or the docstring accompanying that function.

In that vein, you could use more functions. One candidate for this would be get_args, where you can stick all your argparse code. Also the two subprocess calls could be in their own function, since they are completely independent. The least you should do is wrap your code in if __name__ == "__main__":. This way you can import this module from another script without running the tests.

In Python, ; serve (almost) no purpose, even though they are not a SyntaxError. You definitely do not need them at the end of normal lines.

Final code:

import sys
import argparse
import subprocess
import os.path

def get_args():
    parser = argparse.ArgumentParser()
    parser.add_argument("-s", "--projectSuite", help=".pjs that you want to execute", type=os.path.abspath)
    parser.add_argument("-p", "--project", help=".pj that you want to execute", default="ExploratoryTests")
    parser.add_argument("-l", "--logDirectory", help="The directory to log the mht's too", default="C:\TestLogs", type=os.path.join)
    parser.add_argument("-lf", "--logFile", help="The name of the log", default="TextExecuteLog.mht")
    parser.add_argument("-ps", "--psexec", help="Absolute path to psexec", type=os.path.abspath, default="c:\Program Files (x86)\PsTools\PsExec.exe")
    parser.add_argument("-te", "--testExecute", help="Absolute path to testExecute", type=os.path.abspath, default=r"C:\Program Files (x86)\SmartBear\TestExecute 9\Bin\TestExecute.exe")
    parser.add_argument("-m", "--mhtExtractor", help="Absolute path to an mht extractor", type=os.path.abspath, default="C:\extract2htm\extractMHT.exe")
    args = parser.parse_args()
    args.project = "/p:" + args.project
    args.logFile = os.path.abspath(os.path.join(args.logDirectory, args.logFile))
    args.log = "/exportlog:" + logFile
    return args

def execute_test(psexec, testExecute, suite, project, log, **kwargs):
    """Execute testExecute through psexec"""
    command = [psexec,"-i" ,"-accepteula", testExecute, suite, project, "/exit", log, "/r", "/DoNotShowLog", "/SilentMode"]

    #Blank array to hold the TE outcome
    testExecuteError = []
    try:
        result = subprocess.run(args=command, stderr=subprocess.STDOUT, shell=True)
        result.check_returncode()
        testExecuteError.append(result)
    except subprocess.CalledProcessError as e:
        testExecuteError.append(e)
        print("Error code: {0.returncode}".format(e))
    return testExecuteError

def execute_extractor(mhtExtractor, logDirectory, logFile, **kwargs):
    """Descriptive docstring"""
    command = [extractor, logFile, logDirectory]
    try:
        result = subprocess.run(args=command, stderr=subprocess.STDOUT, shell=True)
        result.check_returncode()
    except subprocess.CalledProcessError as e:
        print("Error code: {.returncode}".format(e))

if __name__ == "__main__":
    args = vars(get_args())
    testExecuteError = execute_test(**args)
    execute_extractor(**args)

    sys.exit(testExecuteError[0])

Note that I made all parameters part of the argparse.Namespace to have it all in one place. Afterwards I used vars to convert it to a dict. Then I used keyword arguments to make the function calls easier but at the same time giving the parameters of the function names.

To see how the dictionary unpacking works, have a look at this example:

def f(x, y, **kwargs):
    print x, y, kwargs

def g(x, z, **kwargs):
    print x, z, kwargs

args = {'x': 1, 'y': 2, 'z': 3}
f(**args)
# 1 2 {'z': 3}
f(1, 2)
# 1 2 {}
g(**args)
# 1 3 {'y': 2}

kwargs swallows all left-over keyword arguments and allows in my code above the passing in of the whole args dict, even though both functions need only some of its content.


While I can't answer to your main question about crawling multiple pages at once,
I have a couple of comments about this code.

get_page_links

In this code, it seems that base_url and the if that follows should be outside of the for loop:

links = []
for link in page[1].find_all('a'):
    url = link.get('href')
    base_url = (uparse(page[0]).scheme + '://' + uparse(page[0]).netloc)
    if base_url not in robot_set:
        robot_set.add(base_url)
        rp.set_url(urlparse.urljoin(base_url,'robots.txt'))
        rp.read()
    abs_url = urlparse.urljoin(base_url, url)
    if rp.can_fetch('*', abs_url):
        links.append(abs_url)
links = set(links)

Also, instead of links = set(links),
it would be better to make links a set in the first place.

Rewriting the above code like this would be more efficient:

links = set()
base_url = uparse(page[0]).scheme + '://' + uparse(page[0]).netloc
if base_url not in robot_set:
    robot_set.add(base_url)
    rp.set_url(urlparse.urljoin(base_url, 'robots.txt'))
    rp.read()
for link in page[1].find_all('a'):
    url = link.get('href')
    abs_url = urlparse.urljoin(base_url, url)
    if rp.can_fetch('*', abs_url):
        links.add(abs_url)

get_page

It would be better to specify in the except clause the type of error that can be raised.

get_page_words

This expression contains the non-ascii character «,
and the posted code doesn't contain an encoding declaration:

to_remove = '{}[]«#$%^&*._,1234567890+=<>/\()":;!?'

You can declare utf-8 encoding by adding this line at the top of the file:

# coding=utf-8

to_remove = '{}[]«#$%^&*._,1234567890+=<>/\()":;!?'
for i in to_remove:
    words = words.replace(i, ' ').lower()

One problem here is that words is lowercased in every iteration,
when it would be enough to lowercase just once, before starting the loop.

But the bigger problem is the loop itself:
it would be much more efficient to use a regular expression to get rid of all the unwanted characters, for example like this:

import re
re_nonword = re.compile(r'[{}[\]«#$%^&*._,1234567890+=<>/\()":;!?]')
words = re_nonword.sub(' ', words)

set literals

Instead of this:

bad_words = set(['http','www','com','https','gov','org','edu','imgs'])

As of Python 2.7, you can write using set literals, like this:

bad_words = {'http', 'www', 'com', 'https', 'gov', 'org', 'edu', 'imgs'}

I also put spaces after the commas, to follow PEP8.


Lua OOP and classically-styled prototypal inheritance

Example Usage

The example that you provide does not really help to explain how the code should work. I trust that it does work, but the variables don't exactly match up.

For example, this code here:

B.request_handlers = {
  GetHandler:new(),
  URLEncodedPostHandler:new(),
  MultipartPostHandler:new(),
}

What is B.request_handlers before this? You define the "class" like so in the former code block: B.RequestHandler = B.Object:extend() but now it is lower case and has an underscore.  The same for the three calls inside those brackets.  What does GetHandler call? I don't see a matching method anywhere.  It's all just a little confusing.

Lack of Comments

The first code block is the most important one, because that is where you are establishing the actual Lua inheritance.  The problem with the code it that it is very dense, and there is no explanation whatsoever about how it is expected to be used.

For example, you define the extend method like this:

extend = function(this, that, meta)
  if not that then that = {} end
  if not meta then meta = {} end
  meta.__index = this
  setmetatable(that, meta)
  return that
end,

But then in your example you call it like this:

B.RequestHandler = B.Object:extend()

After reading through the method, I can understand that if extend() is called without any arguments, the arguments will be filled in inside the method. I think that it would be very helpful to have a comment explaining the valid ways of calling the extend method. In most languages, when a method has arguments they must be supplied or the compiler will complain.  Since Lua is a dynamic language, you won't get this checking, and so I believe the comments explaining how to use your OOP implementation become even more important.

This, That, and the other

This code is very hard to understand:

isa = function(this, that)
  for meta in function() return getmetatable(this) end do
    if this == that then return true end
    this = meta.__index
  end
  return this == that
end,

new = function(this, ...)
  local that = this:extend()
  that:constructor(...)
  return that
end,

All of the this, that, then, this == that, return that, etc etc just twists my brain up into a knot.  If I read the code very carefully line by line, I can eventually puzzle out what it does, but I think that this is a failure of the code.

It's true, you probably do not intend anyone to read your inheritance code.  Rather, once you have set it up, it will just work and allow someone using the code to extend objects in a classical OOP way.  However, I still think that by using better variable names, some comments, and a few more lines of code, it could be written in such a way that it could be understood immediately when reading it.

Also consider that someone who is not a native English speaker may struggle with the difference between this and that, and the then keyword does not help the situation.

Something Good

I like this code here:

-- override these
function B.RequestHandler:accept() error "not implemented" end
function B.RequestHandler:process() error "not implemented" end

The comment makes it very clear that these are the methods that should be overridden, and the string that will be returned if they are not will make it very clear what has gone wrong.


I would turn around the logic. The design you have has a lot of the general logic in the main program, and a lot of game-specific logic in the questions module. A better design for reusability would be to put the logic in a module, and the game-specific stuff (basically just the questions and perhaps some very superficial user interface) in the main program.

I would also define a separate class for the collection of questions; so maybe the main game might look like

from Quiz_Questions import QuestionsCollection, Question, PlayerState, QuestionsExhausted

collection = QuestionsCollection()

collection.add('Russia has a larger surface area than Pluto.', 'True', 7)
collection.add('There are as many stars in space than there are grains of sand on every beach in the world.', 'False', 5)
collection.add('For every human on Earth there are 1.6 million ants.', 'True', 8)
collection.add('On Jupiter and Saturn it rains diamonds.', 'True', 9)
collection.add('Scotland’s national animal is the phoenix.', 'False', 4)
collection.add('A banana is a berry.', 'True', 8),
collection.add('An octopus has three hearts.', 'True', 4)
collection.add('There are 10 times more bacteria in your body than actual body cells.', 'True', 9)
collection.add('Rhode Island is the closest US state to Africa.', 'False', 3)
collection.add('Shakespeare made up the name “Sarah” for his play Merchant of Venice.', 'False', 3)

def game():
    print('-' * 20)
    player = PlayerState()
    questions = collection.game()
    while True:
        try:
            question = questions.next()
        except QuestionsExhausted:
            break
        print(question.challenge())
        user_answer = input('Insert answer (Yes or No): ')
        response = player.assess(question, user_answer)
        print(response)
        print('-' * 20)
    print('Congratulations! You have answered all the questions!')
    print('Your final score is {} points'.format(player.score()))
    print('You answered {} questions right'.format(player.right_answers()))
    print('And {} questions wrong.'.format(player.wrong_answers()))

game()

This is just a rough sketch, but hopefully hints at a different design where the classes encapsulate the behavior specific to each, with enough freedom to reassemble things in a different arrangement (multi-player game? Add timeouts?) and also with the user-facing I/O and message strings (mostly) separate from the library module.

This doesn't handle invalid input very elegantly but I'm thinking the way I started this, the Player class would be responsible for some I/O as well, so maybe actually refactor to

...
while True:
    try:
        question = questions.next()
    except QuestionsExhausted:
        break
    player.handle(question)
print('Congratulations! You have answered all the questions!')
...


Removing trailing whitespace from input lines

What you've got is very "old-fashioned-looking" C code, which is only natural for someone working through K&R. :) For example, declaring main() without a return type (that is, with the implicit int return type) is frowned upon these days.

Declaring all your variables — uninitialized! — at the top of your function is also frowned upon, in every language, because uninitialized variables in general are bad. Because uninitialized variables mean they have to be initialized later, which means assignment, which means mutation, which means more stuff for me to keep track of in my head. It's better to delay defining the variable at all until you know what's going to be put in it.

For example:

int len;
int i;
char line[MAXLINE];

while ((len = getli(line, MAXLINE)) > 0) {
    i = len-1;

could definitely be written as

int len;
char line[MAXLINE];
while ((len = getli(line, MAXLINE)) > 0) {
    int i = len-1;

And then personally I'd avoid side effects inside the condition of a while loop. It's very K&R, but it's frequently not very easy to read!

while (true) {
    char line[MAXLINE];
    int len = getli(line, MAXLINE);
    if (len <= 0) break;
    int i = len-1;
    while (line[i] == '\n' || line[i] == '\t' || line[i] == ' ') {
        line[i] = '\0';
        i--;
    }

Now we can flatten that whole business with i into a simple for loop:

while (true) {
    char line[MAXLINE];
    int len = getli(line, MAXLINE);
    if (len <= 0) break;
    for (int i = len-1; (line[i] == '\n' || line[i] == '\t' || line[i] == ' '); --i) {
        line[i] = '\0';
    }

At this point I wonder whether it is in your function's contract that it zeroes out every char in the buffer after the last nonspace character, or if you could avoid writing a lot of those zeroes. One way to do that would be to re-expand the loop:

    int i = len-1;
    while (line[i] == '\n' || line[i] == '\t' || line[i] == ' ') {
        --i;
    }
    line[i+1] = '\0';

Another way would be to factor that code out into a helper function... or just use one of the many standard library functions for manipulating char buffers! Unfortunately, the function you're looking for in this case is spelled strrspn and is actually not standard, even though strspn is.

    *strrspn(line, "\n\t ") = '\0';

(I also notice that Oracle's documentation for strrspn contradicts itself. I assume that on failure it returns a pointer to the end of the string, as shown in their example code, not a pointer to the beginning of the string, as written in their prose description.)

Anyway, writing strrspn from scratch would be a good K&R-style exercise for you!

(len = getli(line, MAXLINE)) looks like a stutter. I don't know why you wouldn't write out the whole word getline... unless you're avoiding the library function, in which case something like read_one_line or get_a_line would be reasonable names to choose.

Don't write MAXLINE on that line, either. Don't give yourself a chance to misspell it or cut-and-paste-error your way into a bug. If you're calling getli with the buffer line, then the second argument should always be the size of the buffer — that is, sizeof line. Sure, that happens to be MAXLINE, just like MAXLINE happens to be 1000; but you should avoid repeating the word MAXLINE for the same reason you should avoid repeating the word 1000. If you call getli(line, sizeof line), you can see right at the callsite — without inspecting any other code — that the call is correct. It is correct by design.

Your getli function has very convoluted control flow. You have a side-effect in the condition again; and then you test for '\n' both inside and outside the loop.
Here's an exercise: Rewrite the code using no loops except while (true). Then see how much clearer you can make the code while preserving that property.

Here's the first pass:

int getli(char s[], int lim) {
    int i = 0;
    int c;
    while (true) {
        if (i >= lim - 1) break;
        c = getchar();
        if (c == EOF) break;
        if (c == '\n') break;
        s[i] = c;
        ++i;
    }
    if (c == '\n') {
        s[i] = c;
        ++i;
    }
    s[i] = '\0';
    return i;
}

Do you spot an opportunity to combine codepaths now? Keep going; you'll find that the code ends up shorter than it started out!


Calling a method only once in an updateScore function in SpriteKit

First,  use properties,  they are your best friend

var rotateToggle = false
{
    didSet
    {
        if(rotateToggle)
        {
            GameEffects.rotateScreenAndHold(worldNode, finishAngle: -0.06, totalEffectDuration: 5.0)
        }
        else
        {
            //if you need something on false, do it here
        }
    }
}

do this for all of your other toggles.

Update for score property:

var score : UInt = 0
{
    didSet
    {
        updateScore()
    }
}

Now we have your case statement, chain them together

switch (score)
{
    case 0:
      levelToggle = true
    case 1:
      levelToggle = false
    case 10,16,25:
      rotateToggle = true
//...etc
}

of course, how you have toggle set up, this requires score increases by 1, and updateScore gets fired afterwards before score can increase by 1 again.