Map unique raw words to a list of code words

My main concern is the use of cw as a "persistent pointer". Specifically, might people be confused when they see for code in cw?

No. Instead, you can just remove the line cw = iter(code_words) as long as it's a native iterable. "Persistent Pointer" isn't a thing in python, because all python knows are Names.

  What should be the typical best practices in this case?

That would be building a dictionary and using it for the actual translation. You're basically already doing this with your code_by_raw, if a bit more verbose than others might. The only real difference would be that, in my opinion, it would be better to first establish the translation, and then create the result.

Except for your premature result generation, I would say your current function isn't bad. It does what it needs to do, it does it well without stupid actions, but it's not very readable. It's said often, I think you need to factor out a bit of code. Specifically, the bit that handles the fact that your inputs don't have to yield unique values, and how you need to handle duplicates.

I would suggest a  generator to handle that. This simplifies the main function a ton. (A comment pointed me towards the unique_everseen recipe, which is a slightly broader function. We don't quite need all it's functionality, but it might be worth the effort if you need some more flexibility.)

def unique(iterable):
    """ Generator that "uniquefies" an iterator. Subsequent values equal to values already yielded will be ignored. """
    past = set()
    for entry in iterable:
        if entry in past:
            continue
        past.add(entry)
        yield entry

def encoder(raw_words, code_words):
    # Create mapping dictionary:
    code_by_raw = dict(zip(unique(raw_words), unique(code_words))
    # Check if we had sufficient code_words:
    if len(code_by_raw) < len(raw_words):
        raise ValueError("not enough code_words")
    # Do translation and return the result
    return [code_by_raw[raw] for raw in raw_words]

I can't completely tell your experience level with python. For result creation, I'm using comprehensions here.

  Might it be better if I used the following instead?

It would not be bad functionally to use a structure like that, but it's still ugly (but opinions may differ). It basically does the same as my unique() generator up there.


Merit of a "Search" class similar to a "Builder" class

You can generify your ItemManager

class ItemManager<T extends Item> {

    List<T> items;

    public ItemManager() {

        items = Database.retrieveItems();

    }
}

The search method can be refactored with this help of CollectionUtils like this

public Collection<Item> search() {
    return getMatchedItems();
}

And her is getMatchedItems()

public Collection<Item> getMatchedItems() {
    CollectionUtils.select(items, new Predicate() {
    public boolean evaluate(Object o) {
        Item item = (Item)o;
        return containsA(item) && containsB(item) && containsC(item) //explain below
    }
    });

    return items
}

IMO its better to create seperate methods like containsA() , containsB()....

Because as you mentioned you have many attributes(let presume 50), then one big if will sooner or later lend you in problems like readability, maintainability and testing.
You may say that creating separate methods may degrade your performance but jvm handles method inlining pretty well.


Current ISS latitude / longitude position

Currently your code is not re-useable. To make it re-useable you should move the logic inside a function.

To do this the function we will have two functions here: _get_data_from_api and get_current_iss_location. Former gets the data from API and latter one passes that data to LatLon class and returns its instance.

The advantage of having two functions here is that the data fetching part and processing part are now isolated and each function is doing a specific thing. Plus having two isolated functions also allows us to test them easily, for example _get_data_from_api function can easily be mocked to return some data in tests and that way our tests won't rely on this external API.

For basic performance improvement we will be using a global session here to re-use previous connections.

Having a separate class LatLon to represent data allows to access the data later on and it can also have other methods that can be used later to do other stuff.

from time import strftime, localtime

import requests

session = requests.Session()

class LatLong:

    def __init__(self, latitude, longitude, timestamp):
        self.latitude = float(latitude)
        self.longitude = float(longitude)
        self.time = strftime('%T', localtime(timestamp))

    @property
    def latitude_hemishere(self):
        return 'N' if self.latitude > 0 else 'S'

    @property
    def longitude_hemisphere(self):
        return 'E' if self.longitude > 0 else 'W'

    def __str__(self):
        return (
            "<Latitude: {self.latitude}° {self.latitude_hemishere} |"
            " Longitude: {self.longitude}° {self.longitude_hemisphere} at {self.time}>").format(self=self)

    __repr__ = __str__

    def pprint(self):
        print('International Space Station position @ {}'.format(self.time))
        print('Latitude: {self.latitude}° {self.latitude_hemishere}'.format(self=self))
        print('Longitude: {self.longitude}° {self.longitude_hemisphere}'.format(self=self))

def _get_data_from_api():
    response = session.get('http://api.open-notify.org/iss-now.json')
    response.raise_for_status()
    return response.json()

def get_current_iss_location():
    data = _get_data_from_api()
    iss_position = data['iss_position']
    return LatLong(iss_position['latitude'], iss_position['longitude'], data['timestamp'])

if __name__ == '__main__':
    get_current_iss_location().pprint()

Now running the module directly will result in:

International Space Station position @ 21:14:51
Latitude: -51.518° S
Longitude: -93.6953° W

We can also import the function in other modules and get the values:

>>> loc = get_current_iss_location()
>>> loc
<Latitude: -51.616° S | Longitude: -89.694° W at 21:15:31>
>>> loc.latitude
-51.616
>>> loc.longitude
-89.694
>>> loc.time
'21:15:31'
>>> loc.pprint()
International Space Station position @ 21:15:31
Latitude: -51.616° S
Longitude: -89.694° W


MySQL query to find country and state of active users

The first way is less error-prone. When you write like this:

"FROM " . MAIN_DB . ".users AS u " .
"INNER JOIN " . MAIN_DB . ".countries AS c ON u.country=c.id " .

... it's easy to "forget" to add a trailing space on the previous line, which is necessary to separate the clauses of the statement.

This might be slightly better, though it might look somewhat awkward:

" FROM " . MAIN_DB . ".users AS u " .
" INNER JOIN " . MAIN_DB . ".countries AS c ON u.country=c.id " .

Here, it's easier to see the space before the expressions, as they are all aligned to the left, your eyes don't have to scan the right ends to see if you put a space there.

With your first version, this problem doesn't exist: the embedded newlines ensure that the clauses are separated, you cannot accidentally forget whitespace in between, it's naturally there. Another advantage of the first version is that it's easier to copy and paste to a query runner program.

Finally, when embedding variables in long strings, I find the interruptions caused by concatenation quite annoying, so I prefer to use formatting with a template, like this:

$query = sprintf(
    "SELECT u.*, COALESCE(c2.name,u.state) AS real_state, c.name AS real_country, bc.alpha3 AS country_iso, bs.code AS state_iso
    FROM %s.users AS u
    INNER JOIN %s.countries AS c ON u.country=c.id
    LEFT JOIN %s.countries AS c2 ON u.state=c2.id
    LEFT JOIN %s.countries AS bc ON c.country_iso_code=bc.alpha2
    LEFT JOIN %s.states AS bs ON bc.alpha2=bs.country_alpha2 AND COALESCE(c2.name,u.state)=bs.name
    WHERE u.active=1 AND u.mail_activated=1 AND u.blesta_id=0",
    MAIN_DB, MAIN_DB, MAIN_DB, BLESTA_DB, BLESTA_DB);

The drawback of this approach is that now the table names are not so easily visible in their contexts, which can invite mistakes in the argument list.

A helper function from this post on Stack Overflow could improve that:

function vsprintf_named($format, $args) {
    $names = preg_match_all('/%\((.*?)\)/', $format, $matches, PREG_SET_ORDER);

    $values = array();
    foreach($matches as $match) {
        $values[] = $args[$match[1]];
    }

    $format = preg_replace('/%\((.*?)\)/', '%', $format);
    return vsprintf($format, $values);
}

Like this:

$query = vsprintf_named(
    "SELECT u.*, COALESCE(c2.name,u.state) AS real_state, c.name AS real_country, bc.alpha3 AS country_iso, bs.code AS state_iso
    FROM %(main).users AS u
    INNER JOIN %(main).countries AS c ON u.country=c.id
    LEFT JOIN %(main).countries AS c2 ON u.state=c2.id
    LEFT JOIN %(blesta).countries AS bc ON c.country_iso_code=bc.alpha2
    LEFT JOIN %(blesta).states AS bs ON bc.alpha2=bs.country_alpha2 AND COALESCE(c2.name,u.state)=bs.name
    WHERE u.active=1 AND u.mail_activated=1 AND u.blesta_id=0",
    array('main' => MAIN_DB, 'blesta' => BLESTA_DB));


C++ Grid gravity simulation opzimization (Kattis Falling Apples)

Avoid allocating large arrays on a stack. According to the restrictions, grid may take up to half a meg. You are dangerously close to stack overflow.
Avoid single letter identifiers. Spelling out row and col wouldn't make your program any slower, but it would make it much more readable.
scanf adds a null terminator to a string. Your char line[c] is one character short. Definite UB.
The problem statement clearly says that

  Merely iterating the gravity rule, a step at a time, will likely take too long on large datasets.

but that is exactly what your code is doing. No optimization may salvage this algorithm. You need to rethink the approach.

I don't want to spell out a solution, but here are few hints:

Columns are totally independent. Do one column at a time.
Do not drop apples. Count them.
It is beneficial to add an artificial row of obstacles at the bottom.


Taking this from an algorithmic perspective, rather than a python perspective (I don't know python well enough to write something sensible in it).

Your algorithm can be expressed as the logic:

Create a virtual list of balanced strings
Starting from every position in the input string...

find every substring from that point.
check to see whether that substring is balanced
if it is balanced, add it to the virtual list.

from the virtual list, pull the longest substring.

Assuming the list remains virtual (yielded instead of real), then your algorithm is essentially space-complexity O(1). If the virtual list is really generated, then the space complexity is O(n2)

The time complexity is something more than O(n2). Probably closer to O(n3). For each b value ( O(n) ) you find all e values (also O(n) ), and then after doing that, you count each 1 and each 0, which is another two O(n) operations.

So, your code runs at O(n3) not O(n2)

Code Review is not really about helping you rewrite the code, but, there are some changes I can recommend...

Firstly, in complexity analysis, doing two O(n) operations one after the other, is still time-complexity of O(n)  ( time complexity of O(n) means that if you double the size of the input data, the time to execute will double.... and that is true if you have 1 O(n) operation or 100 O(n) operations one after the other).

Use this to your advantage.

I can't spend the time to do it all for you, and I am not even certain how to arrive at a fully O(n) solution, but the process you should probably follow is:

scan all the characters once, and count the 1's and count the 0's - O(n)
find out which one is more common (difference between overall counts) - O(1)

call the common character C
call the uncommon character U

scan all characters again, and calculate - O(n)

where the longest sequence of C's is, and how long it is

scan all the characters again, and count the number of U characters before, and after the longest C sequence. O(n)
Decide whether it is possible to contain the complete longest C sequence in the result.
..... from here it gets fuzzy, and I m not sure you can keep it to O(n), but you get the idea.


Checking if an element exists in an array efficiently and return it

Another way, in addition to the ones Joseph mentioned, you could have a list of questions, and simply remove a question when you ask it.

I.e. something like:

function setQuestion() {
  $scope.question = null;

  if(questions.length) {
    var i = Math.random() * questions.length | 0; // get random index
    $scope.question = questions.splice(i, 1)[0]; // remove question and use it
  }
}

And when the question's answered:

questionsUserAlreadyAnswered.push($scope.question);

This way, you don't need to check multiple arrays.

If you need a "master list", you can do init things like so:

var allQuestions = [...],
    unasweredQuestions = allQuestions.slice(0), // copy array
    answeredQuestions = [];

On another note: Don't use for..in to iterate JavaScript arrays. It doesn't work the same as for..in in other languages. Use a regular for loop or the forEach function.


Is this a correct Factory Method implementation?

Don't use dynamic allocation if automatic objects will do:

unique_ptr<Factory> win7fact(new Win7Factory);
unique_ptr<Factory> win8fact(new Win8Factory);

Should be:

Win7Factory win7fact;
Win8Factory win8fact;


Finally nailed it in Perl. In Python this same approach worked no problem, but clearly Perl (or at least the interpreter run on hackerrank) is much slower.

The "trick" is to extract all the substrings already in the form of an array, preferably already sorted, so that there is no need to do the loop another time inside of is_anagram.

Here is my code, admittedly not a beauty, but now it passes all tests.

use strict;
use warnings;

sub get_all_subarr {
    my ($input_string) = @_;

    my %results;

    foreach my $i (1..length($input_string)-1) {
        foreach my $j (0..length($input_string)-$i) {
            push @{$results{$i}}, [sort(split(//,substr($input_string, $j, $i)))];
        }
    }

    return \%results;
}

sub is_anagram {
    my ($arr_a, $arr_b) = @_;

    return 0 if (join('', @{$arr_a}) ne join('', @{$arr_b}));
    return 1;
}

my @results;

my $t = <STDIN>;
chomp $t;

while($t--) {
    my $input_string = <STDIN>;
    chomp $input_string;

    my $count = 0;
    my $strings_arr = get_all_subarr($input_string);
    foreach my $i (keys(%{$strings_arr})) {
        my $max_size = scalar(@{$strings_arr->{$i}})-1;
        foreach my $j (0..$max_size) {
            foreach my $k ($j+1..$max_size) {
                if (is_anagram($strings_arr->{$i}[$j], $strings_arr->{$i}[$k])) {
                    $count++;
                }
            }
        }
    }
    push @results, $count;
}

print join("\n", @results);


Sum an array of video lengths in JavaScript into HH:MM:SS

The global hours, minutes, seconds are icky. Many of the functions you've defined access and change them, in essence causing side-effects.

Besides, this seems over-engineered. The sumArrayIndex function is a strange mix of specific and generic. It's specific because it expects an array of strings with a delimiter and stuff that can parsed to integers and summed. But it's generic in that it stops there. It's already built for this very specific task, but doesn't go all the way. Instead you have to call it twice (and it has to split everything twice, etc.).

You have 3 tasks here:

convert each time code to the smallest common unit (i.e. seconds),
calculate the total,
format it as HH:MM:SS.

Your sumArrayIndex does some of the first two, rather than just one of them, or all of both.

Also having a function like minToSec is a little much. It's literally just a multiplication. In this context, I'd say that 60 doesn't even count as a magic number. So no reason to wrap it in a function; just multiply.

There also some bugginess: Should the final total be, say, 2:00:01, your code will output "2:0:1" because it doesn't pad the numbers.

And lastly, you're parsing seconds as a base16 number in tallyTime? I don't know why you're using parseInt on something that's already an int, and I certainly don't know why you're expecting it to be base16.

Anyway, here's my (very different) take:

function sumDurations(durations) {
  return durations.reduce((sum, string) => {
    var mins, secs;
    [mins, secs] = string.split(":").slice(-2).map(n => parseInt(n, 10));
    return sum + mins * 60 + secs;
  }, 0);
}

function formatDuration(duration) {
  function pad(number) {
    return `${number}`.slice(-2);
  }

  let hours = duration / 3600 | 0;
  let minutes = duration % 3600 / 60 | 0;
  let seconds = duration % 60;
  let minsSecs = `${pad(minutes)}:${pad(seconds)}`;

  return hours > 0 ? `${hours}:${minsSecs}` : minsSecs;
}

Some notes:

It's two functions for the sake of clarity, though they could be combined or be placed inside an outer function to hide that implementation detail (like pad is nested inside formatDuration).
I've combined steps 1 & 2 in sumDurations.
I'm using some array destructuring to pull out the minutes and seconds after splitting the time code string.
Like your code, you'll get wrong results if the time code contains hours as well. Mine ignores them, yours treats them as minutes - and the minutes as seconds. Fixing this is an exercise left to the reader.
I'm using the | 0 trick, which does the same as Math.floor (for positive numbers that fit in 32-bit integers anyway).
I'm using ES6 string templates/interpolation to build the final string.


This is fairly easy to achieve. You need to make use of callback, which can perform the functionality specific to the method, which your generic method then uses. Something like this:

private void UpdatepPeople<T>(ConcurrentQueue<pPeople> people,
                              Func<PersonClass, List<T>> listProvider,
                              Func<T, bool> updateRequiredTest)
{
    var updateCount = 0;
    var lockObject = new object();

    Parallel.ForEach<pPeople>(people, person =>
    {
        listProvider(person).ForEach(item =>
        {
            if (updateRequiredTest(item))
            {
                lock (lockObject)
                {
                    updateCount++;
                }
            }
        });
    });
}

private void UpdatepPeopleContactNumbersIDs(ConcurrentQueue<pPeople> people)
{
    UpdatepPeople<ContactType>(people,
                               person => person.myPeopleContactsList,
                               contact => contact.ID == 0);
}

private void UpdatepPeopleEmailIDs(ConcurrentQueue<pPeople> people)
{
    UpdatepPeople<EmailType>(people,
                               person => person.myPeopleEmailsList,
                               contact => emails.ID == 0);
}


C# function to execute queries in a SQL Server database

SqlConnection implements the IDisposable interface, and so does the SqlDataAdapter class - any type that implements IDisposable should have its Dispose method called; by disposing the connection you don't need to explicitly close it.

The best way to ensure Dispose gets called is to wrap the object in a using block, which is basically compiler magic for a try/finally block.

using (SqlConnection connection = new SqlConnection(connectionString))
{
    connection.Open();
    using (SqlDataAdapter adapter = new SqlDataAdapter(sql, connection))
    {
        DataSet result = new DataSet();
        adapter.Fill(result);
        return result;
    }
}

The SqlAdapter+DataSet way will work on any version of .net - but it's also a pretty dated way to go about it, and since DataSet also implements IDisposable, returning it makes for awkward code, because normally it's the responsibility of the code that creates an IDisposable object to call its Dispose method - but if you do that (or wrap it in a using block), then the client code will run into an ObjectDisposedException when it tries to use the data set.

Slightly more advanced is the ADO.NET SqlCommand, which lets you create SqlParameter objects to send a parameterized command to the SQL Server. This puts the responsibility of dealing with parameters on the database server, where it belongs (Greg Burghardt's answer hints at this).

If you're not constrained to .net 2.0 then the next best thing would be LINQ-to-SQL, or Entity Framework (or NHibernate, or Dapper, depending on your needs): these more modern tools often eliminate the need to ever concatenate SQL strings, or even to explicitly create SqlParameter objects for a parameterized query - for example this:

public User GetById(int id)
{
    using (var context = new MyDbContext(connectionString))
    {
        return context.Users.SingleOrDefault(user => user.Id == id);
    }
}

Generates a SQL query similar to:

SELECT TOP 1 Id, FirstName, LastName, BirthDate, Gender
FROM dbo.Users
WHERE Id = @id

...without ever needing to explicitly create a parameter or concatenate an arbitrary string into a bit of SQL - and C# looks so much better without SQL string literals all over the place! And the best part, it returns a User object ("entity") instead of a DataTable.

But it's great that you're starting with the lower-level stuff: lots of people start with Entity Framework and don't quite understand what's happening under the hood.

Other issues:

The method being static hints at procedural-style. C# is multi-paradigm, but the overwhelmingly dominant one is the object-oriented one: by working with static methods, you're working with types, not objects. The difference will start showing when you start measuring things like coupling and testability.
Method names should be PascalCase, so execute_query should be ExecuteQuery.
query isn't an ideal name for a string containing an SQL statement, because starting in .NET 3.5 a query is a good name for an IQueryable<T> object that's actually building a query via a LINQ provider. Keep it simple: sql is a perfectly acceptable and descriptive name.


You can remove the overloading of properties from constructor.
This way you can improve the code maintaning and readability.

An example of improvement:

class Style {
    // Properties here
}

class Ppt {
    public function __construct(Style $style) {
    }
}

Now you pass just the object of style with all the needed properties.


Improving Makefile and general C++ project structure

Common Conventions

The C++ compiler is named CXX
What you call RESULT is usually named TARGET
In addition to SRC you probably need OBJ
Commands are usually replaced by uppercase version varaiable names that name themselves.

ie. RM not DEL

Building issues

It rebuilds everything every time for two reasons.

The all: rule performs a clean: before it starts. So obviously it has to rebuild everything:

all: clean $(RESULT) ctags
   # ^^^^^^ Get rid of this

RESULT is dependent on SRC and rebuilds the executable from scratch each time. What you need to do is depend on OBJ and then you only need to link the objects together to build the executable. Each object will have its own dependency and only be re-built if the source file changes.

Note: Any common commands should follow the conventions set by the implicit rules:

The implicit rule for linking is:

$(CC) $(LDFLAGS) n.o $(LOADLIBES) $(LDLIBS)

You have a slightly more complex case but I would thus make my link command:

$(RESULT): $(OBJ)
    $(CXX)  $(LDFLAGS) -o $@  $(OBJ) $(LOADLIBES) $(LDLIBS)

In your case you have a set of extra libs I would then add them as follows

LDLIBS  += -lm -lboost_timer -lboost_filesystem -lboost_system
  #    ^^^^  Notice the +=
  #          This makes it easy to add future enhancements to the makefile

The default rule for building *.o files from C++ files is:

$(CXX) $(CPPFLAGS) $(CXXFLAGS) -c

Your header files inclusion is part of the pre-processor stage so should be added to the CPPFLAGS macro.

CPPFLAGS += $(INCLUDE)

Now your object files will build as you expect.

But normally you don't want to build your object files into the same directory as your source files. This is because you can have different build versions. eg you can have a debug build and a release build. You should never mix and match object from different builds. In fact no compiler manufacturer makes any guarantees about object files built with different compiler flags. So any differences in compiler flags can potentially make the object files incompatible. As a result build your object files into different sub directories based on the type of build.

Personally I build release into a directory called release and debug into a directory called debug. I know very boring. I also suffix any targets with their type.

Say I am building the lib plop.

libplop.<version>.so        // release version of plop
libplopD.<version>.so       // debug version of plop
libplopS.<version>.so       // Single threaded version (only build this if there is a specific difference for a single threaded version of the library that can be exploited (rare))
libplopSD.<version>.so      // Single threaded debug version

This way I can install both normal and debug version of a library and explicitly link other code against them.

Anyway back to your make file. To achieve this I normally do

OBJ     = $(patsubst %.cc,$(BUILD)/%.o, $(SRC))
BUILD  ?= .
TARGET  = projectname$(BUILD_EXT)
   # BUIlD_EXT is the suffix I place on executable to tell if it is debug/relese and version information
   # BUILD defines the build type. If this is not specified an unadulterated
   # build is done into the current directory (rather than debug/release)
   # You can do this by directory building target `make target`

## You can add build specific flags like this
## Set BUILD_EXT using the same technique.
CXX_BUILD_FLAGS_debug   = -g
CXX_BUILD_FLAGS_release = -O3
CXXFLAGS       += $(CXX_BUILD_FLAGS_$(BUILD))

all:  debug ctags
  #  by default build debug version
  #  if you want release you must explicitly ask for it

debug:
    $(MAKE) BUILD=debug target
release:
    $(MAKE) BUILD=release target

target: $(TARGET)

$(TARGET): $(OBJ)
    $(CXX)  $(LDFLAGS) -o $@  $(OBJ) $(LOADLIBES) $(LDLIBS)

$(BUILD)/%.o: %.cc
    $(CXX) $^ -c -o $@ $(CPPFLAGS) $(CXXFLAGS)

Organization of files.

I normally put header and source files into the same directory. And I normally break these into directories based on libraries or executable.

But I add an install target into to my makefile that uploads the binary and required header files to a build directory for other projects so they can use it.

# Note this is not real makefile (pseudo make)
# It is designed to show intention not real make instructions.
install:
   $(generate_source_control_tag)
   $(CP) $(TARGET)  $(BUILD)/bin    # used for executables
   $(CP) $(TARGET)  $(BUILD)/lib    # used for libs
   $(CP) $(TARGET_INCLUDES) $(BUILD)/include/$(TARGET_BASE)/

This way if I am working on a project I can leave it (potentially in mid modification) and work on another library without the two being affected. The two will only ever interact after they have been installed into the build directory.

This means I also add the following too my makefile

 CPPFLAGS += -I$(BUILD)/include
 LDFLAGS  += -L$(BUILD)/lib

Now if project B wants to use the library from project A. Then the convention becomes.

 #include "A/A_HeaderFile_1.h"

 LDLIBS += -lA


There's a couple of things to consider when designing a class -> test relationship.

First, I'd try hard to avoid the situation you're setting up where a command class can crash in its own constructor. It's usually a better idea to make sure that constructors are exception-safe if possible, since you never know what calling code may want to do with them. So the first thing I'd consider is simply turning _extract_files() into a regular public method and not calling it in the constructor.  Among other things, it simplifies the test code by allowing you to create test objects without pre-configuring things -- but it's also better in general.

Which leads to number 2: it's better not to rely on the state of a local hard drive in tests. Ideally a test suite runs in a completely reproducible, zero-entropy environment which never changes  -- not even if your build machine is low on hard drive space, the network connection goes down, a user doesn't actually have a writable tempdir, and so on.  These kinds of conditions are often hard to work around, but thinking them through tends to make for better testing  - and that, in turn, leads to better code.

In your case, _files_from_rgx really wants to be operating on the state of a local directory.  However the 'work' is really coming from the way the regex parses the file list.  So, one way to get good coverage is to refactor Command so that the parsing logic works on a file list (or better yet, any iterable since that's more generous).  In this case if you took the regex-filter function out you might get something like this for a start:

def _files_from_rgx(self, rgx):
    """
    Looks for files in the directory `self.dir_path` with names
    matched to the provided regex and returns those files names.
    """
    f = []
    # incidentally since you're not using the recursion here,
    # you could probably replace this with
    # f = os.listdir(self.dir_path)

    for (dir_path, dirnames, filenames) in os.walk(self.dir_path):
        f.extend(filenames)
        break

    return list(self._filter_by_regex(rgx, f))

def _filter_by_regex(self, rgx, inputs):
    """
    loop over <inputs> yielding out values which are full-name
    matches for the regex <rgx>
    """
    compiled_rgx = re.compile(rgx)

    for each_file in inputs:
        matched = compiled_rgx.match(each_file)
        if matched and matched.span()[1] == len(each_file):
            yield each_file

Now you can test the _filter_by_regex function in isolation :

  def test_filter_by_regex(self):
       example = Command(some_test_args):
       known_regex = ".*\.py"
       sample_data = ['a.py', 'b.py', 'c.cpp', 'd.exe', 'e.py']
       results = list(example._test_filter_by_regex(known_regex, sample_data))
       self.assetListEqual(results, ['a.py', 'b.py', 'e.py']

This doesn't require you to actually create temp files and so on (note that you have to be a good citizen if you do create temp files, because if the tests are running on a build server you don't want to be leaking test files all over the place!).

You didn't include source for _file_from_name() and _names_from_file but it would be a good idea to structure them and their tests so you can test them in the same way, without going to disk.  You'll eventually have a function somewhere what will have to call os.listdir() or os.walk and feed those functions -- but at least the part of the logic that is all yours will be covered by tests.

The test you wrote does one thing that's really great, which is guaranteeing the exception raised by an empty file list.  That's a very good test, because it guarantees predictable behavior for users.  You can make that functionality independently testable with a similar small refactor that moves that into a separate function:

def _validate_file_list(self, file_list):
   if not file_list:
        raise FileNotFoundError("No files match")

And a matching test:

def test_validate_raises_on_empty_file(self):
       example = command(some_test_args)
       should_raise = lambda: example._validate_file_list([])
       self.assertRaises(FileNotFoundError, should_raise)

There's another approach you could take, but I would recommend learning how to do the "old-fashioned" way first.  The mock module would allow you to simulate the state of a disk by replacing calls to things like os.walk with a dummy function that return the same results every time without looking at the disk.  It's a very  powerful technique, but it can encourage code that's otherwise not easily testable.  I'd figure out more ways to isolate and test key code paths without mock before adding it in as a final level of testing.

For a good look at the philosophy of breaking out logic and IO, I highly recommend this talk by Python core dev Brandon Rhodes:  https://youtu.be/PBQN62oUnN8


Your solution lacks the use of higher-order functions that are an essential part of Haskell style. Instead of writing explicitly recursive functions that plumb state around (like an impoverished for-loop) seek out, create, and use constructs that have semantic meaning to your problem.

Starting from the small, the problem defines a growth cycle as a either doubling a tree's height, or increasing it by one. We can encode all of that information in Haskell.

type Height = Integer
type Cycle = Height -> Height

monsoon :: Cycle
monsoon = (* 2)

summer :: Cycle
summer = (+ 1)

Next, we know that the cycles begin with a monsoon and thereafter proceed regularly indefinitely into the future.

timeline :: [Cycle]
timeline = cycle [monsoon, summer]

cycle is a function from the Haskell Prelude module which turns a finite list into an infinite list by infinitely repeating it. Now we can take a finite sequence from the front of the list to determine all of the cycles the tree will experience over a given span of time.

-- code fragment
take n timeline :: [Cycle] -- n cycles from planting

Now given the original height of the tree and the cycles it will experience over a given span of time, we can find the resulting height of the tree.

utopian :: Int -> Height
utopian n = foldl (flip ($)) 1 $ take n timeline

We fold over the finite timeline from the left, applying each growth function to the accumulator (starting with the height of the tree at year 0).


Extracting original values from cumulative sum values

Both aren't great. B is really complicated, which is unnecessary. A has this extra negation:

output[-(i+1)] = cumulative[-(i+1)] - cumulative[-(i+2)]

You could have just used positive indices (why iterate backwards anyway?):

output[i+1] = cumulative[i+1] - cumulative[i]

I think sort of thing is best accomplished with itertools. In particular, there's a pairwise recipe:

def pairwise(iterable):
    "s -> (s0,s1), (s1,s2), (s2, s3), ..."
    a, b = tee(iterable)
    next(b, None)
    return izip(a, b)

Which translates directly into what we want to do:

def C(cumulative):
    yield cumulative[0]
    for a, b in pairwise(cumulative):
        yield b-a

This returns a generator, instead of a list, but generators are typically far more useful anyway. I feel that this approach makes it much more explicit what it is you're actually doing: you're yielding the first element, and then the pairwise difference between the rest.


When approaching object oriented programming, I always try to adhere to a few best practices:

Do one thing, and do it well
Don't Repeat Yourself (DRY)
Favor composition over inheritance

There are many more, but I've found these three practices to be the most useful in creating modular code. That being said, let's explore how your class compares.

Do one thing and do it well

Your User class is doing many things:

It is the Domain Model representing a User in the database along with some business logic (hashing the password)
It is getting data from the $_POST, coupling this class to the HTTP request layer of your application.
It is echo-ing a success flag, coupling this call to the standard output in PHP. Furthermore, if something goes wrong on insert, you can't handle that error. You should be throwing exceptions here instead. See Returning status codes from business layer for additional info
It is inserting data into the database.

Worse yet, you appear to have a SQL injection vulnerability since you are using double quotes and variable interpretation in PHP (see Little Bobby Tables for a humorous explanation). You want MYSQL Prepared Statements.

Don't Repeat Yourself

While no code is repeated inside this class, because it is doing multiple jobs it becomes difficult to reuse the code, which could lead to repetitive code later on.

Favor Composition over Inheritance

Your class doesn't inherit from anything, which is good. It doesn't really need to inherit from a super class. Composition (e.g. holding references to other objects specializing in a different but related task) won't help you here either because about 4 layers of an application have been mashed together into one class.

Splitting your class up into multiple classes

While this will seem like overkill at first, the following code will lay the ground work for a modular and testable application.

user.php

This is your domain model representing the database data. This will also contain methods that implement business logic, for instance, hashing the password. It is both data and behavior.

<?php

class User
{
    private const PASSWORD_BCRYPT = "...";

    private $username;
    private $email;
    private $password;

    public User()
    {
    }

    public User($username, $email, $password)
    {
        $this->set_username($username);
        $this->set_email($email);
        $this->set_password($password);
    }

    public function get_username()
    {
        return $this->username;
    }

    public function set_username($value)
    {
        $this->username = $value;
    }

    public function get_email()
    {
        return $this->email;
    }

    public function set_email($value)
    {
        $this->email = $value;
    }

    public function get_password()
    {
        return $this->password;
    }

    public function set_password($value)
    {
        $this->password = password_hash($value, User::PASSWORD_BCRYPT);
    }
}

users_repository.php

The "Users Repository" is a class whose sole purpose is to perform CRUD operations on the users table (Create, Read, Update and Delete). It implements an interface that you can use later on as a test harness.

<?php

interface IUsersRepository
{
    public function add(User $user);
}

class UsersRepository implements IUsersRepository
{
    private static $connection_string;

    public static function set_connection_string($value)
    {
        UsersRepository::$connection_string = $value;
    }

    private $connection;

    public UsersRepository()
    {
        // $connection = new mysqli(...);
    }

    public function add(User $user)
    {
        $statement = $this->connection->prepare('INSERT INTO "users" ("id", "username", "email", "password") VALUES (?, ?, ?, ?)');
        $statement->bind_param("id", NULL);
        $statement->bind_param("username", $user->get_username());
        $statement->bind_param("email", $user->get_email());
        $statement->bind_param("password", $user->get_password());

        if (!$statement->execute())
            throw new Exception("Execute failed: ({$stmt->errno}) {$stmt->error}");
    }
}

If the INSERT into the database fails, an Exception gets thrown. Failing to save data to the database is a hard stop. You really can't go any further, so an exception is the ideal way to communicate this catastrophic error (yes, failed database operations are catastrophic and your application should blow up in your face).

users_controller.php

This begins the Poor Man's MVC. The UsersController handles the HTTP request.

Notice that it has two constructors. The first one, which takes no arguments, instantiates UsersRepository as the object responsible for database operations. The second constructor accepts any object implementing the IUsersRepository interface, which is the interface implemented by UsersRepository. We'll use this later on as a test harness so we can write unit tests for this class without a database.

<?php

class UsersController
{
    private $repository;

    public UsersController()
    {
        $this->repository = new UsersRepository();
    }

    public UsersController(IUsersRepository $repository)
    {
        $this->repository = $repository;
    }

    public function register()
    {
        return new User();
    }

    public function register($params)
    {
        if (isset($params['username'])) {
            $user = new User($params['username'], $params['email'], $params['password']);
            $this->repository->add($user);

            return '/users/index.php';
        }
    }
}

users/create.php

Lastly, this file, which technically does too much here, is the actual "Create new User" page:

<?php

$controller = new UsersController();

if (isset($_POST))
{
    $redirect = $controller->create($_POST);
    header($redirect);
}
else
{
    $user = $controller->create(); ?>

<!DOCTYPE HTML>
<html>
    ...
    <body>
        <form method="post" action="./create.php">
            <input name="username" type="text" value="<?= $user->get_username() ?>">
            <input name="email" type="text" value="<?= $user->get_username() ?>">
            <input name="password" type="password" value="<?= $user->get_username() ?>">
            <button type="submit">Submit</button>
        </form>
    </body>
</html>

<?php } ?>

It instantiates a new UsersController. If the $_POST is set, then it passes that array into the $controller->create method, which performs the actual logic for extracting the user information from the HTTP request and inserting it into the database. Furthermore, it directs the application flow, for instance redirecting the user to another page upon successful insert (which prevents a double-POST of the same information as well).

The UsersController also has a method just called create() which takes no parameters and returns a User object, which gets displayed in the HTML form when the $_POST is not set.

Really, you'd want to take advantage of one of the many fantastic MVC frameworks out there for PHP for the users_controller.php and create.php files, because the MVC application should be responsible for routing an HTTP request to a controller, and then a specific PHP file for the view should provide the web page.

By splitting up your code into layers, you create a modular and testable application:

User - Business logic
UsersRepository - Data storage and retrieval
UsersController - Handle application flow
create.php - a not-so-perfect view

Testing your code

Notice that the UsersController has a constructor which accepts an instance of IUsersRepository. This is the interface implemented by UsersRepository. This allows you to write test code that uses a mock or stub user repository instead of going out to a database.

First, create the "mock users repository" class, which implements IUsersRepository

<?php

class MockUserRepository implements IUsersRepository
{
    public $added_users = array();

    public function add(User $user)
    {
        $this->added_users[] = $user;
    }
}

Next, create your unit test. In this case, I'm using PHPUnit:

<?php

class UsersControllerTest extends PHPUnit_Framework_TestCase
{
    public function addsANewUser()
    {
        $repository = new MockUserRepository();
        $controller = new UsersController($repository);
        $params = array(
            'username' => 'testable',
            'email' => 'test@example.com',
            'password' => 'abc123'
        );

        $redirect = $controller->create($params);

        $this->assertFalse(empty($repository->added_users)));
        $this->assertEquals('testable', $repository->added_users[0].get_username());
        $this->assertEquals('/users/index.php', $redirect);
    }
}

Now you can verify the behavior of your application in an automated and repetitive way. Covering your with unit tests allows you to refactor code or change business rules later on with confidence that you won't introduce bugs.


Storing a fetched list of printers in Angular

"Caching" an Observable

PrintersService is by default a singleton in Angular just like any other service (This is thanks to the way dependency injection container works in Angular). This means you don't need to have the class fields as static. Make them instance fields instead:

private readonly apiUrl = "/api/printers";
private printers: Observable<Printer[]> = undefined;

updatePrinter(printer: Printer) {
  this.printers.subscribe(printers => {
    printers.forEach((element, index) => {
      if (element.id === printer.id) {
        printers[index] = printer;
        this.http.put(this.apiUrl, printer).pipe(
          tap(_ => this.log(`Updated printer id=${printer.id}`)),
          catchError(this.handleError<any>('updatePrinter'))
        );
      }
    });
  });
}

private apiGET(): boolean {
  this.log("GET: " + this.apiUrl);
  this.printers = this.http.get<Printer[]>(PrintersService.apiUrl);
  return PrintersService.printers === undefined;
}

Caching Pattern

I commonly see the following pattern for "caching" observables in various code bases:

const CACHE_SIZE = 1;

this.cache$ = this.requestJokes().pipe(
  shareReplay(CACHE_SIZE)
);

It will not very nicely with the code you have because the pattern addresses the GET calls assuming there's no mutation to the list of Jokes (Printers in your case) such as PUTs, POSTs or DELETEs.

Alternative 1: Observable as BehaviorSubject

You can use a BehaviorSubject<Printer[]> to deal with the state manually. BehaviorSubject is an observable and an observer at the same time. It has methods next(newValue) and getValue() which make manual state control possible.

Alternative 2: More advanced state management

I've been where you are many times. The complexity of manual observable/behaviour subject led me to a conclusion the approach is not scaling well. It holds me back from actually progressing on feature work itself.

My recommendation is to look at a state management library like @ngrx/store. The library ideas revolve around a few concepts one which is Store. The store represents the state of your application that is exposed as an observable. The operations that change (update) the stored are written in a special way (via so called "reducers").

In my experience, the apps written with @NgRx are simpler in building and even simpler in maintenance. You'll need invest some time in picking up the library's approach, API, and so on but the overall entry barrier is relatively low.

Side notes

Use interface instead of class if possible.
Do not use keywords as field names if possible, (i.e. default).
Use is* or are* in member names to denote the boolean nature of the member. (I've seen word "default" used as a verb with the meaning of reset to default(s)).

export interface Printer
{
  id: number;
  name: string;
  isActive: boolean;
  isBlacklisted: boolean;
  isDefault: boolean;
}


A few things to note before I explain improvements

Never forget var when declaring variables. I saw this in the code.

email = subscriberInput.val();

Now, best case scenario is that you declared email somewhere in the higher scopes. However, if it wasn't then JS will declare it as a global, which may have unforseen negative effects.
If you run a site that has translation capabilities, I suggest you don't hard-code messages. Use translation libraries instead. That way, you can globally change language with just a flick of a switch.

Then here's the improved code, given what you have:

subscriberInput.keyup(function (event) {

  // If you intent to prevent the default action of the enter, then I
  // suggest preventDefault() instead of return false
  event.preventDefault();

  // I recommend the "return if fail" pattern rather than "run when true"
  // because it removes unnecessary indentation. It's a case to case basis.
  if (event.which !== 13) return;

  $.post("/en/subscriber/add", {
    // The context of the function is the input box. We can simply use
    // the value property instead of calling out val()
    email: this.value
  }).done(function (data) {

    // Similar to above, use strict comparison as much as possible
    if (data === "success") {

      // Avoid repeating code by factoring out common code. In this case,
      // what set the code apart was the message.
      var message = isEnglish ? "You're subscribed!" : "Sunteti abonat cu success";

      subscribeBox.find('div.row')
                  .remove();

      // jQuery actually has a delay() function which can delay animations.
      // Better than using timers manually.
      subscribeBox.find('h5')
                  .addClass('green-text')
                  .text(message)
                  .delay(5000)
                  .fadeOut('slow');

    } else {

      // Now I don't know why you are doing the alert. If you are debugging,
      // I suggest you use console.log() or a custom logging library.
      alert(data);
    }
  });

});


According to information in VBScript Data Types MSDN article, a 32-bit integer is long variant subtype.

However, the value &h8000 (decimal  -32768) seems to be a 16-bit integer minimal value; it's hexadecimal &hFFFF8000 if converted to long subtype (check Wscript.Echo Hex(&h8000).

This means that your function miscounts 1 additional bit for all long values above 32767 i.e. above 16-bit integer maximal value &h7FFF.

To check the sign bit of a long value correctly, use &h80000000 as follows:

Function countBits(value)
  Dim mask : mask = 1
  Dim i
  countBits = 0                    ' no need to use any auxiliary variable
  For i = 0 to 30
    If ( value And mask ) > 0 Then
      countBits = countBits + 1
    End If
    mask = mask * 2                ' got lost due to your own edit Dec 11 '16 at 14:22
  Next
  If ( value And &h80000000 ) Then ' check the sign bit
    countBits = countBits + 1
  End If
End Function

Sample tests:

Wscript.Echo countBits(2^16)         '  1
Wscript.Echo countBits(2^30)         '  1
Wscript.Echo countBits(1)            '  1
Wscript.Echo countBits(-1)           ' 32
Wscript.Echo countBits(-2)           ' 31
Wscript.Echo countBits(-2147483648)  '  1


JQuery Promise Interface for a (very) simple JSON Parser

Chop up functions longer than 12 lines of code into smaller functions.
Since you're using this, then you can attach the additional functions to the prototype on Parser.

Here's what I came up with.

function Parser(json) {
    var dfd = $.Deferred();
    dfd.notify("Parsing");
    if (json && typeof json === "object") {
        if (json.hasOwnProperty("GetDataSources")) {
            this._handleData( dfd, json.GetDataSources, SourceParser, "Source" );
        } else if (json.hasOwnProperty("GetData")) {
            this._handleData( dfd, json.GetData, DataParser, "Data" );
        }
    } else {
        dfd.reject("There was a problem reading the JSON");
    }
    return dfd.promise();
}
Parser.prototype._handleData = function(dfd, data, OtherParser, type){
    if (data && this.isSuccessfulResponse(data.Response)) {
        dfd.notify("Parsing " + type );
        var otherParser = new OtherParser(data);
        otherParser.done(dfd.resolve);
        otherParser.fail(dfd.reject);
    } else {
        dfd.reject("Parsing "+ type +" Failed");
    }
};


Given an array of integers, return all pairs that add up to 100

okay, my javascript is not great but this should find all pairs in a single pass.

this assumes all values are between 0 and 100, like in the question.

Here is a working JSFiddle that shows the code in action.

var sample_data = [0, 1, 100, 99, 0, 10, 90, 30, 55, 33, 55, 75, 50, 51,
                      49, 50, 51, 49, 51];

var found = {};
var results = [];
for(var i of sample_data) {
    if (found[100 - i] === true) {
        // pair found
        results.push({
            a: i,
            b: 100 - i
        });
    }

    found[i] = true;
}

essentially, found is used as a hashset to see if the required reciprocal has already occurred in the array.

This makes the code more concise while being as easy to understand, therefore  more maintainable. The code is also likely to have superior performance to code with nested loops.


Multiple returns in a method or one return

It's debatable. Or perhaps I'd say: it's very dependent on context.

If you return early (as you do here), I think it's probably good to return []. When you read the code, you see the conditional and its effect: an empty array is returned, period. To me early returns feel a bit like throwing exceptions - not in the sense that there's an error, but just "this is a special case; do something special and return control to the caller (i.e. exit the function)"

The other way to go about it is, of course, to return at the end of the function, and treat the steps before that as optional (or "conditional" to be exact). They may do something to the customerIds array, or they may not. Regardless, we return the array at the end, whether it's empty or not.

(note: I'm using all these words, like "normal" or "special", very lightly. Don't read too much into them)

Either way is perfectly acceptable, so it's pretty much up to you. You've got two dependent conditionals, so returning early is a good way to avoid a ton of indentation and nesting.

However, if the return value "changes" when the function returns early, and you don't otherwise declare variables at the top, I'd skip declaring customerIds there too. If the idea is to declare the thing you'll return, but then you don't return that specific object anyway, it's misdirection. Though again, this is really on the margins of "does it matter?" because you're always returning an array, so it's not like the var declaration is totally false.

But, but, in your case, the simplest solution might be to not have that declaration at all. If you expect the code to be read, and people to see the return [];, you can also expect them to read to the end of the function anyway, and see what it would otherwise return. No need for the return-type pseudo-declaration at all. Especially as it's not needed at all (the concat call is pointless). (see update below the code)

function getCustomerIds() {
    var linkedContact = getLinkedContacts(this);
    if (linkedContact === null) {
        return [];
    }
    var linkedResource = getLinkedResources(linkedContact);
    if (linkedResource === null) {
        return [];
    }
    return getLinkedCustomers(linkedResource);
}

Update: As 200_success points out in the comments, the semantics do change when returning without using concat. However, I'd probably still prefer a more direct route than declaring an empty array (that I might not use for anything), and using concat. I'd probably do this

return getLinkedCustomers(linkedResource).slice(0);

to get a copy of the array returned by getLinkedCustomers(). End of update

No declaration necessary. Your function's name (which is plural) should already be a strong enough hint that an array will get returned. Or, you know, add a comment... that's a pretty good way to deal with this too, now that I think about it.

So basically, I think it's your custom of "kinda' sorta'" declaring the return type that's interfering here. Since such a declaration is not exactly binding or enforced by JS (at all), the early returns muddy the picture.

Or (for fun) if you just want to confuse people for a moment :)

function getCustomerIds() {
    var linkedContact = getLinkedContacts(this),
        linkedResource = !linkedContact || getLinkedResources(linkedContact);
    return linkedResource ? getLinkedCustomers(linkedResource) : [];
}

Update: If I'm correct in assuming that all the getLinked...() functions return similar types, then we can deduce that they all return either an array or null.

If that's the case, then both my suggestion (not the joking one above, but the actual one higher up), and your code may give some unexpected results.

The original customerIds.concat(linkedCustomersIds) will result in [null] - not an empty array.

My code will either return the null directly, or - if you use the slice call - it'll just fail, as null doesn't have a slice function.

So there should perhaps be a 3rd conditional for null-checking the return of getLinkedCustomers(linkedResource) and deciding what to return.

But again, this is me making some straight-up guesses at how the rest of the code works. No idea if it's actually an issue.

However, it's confusing that your variables are singular, but hold the return values of functions that are plural. Given the plural function names, I'd assume the return values are (nominally) arrays, but given the singular variables, I'd assume something else (anything else, really).


Detect whether a string has a letter 'b' 3 characters after 'a'

Using a regular expression is definitely the way to go — that's exactly what they are good at doing.  The following regex looks for 'a', followed by any number of spaces, followed by a non-space character, followed by any number of spaces, followed by a non-space character, followed by any number of spaces, followed by 'b'.

function bThreeAfterA(str) {
    return /a *[^ ] *[^ ] *b/.test(str);
}

console.log('after badly', bThreeAfterA('after badly'));
console.log('Laura sobs', bThreeAfterA('Laura sobs'));

You should be aware, though, that you are using .some() suboptimally.  The .some(callback) method stops executing as soon as the callback returns a true value.  But you've already built trueOrFalse by analyzing the entire string, instead of taking advantage of that short-circuiting.  Therefore, if you use .some(), you shouldn't also use .map().

function bThreeAfterA(str) {
    // Array of characters without spaces
    var chars = str.split(' ').join('').split('');
    return chars.some(function(c, i, chars) {
        return c == 'a' && chars[i + 3] == 'b';
    });
}

console.log('after badly', bThreeAfterA('after badly'));
console.log('Laura sobs', bThreeAfterA('Laura sobs'));


Check if multiple lists contain an element from a large external list

In the question, and as suggested by Mathias Ettinger, the reasoning behind locating an \$O(1)\$ search time-complexity as opposed to the current \$O(n)\$ complexity is correct.

However, the best approach is to use the set data structure instead of the list structure. Sets have \$O(1)\$ search time complexity since they are implemented using hash tables (https://wiki.python.org/moin/TimeComplexity) and they are similar to a list conceptually, and thus make much more sense than complicating things by using a dictionary.

The code (with the large dimensions mentioned in the question) runs in under 10 seconds like so:

list1 = [['hello', 'my', 'name'], ['this', 'is', 'stack', 'exchange']]
key_words = ['hello', 'name', 'stack']

def get_features(sentence, key_words):
    return [word for word in sentence if word in key_words]

f = []
key_words = set(key_words)
for sent in list1:
    f.append(get_features(sent, key_words))


There are a few performance tricks we can apply here:

add __slots__ to the class definition should help with memory and performance as well:

class _RegExLib:
    """Set up regular expressions"""
    # use https://regexper.com to visualise these if required
    _reg_school = re.compile(r'School = (.*)\n')
    _reg_grade = re.compile(r'Grade = (.*)\n')
    _reg_name_score = re.compile(r'(Name|Score)')

    __slots__ = ['school', 'grade', 'name_score']

    def __init__(self, line):
        # check whether line has a positive match with all of the regular expressions
        self.school = self._reg_school.match(line)
        self.grade = self._reg_grade.match(line)
        self.name_score = self._reg_name_score.search(line)

using next() instead of .readline() should be faster as it uses a lookahead buffer internally:

with open(filepath, 'r') as file:
    line = next(file)
    while line:
        reg_match = _RegExLib(line)

        if reg_match.school:
            school = reg_match.school.group(1)

        if reg_match.grade:
            grade = reg_match.grade.group(1)
            grade = int(grade)

        if reg_match.name_score:
            value_type = reg_match.name_score.group(1)
            line = next(file, None)
            while line and line.strip():
                number, value = line.strip().split(',')
                value = value.strip()
                dict_of_data = {
                    'School': school,
                    'Grade': grade,
                    'Student number': number,
                    value_type: value
                }
                data.append(dict_of_data)
                line = next(file, None)

        line = next(file, None)

Some of the code style and other notes:

file is a builtin (in Python 2.x only), consider a different variable name
define your regular expression strings as raw strings
you can probably replace .* wildcard with a more concrete \d+ for the "grade" regex: Grade = (\d+)\n


VBA - recursive combination listing based on Excel columns

I continually get Subscript Out of Range at

vOut(outCnt, arrayIndex1) = Split(Mid$(s, 2) & "," & arr(i, lInd), ",")(arrayIndex1 - 1)

ByRef

Sub Arrangements(ByRef arr, ByVal s As String, ByVal lInd As Long, ByRef outCnt As Long, ByRef vOut As Variant)

Arrangements arr, s & "," & arr(i, lInd), lInd + 1, outCnt, vOut

Why are you passing all these arguments ByRef? Especially because you're calling the procedure from within the procedure. By definition the arr passed ByVal will always persist as will the outCnt. The vOut is being passed back as well, so it doesn't need to be ByRef.
Do you see what I mean? If you had this -
Sub main()
    Dim i As Long
    i = 1
    adding i
End Sub
Sub adding(ByVal i As Long)
    If i > 0 Then i = i + 1
    adding i
End Sub

The i would increase every time it's passed back. What ByRef would be used for is making changes to something that isn't passed back.
That being said, why are you calling your procedure from within your procedure anyway?

Sub Arrangements(ByRef arr, ByVal s As String, ByVal lInd As Long, ByRef outCnt As Long, ByRef vOut As Variant)
For
  If
    If lInd = UBound(arr, 2) Then
   Else
       vOut(outCnt, lInd) = arr(i, lInd)
       Arrangements arr, s & "," & arr(i, lInd), lInd + 1, outCnt, vOut
End If

Seems to me if your second If isn't true, maybe you need to go to the next For? Or does it need to re-perform the entire process again?
Maybe I'm having trouble understanding it. I have no idea what lInd is.
Variables
Give your variables meaningful names. This makes following the code easier and it also makes future you happy that you can take a look and know what is happening without tracing the entire procedure.

arr - tell me of what. e.g. arrayOfCombinations()
outCnt = it's a count of what and where is it going out?
vOut - once again, what is this?
s -?
arrayIndex1 - why is this #1? Is there a second one? Try to never put numerical digits in a variable's name
lInd - this is created as a ByVal argument and then sent back to itself as its own value. I - what?!

I'm not being mean, but how would I ever be able to figure out what lInd is if it's never truly defined and has a name that doesn't tell me much. That's very poor logic (of the procedure, not you).
Magic numbers
I see the number 1,000,000 twice. What is it? The number of combinations?
Const TOTAL_COMBINATIONS as Long = 1000000

Or name it whatever it is.


Maximise adjacent numbers in a functional style (PE#11)

You can parse your grid using some very useful functions in kotlin-stdlib:

val numbers = grid.lines().map { line -> line.split(' ').map(String::toInt) }

Personally I prefer "row/column" instead of "y/x". Thinking "y" before "x" is unnatural.
You can always define your own methods too which can improve readability. e.g.:

fun Iterable<Int>.product(): Int = reduce(Int::times)

Instead of using Math.max you might check to see if the given product is greater than the currently known maximum product and then assign it if it is. This is minor but personally I prefer to avoid unnecessary assignments.
I wouldn't worry about trying to do all of this in one pass. Each directional slice has different row/column bounds so I think four separate loops is the clearest/cleanest.
You can remove some duplicated code when it comes to calculating the product and updating the maximum.

e.g.:

var product = 0

fun maximizeProduct(numbers: List<Int>) {
    with(numbers.product()) { if (this > product) product = this }
}

// Horizontal
for (y in 0 until numbers.size)
    for (x in 0 until numbers[y].size - 3)
        maximizeProduct(numbers[y].slice(x..x + 3))

// Vertical
for (y in 0 until numbers.size - 3)
    for (x in 0 until numbers[y].size)
        maximizeProduct(numbers.slice(y..y + 3).map { it[x] })

// Down Right
for (y in 0 until numbers.size - 3)
    for (x in 0 until numbers[y].size - 3)
        maximizeProduct(numbers.slice(y..y + 3).mapIndexed { i, it -> it[x + i] })

// Down Left
for (y in 0 until numbers.size - 3)
    for (x in 3 until numbers[y].size)
        maximizeProduct(numbers.slice(y..y + 3).mapIndexed { i, it -> it[x - i] })

println(product)


The most glaring problem in your code here, is that you open and close the file on each call to the merge function. You should instead open it once in the calling function, and pass in the already-open files to the merge method. The open and close operaions are both slow, and should be minimized.

I recommend that you re-work that part of the solution, then re-post as a 'follow on', but then also include the calling part of the function in to your question so that more of the overall structure of your solution can be reviewed.

The variables a, b, l, r, m i, j, and k are also all horrible names... you should rename them to be 'self-documenting'. They should be called what they are. The variable l is particularly problematic because it is so easy to confuse with 1.

I struggle to understand the difference between the a and b files. It appears that you are copying the first chunk of data from the a-file to the b-file, and then merging from two different places in the b file back to the a-file.

I would recommend instead having two read-poitns on the b file and then write back to the A. The extra copy is slowing you down too. There's no need to copy from a-to b if you can jsut swap them...


That is a very good start.  However, look into moving the loop to get all the books into the GetAllReservations() method in the concrete class that implements IReservationRepository.  When I see the method name "GetAllReservations" in the interface IReservationRepository I would expect it would return all the reservation DTOs fully populated.  You can still keep the logic separate in the repository, it would just have a call to GetByBookId method.  This means the constructor on MyOracleReservationRepository would have to change, but you can hide that in a factory.

namespace LibraryRepository
{
    public interface IReservationRepository
    {
        List<LibraryDTO.Reservation> GetAllReservations();
    }

    public interface IBookRepository
    {
        LibraryDTO.Book GetBookByID(int bookID);
    }

    public class LibraryRepositoryFactory
    {
        public virtual IReservationRepository GetReservationRepository()
        {
            IBookRepository bookRepo = GetBookRepository();

            return new MyOracleReservationRepository(bookRepo);
        }

        public virtual IBookRepository GetBookRepository()
        {
            return new MyOracleBookRepository();
        }
    }

    public class MyOracleReservationRepository : IReservationRepository
    {
        private IBookRepository m_BookRepo;

        public MyOracleReservationRepository(IBookRepository bookRepo)
        {
            m_BookRepo = bookRepo;
        }

        public List<LibraryDTO.Reservation> GetAllReservations()
        {
            List<LibraryDTO.Reservation> reservations = GetReservationsFromDatabase();

            foreach (LibraryDTO.Reservation reservation in allReservations)
            {
                int bookID =reservation.ReservedBook.BookID;
                LibraryDTO.Book book = bookRepository.GetBookByID(bookID);
                reservation.ReservedBook = book;
            }
            return allReservations;
        }

        private List<LibraryDTO.Reservation> GetReservationsFromDatabase()
        {
            int databaseValueResID1 = 1;
            DateTime databaseValueResDate1 = System.Convert.ToDateTime("1/1/2001");
            int databaseValueResBookID1 = 101;

            List<LibraryDTO.Reservation> reservations = new List<LibraryDTO.Reservation>();

            LibraryDTO.Reservation res = new LibraryDTO.Reservation();
            res.ReservationID = databaseValueResID1;
            res.ReservedDate = databaseValueResDate1;
            res.ReservedBook = new LibraryDTO.Book();
            res.ReservedBook.BookID = databaseValueResBookID1;
            res.ReservedBook.Author = null; //Set as null as we don't have values in Reservation DAL
            res.ReservedBook.BookTitle = null; //Set as null as we don't have values in Reservation DAL

            reservations.Add(res);
            return reservations;
        }
    }

    public class MyOracleBookRepository : IBookRepository
    {
        public LibraryDTO.Book GetBookByID(int bookID)
        {
            LibraryDTO.Book book = null;
            if (bookID == 101)
            {
                book = new LibraryDTO.Book();
                book.BookID = 101;
                book.Author = "First Author";
                book.BookTitle = "Book 1";
            }
            return book;
        }
    }
}

Now your ReservationManager would look something like this:

namespace LibraryBL
{
    public class LibraryBLFactory
    {
        public ReservationManager GetReservationManager()
        {
            var repoFactory = new LibraryRepository.LibraryRepositoryFactory();
            LibraryRepository.IReservationRepository reservationRepo = repoFactory.GetReservationRepository();

            return new ReservationManager(reservationRepo);
        }
    }

    public class ReservationManager
    {
        LibraryRepository.IReservationRepository reservationRepository;

        public ReservationManager(LibraryRepository.IReservationRepository resRepositroy)
        {
            reservationRepository = resRepositroy;
        }

        public List<LibraryDTO.Reservation> GetAllReservations()
        {
            return reservationRepository.GetAllReservations();
        }

    }

}

I am sure there is a bit of refactoring that can be done but hopefully you get the idea.

The idea of the repository is to hide how the data is stored and retrieved from other components.  In this particular example ReservationManager.  How would the ReservationManager know that after it got all the reservations it would need to go back to the repository and load all the books?  What if there was another consumer of the repository?  How would it know to do that?  The fact you added a comment to indicate why a for loop is needed should be an indication the loop was not in the right place.

As for your other questions.  I am not sure what your intention is for Generics, I cannot see a good place where it could be used.  As for using data contracts, I would not be in favor of that.  The LibraryBL namespace should exist in it's own set of class libraries another service library should invoke through interfaces.  The service library should be the one consumed directly by the WCF service.  And it would map your DTOs to the Data Contracts.  This will keep your code decoupled, because if you ever had to change your logic in LibraryBL you wouldn't really have to worry about how it affects the WCF service, unless you alter a property in the DTOs, but then you would only have to worry about the mapping.


Multiview for inserting and editing users

It certainly wouldn't do any harm and if you users have a backend that does not do something like encode when the user's name goes into the database then this will prevent any nasties.


A few things that I noticed:

Nesting

You're nesting selects two levels deep. That's usually a bad sign for most of the data mining and report generation I had to do. You might be able to get rid of the innermost Select with some clever thinking.

This may greatly improve performance for that query.

Use a CTE

You can possibly simplify the second select into a "Common Table Expression" (CTE) like so:

WITH (SELECT
    Region
    , Year
    , Month
    , SalesPersonNR
    , COUNT(SalesNumber) as total
    , expr1
FROM  -- ...

Formatting

As you may have noticed above I personally prefer the leading comma style for column specifications. I find it easier to maintain, because you can just delete a single row and can be sure the SELECT (or whatever else) is still valid.

Additionally I strongly recommend you indent your Statements according to the level of nesting they're in.

Usually I follow the folllowing structure

SELECT col1
    , col2
FROM tableName
JOIN otherTable ON tableName.id = otherTable.foreign
WHERE condition
    && condition
HAVING condition
    || condition
GROUP BY col1
    , col2
ORDER BY col1

Any time you increase the level of nesting, I'd increase the indent, too, to make it easier to follow.

Naming

Your table aliases are .. suboptimal. I assume this may be because you anonymized them, but A and B are really not great table names.

Same applies to expr1 and x

Magic numbers

What's the idea behind Status 99? If I read that query I have no idea which sales you're actually selecting. Drop a comment there (or even run a self-documenting select onto a named table)


First since you only care about tif extensions just pass that into the Directory.GetFiles

foreach (string file in Directory.GetFiles(path, "*.tif"))

No need for an if statement checking extension

Also you don't need to create a FileInfo object to get the name of the file use

string filename = Path.GetFileName(file);
filename = filename.Substring(0, filename.IndexOf('.'));

returns you the file name without the extension so you don't need to do the array manipulation.

If me since there is only 6 values I wouldn't do a loop statement.  You need to code each switch and set each property so the loop isn't really helping you save code nor time.  Just set them.  if they need to change in the future it's less code to change and the loop and switch would need to change as well.

string filename = Path.GetFileName(file);
filename = filename.Substring(0, filename.IndexOf('.'));
string[] filenameAsArray = filename.Split('_');
if (filenameAsArray.Length == 6)
{
    document.part1 = filenameAsArray[0];
    document.part2 = filenameAsArray[1];
    document.part3 = filenameAsArray[2];
    document.part4 = filenameAsArray[3];
    document.part5 = filenameAsArray[4];
    document.part6 = filenameAsArray[5];
}
bool validationResults = document.ValidateFields();
if (validationResults)
{
    File.Move(file, @"path\to\destination");
}
else
{
    File.Move(file, @"path\to\errorbin");
}

I also changed your object from do to document because do to most English speakers is a verb and they not going to think it stands for document object.  Again for years later whoever looks at this to make it easier to understand.


Not much to review ;)

As @Dave mentioned, __ is a bit of an odd named, for sure it does not give away what it does
You might consider storing __.params separately in this.parameters or this.options

I would need more code to give deeper, insightful comments ;)


how can this be refactored?

Improved application of Single Responsibility Principle, encapsulation, and domain modeling.

Start With Good Data Structures

public class StoreTimes {
    public DayOfWeek Day {get; set;}
    public string StartTime { get; set; }
    public string EndTime { get; set; }
}

It is clear that an "hours of operation" must consist of a day along with open and close times.
So make a proper structure (class) that expresses this concept.
A complete data structure tends to be very flexible. In our case it will solve the problem @mjolka pointed out.
It simplifies the hell out of client code. i.e. dealing with "a list of key-value-pairs of days and store-times"

Ensure Objects Are Constructed Correctly

Constructor parameters forces the client (code) to supply the required things.
Arguments (the values passed via parameters) are easily validated up front before attempting to use bad or incomplete data.

.

public StoreTimes(DayOfWeek theDay, string startTime, string endTime)
    {
        Day = theDay;
        StartTime = startTime ?? "0000";
        EndTime = endTime ?? "0000";
    }

Don't allow Client to Arbitrarily Change State

non-public setters.

.

public class StoreTimes {
    public DayOfWeek Day {get; protected set;}
    public string StartTime { get; protected set; }
    public string EndTime { get; protected set; }

Use Domain Language

Stores open and close, they do not start and stop.

public class StoreTimes {

    public DayOfWeek Day {get; protected set;}
    public string OpenTime { get; protected set; }
    public string CloseTime { get; protected set; }
}

Make Cohesive Classes

OO programming is an exercise in self-reliance, for the classes. A class should know how to do things that is should be doing itself. The client should only have to tell to do it.

At a restaurant you tell them to cook food, you don't do it for them.
You ask to see a menu, you don't write a menu for them.

.

public class StoreTimes {
    public string HoursOfOperation()
    {
        return String.Format("{0} : {1} to {2}", Day, OpenTime, CloseTime);
    }
}

New Class Applying All the Points Above

public class HoursOfOperation
{
    protected List<StoreTimes> TheHours { get; set; }

    public HoursOfOperation()
    {
        TheHours = new List<StoreTimes>();
    }

    public void Add(StoreTimes newOpsHours)
    {
        if (newOpsHours == null) return;

        TheHours.Add(newOpsHours);
    }

    public override string ToString()
    {
        StringBuilder me = new StringBuilder();

        foreach (StoreTimes openTime in TheHours)
            me.AppendLine(openTime.HoursOfOperation());

        return me.ToString();
    }
}

Lessons Learned Applied to Store Class

public class Store
{
    protected int StoreID { get; set; }
    protected string StoreName { get; set; }
    protected HoursOfOperation OperatingHours { get; set; }

    public Store(string storeName, HoursOfOperation operatingHours, int storeID ){
        StoreID = storeID;
        StoreName = storeName ?? "NoNameStore";

        if (operatingHours == null)
            throw new ArgumentNullException("Operating Hours is null");

        OperatingHours = operatingHours;
    }

    public string HoursOfOperation()
    {
        return this.OperatingHours.ToString();
    }

    public override string ToString()
    {
        StringBuilder me = new StringBuilder();
        me.AppendLine(string.Format("Store {0}  ID {1}", StoreName, StoreID ));
        me.AppendLine("Hours Of Operation:");
        me.AppendLine(HoursOfOperation());

        return me.ToString();
    }
}

Client Code

   static void Main(string[] args)
    {
        HoursOfOperation opsHours = new HoursOfOperation();
        opsHours.Add(new StoreTimes(DayOfWeek.Monday, "0900", "1130"));
        opsHours.Add(new StoreTimes(DayOfWeek.Monday, "1300", "1800"));
        opsHours.Add(new StoreTimes(DayOfWeek.Saturday, "0800", "2100"));
        opsHours.Add(new StoreTimes(DayOfWeek.Sunday, null, null));

        Store CostingTons = new Store("CostingTons", opsHours, 1);
        Console.WriteLine(CostingTons.ToString());
        Console.ReadLine();
  }


Is this the right way to do lambda method chaining in python?

You're better off doing an inline list comprehension.
li2 = [name.capitalize() for name in li if name == "amol"]

For the lambda implementation itself, you can leave the map and filter generators until only when you want to convert back to list:
li2 = filter(lambda n: n == "Amol", map(str.capitalize, li))
print(list(li2))


Android Gradle script in multi-module project

Let's see what we have here:

sourceSets.androidTest.allJava.each {
    ...
}

sourceSets leads to SourceSetContainer
.androidTest leads to a single SourceSet
.allJava leads to the allJava property on a SourceSet which leads to a SourceDirectorySet

Now, while there is not a .size() method or anything like it in SourceDirectorySet, or any of the super interfaces of that, there is some Groovy magic that provides a .size() method for all Iterables.

This means that you should be able to do the following:

def androidTestJavaSourceFileCount = sourceSets.androidTest.allJava.size()

Don't be fooled by this one-liner though, the source of the Iterable.size() method is:

public static int size(Iterable self) {
    return size(self.iterator());
}

which calls:

public static int size(Iterator self) {
    int count = 0;
    while (self.hasNext()) {
        self.next();
        count++;
    }
    return count;
}

So it will do the same thing as your previous code did, but it will clean up your part of the code.


Workflow and Design Feedback on an Add / Edit Form

This is my feedback, Please let me know if I am misinterpreting :

1) Storing the entire PersonEntity object in viewstate will have following problems

It will make viewstate heavy and thus make page load slower.
It may not represent the current state of the object.

You can store the personId, recreate personentity object and then call save.
A lot more about viewstate here http://www.freshegg.com/blog/creating-lean-fast-web-pages-view-state_3377

2) You can make the LoadById method static


Hash Tables: Ransom Note - Hacker Rank in Javascript

Performance

There are several performance issues in the posted code:

The loop over noteKeys continues even after it knows that a word is missing. It should stop.
The map of counts of words in the magazine is unnecessary internal storage. A likely question for discussion during an interview is "what if the magazine doesn't fit in memory, how would you change your implementation?". Food for thought.

Program organization

It's best when a function does one thing.
compareNoteMag does two: computes if there are enough words in the magazine, and prints "Yes" or "No".
This should have been a function returning boolean,
with an appropriate descriptive name.

Safety

The second if should have been an else if here:

if(!mag[key]) string = 'No'
if(mag[key] < note[key]){
    string = 'No'
}

Not only this is inefficient (the second if is sometimes evaluated unnecessarily), the expression in the second if is sometimes a comparison between undefined and a number, which can be confusing and lead to mistakes.

Naming

Some of the names could be better:

getCount returns a map of counts (plural), so I would rename to getCounts (plural).
getCount takes an array of words: words would be a natural and intuitive name instead of array.
compareNoteMag computes the answer to print: answer would be a natural and intuitive name instead of string.

Technique

Instead of this:

let count = counts[word]
counts[word] = count ? counts[word] + 1: 1;

A common technique in JavaScript:

counts[word] = (counts[word] || 0) + 1;

Attention to detail

This is very minor, but I notice some sloppiness:

Semicolons are mostly omitted but not always
The semicolon is unnecessary at the end of function checkMagazine(...) { ... };
The comment // Complete the checkMagazine function below. should not be there
The common convention is to put spaces around the parentheses in for(...){ and if(...){


Few points.

There is no need to create a object for SelectionSort. Declare it static.

selectionSort(arr);

Your variable names should be made better. Its not recommended to use 'out' as variable, or int a[]. You can use int inputArr[] as thats more descriptive. Imagine someone is reading your code then if you make their life easy by making your code more descriptive then job is done faster.

You can create a seperate swap method. This will structure your code properly. You can reuse the swap method for other sorting methods too.

private void swap(int out, int min)
{
int temp = a[out];
a[out] = a[min];
a[min] = temp;
}


Use var myKeys = Object.keys(myObject) to get the keys.
Check if a myString exist in the array myKeys using native

var matchingKey = myKeys.indexOf(myString) !== -1

Processing an array? Use this:

var matchingKeys = myKeys.filter(function(key){ return key.indexOf(myString) !== -1 });

Get the value using myObject[matchingKey].

Processing an array? Use this:

var matchingValues = matchingKeys .map(function(key){ return myObject[key] });

No need to check array-boundaries or sizes. It's 100% boundary-safe.


Jasmine unit tests for a function to set attributes on an HTML element

First, your function should follow JS conventions and be camelCase, not PascalCase. The latter is for constructors, but this is just a function. So for the following I'll call it setAttributes instead.

As for your tests/spec: You should structure - and name - them differently. Treat the descriptions as documentation you can read through.

E.g. the describe call should just be describe("setAttributes()"). It's a headline, not a paragraph. The following tests are what constitute the actual description. So, your describe call should not itself do anything but say what's being described. That could be a concept (e.g. describe("user login flow")) but in this case, it's a named function, so let's "describe setAttributes()":

Your function has one primary purpose: It sets multiple attributes. However, you aren't actually testing that! I'd say the most important test would be:

it("sets an element's attributes from an object of names and values", function () {
  var element = document.createElement('div');
  setAttributes(element, {
    style: 'color: red',
    id: 'testElement'
  });
  expect(element.getAttribute('style')).toEqual('color: red');
  expect(element.getAttribute('id')).toEqual('testElement');
});

(Sidenote: There are two expectations in this test case, but you should usually aim for just one expectation for each test case, if you can. This forces you to be precise in your tests, and not test too much at once. Same rule as for writing functions: Do one thing, and do it well. Here, having two expectations isn't too bad, since they're not really testing different things, but in most other cases, aim for just 1.)

Secondly, setAttributes() should return the element it was given:

it("returns the element", function () {
  var element = document.createElement('div');
  expect(setAttributes(element, {})).toEqual(element);
});

And done. That's your function's specification.

For now, anyway. If you have bugs later, start by writing a test case triggers the bug, but also describes how it should work (and therefore fails when you run it), and then fix you code until the tests start passing again. That way, you test suite grows naturally, based on actual bugs encountered.

What you're doing right now is testing the negative again and again, which, yes, is pointless. Your tests basically say that the return value is not specifically zero, it's not a specific string, it's not a given boolean, etc.. You can do that forever.

In short: There's an infinite number of things it shouldn't be, and you can't check them all.

So what you want to check is the positive instead: Does it do what it should do. I.e. do I get the element back that I passed in? If yes, then you can be pretty confident that you're not going to get numeric zero all of a sudden. Especially since you keep calling your function with the same arguments; unless JavaScript is suddenly completely broken, you will get the same result. And it'll never be zero, a string, a boolean, or... etc..

You basically only want to test negatives when you don't know the exact value you're expect, but know that's there's one thing it certainly shouldn't be. Again, consider how your spec is worded; in some cases you want it to document what definitely won't happen, and then cover what will happen in another test.

So if you for instance have a function that is made to be resilient against bad input, you could make spec that says that it won't throw a TypeError on bad input, and one that says it will return null (or something).

So in all:

describe('setAttributes()', function () {
  it("sets an element's attributes from an object of names and values", function () {
    var element = document.createElement('div');
    setAttributes(element, {
      style: 'color: red',
      id: 'testElement'
    });
    expect(element.getAttribute('style')).toEqual('color: red');
    expect(element.getAttribute('id')).toEqual('testElement');
  });

  it("returns the element", function () {
    var element = document.createElement('div');
    expect(setAttributes(element, {})).toEqual(element);
  });
});

Try reading the spec out loud:

  setAttributes() sets an element's attributes from an object of names and values [and] returns the element


The first point would be the separation of concerns: every entity (function, class …) should only be responsible for one task (except, ironically, printing the result).

In your code, one function (conversion) is responsible for everything. Try separating the different issues:

user input
conversion
output

Granted, for such a very small program the result will almost certainly be larger. But it will also be much clearer and easier to extend.

Next up, your use of recursion. I love recursion as much (more!) as the next programmer but in your case a few loops wouldn’t hurt readability.

A third point, magic constants should be avoided. Your use of the conversion factors 2.2 and 1.45 is innocuous enough but it soon becomes a problem in larger programs. Use properly-named constants instead.


Async Log4net logging handler - High CPU usage on async call

I haven't used Autofac, but with Ninject you can use the logging extensions to automagically inject an ILogger into any class' constructor, so you can stick to constructor injection (which is always preferable over all other kinds of dependency injection techniques), with the only drawback that you have a using Ninject.Extensions.Logging; dependency on your module (given that your current code already has dependencies on Log4Net, I don't see it being an issue though).

I haven't used Log4Net either, but you might be reinventing the wheel.

With NLog it looks like you simply put an async attribute in your targets configuration and you're set:

<targets async="true">

And with the IoC container auto-resolving ILogger for you, all you really need is to constructor-inject an ILogger into whatever class requires logging functionality.

I would look into implementing similar functionality with Autofac/Log4Net, or see what I get with Ninject/Log4Net or Ninject/NLog logging extensions, both available through NuGet.


Validating a search string for an SQL text search

Laravel's Eloquent uses PDO's parameterized queries, so you get the same level of protection from SQL injection attacks as using PDO directly. Which is to say, you don't generally need to worry about sanitizing your input with Laravel.

The main exception being if you ever use Eloquent's raw query. You should avoid passing user-supplied input to a raw query at pretty much any price, but that's where you'd need to sanitize.

As for the input validation, you should just be using it to make sure the inputs conform to your requirements. For instance, ensuring that something looks like an email or is alphanumeric. Or to make sure inputs correspond to an extant record in the database. It's almost never necessary to use it for input sanitization and will probably end up confusing both you and the user, later on.


I like the idea of drawing it using arc segments, but it's probably not the most efficient. There's no doubt some extra loss in coverting to a CSS-style rgb(...) string, which then has to be parsed again. And, as you say, it causes the Moiré artefacts. Otherwise, the code looks pretty good.

While I won't guarantee it's always faster (and there are some browser support issues), you can try drawing the the color wheel pixel by pixel to an ImageData object, and pushing that to the canvas all at once. It seems faster, but I've also left out clipping the data to a wheel shape here.

You might also consider a square color field (rather than a wheel). That what this is of course, but the corners of it aren't too useful. Photoshop, for instance, defaults to a square field with saturation / lightness along the axes, and the slider determining the hue.

But Photoshop also has an RGB mode, where two of the components are used for the axes, and the last one is controlled by the slider. If you can construct a field that only requires you to update 1 byte per pixel, that'd probably be faster still.

var canvas = document.getElementById("picker");
var context = canvas.getContext("2d");
var rangeEl = document.getElementById('range');

function drawPicker() {
  // grab the current ImageData (or use createImageData)
  var bitmap = context.getImageData(0, 0, 150, 150);

  for (var y = 0; y < 150; y++) {
    for (var x = 0; x < 150; x++) {
      // offset for the 4 RGBA values in the data array
      var offset = 4 * ((y * 150) + x);

      var hue = 180 + Math.atan2(y - 75, x - 75) * (180 / Math.PI);
      var saturation = Math.sqrt(Math.pow(y - 75, 2) + Math.pow(x - 75, 2)) / 75;
      var value = rangeEl.value;

      saturation = Math.min(1, saturation);

      var hsv = hsv2rgb(hue, saturation, value);

      // fill RGBA values
      bitmap.data[offset + 0] = hsv[0];
      bitmap.data[offset + 1] = hsv[1];
      bitmap.data[offset + 2] = hsv[2];
      bitmap.data[offset + 3] = 255; // no transparency

    }
  }

  // update the canvas
  context.putImageData(bitmap, 0, 0);
}

// setup
drawPicker();
rangeEl.addEventListener("change", drawPicker);

// =========

function hsv2rgb(h, s, v) {
  var c = v * s;
  var h1 = h / 60;
  var x = c * (1 - Math.abs((h1 % 2) - 1));
  var m = v - c;
  var rgb;

  if (typeof h == 'undefined') rgb = [0, 0, 0];
  else if (h1 < 1) rgb = [c, x, 0];
  else if (h1 < 2) rgb = [x, c, 0];
  else if (h1 < 3) rgb = [0, c, x];
  else if (h1 < 4) rgb = [0, x, c];
  else if (h1 < 5) rgb = [x, 0, c];
  else if (h1 <= 6) rgb = [c, 0, x];

  var r = 255 * (rgb[0] + m);
  var g = 255 * (rgb[1] + m);
  var b = 255 * (rgb[2] + m);

  return [r, g, b];
}
<canvas id="picker" width="150" height="150"></canvas>

<input id="range" type="range" min="0" max="1" value="1" step="0.01">

Alternative to that: Cheat! Make the CSS background of the canvas black, and fade the color wheel in front of it using the slider. That's one way to get away with just changing 1 byte per pixel, and letting the browser handle compositing.

var canvas = document.getElementById("picker");
var context = canvas.getContext("2d");
var rangeEl = document.getElementById('range');
var colorWheel = null;

// update the alpha bytes of the color wheel
function updatePicker() {
  var bitmap = context.getImageData(0, 0, 150, 150);
  for (var y = 0; y < 150; y++) {
    for (var x = 0; x < 150; x++) {
      var offset = ((y * 150) + x) * 4 + 3;
      colorWheel.data[offset] = rangeEl.value | 0;
    }
  }

  // update the canvas
  context.putImageData(colorWheel, 0, 0);
}

// setup
drawPicker();
rangeEl.addEventListener("change", updatePicker);

// =========

function hsv2rgb(h, s, v) {
  var c = v * s;
  var h1 = h / 60;
  var x = c * (1 - Math.abs((h1 % 2) - 1));
  var m = v - c;
  var rgb;

  if (typeof h == 'undefined') rgb = [0, 0, 0];
  else if (h1 < 1) rgb = [c, x, 0];
  else if (h1 < 2) rgb = [x, c, 0];
  else if (h1 < 3) rgb = [0, c, x];
  else if (h1 < 4) rgb = [0, x, c];
  else if (h1 < 5) rgb = [x, 0, c];
  else if (h1 <= 6) rgb = [c, 0, x];

  var r = 255 * (rgb[0] + m);
  var g = 255 * (rgb[1] + m);
  var b = 255 * (rgb[2] + m);

  return [r, g, b];
}

// create the initial color wheel
function drawPicker() {
  colorWheel = context.getImageData(0, 0, 150, 150);

  for (var y = 0; y < 150; y++) {
    for (var x = 0; x < 150; x++) {
      var offset = 4 * ((y * 150) + x);

      var hue = 180 + Math.atan2(y - 75, x - 75) * (180 / Math.PI);
      var saturation = Math.sqrt(Math.pow(y - 75, 2) + Math.pow(x - 75, 2)) / 75;

      saturation = Math.min(1, saturation);

      var hsv = hsv2rgb(hue, saturation, 1); // fixed value

      colorWheel.data[offset + 0] = hsv[0];
      colorWheel.data[offset + 1] = hsv[1];
      colorWheel.data[offset + 2] = hsv[2];
      colorWheel.data[offset + 3] = 255;
    }
  }
  context.putImageData(colorWheel, 0, 0);
}
#picker {
  background: black
}
<canvas id="picker" width="150" height="150"></canvas>

<input id="range" type="range" min="0" max="255" value="255" step="1">

Or better yet: Cheat completely. Just put an image of a color wheel on top of a black box, and fade the image using the slider. No canvas, no pixel manipulation, none of that.

var wheel = document.getElementById("wheel"),
    slider = document.getElementById("slider");

slider.addEventListener("change", function () {
  wheel.style.opacity = this.value;
});
#picker {
  width: 150px;
  background: black;
}

<input id="slider" type="range" value="1" min="0" max="1" step="0.01">

That's probably the simplest :)

Note: I haven't benchmarked any of the above. That's left as an exercise to the reader.


To answer the direct question, the obvious way is to have an All method which takes a params Expression<Func<T, bool>>[] expressions argument. You'll still have a 30-line call, but you won't have And on each line. There are potential side-benefits:

It should reduce the amount of wrapping an expression just to unwrap it immediately.
If you decide that you want AndAlso instead of And (is there any reason for not using it?) then it's much simpler to change.

I would also be tempted to favour a refactor which allows null expressions. If instead of

public Expression<Func<Project, bool>> GetProjectByProjectId()
{
    return prj => FilterCriteria.ProjectId == null || prj.ProjectID == FilterCriteria.ProjectId.Value;
}

you have

public Expression<Func<Project, bool>> GetProjectByProjectId()
{
    return FilterCriteria.ProjectId == null ? null : (prj => prj.ProjectID == FilterCriteria.ProjectId.Value);
}

(with whatever type coercion is necessary to make that compile - I haven't tested it) then you can filter out null expressions in All. This both simplifies the expression constructed and the SQL generated from it, which might improve performance and might also make debugging it easier.

Finally, two comments about SubstExpressionVisitor:

    public Dictionary<Expression, Expression> subst = new Dictionary<Expression, Expression>();

On the basis of coding to the interface rather than the implementation, the type should be IDictionary<Expression, Expression>. I personally would also prefer to hide the field and take the substitutions as arguments to the constructor rather than by modifying a public field after construction, but that's a subjective issue of style. If making that change, the type can become IReadOnlyDictionary<Expression, Expression> instead.

    protected override Expression VisitParameter(ParameterExpression node)
    {
        Expression newValue;
        if (subst.TryGetValue(node, out newValue))
        {
            return newValue;
        }
        return node;
    }

Too many people use Contains and then [key] rather than TryGetValue, so thumbs up for getting that right. I would observe, though, that foo.TryGetValue(key, out val) ? val : defaultVal is a common enough pattern that you could consider factoring it out as an extension method to IDictionary<K,V> and IReadOnlyDictionary<K,V>.