Collect subsequences of consecutive integers

@Peter Taylor has a good idea, but there is more to be improved

You should add some tests

Python is often described as Batteries included
This is a perfect example to use itertools.groupby()

from itertools import groupby
import doctest

def extract_seq(seq):
    """
    Splits sequence into consecutive lists

    args:
        seq (list): A sorted sequence

    >>> extract_seq([1,2, 4,5, 8,9])
    [[1, 2], [4, 5], [8, 9]]

    >>> extract_seq([1,2,3])
    [[1, 2, 3]]

    >>> extract_seq([1,2,3,7,8,9])
    [[1, 2, 3], [7, 8, 9]]

    >>> extract_seq([1,2,3,10])
    [[1, 2, 3], [10]]
    """
    return [
        [x for _, x in g]
        for k, g in groupby(
            enumerate(seq),
            lambda i_x : i_x[0] - i_x[1]
        )
    ]

if __name__ == "__main__":
    doctest.testmod()


Here is a list of found issues, in order of occurrence:

importing from your custom can_convert is fine. You should stick to modularization like that for bigger projects. However, here, it actually seems to be in the way of things. It checks if the string can be converted, but does not actually do any conversion. We later have to do the conversion again anyway. This can be combined.
Currently, the welcome message and actual random range can deviate. You already know what f-strings are, so they should be used to keep that in line.
Use randint to include both bounds, as opposed to randrange, which would stop one short of upper_bound, as you already noted.
input will already return a string, no need to convert via str.
By inverting the logic in checking for "pass", a whole block of indentation can be spared. Just check if user_inp == "pass" and break if True. All following code can be one level less indented, since no else statement is required.
Now for the main logic. The try block has moved here, away from the import. Notice that the logic is still the same. However, we assign to number immediately without checking first. If this fails, the except block is triggered.

Note that int("2.2") will also be a ValueError despite "2.2", to humans, clearly being a number. So the error message should specify whole numbers to be more precise.
The added else block is run if no exception was raised, so in the normal game.
number is now available, so no need to call int conversion repeatedly.

Keep try blocks short and concise and never leave an except statement bare. You did this fine! Lastly, try to not nest try/except blocks.
There was a typo for attempts.

import random

upper_bound = 100
lower_bound = 1

welcome_msg = f"""Welcome to GuessTheNumber Game!
I'm thinking of a number between {lower_bound} and {upper_bound}.
Can you guess this number?

[?] Type 'pass' to exit game.
"""
print(welcome_msg)

number_to_guess = random.randint(lower_bound, upper_bound)
attempts = 0

while True:
    user_inp = input("Your choice: ")
    if user_inp == "pass":
        break
    try:
        number = int(user_inp)
    except ValueError:
        print('Enter whole numbers only!')
    else:  # no exception occurred
        attempts += 1
        if number < number_to_guess:
            print("Your guess is too small.")
        elif number > number_to_guess:
            print("Your guess is too high.")
        else:
            print("Correct! You won the game!")
            print(f"Attempts: {attempts}")
            break


The string looks like a filename, so you should use the library functions for handling file paths instead of rolling your own.

File.basename(str, '.txt').split('_')


Decorators usually add more functional behaviors so if you had a method drive and TurboClass has a new functionality driveFast, you could probably call the Cars driveFast and then the some other special logic which turboClass has to do eg. checkTyres after driving fast. Here since you want the object construction to do it, this is the right way.


Counting significant figures in a number

num_list is better named digits; don't describe the type, describe the value.

test's output is unhelpful; the result should be much more obvious, especially when it fails. Keeping along the given lines, I might do something like

def test():
    def should_be(string, count):
        assert count_sig_figs(string) == count
        print("{!r} has {} sf.".format(string, count))

    should_be('1', 1)
    should_be('10', 1)
    should_be('100', 1)
    should_be('1004', 4)
    should_be('10004', 5)
    should_be('105', 3)
    should_be('01', 1)
    should_be('1.2035', 5)
    should_be('001.09508', 6)
    should_be('0.00110', 3)

Of course, a proper test suite is probably better.

This isn't idiomatic:

for index in range(len(digits)):
    try:
        fig = int(digits[index])
        if fig != 0:
            sig_fig_count +=1
        elif check_zero_sig(index, digits, sig_fig_count):
            sig_fig_count += 1
    except:
        continue
return sig_fig_count

Bare try...excepts are bad. Even

except (ValueError, IndexError):

is bad. Primarily this is because you're spreading out the checks where they don't belong and it should instead be

try:
    fig = int(digits[index])
except ValueError:
    continue

if fig != 0:
    sig_fig_count +=1
else:
    try:
        zero_sig = check_zero_sig(index, digits, sig_fig_count)
    except IndexError:
        continue
    if zero_sig:
        sig_fig_count += 1

But then because... why is check_zero_sig throwing an IndexError? Surely that's a bug.

Before we get there, quickly change this to

for index, digit in enumerate(digits):
    if digit == ".":
        continue
    if digit != '0':
        sig_fig_count += 1
    else:
        sig_fig_count += check_zero_sig(index, digits, sig_fig_count)

That's pretty much what we want expressed more simply.

check_zero_sig's except also needs updating:

try:
    decimal = digits.index('.')
except ValueError:
    ...
else:
    return index > decimal and sig_fig_count > 0

I don't get why you don't just mutate index instead of using new_index.

This can be simplified:

if digits[index] == '.' and sig_fig_count > 0:
    return True
elif digits[index] == '.' and sig_fig_count == 0:
    return False

to

if digits[index] == '.':
    return bool(sig_fig_count)

The

elif digits[index] != '.' and sig_fig_count > 0:

and

elif digits[index] != '.' and sig_fig_count == 0:

branches are the same.

This then simplifies to

if digits[index] == '.':
    return bool(sig_fig_count)
elif digits[index] != '.':
    digit = int(digits[index])
    return bool(digit) or check_zero_sig(index, digits, sig_fig_count)
else:
    return False

which is just

if digits[index] == '.':
    return bool(sig_fig_count)
else:
    digit = int(digits[index])
    return bool(digit) or check_zero_sig(index, digits, sig_fig_count)

Then we see

elif index == len(digits):
    return False

cannot fire until after index += 1. This lets us fix our IndexError.

Back to count_sig_figs. The documentation is not written idiomatically. Documentation should be to the point, direct and not talk in third person.

def count_sig_figs(digits):
    '''Return the number of significant figures of the input digit string'''
    sig_fig_count = 0
    for index, digit in enumerate(digits):
        if digit == ".":
            continue
        if digit != '0':
            sig_fig_count += 1
        else:
            sig_fig_count += check_zero_sig(index, digits, sig_fig_count)
    return sig_fig_count

Back to check_zero_sig, which is now

def check_zero_sig(index, digits, sig_fig_count):
    '''Checks for significance in a zero from a list'''
    try:
        decimal = digits.index('.')
    except ValueError:
        if index == 0:
            return False
        index += 1
        if index == len(digits):
            return False
        if digits[index] == '.':
            return bool(sig_fig_count)
        else:
            digit = int(digits[index])
            return bool(digit) or check_zero_sig(index, digits, sig_fig_count)
    else:
        return index > decimal and sig_fig_count > 0

The if digits[index] == '.' check can't fire; we're in the except. Then

digit = int(digits[index])
return bool(digit) or check_zero_sig(index, digits, sig_fig_count)

can be

return digit != '0' or check_zero_sig(index, digits, sig_fig_count)

and the recursion, which would end up re-calling .index, can just be

if index == 0:
    return False
return any(digit != '0' for digit in digits[index+1:])

index == 0 is a very strange special case; you deal with 01 but not 001 or 00001. I suggest removing the special-case and using sig_fig_count.

Now we have

def check_zero_sig(index, digits, sig_fig_count):
    '''
    Returns if a zero digit at a given position is significant,
    given a count of significant digits preceding it.
    '''
    if not sig_fig_count:
        return False
    try:
        decimal = digits.index('.')
    except ValueError:
        return any(digit != '0' for digit in digits[index+1:])
    else:
        return index > decimal

Most of this complexity is avoidable by using string operations and splitting cases:

def count_sig_figs(digits):
    '''Return the number of significant figures of the input digit string'''

    integral, _, fractional = digits.partition(".")

    if fractional:
        return len((integral + fractional).lstrip('0'))
    else:
        return len(integral.strip('0'))

That's the entirety of the logic!

Another test case you should consider is 100.. Does that have 1 significant figure or 3?


Spreading out model across multiple classes a good idea?

Model classes

Model classes should not have any hard dependency in their constructors especially not a PDO or similar stuff. Model can not have any business rule they are containing only data and that's all. Ofcourse if you have some mapping between model classes (Author <--> Books) then some of the relation informations have to be in the model (Mediator!) classes but that's all.

Model classes should not have any knowledge about their storage type. It can be anything not just az SQL database handled with PDO. They can not have self managed CRUD methods (if they does then they become Active Record(s) which is a really bad thing (~God object), antipattern) (maybe factory methods are okay but these methods also can not have anything heavy (query database through a horrible static global accessible thing) inside).

CRUD

Mapped entities should have some kind of top level storage (like in .NET an ObjectContext) which is dealing the storage problem; handling SQL connections and providing transaction capabilities with an Unit of Work (Commit, Rollback) solution. The top level storage can have for example mapped SQL tables for querying from the underlaying storage, inserting into it, deleting from it. Other thing is that this top level storage is the responsible to track the changes in the entities. Of course we have to put some stuff in every model in the system (Mediator #2!) to handle this session but the main logic still in a state manager not in the models.

Repository

Repositories are tricky. With powerful ORM systems we can forget them if the business logic doesn't require another application level. What should a repository contain? Add, Delete, Query methods but no Save() or anything else (not even named query methods like: GetUserByEmailAddress($email)). You can use DAO classes for named queries which are operating one or more repository or on the entire object tree. Do not forget: not every entity has a repository! Only aggregate roots can have repository, for example: an Order model can have a repo but an OrderDetail can not becouse it belongs to an Order model and can not exists whithout it.

Static

In your example your are using in static context object which are containing state (your repository stuff) information which is bad. You will introduce a global state whith them which will bring more harm to your code then good (global hyperspace is inreliable, anyone can modify it).


Is there a better to do this (infer the alignment of double)?

Curious as to why size_t type is used.  The first choice should be ptrdiff_t.

  When two pointers are subtracted, ... and its type (a signed integer type) is ptrdiff_t defined in the <stddef.h> header.  C11dr §6.5.6 9

Since alignment is certainly something less than 127, any integer type will do for our purposes.  Could use int, unsigned, size_t.

The cast of &(lSpecimen.mWedge) is redundant other than to remove the const

My GCC 4.5.3 compiler says the const on the return type is ignored.

int getDoubleAlignment(void) {
  const DoubleAligned lSpecimen;
  const int lOffset = (char*)&(lSpecimen.mMember) - &(lSpecimen.mWedge);
  return lOffset;
}

Under what (reasonable) circumstances could this not work?  (Nothing to add)
Under what (plausible) circumstances could this not work? (Nothing to add)

What is the actual or practical complete list of types I would need to do this with to determine universal alignment?

Rather than use double, use max_align_t if available.  max_align_t is not in C99.

  "... max_align_t which is an object type whose alignment is as great as is supported by the implementation in all contexts ..."  C11dr §7.19 2

Otherwise instead of double use a union of all non-narrow types:

union {
  int i;
  long l;
  long long ll;
  float f;
  double d;
  long double ld;
  float _Complex fc;
  double _Complex dc;
  ptrdiff_t pd;
  // Suspect the below are the only 4(5) code may truly need
  long double _Complex ldc;
  void *p;
  void (*pf)();
  intmax_t im;
  // C11
  max_align_t ma;
} OneOfEach;

void* might be narrower than void (*pf)() so void* will not work for all pointers.


Java has a couple of constants available that are commonly used in situations like this:

    int max = Integer.MIN_VALUE;
    int min = Integer.MAX_VALUE;

Using these two values ensures you cover the possible spectrum of valid input values.

In addition, a sadistic user would enter 5 large values, and cause your total value to overflow, creating a bad result. You should declare your total as a long value:

    long total = 0;

That will prevent overflow (unless you have 2 billion input values.... ;-)

Finally, the last statement is a problem, it does integer division:

        average = total / num.length;

It should convert one of the values to a double, so that floating-point division happens instead:

        average = total / (double)num.length;


This answer is going to talk about your style, and best practices. Some of it may be subjective. I'll go through the code line by line.

use strict;
use warnings;

Good job! Modern Perl programs should always start with strict and warnings. If you want more detailed information on what's wrong, you can add use diagnostics, but remember to remove it before you ship, because it is really heavy and just a debugging aid. You can also turn on use warnings 'all'.

my ($hostname, $username, $password, $port, $command) = ("bandit.labs.overthewire.org", "bandit0", "bandit0", "2220", "");

As you can see from the scrollbar, this line is long. For two or three variables, assigning them as a list is fine, but in this case, I would break them into five statements. It's way clearer, especially since $username and $password have the same value.

In addition, you do not need to put numbers into quotes, unless you need a leading zero (i.e. for a ZIP, ISBN or EAN code).

Some people prefer single quotes '' when there is no interpolation, but that really doesn't matter and is a question of personal preference. For the empty string, I prefer to use q{}, as that clear shows the intention of an empty string, over a forgotten string or a typo that made it to production, because of the pair delimiters.

my $hostname = 'bandit.labs.overthewire.org';
my $username = 'bandit0';
my $password = 'bandit0';
my $port     = 2220;
my $command  = q{};

$user =~ s/(\d+)\z/ $1 + 1 /e;

This line is fine, but I would like to point out the /x modifier, that allows you to use comments on the pattern part of the substitution. You can also change the delimiter, so this could (but in this case should probably not) become:

$user =~ s{
    (\d+) # grab lots of numbers
    \z    # at the end of the line
}{ $1 + 1 }ex;

my ($pass, $user, $c) = @_;

What is $c? I guess it is the command, but it's not as nicely verbose as your other variables. I do a lot of work with the Catalyst framework, and to me, $c is hardwired to the Catalyst context object. One-letter variable names are seldom the right choice. Always go for clarity.

my $ssh = Net::SSH::Perl->new("$hostname", port=>$port, debug=>0);

You do not need to quote variable names. New Perls will probably optimize this away, but it's ugly and might introduce bugs. You've done that a couple of times (though not consistently), and I'm not going to mention it again.

Also, your spacing here is inconsistent. You are using spaces around the equal (=), so why not do the same around the fat comma (=>)?

my $ssh = Net::SSH::Perl->new($hostname, port => $port, debug => 0);

my ($stdout, $stderr, $exit) = $ssh->cmd("$c");
my $new_pass = $stdout;
return $new_pass;

You only need the $stdout variable. The other two are discarded, so there's no reason to assign them at all. You can assign things to undef in a list context, and those parts of the list on the right hand side will be thrown away.

my (undef, $two, undef) = (1, 2, 3);

Since you only want the first value, there's no need to even do that. In list context the assignment will start with the right-most values, and all the later elements on the RHS will be discarded if there is nothing to assign them to on the LHS.

The assignment to $new_pass is not necessary because you just return that value. I know it has some kind of documentary value, but you can just name your variable like that in the first place. $stdout and $stderr only need to be named that if you need to distinguish them. But in this case, you don't care how Net::SSH::Perl got your password.

my ($new_pass) = $ssh->cmd($c);
return $new_pass;

However, you might want to check for errors somehow. Not sure what those commands return. You maybe want to keep $exit to look at it.

my %hash;

This is a really bad name. The sigil % already tells us it's a hash. This is like naming your child Child. The hash holds levels and commands. Why not name it %commands?

open CONFIG, "commands.txt" or die;
while (my $line = <CONFIG>) {
    # ...
}

This is a big one. Don't use glob filehandles. The CONFIG thing is global. It's the same everywhere in your program. Even in modules from CPAN. If you were to call some function while that CONFIG filehandle is opened, and that module were also using one with the same name, it would interfere with yours. Instead, use a lexical filehandle.

This has the added benefit that you don't have to use close explicitly in this case, as the lexical variable will go out of scope at the end of the function call anyway, and thus the filehandle will implicitly be closed.

You also should use the three-argument form of open. Your two arg without a mode implies reading, which is fine with this fixed string filename here. But what if the name comes from somewhere else? What if it's |rm -rf /? Ooops. You just opened a pipe to a command that deleted your harddrive. Bad luck. Instead, always pass the mode as the second argument, and that kind of thing cannot happen.

You do have or die, which is good, but your error will be Died at .... That's not very helpful. Instead, at least include the error that was returned by open, which is in $!. Putting the filename in that error message is helpful too.

open my $fh, '<', 'commands.txt' or die "Can't open commands.txt: $!";
while (my $line = <$fh>) { ... }

my ($lvl, $cmd) = split /\:/, $line;

The colon : is not a meta character in regex. You don't need to escape it.

 my $bandit_count = scalar(keys %hash);

You don't need the parentheses here. In fact, you hardly ever need them in Perl, which makes the code read more like English.

And there's the %hash again... Actually it's another %hash. Using the same variable name in an outer and inner scope it not the best idea, as it's ambiguous.

my $bandit_count = scalar keys %hash;

while ($bandit_count-- > 0)
{

Your style of placing the curlies is inconsistent. Above you had them on the same line as the while loop, now they are in the same line. Decide for one.

That loop could be implemented in a lot of ways. Since you don't care about the loop variable, the way you picked is not very Perlish at all. In fact, you're prone to getting an off-by-one error (or two?). The $foo-- decrement operator returns the current value of the variable, and then reduces it by one. Is that really what you need here?

It would be easier to get rid of the $bandid_count variable completely, and simply use a range with a for or foreach loop. Both are the same syntactically. Some people prefer the short for, but I think for the non-C style loop the foreach is easier to read.

foreach (1 .. keys %commands) {
     # ...
}

You can leave out the [scalar][9] here as well, as the .. range operator already puts keys into scalar context.

print "$username:$password\n";

Instead of print and the newline, you can use say, which is shorter and includes the trailing line break. It's included in Perls starting from version 5.10 and needs to be turned on with use feature 'say' or a feature bundle.

In total, this code is pretty solid. If you really just started learning, you've done a great job. But then you are probably an experienced developer already, which is worth a lot.

If you wanted to replace the whole read_file sub with something more Perlish, you could do this:

sub read_file {
    return do {
        open my $fh, '<', 'commands.txt'
            or die "Can't open commands.txt: $!";
        map { chomp; split /:/ } <$fh>;
    };
}

The code makes use of a couple of features. The do block groups a number of statements into one. It's useful to know that every block's return value is always the return value of the last statement. That's also true for subs, so if there is no explicit return, chances are something will go wrong at some point.

map is like a foreach loop, and you probably know it from other languages that implement the map and reduce pattern. In the block, the topic variable $_ is used to iterate the list that comes out of the <$fh> diamond operator, which will just return the whole file as a list of lines in list context. Both chomp and split work on $_ implicitly if no arguments are passed, which makes this really concise.

If I may suggest another resource, the book Beginning Perl by Curtis "Ovid" Poe is great for developers that want to switch to Perl, as it has a nice walk-through, but does not explain everything down to how programming works, like Learning Perl would. To get more of the new stuff that's in the language, take a look at chromatic's Modern Perl, which is available for free online. Finally, there are some nice tricks in Effective Perl by Joseph N. Hall, Joshua A. McAdams and brian d foy. Several of the authors of these books are frequent users on Stack Overflow too.

Finally, something that might make your life a bit easier as these challenges get harder is Object::Remote. It allows you to execute Perl code on remote systems via SSH, without installing any modules there. You can use everything you have locally as long as it's compatible with the Perl that's there remotely, down to 5.8. Here is a great talk about it by the author from the German Perl Workshop 2014.


To eliminate nested loop you may apply regex checking for invalid symbol from the start -- it would be O(n) + O(n) = O(n):

def balanced? string
  return false if string.length.odd?
  return false if string =~ /[^\[\]\(\)\{\}]/

  pairs = { '{' => '}', '[' => ']', '(' => ')' }

  stack = []
  string.chars do |bracket|
    if expectation = pairs[bracket]
      stack << expectation
    else
      return false unless stack.pop == bracket
    end
  end

  stack.empty?
end


As you suspected, you don't need append for this problem. The trick is to notice that if, for example, your goal is to create the list '(1 2 3), then writing (list 1 2 3) is more straight-forward and more efficient than writing (append '(1) '(2) '(3)).

With that in mind, consider the following insertion function:

(define (insert BST n)
  (cond
    ((null? BST)
     (list empty n empty))
    ((< n (cadr BST))
     (list (insert (car BST) n) (cadr BST) (caddr BST)))
    ((> n (cadr BST))
     (list (car BST) (cadr BST) (insert (caddr BST) n)))
    (else BST)))

To run a small test, suppose you have the list '((() 3 ()) 5 (() 8 (() 10 ()))), which looks as follows:

then inserting 7 to it, ie. (insert '((() 3 ()) 5 (() 8 (() 10 ()))) 7) will produce '((() 3 ()) 5 ((() 7 ()) 8 (() 10 ()))), which looks as follows:


Text-based Tic Tac Toe with DRY and PEP-8

Input validation

Input validation is not really related to Tic-Tac-Toe, but is shared among all console programs.

The code to get the correct coordinates should be moved into a function in its own file to reuse it in the future and make the tic-tac-toe script even shorter.

Names

askmove does a complete turn, so I would call it: make_turn

Refactor

The check for the winning position is very much isolated from the rest of the program. I think the program would gain a lot of readibility if you wrote a is_won function.


First off, the Include is ignored because you project the result to a new result (Select(x => new{ ... }). So you may as well remove it.

Then, there is some room for improvement by eliminating the repetitive parts. It's much easier to do this in query syntax:

var customers = from cst in _customerRepository.All() // Assuming this method exists
    let lastInvoice = cst.Invoices.OrderByDescending(t => t.InvoiceDate).FirstOrDefault()
    let upaidInvoices = cst.Invoices.Where(t => !t.IsPaid)
    select new
      CustomerListEntity {
        Name = cst.Name,
        CustomerID = cst.CustomerId,
        RecentInvoiceRef = lastInvoice.Ref ?? string.Empty,
        RecentInvoiceAmount = lastInvoice.Value,
        UnpaidInvoicesNumber = upaidInvoices.Count(),
        UnpaidInvoicesTotalAmount = upaidInvoices.Sum(k => k.Value)
    });

The let keyword defines a local variable that can be reused in the LINQ statement. As you see, this greatly improves the readability of the code.

It also slightly improves the generated SQL query, because Ref and Value will now be retrieved in one subquery instead of two. Unfortunately, this doesn't apply to Count and Sum, because aggregates require separate subqueries.

Also, note that the null checks are removed. The LINQ statement is translated into SQL and executed in the database. SQL doesn't have this null reference concept.

One last point is that there is broad consensus on the additional repository layer being totally redundant. Consider removing and query directly on the context.


Check for null in WeakReference everytime you need to use it is necessary because the GC process can clear the instance anytime during your work. Besides, onPreExecute, onPostExecute and doInBackground is run in the different thread, there is also some delay when changing thread and the instance may be cleared in between too.

I have a helper method that does the check for null in WeakReference and its instance. Use it if you like:

public <T> T getInstance(WeakReference<T> weakReference) {
    if (weakReference == null || weakReference.get() == null)
        return null;

    // Do more check for your activity if need

    return weakReference.get();
}


Injecting the Repository into the business layer

Injecting Repositories or anything else into the business layer is an overall bad idea. You business layer should be free from framework mechanisms. You will pollute your most important parts with unneccessary complexity.

What you can do is to place DI in certain objects that will initialize the dependencies around your business layer. So you may use a "RepositoryProvider" as an DI dependent artefact that will initialize a "RepositoryRegistry" which is DI independent and is used by the business layer.

Keep your your business layer clean.


Is this String concatenation creating extra String?

Enter the mad world of optimization!

The Java compiler and HotSpot just-in-time compiler are programmed with a great many rules for optimizing performance of Java code.

In general, it is NOT worth your time to fret about the performance impact of these kinds of details as they are often taken care of. Rather, just try to write clean readable code that solves the problem. If performance is an issue, profile the code and then optimize the bottlenecks.

Now, to address this specific issue, I am virtually certain that constant expressions are resolved by the Java compiler at compile time. I would be very surprised if the byte code for your two queries wasn't identical.


Prompting user for connection parameters to SQL Server

It doesn't matter what you do with this code, the ConnectionString can always be accessed in the SqlConnection.

As a test, here's what you should do:

Launch your application outside of Visual Studio. Take a new instance of Visual Studio and go to 'Debug' -> 'Attach to Process...' and then locate the process for your programme.

Once you have done this, run your programme until the SQL point, then hit the 'Pause debugging' button in Visual Studio.

Once you have done this, go to the Diagnostic Tools and take a memory snapshot. While it's paused you can view the memory dump of the snapshot.

You should see a record appear in the 'Memory Usage' tab of the diagnostic tools, click the 'Objects (Diff)' link.

A new tab will open with a lot of fun stuff, order everything by 'Object Type' and find SqlConnection.

Double click your SqlConnection line.

Click the first line (assuming you have one) in the new portion of the window, then click 'Referenced Objects' at the bottom section of the window.

It should show you a bunch of objects, find SqlConnectionString, and drill down into it.

You should see several string objects. Go ahead and hover each one, one will be your password.

Of course, the string object on the SqlConnection itself will also be a connection string and have your password in it.

Bottom line: the client computer is always compromised in your eyes. You can do no more to secure it at this point, with an instance of Visual Studio (which this can be done more effectively with a separate programme that hides on the client computer) I can view your connection string and extract it with little to no effort required. (This whole process takes ~10 seconds once you have used the 'Break All' on the programme.)

My advice to you: don't take unnecessary security precautions to ensure your work is 'safe in memory' -- there's no such thing. All memory can be dumped, and even if it's only in memory for ~0.00001ms, an attacker can still find it. Just worry about building your application, let the memory security be handled by the user. :)

On to the code review:

YesNo is a bad variable name; C# rules are camelCase for local members. It should also be named something related to what it does: windowsCredentials.
You say data sanitization isn't important, but what happens if the user enters a semicolon then another ConnectionString property? You should always validate input.
You should be using the SQLCon, not creating it like you are. (using (var connection = new SqlConnection(ConnectionString))), same with SqlCommand and SqlDataReader if/when/where you use them.

Why?

The SqlConnection type is an IDisposable - that means, it's capable of being automatically closed and deallocated.

In the case of SqlConnection, IIRC there are unmanaged objects that need disposed, cleared and cleaned up before the connection is done. There are resources it uses that are not automatically freed.

If you look at the API of SqlConnection, it has a Close() method which indicates that it will disconnect from the database and free it's resources. This is important because if you forget the Close() call, then your database connection will hang open. This has performance implications for your programme, database and network.

So, we use using (...) construct for it which is a fancy way of writing:

var connection = new SqlConnection();
try
{
    /* Inner code here */
}
finally
{
    ((IDisposable)connection).Dispose();
}

The Dispose() method of SqlConnection then handles closing and flushing all the connection information.

On to your requests:

  From a security perspective, would this code be acceptable in a SQL Importer?

As far as security goes - maybe. I'm not a security expert, but I would be wary of having no input validation here. At the very least, do not allow input with a semicolon (;), as that can totally mess with your connection string.

  Besides data sanitation, is there a best practice somewhere that I'm missing in this?

I mentioned them above, but generally speaking you're trying to solve a problem that we have accepted as a non-issue. Worry about the features of the application, then about the security. Your job as a programmer is not to secure the information in RAM, but to secure it:

On disk / file system
In the database
Over the network

Don't worry so much about the password sitting in memory. Yes, it's generally safer to keep it in memory as shortly as possible, but that goes along with Single-Responsibility Principle, the Principle of Least Astonishment, etc. Your password (and all variables) should be as short-lived as possible. (I.e. Don't do like the old C code used to with 100 variables defined at the top of the file.)

I have turned this answer into a guide on my blog for further explanation.


Using timeit, we can make changes and see how it affects the speed. Baseline:

>>> def search1(word,source):
    places = []
    for x in range(0,len(source)):
        if source[x] == word[0]:
            if source[x:x+len(word)] == word:
                places.append(x)
    return places

>>> timeit.timeit("search1(word, source)",
                  setup="from __main__ import search1, search2, search3; import re; word, source = 'foo', 'foobarbaz'*1000",
                  number=10000)
15.60436644058882

Note the following:

You calculate word[0] and len(word) on every loop, these can be factored out;
A list comprehension is generally faster than for looping and appending; and
enumerate allows you to get the index and character from source simultaneously.

With those in mind:

>>> def search2(word, source):
    """Find every index in source at which word starts."""
    firstchar = word[0]
    wordlen = len(word)
    return [index for index, char in enumerate(source)
            if firstchar == char and word == source[index:index+wordlen]]

>>> timeit.timeit("search2(word, source)",
                  setup="from __main__ import search1, search2, search3; import re; word, source = 'foo', 'foobarbaz'*1000",
                  number=10000)
9.99040215947457

I have also added a docstring to explain what the function does.

However, if you really want efficiency (and readability, for that matter), ignoring regular expressions is a bad move:

>>> def search3(word, source):
    """Find every index in source at which word starts."""
    return [m.start() for m in re.finditer(word, source)]

>>> timeit.timeit("search3(word, source)",
                  setup="from __main__ import search1, search2, search3; import re; word, source = 'foo', 'foobarbaz'*1000",
                  number=10000)
2.1652778798459167


Loop through column cells and compare value with a Collection

For a lot of data dictionaries are faster than collections, and much more convenient as @RubberDuck mentioned. Lookup times are almost instantaneous, IF you do a lot of lookups, but both collections and dictionaries are slow to initialize

In your case though, the lookups are insignificant. The main issue is caused by updating the cell format in every iteration. I converted the range to an array to minimize interaction with the sheet. Here are the results using the same data set:

1. (Rng) - Total rows: 100,001 in  8.851 sec
2. (Arr) - Total rows: 100,001 in  0.398 sec

1. (Rng) - Total rows: 500,001 in 43.578 sec
2. (Arr) - Total rows: 500,001 in  2.023 sec

   Invalid cells: 77,780 (out of 100,000)

.

the code

Option Explicit

Public Sub CompareValues()

    Const MAX_SZ    As Byte = 240
    Const COL_ID    As Byte = 1
    Const COL_LTR   As String = "A"
    Const FIRST_ROW As Byte = 2

    Const SRCH_LST  As String = "%,%O,D,CMS"
    Const LST_START As Byte = 0
    Const LST_END   As Byte = 3 '165

    Dim ws As Worksheet
    Dim thisRow As Long
    Dim thisItm As Long
    Dim invalidSet As Long
    Dim tmp As String
    Dim lastRow As Long
    Dim colArr As Variant
    Dim searchItem As Variant
    Dim isValid As Boolean
    Dim invalidArr() As String

    Set ws = ActiveWorkbook.Worksheets("Sheet1")
    With ws
        lastRow = .Cells(.Rows.Count, COL_ID).End(xlUp).Row
        colArr = .Range(.Cells(FIRST_ROW, COL_ID), .Cells(lastRow, COL_ID)).Value2
    End With
    searchItem = Split(SRCH_LST, ",")
    ReDim Preserve invalidArr(lastRow)
    invalidSet = 0

    For thisRow = 1 To lastRow - FIRST_ROW + 1
        isValid = False
        For thisItm = LST_START To LST_END  'compare
            If colArr(thisRow, COL_ID) = searchItem(thisItm) Then
                isValid = True
                Exit For
            End If
        Next
        If Not isValid Then     'if not valid build string of addresses (A1,A3,A7,...)
            tmp = tmp & COL_LTR & thisRow + FIRST_ROW - 1 & ","
            If Len(tmp) > MAX_SZ Then               'if string length > 240, split it
                invalidArr(invalidSet) = Left(tmp, Len(tmp) - 1)   'remove last comma
                invalidSet = invalidSet + 1
                tmp = vbNullString
            End If
        End If
    Next

    If Len(tmp) > 0 Or invalidSet > 0 Then      'if there are invalid values
        Dim totalSets As Long
        totalSets = invalidSet

        If Len(tmp) > 0 Then                    'store the last set, if tmp not empty
            invalidArr(invalidSet) = Left(tmp, Len(tmp) - 1)
        Else
            totalSets = totalSets - 1
        End If
        ReDim Preserve invalidArr(totalSets)    'cleanup (not needed)
        With ws
            For invalidSet = 0 To totalSets     'change cell colors in sets of ranges
                .Range(invalidArr(invalidSet)).Interior.Color = 192
            Next
        End With
    End If

End Sub


Getting days count from start day and number of day for a group of days in Java 8

You came far to a good solution.

public void getDayCount(List<DaySpan> daySpans){
    Map<LocalDate, Long> freqs = daySpans.stream()
            .flatMap(daySpan ->
                IntStream.range(0, daySpan.numberOfDays)
                    .mapToObj(i -> daySpan.date.plusDays(i)))
            .collect(
                Collectors.groupingBy(Function.identity(), Collectors.counting()));

    freqs.entrySet().stream()
            .sorted(Map.Entry::getKey)
            .forEach(System.out::println);
}

Creating a Stream<LocalDate> for some DaySpan is simply realized by an IntStream.range.
A flatMap makes a larger Stream of LocalDates.
Turning them into a counting map you already did.
Sorting them on the date remains.
You printed the entries as Entry, so I kept it that way.


What comes to performance, in order to merge two subranges, you copy them first and then merge. All in all, you make \$\Theta(n \log n)\$ array component copies. One thing to remedy this is to allocate the entire range before actual sorting, copy the input array contents to it and proceed making the array swapping: at one recursion level, you merge from one array to another, at the next recursion level you merge in opposite direction. See what I mean:

#include <stdio.h>
#include <stdlib.h>
#include <time.h>

static void getArray(int* values, int len)
{
    for (int i = 0; i < len; i++)
    {
        values[i] = rand() % 100;
    }
}

static void printArray(int* values, int len)
{
    for (int i = 0; i < len; i++)
    {
        printf("%i ", values[i]);
    }

    printf("\n");
}

static void merge(int* source_left,
                  int* source_middle,
                  int* source_end,
                  int* target)
{
    int* left      = source_left;
    int* right     = source_middle;
    int* left_end  = source_middle;
    int* right_end = source_end;

    while (left < left_end && right < right_end)
    {
        *target++ = *left < *right ?  *left++ : *right++;
    }

    while (left < left_end) *target++ = *left++;
    while (right < right_end) *target++ = *right++;
}

static void mergesort_impl(int* source, int* target, int len)
{
    if (len < 2) return;

    int left_range_length = len / 2;
    /* No copying here whatsoever. */
    mergesort_impl(target,
                   source,
                   left_range_length);

    mergesort_impl(target + left_range_length,
                   source + left_range_length,
                   len - left_range_length);

    merge(source,
          source + left_range_length,
          source + len,
          target);
}

static void mergesort_(int* array, int len)
{
    if (len <= 1) return;

    int* aux = malloc(sizeof(int) * len);
    memcpy(aux, array, sizeof(int) * len);
    mergesort_impl(aux, array, len);
    free(aux);
}

int main(void)
{
    int len = 10;
    srand(time(NULL));

    int* array = malloc(sizeof(int) * len);
    getArray(array, len);

    printf("Before sort: ");
    printArray(array, len);

    mergesort_(array, len);

    printf("After sort: ");
    printArray(array, len);

    return 0;
}


Calculate divisors using prime factorization

Bugs

FindPrimes is one of the better sieve implementations I've seen posted on this site, but it has an out-by-one bug. Here's some test code, which you can tidy up in a unit testing framework if you want to do things properly:

    var df = new DivisorFinder(72);
    int[] knownPrimes = new int[] { 2, 3, 5, 7, 11, 13, 17, 19, 23, 29, 31, 37, 41, 43, 47 };
    for (int limit = 1; limit <= 50; limit++)
    {
        var expected = knownPrimes.Where(p => p < limit);
        var observed = df.FindPrimes(limit);
        if (!expected.SequenceEqual(observed)) Console.WriteLine($"Error for limit {limit}");
    }

FindDivisors has a more alarming bug. What are the divisors of 72? According to this code, they include 144...

Structure

FindPrimes should be static: nothing in that method depends on instance fields or properties.

FindPrimeFactors modifies state, so it's not idempotent. Calling FindDivisors twice on the same object should give the same results, but it doesn't.

Frankly, this class is an example of OO purism at its worst. There's no good reason for any of the methods to be non-static.

IMO the dependency relationship between FindDivisors and FindPrimeFactors is wrong: FindPrimeFactors should be responsible for calculating which primes it needs to use. Consider that if you want to have multiple methods which use the prime factors in different ways, you shouldn't have to copy-paste the line

    var primes = FindPrimes((int) (Math.Sqrt(_number) + 1));

into every single one.


Is it a good exercise, or the algebra jumps out like Jack off the box?

The algebra is fine. That size_t x1_minus_x2 = x1_plus_y - x2_plus_y line wouldn't be necessary (IMHO even reduces readability), but apart from that it's reasonable.

However, your solution is worse than it needs to be. It merely stays in-place, while the optimal solution to this problem can treat the input as read-only and limit the working-set to constant space.

Your solution also depends on list_reverse being implemented as in-place and manipulating the original list. Depending on such behavior is IMHO pretty bad design. In fact, if I hadn't checked the source of list_reverse, I would have expected it to return a reversed copy instead. Which would obviously break your solution.

I would expect that your students will mostly just reverse both lists, traverse in lockstep and identify the last common node that way. Students have a tendency to trade algebra for brute force, if possible.

Smarter students will just read the length of both lists, skip the length difference, and then walk in lockstep until they find the common element. Which then also would fit better within the space complexity requirements, and requires no manipulation of the lists.


If you are keeping a reference to an object you should pass that object to the constructor as a reference:

mutexLocker (pthread_mutex_t    argMutexVariable)
                    //     ^^^  Missing reference.

You return the state of the lock operation:

int getMutexLockStatus ()

But if the mutext failed to lock what are you going to do.
If the mutext fails to lock there is probably something fundamentally wrong so you should probably stop the current action by throwing an exception.

You can't conditionally return a value.
You must always return something from a function that expects to return value.

pthread_mutex_t getLockedMutex ()
{
    if (mutexStatus >= 0)
        return mutexA;

    // What happens if the code gets here?
    // Letting a function return without an explicit return (in a non void method)
    // is undefined behavior.
}

Also I see no point in testing the status.
If the lock failed to lock you should have exited the code much earlier than this. If the lock fails throw an exception.

Your test is also incorrect (here and the destructor)

if (mutexStatus >= 0)

If the mutex did lock then it returned 0 and thus the mutexStatus is zero.


Notify one of hundreds of viewmodels with MVVM Light messenger

It should be up to that ViewModel how it handles messages and if it wants to ignore them or act on them.  So that rules out option 3.  That just leaves option 1 and 2.  Option 2 will work but as a developer I wouldn't normally think that setting a property of IsSelected to have side effects and we all know side effects are bad.  I just think it adds a bit more in the maintenance that doesn't need.   I typically just do option 1 and I typically have one method in a class that handles all the registrations for the messages and what they should do.  Just makes maintenance easier if you have a message issue you know where they are all getting registered at.

And since this is CodeReview and all code is subject to review I would recommend you get rid of "magic" strings and make a class that holds them all.  Something like

public static class Messages
{
    public const string Refresh = "REFRESH";
}


You might prefer to not use polymorphism at all if you want to make sure that your types remain unique. Consider something more like:

class Node
{
private:
    list<Node*> children ;
    int v ;

public:
    Node( int iv ) : v(iv) { }

    void add( Node* child )
    {
        children.push_back( child );
    }

    void print( const char* msg )
    {
        printf( "REGULAR CHILD %d in %s\n", v, msg );
        for( Node *sn : Node::children )
        {
            sn->print( "regular" );
        }
    }
};

class SpecialNode // no inheritance
{
private:
    list<SpecialNode*> children;
    int v;

public:
    SpecialNode( int iv ) : v(iv) {}
    void add( SpecialNode* child )
    {
        children.push_back( child ) ;
    }

    void print( const char* msg )
    {
        printf( "SPECIAL CHILD %d in %s\n", v, msg ) ;
        for( SpecialNode *sn : children )
            sn->print( "special" ) ;
    }
};

class Composite // no inheritance
{
private:
    Node nodes;
    SpecialNode specialNodes;

public:
    Composite( int iv ) : nodes(iv), specialNodes(iv)
    void addSpecial( SpecialNode* child )
    {
        specialNodes.add( child ) ;
    }
    void add( Node * child)
    {
        nodes.add(child);
    }

    void print( const char* msg )
    {
        nodes.print(msg);
        specialNodes.print(msg);
    }
};


Preparing a walkthrough: Freeze effect :s

The problem here isn't going to just be that the segue is performed slowly.  At a certain point, if you add enough view controller's views to this scroll view, it's very likely that you'll run into memory issues as well, particularly on mobile devices.

We have a scroll view.  Only one screen's worth of the scroll view can be on the screen at any given time.  But in viewWillAppear, we are attempting to load several screen's worth of views onto our scroll view all at once.  This takes time.  And the more views you are adding to it, the more time it takes.

Consider UITableView and UICollectionView are both direct subclasses of UIScrollview.  How do these guys handle adding views to the scroll view?  They do it one at a time, as needed.

If you put a breakpoint or any sort of print statement in the cellForRowAtIndexPath: (or the collection view equivalent), you'll see that this method is only called for any given cell just as that index path is about to scroll on screen.

Using your approach of adding at the views at the same time means you'll holding them all in memory at once, so there's a top end cap on how many views you can stick on there, and it means you're loading them all at once, so there's a big block of time where it takes to run through adding them to the scroll view.  With a table view or collection view, it's not necessarily any faster to add each individual view to the screen, but only the ones necessary are added at once.  Moreover, as views scroll off the screen, they're released from memory as new ones are added.  So you can have an unlimited number of views on the collection view without running out of memory and without it taking an unmanageably long amount of time to load.

Instead of re-inventing the wheel and try to implement some UIScrollViewDelegate methods by hand to add and remove your views as they're needed, I instead recommend that you change your scroll view to a collection view and work with that.


There's an important principal in programming called Don't Repeat Yourself (or DRY). This says that if there is any repeated code, then that is usually a bad thing. So let's identify the non-repeated code, and start with that. This is the stuff that changes:

desktop
contacts
documents
Downloads
Music
Pictures
Videos

We could put that in a file, but that's probably overkill here, so let's just put that in an array:

$dirNames =
    "desktop",
    "contacts",
    "documents",
    "Downloads",
    "Music",
    "Pictures",
    "Videos"

Now we can just operate on the array:

foreach ($dirName in $dirNames)
{
    Remove-Item -Path "C:\users\test_removal_account\$dirName\*" -Recurse -WhatIf
}

Notice I put -WhatIf on the Remove-Item. That just makes the command tell you what it is going to do instead of actually doing it. This is useful for testing purposes. You can remove it when you are happy the script does what you want.

This script as it stands is dangerous. Recursive deletes must be treated with respect. If you accidentally blow away someone's work, you could ruin their life. You may even accidentally blow away all your own work and ruin your own life. You have to think about this sort of thing as a responsible engineer.

Of course, the "test_removal_account" makes the script pretty benign as it stands, but I don't know what you are going to replace that with in the real script.

I don't think a script ought do anything destructive if you just run it.

If it were me, I would put in a confirmation check before deleting. I would get the user to type in something, such as "really delete" before doing the delete.


On Threading

lockfree code is rarely easy to understand.

I would beg to differ. It usually just hides the complex situations.
On Spin Locks.
Spin locks are generally not a good idea. You should be really sure that a thread caught in a spin lock will escape quickly (otherwise you are going to melt your processor).
But you mitigate the problem by using std::this_thread::yield() so this is not such a huge problem. So it is not what I would call a classic spin lock. More a spin with yield.
Guaranteed escape
The only issue I see is that you don't guarantee escape of the lock for a particular thread.
Some particular unlucky thread may be caught trying to obtain a lock while other threads zip passed and keep getting the lock forcing the unlucky thread to keep re-trying to get out of the acquire.
So your spin lock has the potential for some resource starvation (or worst case making the code serial).
This is usually achieved by maintaining an order.
Documentation
You need to put the explanation into the code (as comments). Putting it as an explanation for this site was nice and allowed me to understand the code. But without it the code is undecipherable.
RAII
You have designed your code that requires matched calls to method (this is bad practice). Also you have not show RAII lockers for this class so we must assume its use is not exception safe.
All four of your methods:
        void AcquireReaderLock()
        void ReleaseReaderLock()
        void AcquireWriterLock()
        void ReleaseWriterLock()

Should be private members. You should only allow access to these methods via an RAII lock guard. Look at std::lock for an example.
Best practice
Minor best practice violations:
// One declaration per line
int computedValue, initialValue;

// Why not initialize these on declaration.
initialValue = 0;
computedValue = -1;
// There is no need to ever set them again.


I'd never thought of using reduce for this. Neat! However, you can simplify mk-balanced? a little.

str->chars is redundant: reduce and the other sequence functions accept strings as such.
You can use reduced to short circuit a reduce.
The reduced can return any non-empty sequence: no need for [:false].

The simplified version is ...

(defn mk-balanced? [table]
  (fn [s]
    (let [opens (set (vals table))
          closes (set (keys table))]
      (empty?
        (reduce
          (fn [stack cur]
            (condp contains? cur
              opens (conj stack cur)
              closes (if (and (seq stack)
                              (= (peek stack) (table cur)))
                       (pop stack)
                       (reduced [nil]))
              stack))
          []
          s)))))


Comments

It is of course better to write self-documenting code that does not require comments.  However, sometimes we can't quite meet that goal.

    float size;
    int divisions;

What are these?  My first thought is that they are the size of a grid square in pixels or some other unit and the number of grid squares in a row or column.  I don't know what the unit is for the size.  And my length/width hypothesis gets confused when I see code like

                grids.add(x + y * divisions * divisions);

Why divisions * divisions?  If it were just divisions, I'd think that this was just a Cartesian plane.  Then y would represent a row, and x would be the place in the row.  So basically converting the two-dimensional grid into one-dimension.  So a value of divisions - 1 would be next to divisions in the single dimension but would represent the last element of the y = 0 row and the first element of the y = 1 row in two dimensions.

As is, this means something different, and I'm not sure what.  Comments could help clarify this, not just for me as a reviewer but for you in six months when you've forgotten why you did things this way.  You don't need to comment every line, but if you feel clever for the way you did something, comment that.  Because the next person who edits the code (which may be you) won't be nearly as clever as you are at that moment.

What's an Obj?

You don't include the code for Obj, so I'm not really sure what it is.  A less generic name might help.

Fields vs. local variables

    private Set<Obj> retrievedObjects = new HashSet<>();

Why is this an object field?  It's used in exactly one method, cannot be retrieved outside that method, and is reset in the method.  If you replaced it with a local variable, nothing else about this code would change.

The one difference is that if you call retrieve a second time, the values in the set might change.  That seems rather confusing.  You could be iterating over the set, trigger a retrieve, and your iterator is now pointing to an invalid location.  That seems more problematic than keeping up to date is helpful.  Particularly as retrieve takes different parameters.  So the set may change because the parameters are different rather than because what the set represents has changed.

Sparse

This grid is sparsely populated.  You use a Map to hold it, so you only need to remember the locations where it is populated.  This saves a lot of memory at the cost of some speed.  If you find yourself with plenty of memory and constrained by time, consider switching to a different data structure, e.g. a two dimensional array.

Another question is if the Map helps you at all.  You could simply iterate over the list of objects instead.  That might be faster, as you only need to know if the objects conflict.  You actually check each possible conflict and then merge the results.  So if an object is in a hundred squares and another is in fifty, you might do fifty merges.  Consider

for (Obj o : allObjects) {
    if (distance(o, obj) < o.radius + obj.radius) {
        retrievedObjects.add(o);
    }
}

That checks more objects but may do fewer merges.

A side effect of this is that the radius would become an actual radius.  I would expect a radius to be part of the definition of a circle, but in the current code, it defines a square.  If you really want a square, you can still simplify things.  Consider

    int objMinX = Math.max(0, obj.x - obj.radius);
    int objMinY = Math.max(0, obv.y - obj.radius);
    int objMaxX = Math.min(divisions - 1, obj.x + obj.radius);
    int objMaxY = Math.min(divisions - 1, obv.y + obj.radius);
    for (Obj o : allObjects) {
        if (hasOverlap(objMinX, objMaxX, o.x - o.radius, o.x + o.radius)
                && hasOverlap(objMinY, objMaxY, o.y - o.radius, o.y + o.radius)) {
            retrievedObjects.add(o);
        }
    }

and later

public static boolean hasOverlap(int minA, int maxA, int minB, int MaxB) {
    return maxA >= minB && minA <= maxB;
}

Another option would be to switch from Map<Obj> to NavigableMap<NavigableMap<Obj>>.  Then you could do the query more like a two dimensional array.

The important thing is to test these to see which performs best for you.  This is not a case of a one size fits all solution.  The best solution is going to be dependent on the exact characteristics of your data.  How sparsely filled is your grid?  How many objects are there?  Two large objects in a grid would suggest that iterating over all objects is best.  A grid where every square is covered but with small objects might suggest a different solution.


Can anyone suggest a more idiomatic way of representing this?

The equations for the ALU have the form

(out !! i, cout !! i) = aluc (a !! i) (b !! i) (cout !! (i - 1))

so we should be able to express all of the equations with a single equation that manipulates lists. If only cout !! (i - 1) would be something !! i, this would be easier because all the indexes would be uniform. We can make the indexes more uniform by defining cin = c0 : cout, and then the equations become:

(out !! i, cout !! i) = aluc (a !! i) (b !! i) (cin !! i)

Now we can make a loop over i to implement alu32 as follows:

alu32 c0 c1 c2 a b =
  let (out, cout) = unzip [alu1 c0 c1 c2 (a !! i) (b !! i) (cin !! i) | i <- [0 .. 31]]
      cin = c0 : cout
      ...
  in ...

This should solve the main question: How to implement alu32 without repeating all these equations. On the way, I also replaced the translate with an unzip.

However, we can do even better if we avoid the index computations. There are two problems with the indices: Index lookup is slow and awkward compared to just looping over the elements of a list. And the loop over the indices forces us to choose a fixed upper bound in the [0 .. 31] expression.

To avoid the indices, we note that the list comprehension

[alu1 c0 c1 c2 (a !! i) (b !! i) (cin !! i) | i <- [0 .. 31]]

just loops over three lists in parallel, a, b and cin. This parallel loop over three lists can be expressed with zipWith3 as follows:

unzip (zipWith3 (alu1 c0 c1 c2) a b cin)

Or you activate GHC's extension for parallel list comprehensions by putting {-# LANGUAGE ParallelListComp #-} near the top of the file, and then use:

unzip [alu1 c0 c1 c2 ai bi ci | ai <- a | bi <- b | ci <- cin]

The latter will desugar to the former. Clearly, this variant of the code doesn't use any indices anymore. This also means that it works for arbitrary bitvectors, not just 32bit ones. We only have to adapt the rest of the alu32 implementation to avoid indices, too. For example:

alu :: Wire -- Control Bit 0
  -> Wire -- Control Bit 1
  -> Wire -- Control Bit 2
  -> ByteWire -- n Bit Wire addend
  -> ByteWire -- n Bit Wire addend
  -> (ByteWire, Wire, Wire, Wire)
alu c0 c1 c2 a b =
  let (out, cout) = unzip (zipWith3 (alu1 c0 c1 c2) a b cin)
      cin = c0 : cout
      overflow = xorGate (take 2 (reverse cout))
      isZero   = map (all (==False)) out
      negative = last out
  in (out, overflow, isZero, negative)

This version of alu is concise and works for bitvectors of arbitrary length.

I'm not really sure about map (all (== False)) out to compute that all bits are zero. Maybe I would use something like map (all not) out or map (not . or) out. Note sure. Or maybe I would define a gate for deciding that something is all zeros and then use that one.

  Also, if you feel like browsing the repository, feel free to make any suggestions.

Consider adding a cabal file. When I cloned your repo in order to test the above suggestion, I was confused that cabal sandbox init && cabal build didn't work. You can just run cabal init in the repository and it should ask you questions to help you make the cabal file.
Consider making the Wire type more abstract.

Currently, you have:

type Wire = [Binary] -- A list of discrete states

Instead, you could use:

newtype Wire = Secret [Binary]

The benefit of this definition is that it becomes harder to "cheat" and use list operations instead of gates. Later, when you are finished defining the basic gates, you can make a module that exports the Wire type and the gates, but not the Secret constructors. This way, you force the users of that module to use the gates and only the gates to compose wires.


Handling data returned from fetch_assoc()

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

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

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

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

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

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

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

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

Hope this helps!


Project Euler 46: Finding a counterexample to a Goldbach conjecture

Optimization

It seems that you have tried to optimize your code using the sieve of Eratosthenes; however, the sieve of Eratosthenes is only useful if there is a given upper bound in the problem statement.

An optimal solution can be achieved for this problem via brute-force in the following manner (pseudocode):

n = ... # the smallest odd composite number

loop forever:
    solution_found = False
    for all possible x:
        possible_prime = n - 2 * x ** 2
        if is_prime(possible_prime):
            solution_found = True
            break
    if not solution_found:
        print("Answer: " + n)
        break
    n = n + ... # what to increment by?

Note that the above is merely pseudocode. For the line for all possible x, it is possible to bound x. Also (and most importantly), the is_prime() method needs to be cleverly thought out, possibly caching values.

Note: I just tested my implementation based on the above pseudocode and got the answer instantaneously. Too bad Goldbach didn't have a computer!


Efficiently remove duplicate elements from the list

I think you used a good way to filter out the duplicates, but I don't see why you feel the need to call .ToList() after that. A HashSet is perfectly iterable.
As such, I believe I would write your lengthy code somewhat differently as:
public async Task<IReadonlyList<T>> Execute<T>(IEnumerable<int> ids, Policy policy, Func<CancellationToken, int, Task<T>> mapperFunc) where T : class
{
    return (await Task.WhenAll(
            new HashSet<int>(ids).Select( id => ProcessData( policy, ct => mapperFunc( ct, id ) ) ) ) )
        .Where( response => response != null )
        .ToList();
}

It will still remove all the duplicates, await all entries using the Task.WhenAll and remove the null values.
As for knowing if it is more efficient I can't tell without testing. I think it is more readable, though to be fair, I would probably rather assign the results to a variable before filtering them out, saving you some brackets.
public async Task<IReadonlyList<T>> Execute<T>(IEnumerable<int> ids, Policy policy, Func<CancellationToken, int, Task<T>> mapperFunc) where T : class
{
    var responses = await Task.WhenAll(
            new HashSet<int>(ids).Select( id => ProcessData( policy, ct => mapperFunc( ct, id ) ) ) );

    return responses
        .Where( response => response != null )
        .ToList();
}


It depends. (As usual.)

The recursive version is easier to read an maintain, and does the job fine for relatively small data sets.

However, when the data set grows large, you create a new stack frame for each recursive call, which may lead to great stack memory usage up to stack overflow. Furthermore, this is comparatively slow.

The second version uses constant stack size and puts all the real memory usage in the heap where it should be. Not using recursion, it is also bound to be faster (on a micro-level only).

Thus, know the problem you solve: for small sets, write nice code and use your version, for big data sets, optimize by unrolling and use the second version.

(BTW: in my university courses, it was generally recommended to use the second version for NP problems due to raw speed - albeit in C at that time.)


Python sum of multiples of some numbers in a range

there is some room for improvement here, I tried to adapt a bit your solution without changing it a lot, to make it a bit less repetitive and more understandable
Will explain you some steps :)
Reading input
First of all, you are reading the input from the user on a few places, always in the same way, asking for input again unless an integer is provided. To avoid repetition, this functionality can be extracted into a function
def read_integer(input_message):
    while True:
        try:
            number = int(input(input_message))
            return number
        except ValueError:
            print('Integer Values ONLY')

You just pass the message you want to appear on screen and it will ignore all non integer values, asking for input again
Reuse of variables
Sometimes, you can use same variable for different purposes.
In order to read input in a loop, you can use the same variable that is introduced in the loop as an index, avoiding the creation of the new variable integers only for that
So this code
integers = 1
for y in range ( num_mul ):
    multiples.append ( int ( input ( 'Please input your No.{} integer here: '.format(integers) ) ) )
    integers = integers + 1

Can be reduced to
for y in range ( num_mul ):
    multiples.append ( int ( input ( 'Please input your No.{} integer here: '.format(y+1) ) ) )

Avoid unnecessary code
In the end, there is an else condition that simply continues to next iteration. That will also happen even if the else condition does not exist, saving you some unnecessary lines of code
A bit later, you turn all multiples into a set to avoid repetition. That can be handled before, inserting only if they are not already in the list
if (k % multiples[i]) == 0 and k not in sums:
    sums.append(k)

Naming
You could use a bit better naming in variables and so, I modified a bit some of them so you can check but things like num_mul can be better understood with longer naming, like multiples_count
Summary
You can check how your solution would look like with this features, I applied the changes that I suggested you:
def read_integer(input_message):
    while True:
        try:
            number = int(input(input_message))
            return number
        except ValueError:
            print('Integer Values ONLY')

def main():
    # Initialize variables
    multiples = []
    sums = []

    # Determines the number of multiples from user
    input_message = 'How many multiples are you using in total?: '
    multiples_count = read_integer(input_message)

    # Takes multiples and store in list
    for y in range(multiples_count):
        input_message = 'Please input your No.{} integer here: '.format(y+1)
        new_multiple = read_integer(input_message)

        multiples.append(new_multiple)

    # determine the start and stop of the range from user
    input_message = 'Start of range: '
    start = read_integer(input_message)

    input_message = 'End of range: '
    stop = read_integer(input_message)

    # Figures out the multiples of the numbers given between the start and end range
    for i in range(multiples_count):
        for k in range(start, stop):
            if (k % multiples[i]) == 0 and k not in sums:
                sums.append(k)

    x = sum(sums)  # Gets rid of duplicated values and sums the list of values

    print('Your SUM total is {}.'.format(x))

if __name__ == "__main__":
    main()


The code is quite monolithic: calculation and presentation are separate concerns, and while I appreciate the context given by showing the presentation, it would be better to make clear that they're separate concerns by separating them with whitespace and commenting briefly.

Also on structure: separate constants (h) from variables (v, x, t).

    v=v-(10.0/1.0)*x*h    #k=10.0, m=1.0
    x=x+v*h
    t=t+h

Two things: firstly, what are k and m? It seems that they should be constants with comments explaining their physical significance.

Secondly, this isn't Euler's method. Euler's method is \$\vec{x}_{n+1} = \vec{x}_n + hf(t_n, \vec{x}_n)\$ where \$f = \frac{d\vec{x}}{dt}\$. Here \$\vec{x} = (v, x)\$ and \$f(t, v, x) = (\frac{dv}{dt}, \frac{dx}{dt}) = (-\frac{k}{m}x, v)\$. In other words, to be Euler's method you should update x according to the previous value of v, not the updated value of v. Since you're using Python, you can take advantage of simultaneous assignment:

    v,x=v-(k/m)*x*h,x+v*h
    t=t+h

(As it happens your buggy implementation works better than Euler's method, but if it was intended to implement Euler's method then it's still technically buggy).


This exists already:

It's not the prettiest to use but it's there.

import java.time.*;
import java.time.format.*;
import java.util.*;

public class Example {

    public static void main(String[] args) {
        Month month = Month.of(2);
        System.out.printf("%s", month.getDisplayName(TextStyle.FULL, new Locale("en")));
        // outputs:
        // February
    }
}

Don't re-write code that's in your standard libraries :)


Switching functionality on and off in a Scheduled Thread

you could also look at using a fixed thread pool. That way, if one thread becomes blocked for some reason, say the disk became full, the threads would wait in a queue until the blocked thread terminated. That way you wouldn't loose any unique id's or run into memory problems if too many threads became blocked.

Hope that helps,

Ross


One of the unfortunate things you'll notice about coding challenges is that the requirements often force you to write code that isn't ideal. I'm going to call these out, even though in this case they're outside of your control.

Silly requirement: The method is named solution. This name tells us nothing about the behavior of the function. Also (although less importantly), it goes against the C# method naming convention of PascalCase. A much better name would be something like IsPermutation.

Silly requirement: The parameter is named A. Single-letter variable names are almost never a good idea. Also (although less importantly), it goes against the C# parameter naming convention of camelCase. It's hard to be descriptive about the context here (since it's just a coding challenge and not a "real life" business problem), but even something like values would be better.

Silly requirement: The return type of the function is int. The given array is either a permutation of 1..N or it isn't. We don't have a range of possible return values; we have a true/false condition. A better return type would be bool.

Algorithm inefficiency: There's no need to initialize two new int[]. As you discovered, one new array is sufficient to track unique values.

Algorithm inefficiency: Again, you've already discovered this. If an array has the correct elements in some order, its sum will necessarily match that of the unordered array. Meanwhile the converse is not true; there are many arrays with the same sum which are not permutations. So you needn't bother tracking the sum at all.

Syntax: If you take only one piece of my advice, let it be this one: for loops are not the answer. If you are iterating over the indexes of an array, and the only thing you use the index for is to grab the ith element, use a foreach. The situation where you actually need to know the index, and therefore need a for loop, is very rare.

Semantics: Does an array of int take less space in memory than an array of bool? I don't know, and I don't care. If all I'm tracking is true/false values, I'm going to choose a bool[]. Because that choice tells the person reading the code why I created the variable.

Formatting: Another major difference between formatting conventions in C# and Java (besides the PascalCase/camelCase conventions I already mentioned) is C#'s convention of putting opening curly braces on the next line (known as K&R style).

With all of that advice applied, and some XML documentation comments (which enable Intellisense information when hovering or typing in Visual Studio), we'll end up with something like this:

/// <summary>
/// Check whether the given array contains each integer 1..N exactly once.
/// </summary>
/// <returns>
/// True if <paramref name="values" /> is a permutation of 1..N,
/// False otherwise.
/// </returns>
public static bool IsPermutation(int[] values)
{
    var seen = new bool[values.length];

    foreach (var value in values)
    {
        if (value < 1 || value > values.length)
        {
            // Out of range: not a permutation
            return false;
        }
        else if (seen[value - 1])
        {
            // Duplicated value: not a permutation
            return false;
        }
        else
        {
            // Value is OK. Mark as seen.
            seen[value - 1] = true;
        }
    }

    // All values in range, no duplicates: a valid permutation
    return true;
}


How do I refactor this basic form handler

I know you're asking for "a basic connection class", but you really need a few classes. Currently, your code has all of these (tightly-coupled) responsibilities:

Database access
Error reporting
Form validation
Data persistence
And I assume you're rendering output as well

To move past the difficulties you're facing, you need to separate these concerns into loosely-coupled objects. The prevailing architectural pattern to accomplish this is with the MVC pattern, but this is out of the scope of your question.

With this in mind, I would start with trying to isolate database access. To decouple your data from the rest, you need to put all of your database access in a separate class or set of classes. Here, there are two key patterns: object-relational mapping (ORM) and the database repository.

With ORM, you essentially try to hydrate database records into PHP objects – if you want to create a new record in the database, you create a PHP object and call a method to persist it to the database; if you want to update a record, you fetch a record, convert it to a PHP object (hydration), update the PHP object, then persist it; to delete, you either obtain a reference to a record or create a dummy PHP object, call a method to delete this record, then call another method to persist this change.

ORM has a lot of surface area; it typically relies on a database abstraction layer, requires you to write a new class for every type of record you'd like to persist, and can confuse beginners when you throw relations into the mix. While this definitely takes some investment, it provides a path to a lot of other benefits: code reuse, testing, automatic schema generation, migrations, validation, etc.

Another pattern for data persistence is the database repository. Here, all you're doing is wrapping your queries in methods and grouping these methods into classes based on which records the queries operate on. For instance, a UserRepository class might have the following methods: UserRepository::createUser(), UserRepository::getUserById(), UserRepository::getUsersByZipcode(), UserRepository::updateUser(), UserRepository::deleteUser(). Each one of these methods would accept a set of PHP parameters (arrays or ORM object and integers), perform database queries based on this data, and return the results (possibly hydrating them).

ORM and database repositories work very well together. I've personally used Doctrine 2's ORM and DBAL as well as Aura SQL's DBAL and would recommend both. I personally would start with creating a connection object with Aura SQL, then create your own class called UserRepository with the methods I listed above. Instantiate the UserRepository class by passing the AuraSQL connection object to it's constructor, then call UserRepository::createUser() to persist the user data from your form. After this, I would consider how to improve your form validation and look into using a template system like Twig.


Wrapping certain elements in an array with an array with PHP

You can try something like this, note that it is untested:

<?php
function createControls($parentView) {
    $newArr = $parentView;
    foreach ($parentView as $k => $v) {
        $newArr[$k] = ['type' => 'control'];
        if (isset($v['childViews'])) {
            $v = createControls($v['childViews']);
        }
        $newArr[$k]['childViews'] = $v;
    }
    return $newArr;
}
?>

This function uses reccursion to do the same as your code above. It takes one array of Key Value pairs and reassigns the values to wrap the old values into an array with the following format:

array ( 'type' => 'control', 'childViews' => $oldValue );

Basically, instead of nesting a new loop each time the depth of your input array changes like in your function above this function will just pass the old childValues back into itself and assign the result as the new childValues array.


line_to, fill_rect, and clear_rect functions

If I understand this correctly, you want to be able to call these methods with two different signatures:

method(x: .., y: .., height: .., width: ..)
method(rect: ..)

There has to be some logic in the methods to decide which signature to use. I like your current solution for that. Using metaprogramming would make things too complicated.

Since the signatures are different, you could argue they are actually two different methods: #line_to and #line_to_rectangle for example. You could simply define two different methods:

def line_to(x, y)
  `#{@context}.lineTo(#{x}, #{y})`
end

def line_to_rectangle(rectangle)
  line_to(rectangle.x, rectangle.y)
end

The best solution would be to only expect a single signature in the first place.

One way to do that is to use ** to pass a rectangle object as keyword arguments. For this to work you have to define a #to_hash.

class Rectangle
  def to_hash
    { :x => self.x, :y => self.y, :width => self.width, :height => self.height }
  end
end

...

line_to(**rectangle)

You'll have to make sure that #line_to takes all keys returned by Rectangle#to_hash. That means it should also a width and height keyword argument. You can do this by taking them explicitly or implicitly (def line_to(x: 0, y: 0, **discard)). If you don't take those unnecessary arguments it will raise an exception. Hardly ideal.

Another way to do that would be to create a second type of object to replace the keyword arguments (x, y, width and height). It should have the same interface as rectangle. I'd recommend not complicating things this much, though, and pick a single signature.


Use a Set.

private <E> boolean hasDuplicates(E[] array) {
    Set<E> set = new HashSet<E>();
    for(E e : array) {
        if (!set.add(e)) {
            return true;
        }
    }
    return false;
}

The add() method of a set returns false if the object already exists in the set.


This looks pretty good.

Use mem-initializer lists

When you write

slice(T* array, size_t length) {
    this->m_array  = array;
    this->m_length = length;
}

you're default constructing m_array and m_length and then assigning them. For pointers and integral types, this isn't exactly expensive, but it's a waste and may as well not get into that habit. Prefer directly constructing:

slice(T* array, size_t length)
: m_array(array)
, m_length(length)
{ }

Your other constructor can on the one hand just delegate to this one:

slice(T* array, size_t length, size_t skip)
: slice(array + skip, length)
{ }

but on the other hand doesn't really need to exist anyway. Just let the user pass in where he wants to start. This strikes me as just bloaty.

const functions

size() should be const, and you're missing T const& operator[](size_t) const.

You could also provide a T* data() and T const* data() const, which in this case just alias begin().

Additionally, consider providing a different constructor so that I can construct a slice<const T> from a slice<T>. That's something that makes sense.

throwing

If you're throwing due to out of range indexing, you should throw std::out_of_range. But generally, avoid having operator[] throw. This is what you're going to use most often so it's best to make it as simple as possible. If you want to provide a throwing alternative, the typical pattern followed by the standard library is to provide an at() function that does the bounds checking and then forwards along to operator[].

constexpr

Everything here can be constexpr. Just go overboard.


Optimise compare and match method using scripting.dictionary in VBA

This implementation actually looks really solid. A couple stylistic and micro-performance issues I would personally address:

Array base: I'd remove Option Base 1 and explicitly declare your array dimensions. The problem with using Option Base 1 is that it is a module level option, and it "taints" all of the code in the module with a non-default setting.  I consider it similar to using #define foo bar in C - it alters the semantics of your code. There's not even really a reason to use it in this case because you are always using the LBound to initialize your loop counters anyway. Changing this line...

ReDim arrayToPaste(UBound(secondArray, 1), 1)

...to...

ReDim arrayToPaste(1 To UBound(secondArray, 1), 1 To 1)

...makes it immediately obvious that the array is 1 based without having to scroll to the top of the module or remember that the base is non-default.

Variable naming: firstArray and secondArray are about as meaningful as a1 and a2. Something like workingArray and comparisonArray would make it a little more obvious what your code is dealing with.

Worksheet references: You are currently using magic numbers to locate the worksheets that you're operating on, and the indexes are based on the ordinal of where the sheet is in the collection.  If this is intended to operate on two fixed worksheets, use their class names instead.  This has two benefits: first, you aren't using 2 procedure calls (Worksheets and the implicit Items) to get a reference that you already know and have access to .  Second, your Sub won't break the first time you add a new worksheet. For example, use With Sheet1 (or whatever it is) instead of With ThisWorkbook.Worksheets(1).

Binding:  There is no reason to late bind to the scripting runtime.  The interface hasn't changed in this century, and is unlikely to. This is a pretty good performance hit in exchange for absolutely zero upside. Add a reference to Microsoft Scripting Runtime and declare it explicitly:

Dim dict As Scripting.Dictionary
Set dict = New Scripting.Dictionary

Use With appropriately: You're doing this for your worksheets, but it doesn't actually do anything for you:

With ThisWorkbook.Worksheets(1)
    firstArray = .Range("A1").CurrentRegion
End With

You use it exactly once inside the block, so this is exactly the same thing as:

firstArray = ThisWorkbook.Worksheets(1).Range("A1").CurrentRegion
'...or even better (see above)...
firstArray = Sheet1.Range("A1").CurrentRegion

What With does is hold an object reference to avoid having to dereference it multiple times.  If you only use it once, it's reduced to 2 extra lines of code and 1 extra indentation level.

That said, the place where it is appropriate is with your Dictionary, and it isn't used there. This is much better:

With New Scripting.Dictionary   'You don't even need the variable.
    .CompareMode = vbTextCompare

    For i = LBound(firstArray, 1) To UBound(firstArray, 1)
        dictKeyForLookup = firstArray(i, 4)
        dictValueToReturn = firstArray(i, 3)
        If Not .Exists(dictKeyForLookup) Then
            .Add Key:=dictKeyForLookup, Item:=dictValueToReturn
        End If
    Next i

    Dim arrayToPaste As Variant
    ReDim arrayToPaste(1 To UBound(secondArray, 1), 1 To 1)
    For i = LBound(secondArray, 1) To UBound(secondArray, 1)
        If .Exists(secondArray(i, 1)) Then
            arrayToPaste(i, 1) = .Item(secondArray(i, 1))
        End If
    Next i
End With    'Dictionary goes out of scope here.

Named arguments: There is no reason to use them here:

dict.Add Key:=dictKeyForLookup, Item:=dictValueToReturn

If you are early bound, the name information is available via both Intellisense and the object browser.  If you are late bound, it actually adds more overhead for the runtime to resolve them. Just use the ordinal argument positions:

dict.Add dictKeyForLookup, dictValueToReturn

They're also a potential bug source because they are resolved at runtime for late bound objects (which is the same reason you take a performance hit).  That means that if you have a typo in the name (i.e. Keey:=foo), it won't be caught by the compiler.  It is also dependant on how the typelib is defined for the class.  For example it's possible for some late bound objects to introduce errors by specifying the same variable name multiple times.  Excel seems to be prone to this type of error:

Debug.Print WorksheetFunction.Sum(Arg1:=1, Arg1:=42) 'Prints 1


Calculating the result of a recurrence function

Avoid the result variable

The result variable is only ever incremented once and then returned. You can simplify by returning directly:

if (x % 2 != 0) {
  return function((x-1)/2) + function((x/2)-1);
} else {
  return function(x/2);
}

Eliminating the variable should also speed-up the program if the compiler did not optimize it out already.

Memoization

Memoization essentially stores already computed values for later re-use trading space for run-time. Memoization has already been implemented in C++ so you can just make use of it.


Evaluating an arithmetic expression from a binary tree

This is not a search tree.  You can call it an abstract syntax tree, or just a binary tree.

You can use a dispatch table to evaluate ops.

 dispatch = {
      '+': (a,b) => a+b,
      '*': (a,b) => a*b
 }
 ...
 if (node.left && node.right && node.value in dispatch) dispatch[node.value]( traverse(node.left), traverse(node.right) );


Your approach is fundamentally flawed, I'm afraid. You're not using Rails' abilities to your advantage at all.

For a simple create operation, your controller method rarely needs to be more than 10 lines or so. There's way too much going on in yours.

Start by nesting your routes:

 resources :nutrients do
   resources :recommendations
 end

Run rake routes to see how the routes now look. Basically, a recommendation will have a URL like /nutrient/:nutrient_id/recommendation/:id.

This means you'll have to make some changes to links in your views and your redirections. Where you could previously write recommendations_path and so on you'll now have to write nutrient_recommendations_path(@nutrient) and so on. Again, rake routes will show you what helpers you now have.

In your recommendation form, you'll have to do something like:

 <%= form_for [@nutrient, @recommendation] %>

so the form's action URL will be correct. An get rid of the hidden input that I assume you've added to the form as recommendation[nutrient_id].

Inside the RecommendationsController, you can always get the "parent" nutrient by saying

 @nutrient = Nutrient.find(params[:nutrient_id])

as that param is given in the URL.

In the end, your create method will look like

def create
  @nutrient = Nutrient.find(params[:nutrient_id])
  @recommendation = @nutrient.recommendations.new(params[:recommendation])

  if @recommendation.save
    redirect_to @recommendation, :notice => "Recommendation created."
  else
    render 'new'
  end
end

See something like this post for a more thorough diff-like walkthrough of the changes to make.

Note, that if Recommendation has further nested resources, you should use "shallow" routes, but that's left as an exercise to the reader.


Functional vs Object Oriented Approach for UI codes

Object oriented doesn't really makes sense to me here. That FormattedError class doesn't even represent anything, just transforming error constructor parameter to another class.

Also you are calling constructor, that then returns instance of something else? I don't know javascript that much so maybe it's some unknown construct to me, but that looks like something terribly wrong.

Edit:

I see no relevance to code being UI code or not.


I would say you are wrong!!!

Let me explain...

If there is a user named 'tempuser'@'localhost' and you are trying to call DropUserIfExists('tempuser'), the above procedure may produce error something like:

Error Code: 1396 Operation DROP USER failed for 'tempuser'@'%', since there is no user 'tempuser'@'%'

A workaround for this problem is to pass both username and host name to the SP (pass a null host name if you want to delete all users named tempuser)

The following SP will help you to remove user 'tempuser'@'%' by executing CALL DropUserIfExistsAdvanced('tempuser', '%');

If you want to remove all users (say 'tempuser'@'%', 'tempuser'@'localhost' and 'tempuser'@'192.168.1.101') execute SP like CALL DropUserIfExistsAdvanced('tempuser', NULL); This will delete all users named tempuser!!! seriously...

Now please have a look on mentioned SP DropUserIfExistsAdvanced:

DELIMITER $$

DROP PROCEDURE IF EXISTS `DropUserIfExistsAdvanced`$$

CREATE DEFINER=`root`@`localhost` PROCEDURE `DropUserIfExistsAdvanced`(
    MyUserName VARCHAR(100)
    , MyHostName VARCHAR(100)
)
BEGIN
DECLARE pDone INT DEFAULT 0;
DECLARE mUser VARCHAR(100);
DECLARE mHost VARCHAR(100);
DECLARE recUserCursor CURSOR FOR
    SELECT `User`, `Host` FROM `mysql`.`user` WHERE `User` = MyUserName;
DECLARE CONTINUE HANDLER FOR NOT FOUND SET pDone = 1;

IF (MyHostName IS NOT NULL) THEN
    -- 'username'@'hostname' exists
    IF (EXISTS(SELECT NULL FROM `mysql`.`user` WHERE `User` = MyUserName AND `Host` = MyHostName)) THEN
        SET @SQL = (SELECT mResult FROM (SELECT GROUP_CONCAT("DROP USER ", "'", MyUserName, "'@'", MyHostName, "'") AS mResult) AS Q LIMIT 1);
        PREPARE STMT FROM @SQL;
        EXECUTE STMT;
        DEALLOCATE PREPARE STMT;
    END IF;
ELSE
    -- check whether MyUserName exists (MyUserName@'%' , MyUserName@'localhost' etc)
    OPEN recUserCursor;
    REPEAT
        FETCH recUserCursor INTO mUser, mHost;
        IF NOT pDone THEN
            SET @SQL = (SELECT mResult FROM (SELECT GROUP_CONCAT("DROP USER ", "'", mUser, "'@'", mHost, "'") AS mResult) AS Q LIMIT 1);
            PREPARE STMT FROM @SQL;
            EXECUTE STMT;
            DEALLOCATE PREPARE STMT;
        END IF;
    UNTIL pDone END REPEAT;
END IF;
FLUSH PRIVILEGES;
END$$

DELIMITER ;


Assuming you are going to put this in a header file don't do thus

using namespace std;

See: https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice

These should be const as they don't change the state of your auto_ptr

T& operator * (){ return *m_p; }
T* operator -> (){ return m_p; }

The destructor

~auto_ptr()
{
    if(m_p)            // No need to check for NULL
                       // delete on a NULL pointer is valid.
        delete m_p;

    m_p = NULL;        // This is pointless.
                       // The object is about to leave scope.
}

Why the sudden use of this!

template <typename T>
T* auto_ptr<T>::release()
{
    T* temp = this->m_p;
          //  ^^^^^^
    this->m_p = NULL;
 // ^^^^^^
    return temp;
}

Assignment not exception safe.

template <typename T>
auto_ptr<T>& auto_ptr<T>::operator=(auto_ptr& other)
{
    if(this == &other)
        return *this;

    // This is not exception safe.
    // T is completely unknown to you.
    // Therefore you must assume the person who wrote T is an idiot
    // and therefore may throw an exception from there destructor.
    delete m_p;

    // If the above statement throws then your object is now in an undefined
    // state with m_p pointing at an invalid object.

    m_p = other.release();
    return *this;
}

The correct way to do this.

template <typename T>
auto_ptr<T>& auto_ptr<T>::operator=(auto_ptr& other)
{
    if(this == &other)
        return *this;

    auto_ptr<T> tmp = release();
    m_p = other.release();

    // After the state of your (and other objects)
    // are completely defined and are in a good state.
    // then you can tidy up any left over objects.

    return *this;

    // the destructor of tmp should now release it properly.
}


Function to show or hide validation error messages

You could start by minimizing your function calls. like so

ORIGINAL

$.fn.forceNumeric = function(){
    return this.each(function(){
        $(this).keyup(function(){
            if (!/^[0-9]+$/.test($(this).val())){
                $(this).val($(this).val().replace(/[^0-9]/g, ''));
            }
        });
    });
};

OPTIMIZED

$.fn.forceNumeric = function(){
    return this.each(function(){
        $(this).keyup(function(){
            if (!/^[0-9]+$/.test(this.value)){
                this.value = this.value.replace(/[^0-9]/g, '');
            }
        });
    });
};

  Just in this little bit you have saved on 3 Function calls.

And on the larger part.

OPTIMIZED

function showHideErrorBox(theFriendlyName, check){
     var topLevelDiv,
         numDivsAfter;

        if(check === 0){
        // REMOVE THE OLD ERROR MESSAGE IN THE ERROR BOX
            $('.form-validation-errors').find('div').each(function() {
                var $this = $(this);
                if( $this.html().indexOf(theFriendlyName) !== -1 ){
                    $this.remove();
                }
            });
        }

        //  CHECK WHETHER THE ENTIRE MESSAGE BOX CAN BE REMOVED NOW
        topLevelDiv = $(document.getElementById('alert-form-errors'));
        numDivsAfter = topLevelDiv[0].getElementsByTagName('div').length;

        if(check === 0 && numDivsAfter <= 1) {
            return topLevelDiv.hide();
        }

        if(check === 1 && numDivsAfter >= 1) {
            return topLevelDiv.show();
        }
}

  You have also saved on 3 Function calls here too;

Just remember that $ is a function and it's better to call a function once than twice right?

if you ever have something like this

$(this).show();
$(this).val('hello');
$(this).hide();

It should be like this

var $this = $(this);
$this.show()
this.value = 'hello';
$this.hide();