Count words with given prefix using Trie

I think the important thing to notice here is that you don't actually need a Trie. The partial queries are not just substrings of the names, they are prefixes. It should therefore be faster to just built a dictionary which directly counts how often each prefix occurs.

Something as simple as this is sufficient (just tested it, it passes all testcases):

from collections import defaultdict

def contacts(queries):
    prefixes = defaultdict(int)
    for op, name in queries:
        if op == "add":
            # maybe not the best/nicest way to iterate over slices,
            # but probably the easiest
            for i in range(1, len(name) + 1):
                prefixes[name[:i]] += 1
        else:
            # gives 0 if name is not in prefixes, no need for `get`
            yield prefixes[name]

This is \$\mathcal{O}(len(name))\$ for each add and \$\mathcal{O}(1)\$ for each find. And since the length of all names is less than 22, the for loop will be very fast. It does not even seem to matter that slicing the string might create additional copies.

In contrast, your code is also \$\mathcal{O}(len(name))\$ for each add, but it is also \$\mathcal{O}(len(name))\$ for each find, as far as I can tell. You need to actually traverse the Trie to find the count. Apparently that, maybe together with the overhead of looking up class methods, is enough to reach the time limit for one of the larger testcases.

Stylistically your code looks very good. The only thing to improve there is adding some docstrings to your methods which explain how to use them.

If you were worried about Python 2 backward compatibility you should inherit from object, but nowadays that is probably less of a concern (especially with one-off code for a coding challenge).


Improving many for loops, any more efficient alternative?

for i in range(10):
    for j in range(10):
        print i*j,
    print ""

Something like this, perhaps?


Spotify API GET curl with auth using php - is this code good?

There's not much you can do wrong here. It works. I don't want to go into details of how to handle Spotify's API, but I would like to comment on your code.
One thing I noticed is that, if there's an error, you call curl_close() twice. That looks like a bug.
You're using curl two times, and in quite a different way. The first time you set individual options with curl_setopt(), the second time you combine all options and call curl_setopt_array(). Your code would be more consistent if you used the same method both times.
Better still, why not make a simple function to do the curl call for you? Something like this:
function callSpotifyApi($options)
{
    $curl  = curl_init();
    curl_setopt_array($curl, $options);
    $json  = curl_exec($curl);
    $error = curl_error($curl);
    curl_close($curl);
    if ($error) {
        return ['error'   => TRUE,
                'message' => $error];
    }
    $data  = json_decode($json);
    if (is_null($data)) {
        return ['error'   => TRUE,
                'message' => json_last_error_msg()];
    }
    return $data;
}

This function combines all the things that are common between the two curl calls, and therefore saves you writing code twice, and the reader reading it twice. This function also checks if the returned json code was valid and tells you what was wrong. You would normally use json_last_error() to detect json errors, but if a Spotify call returns null then there's surely an error.
Given the above function, the first curl call would now become:
$headers  = ['Authorization: Basic '.base64_encode($client_id.':'.$client_secret)];
$url      = 'https://accounts.spotify.com/api/token';
$options  = [CURLOPT_URL            => $url,
             CURLOPT_RETURNTRANSFER => TRUE,
             CURLOPT_POST           => TRUE,
             CURLOPT_POSTFIELDS     => 'grant_type=client_credentials',
             CURLOPT_HTTPHEADER     => $headers];
$credentials = callSpotifyApi($options);
$token = $credentials->access_token;

I haven't tested this code, so there might be errors. The second call will now be very similar:
$headers  = ['Content-Type: application/json',
             'Authorization: Bearer '.$token];
$url      = 'https://api.spotify.com/v1/audio-features/'.$spotify_track;
$options  = [CURLOPT_URL            => $url,
             CURLOPT_RETURNTRANSFER => TRUE,
             CURLOPT_FOLLOWLOCATION => TRUE,
             CURLOPT_ENCODING       => '',
             CURLOPT_MAXREDIRS      => 10,
             CURLOPT_TIMEOUT        => 30,
             CURLOPT_HTTP_VERSION   => CURL_HTTP_VERSION_1_1,
             CURLOPT_CUSTOMREQUEST  => 'GET',
             CURLOPT_HTTPHEADER     => $headers];
$features = callSpotifyApi($options);
var_dump($features);

Writing calls to Spotify consistently this way makes it easier to read the code, and find and correct bugs.
I didn't really review how you use the curl options, or whether this is the best way to use the Spotify API. Others may have more experience with this.


simple shared pointer implementation in single threaded environment

Your implementation misses to implement pointer semantics in the form of an overloaded operator->() and operator*().

Isn't that the whole point of a pointer surrogate?


Just fire the task without async/await.

private void InitMethod(ServiceControl serviceControl)
{
    if (serviceControl != null)
    {
        Task.Factory.StartNew(() => serviceControl.Execute());
    }
}

In a proper F&F task all exception handling (including a final catch, logging, notifications) is done by the task itself, so you don't need exception handling that async/await provides.

Make the method name reflect what it does. I wouldn't mind a name like FireAndForgetXyz


Using PHP's __call() to emulate a Class instead of Extending

A simple workaround, assuming that you want to keep the class a Singleton:

class Database {
    private static $instance;
    private $PDOInstance;

    public static function getInstance(){
        if(!self::$instance){
            try{
                self::$instance = new self();
            }catch(PDOException $e){
                return false;
            }
        }
        return self::$instance->getPDOInstance();
    }

    public function getPDOInstance() {
        return $this->PDOInstance;
    }
}

Well, more of a factory method now, which is silly. But not as silly as __call() / call_user_func_array. If you absolutely need to keep the class's signature as it is, go for it.

If not, a PDO wrapper would be the better approach.


There is no need to urldecode() the url parameters, Cake does this for you. In fact,  you are decoding twice, which is usually a bug.

$url actually contains the hostname. That is very confusing and you should rename that.

Also, that is really deep nesting for such a simple piece of code and a lot of code duplication.

I implemented pretty much the same functionality once.

If you only want to redirect to internal resources, maybe an option would be to only allow relative urls (e. g. http://example.com/login?next=/dashboard). That would save you a lot of trouble:

$defaultRedirect = $this->Auth->redirect();

if(empty($this->params['url']['next'])) {
    return $this->redirect($defaultRedirect);
}

$path = $this->params['url']['next'];

if (strpos($path, '/') === 0) {
    return $this->redirect($path);
}

$this->redirect($defaultRedirect);

This also works around the inconsistencies of $_SERVER['SERVER_NAME'].

Why are you expecting weird urls?

Consider matching the path to your defined routes if you want to redirect to actions of your application only.


So, my first attempt at this solution was very slightly more efficient.  With 2 more instructions, I managed 39 fewer cycles.  I'm not exactly certain right now what the difference is (it's pretty close), but I'm not concerned with that right now.

Thinking about this problem some more, I realized there's a mathematical trick going on here that we can make use of to drastically decrease both the number of instructions and the number of cycles.

A - B == -1 * (B - A)

That is, instead of subtracting left from right for the left column and right from left for the right column, we can just subtract once, pass the positive result to one column and the negative result to the other column.

Using this trick, I'm also able to eliminate an entire node.

As you can see, 201 cycles 5 nodes, 10 instructions.

So, starting with the top left node, we pull A into memory, subtract B, then pass it down.  Middle left just passes it down again.  Bottom left saves it to ACC, passes it down, negates it and passes it right.  The top right node just reads in B and passes it left.  The bottom right node just moves stuff from left to down.

Three of the five nodes are just there to move the values around.

Here's the source...

TOP RIGHT

MOV UP, LEFT

TOP LEFT

MOV UP, ACC
SUB RIGHT
MOV ACC, DOWN

MIDDLE LEFT

MOV UP, DOWN

BOTTOM LEFT

MOV UP, ACC
MOV ACC, DOWN
NEG
MOV ACC, RIGHT

BOTTOM RIGHT

MOV LEFT, DOWN


Updating a dictionary of trajectories after a step has been made

Basic Review

You should import print_function from __future__. So you can use print like you can in Python 3.

Your variable names are poor.

j should be _.
What do j, k and d mean?
Why not just type out parts rather than use pts?

The way you're generating key value pairs in not amazing. If you make a function to build 20 keys then it would be much easier to understand. This is as things are split into two core aspects.

Building key
Building the dictionary

You should really use some functions.

You should really follow PEP 8. Your code is really hard to read because it looks like a block of characters, rather than a Python program.

Functional changes

A trajectory is defined as a list of numbers separated by _.

You should make it a tuple of numbers, (3, 7, 2, 5).

I also save the trajectories in separated dictionaries if they have different numbers of steps.

I see no reason to do this.

You may benefit from using a trie instead.
Since you're just printing the new dictionaries it doesn't make much sense.
However it looks exactly like what you want datatype wise.
I have included a build and an as_dict method to make understanding how it works a little simpler. You can easily remove the need for build by using it to build the trie directly from generate_keys.

from __future__ import print_function
import random

def generate_keys(amount, choices):
    for _ in range(amount):
        yield tuple(
            random.choice(choices[i])
            for i in range(random.randint(1, len(choices)))
        )

class Trie(dict):
    value = DEFAULT = object()

    @classmethod
    def build(cls, mapping):
        trie = cls()
        for keys, value in mapping.items():
            node = trie
            for key in keys:
                node = node.setdefault(key, cls())
            node.value = value
        return trie

    def _as_dict(self, path):
        for key, value in self.items():
            keys = path + (key,)
            if value.value is not self.DEFAULT:
                yield keys, value.value
            for item in value._as_dict(keys):
                yield item

    def as_dict(self):
        return dict(self._as_dict(()))

paths = {}
for key in generate_keys(20, [range(3), range(4), range(5)]):
    paths[key] = random.randrange(10)

trie = Trie.build(paths)
paths_new = trie[1].as_dict()

# Sort the output so it's easier to read
print('Initial dictionary =', dict(sorted(paths.items())))
print('Inital Trie =', trie)
print('Updated dictionaries =', dict(sorted(paths_new.items())))


The key difference is in a technic called "memoization". According to Wikipedia:
In computing, memoization or memoisation is an optimization technique used primarily to speed up computer programs by storing the results of expensive function calls and returning the cached result when the same inputs occur again.
So basically, in the first function, you save all the previous results while on the second function you need to calculate everything again from the beginning for every new call.
A quick search on the internet gives this tutorial if you wish to learn more.


Return the first number found greater than the provided input number (13 digits)

I'm having issues with it being very slow.

What makes your script code slow is that you're reading in the file yourself with the while loop, and apply grep to each single input line, instead of passing grep the file itself and let it just do it's job.

No matter what you want to search for with grep, you should always first pass your input to it with a single call, and inspect the results afterwards.

  For the output I wanted the line number in the file (for slicing) as well as the original line (to confirm/inspect).

grep already has this feature intrinsically (at least as it says from this documentation):

    -n, --line-number
           Prefix each line of output with the line number within its input file.

you simply can do what you want using this option.

Thus you can get rid of your while loop and count variable to determine the line number yourself.


Extract higher value of pair from JavaScript array

Rewrite
Here's a functional approach, with notes inline as comments.
Note that the internal variables are all essentially simple "helper" functions or calculations, which allow us to write the entire algorithm that you're seeking as a single statement (the final return).
The basic idea is to first group together the pairs of "twin" elements.  Once we have that, we just go through the pairs and select the one with the highest weight.
function bestOfPairs(input) {
  var pairId   = elm    => elm.id.split('-').sort().join(''),        // creates an id that's the same for both elements of a "pair"
      byWeight = (a, b) => parseInt(b.weight) - parseInt(a.weight);  // reverse sort by weight: best candidate will be first in array
      grouped  = input.reduce((m, x) =>
        (m[pairId(x)] = (m[pairId(x)] || []).concat(x)) && m         // groups together elements as pairs, based on their id
      , {});

  return Object.keys(grouped).map(id => {                            // each "id" here is the "pairs" id, that is, the id being used to create the groups
    var sorted  = grouped[id].sort(byWeight),
        invalid = sorted[1] == undefined ||
                  sorted[0].weight == sorted[1].weight;
    return invalid ? null : sorted[0];                               // returns the element with higher weight, or null if they're equal, or there's only one
  })
  .filter(x => x !== null);                                          // this removes elments with the same weight
}

Output
[ { id: 'A-B', x: '1', y: '0', weight: '100' },
  { id: 'A-C', x: '2', y: '0', weight: '77' },
  { id: 'A-D', x: '3', y: '0', weight: '95' },
  { id: 'B-C', x: '1', y: '2', weight: '3' },
  { id: 'D-F', x: '5', y: '3', weight: '18' } ]

Finally, note that I'm using ES6 syntax, which is supported by modern browsers, but if you needed support for older browsers you'd need to transpile it using Babel.


System for inputting and monitoring worker shifts

One thing to improve on (if your PHP version allows it), is to consider using PHP's built in password_hash() function. Coupled with password_verify(), it eliminates the need to salt and hash passwords manually. More information can be found in the documentation.


Priority Queue Linked List Implementation

A Priority Queue is synonymous with a heap so I'm assuming that the implication in this assignment was to implement a maximally efficient data structure.

The optimal implementation is spelled out in Wikipedia: http://en.m.wikipedia.org/wiki/Heap_(data_structure)

If it were me, I would use a tree instead of a list. There is no Node class. A Heap object has two nullable Heap children, and every operation is recursive in nature. Length, for example, would just return each child's length or 1 if no children existed. The other operations would take advantage of the tree's structure in order to return results in logarithmic time.


First of all congratulations on a good start to C++ and SFML. You should consult good C++ design and programming practices such as https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md and/or https://www.gitbook.com/book/lefticus/cpp-best-practices/details. But I will point out a few things quickly here:

Typically the hpp file would consist of declarations and the implementation would be done in cpp file. This is recommended for significant implementations such as TimeToNewYear() ctor and the get_counter() method.
Just as you are scoping std::string, doing the same for time, tm, etc. as std::time, std::tm is a good idea - improves readability of the code.
You can also pretty much take all of the contents of your ctor and initialize all the variables as TimeToNewYear::TimeToNewYear() : m_nowTime {std::time(0)}, m_locTime {std::localtime(&m_nowTime)}, ... {} etc. But watch out for the order of initialization which depends on the order of member specification!
You can always return a reference to m_counter by having const std::string& TimeToNewYear::get_counter() so that clients cannot modify the contents. Haven't looked but am almost positive sf::Text::setString method takes a const std::string&. Saves time on making temporaries.

I am not an expert on the SFML side, but having used other windowing systems, these are general guidelines:

I would derive from sf::RenderWindow or compose it in your application specific class such as class MyAppWindow () : m_sfWindow {sf::VideoMode(), ...}, ... {} where m_sfWindow is a private member of your class.
This would permit you to have the event loop inside MyAppWindow::display() or some such method. It is likely that you can sf::RenderWindow::draw() inside the event loop in an already displayed sf::RenderWindow, thus avoiding the sf::RenderWindow::display() call.
It is also possible to have a New Year Countdown button, which when clicked, displays the countdown text box. You can get rid of it with a Close button.

Welcome to C++ (and SFML) development! After doing C++ for 20 years, I am still learning, and am in it for the long haul. Good luck!


Finding capitalized characters and printing their position & ASCII value

Nits

Check out map:

(def string-to-ascii (str)
  (map 'list 'char-code str))

Note that < takes &rest so you can write (< 65 i 90) instead of (and (> i 65) (< i 90)) (you also want <= instead).

Furthermore, return-capital-positon is quadratic in length of the input string - it can be made linear instead.

do- is a usually used as a prefix for iteration macros.
Also, you call string-to-ascii in do-hash twice; you can use let to remove one call.

Your line breaks in hash-table-alist are very confusing.

You can use delete instead of remove in is-capitalized because hash-table-alist returns a fresh list.

Summary

I think your code is overkill.

(defun is-capitalized (str)
  (loop for char across str for code = (char-code char) for pos upfrom 0
     when (<= 65 code 90)
     collect (cons pos code)))

This has the added benefit that the return value increases in pos (i.e., it returns the reverse of your function).

Alternatively, you can actually avoid constants:

(defun is-capitalized (str)
  (loop for char across str for pos upfrom 0
    when (char= char (char-upcase char))
    collect (cons pos (char-code char))))


If you're not worried about performance, you can replace all the manual work by using the <regex> facilities, which results in a considerable reduction of code to test and maintain.

#include <regex>

source = std::regex_replace(source, std::regex("€"), "\x80");

I would still keep it in a separate function to make it easy to change the implementation afterwards.


Go version of Advent of Code, day 5 - Dealing with runes

Your code is pretty idiomatic. The only obvious style thing I'd change is the end of the evaluateIndex function:

if strings.HasPrefix(hash, "00000") {
        return []rune(hash)[5]
}
return 0

you could also write if letter := evaluateIndex(roomId, i); letter != 0 { but it's not that important.

Now, the rune/string/byte question is the interesting one. md5 deals with []byte only; and converting strings to bytes and back is expensive (new memory allocations each time). So I'd change your code to use []byte instead of string, and byte instead of rune. The evaluateIndex function becomes faster:

var prefix = []byte("00000")

func evaluateIndex(roomId []byte, index int) byte {
    bytes := append(roomId, []byte(fmt.Sprint(index))...)
    hash := md5Hash(bytes)
    if bytes.HasPrefix(hash, prefix) {
        return hash[5]
    }
    return 0
}

func md5Hash(bytes []byte) []byte {
    rawHash := md5.Sum(bytes)
    encHash := make([]byte, md5.EncodedLen(len(rawHash)))
    hex.Encode(encHash, rawHash)
    return encHash
}

I put zeroes outside the func, to avoid converting it over and over. You still have a few conversions & allocations, but there's no way around them. The other functions can be modified easily to accommodate the type change.


don't change the input unless explicitly stated

partial.sort() works in-place. This might cause some nasty surprises for someone who uses this function and did not expect this.

The reason you sort this is also only to take the last element. max(partial) would suffice here. If partial is sorted, you can work with a different algorithm using the sortedness

set comprehension

a set comprehension is nice, but here a set(range(partial[-1] + 1)) would suffice

set.difference

there is no need to cast partial to a set

so the complete method reduces to

return set(range(max(partial) + 1)).difference(partial)

alternative method

If partial is sorted, you can work with 2 iterators

from itertools import count

def compare_to_complete_generator(partial):
    complete_iterator = count()
    partial_iterator = iter(partial)

    element_complete= next(complete_iterator)
    element_partial = next(partial_iterator)

    while True:
        while element_partial != element_complete:
            yield element_complete
            element_complete = next(complete_iterator)
        element_complete = next(complete_iterator)
        try:
            element_partial = next(partial_iterator)
        except StopIteration:
            return

list(compare_to_complete(partial))

[0, 5, 7, 9]

For large lists, this has the advantage that you don't have to look for the max and construct a set with max elements. This also accepts any sorted iterable or iterator, not just lists. if the input is not sorted, you can use sorted, but then you lose the advantages of this method.


I see at least one big problem in your method: there is no separation of concerns. Everything is mixed into this method: common rule for password double-checking, business rules for password validation and UI calls for user response.

Extracting the business logic

The business logic in your case if the password validation specific rules, as you named them rule1, rule2a, rule2b, rule2c and rule2d.
Everytime you will need a password validation, you will need these rules to validate a password, therefore I would extract these rules into a BO.

import java.util.function.Predicate;
import java.util.stream.Stream;

public final class Password {
    private final String value;
    private final Predicate<String> rule;

    public Password(String value) {
        this.value = value;
        Predicate<String> rule1 = s -> s.length() >= 8 && s.length() <= 50;
        Predicate<String> rule2a = s -> !s.equals(s.toLowerCase());
        Predicate<String> rule2b = s -> !s.equals(s.toUpperCase());
        Predicate<String> rule2c = s -> s.codePoints().anyMatch(Character::isDigit);
        Predicate<String> rule2d = s -> s.codePoints().anyMatch(i -> !Character.isAlphabetic(i));
        Predicate<String> rule2 = s -> Stream.of(rule2a, rule2b, rule2c, rule2d)
                                             .filter(p -> p.test(s))
                                             .count() >= 3;
        this.rule = rule1.and(rule2);
    }

    public boolean isValid() {
        return rule.test(value);
    }
}

If the rules are likely to change, it would also be possible to refactor this code and take the "final predicate" as a constructor argument.

Extracting "standard" logic

In your case, it is only the rule that assert the password and the confirmation password are equals. This is not a business-specific rule but rather a commonly accepted rule everytime a user creates a password. Moreover it seems you have a "validation flow" defined (first check the equality between the two password, then check if the password in itself is valid) that can be extracted into a separate class

public final class PasswordInteractor {
    public ValidationResult validatePassword(String pwd, String confirmPwd) {
        if (!pwd.equals(confirmPwd)) {
            return new ValidationResult("Passwords must match.", false);
        }

        if (!new Password(pwd).isValid()) {
            return new ValidationResult("Password does not follow instructions above.", false);
        }

        return new ValidationResult("Password set successfully", true);
    }
}

I created a DTO ValidationResult to transmit to the upper layer a composite result since it can be more than true/false.

public final class ValidationResult {
    private final String message;
    private final boolean success;

    public ValidationResult(String message, boolean success) {
        this.message = message;
        this.success = success;
    }

    public String getMessage() {
        return message;
    }

    public boolean isSuccess() {
        return success;
    }
}

Extracting UI calls

Finally the UI calls must stay in the UI-part, namely into the JavaFX controller. Typically it could looks like this (simplified):

import javafx.fxml.FXML;
import javafx.scene.control.Label;
import javafx.scene.control.TextField;
import javafx.scene.paint.Color;

public final class PasswordController {
    @FXML
    private Label responseLabel;

    @FXML
    private TextField pwd;

    @FXML
    private TextField confirmPwd;

    public void validateForm() {
        // validate other parts of the form

        ValidationResult result = new PasswordInteractor().validatePassword(pwd.getText(), confirmPwd.getText());

        if (result.isSuccess()) {
            responseLabel.setTextFill(Color.GREEN);
        } else {
            responseLabel.setTextFill(Color.RED);
        }
        responseLabel.setText(result.getMessage());
    }
}


In the above code, you missed counting the in-between null nodes, that is why, the result is wrong for the input [2,1,4,3, null,5].

You approached the problem using DFS(Depth First Search) algorithm which is useful to find the height of a tree.

BFS (Breadth-first search) algorithm would be the best suitable choice for this problem.


A little explanation first. I have written your code in a different way. This does not mean that my code is perfect in any way. I just wanted you to see other options and possibilities to achieve the same result. I'm open for comments if supported by valid arguments.

First of all I suggest you use another naming convention. This way, the name of your controls and variables have a better meaning. For example 'cVas', I'd rename that to 'DrawingCanvas' or 'DrawingSurface'.

Then there's the structure of your code. You have different shapes that have a similar structure and share a certain functionality. In situations like that, you can start thinking about inheritance and/or the use of interfaces.

Below you'll see the code that I have written. Again, this code is not perfect but it shows how it can be done. And to be honest: I think my code is easier to read, shorter and also easier to expand.

I have used an interface, from which both class derive. This way it was easy to create a Pen-object that will draw an IDrawable object. Using the Pen-object also de-couples your UI from the shape-objects. In the constructor you pass the UIElement (in this code only Canvas can be passed through) on which the pen will draw: they are coupled but in this case this is needed.

Due to the fact of the interface and pen-object, the code in your MainWindow.xaml.cs will also be a lot cleaner. You only need to instantiate one IDrawable-object and depending on the mouse-actions you provide what is needed.

I hope you find this useful and helpful! ;)

XAML:

I have not changed much about the XAML except for the names of the controls and I used a StackPanel instead of a Grid with columns.

IDrawable

public interface IDrawable
{
    void Draw(Point location);
}

MyCircle

//Since you didn't provide the code of your circle-class
//I quickly wrote a dummy-circle class, it works but not
//flawless (X and Y coordinates)
public class MyCircle : IDrawable
{
    public Ellipse Circle { get; private set; }

    public MyCircle(Point location)
    {
        Circle = new Ellipse
        {
            Stroke = Brushes.Black,
            StrokeThickness = 2,
            Margin = new Thickness(location.X, location.Y, 0, 0)
        };
    }

    public void Draw(Point location)
    {
        if(Circle != null)
        {
            Circle.Width = location.X - Circle.Margin.Left;
            Circle.Height = location.Y - Circle.Margin.Top;
        }
    }
}

MyLine

public class MyLine : IDrawable
{
    public Line Line { get; private set; }

    public MyLine(Point location)
    {
        Line = new Line
        {
            Stroke = Brushes.Black,
            StrokeThickness = 2,
            X1 = location.X,
            X2 = location.X,
            Y1 = location.Y,
            Y2 = location.Y
        };
    }

    public void Draw(Point location)
    {
        Line.X2 = location.X;
        Line.Y2 = location.Y;
    }
}

Pen

public class Pen
{
    public Pen(Canvas holder)
    {
        _holder = holder;
    }

    private readonly Canvas _holder;

    public void Down(IDrawable obj)
    {
        //if more shapes are added:
        //better use a switch-statement
        if(obj is MyCircle)
            _holder.Children.Add((obj as MyCircle).Circle);
        if(obj is MyLine)
            _holder.Children.Add((obj as MyLine).Line);
    }

    public void Draw(IDrawable obj, Point location)
    {
        obj.Draw(location);
    }
}

MainWindow

public MainWindow()
{
    InitializeComponent();
    KeyDown += MainWindowKeyDown;
    DrawingSurface.MouseMove += DrawingSurfaceMouseMove;
    DrawingSurface.MouseDown += DrawingSurfaceMouseDown;
    DrawingSurface.MouseUp += DrawingSurfaceMouseUp;
    _pen = new Pen(DrawingSurface);
}

private IDrawable _dr;
private readonly Pen _pen;

private void DrawingSurfaceMouseUp(object sender, MouseButtonEventArgs e)
{
    _dr = null;
}

private void DrawingSurfaceMouseDown(object sender, MouseButtonEventArgs e)
{
    if(LineRadioButton.IsChecked == true)
        _dr = new MyLine(Mouse.GetPosition(DrawingSurface));
    else if(CircleRadioButton.IsChecked == true)
        _dr = new MyCircle(Mouse.GetPosition(DrawingSurface));

    _pen.Down(_dr);
}

private void DrawingSurfaceMouseMove(object sender, MouseEventArgs e)
{
    if(_dr != null)
        _pen.Draw(_dr, Mouse.GetPosition(DrawingSurface));
}

private void MainWindowKeyDown(object sender, KeyEventArgs e)
{
    if(e.Key == Key.Enter)
        DrawingSurface.Children.Clear();
}


I cannot inject the dependency's in the constructor as the OnlinePaymentTransactions are retrieved from using nhibernate.

There's your problem. Your OnlinePaymentTransaction class is/should-be a POCO whose job is to convey data. The Complete() method doesn't belong on that type, it's breaking SRP and making your life much harder than it needs to be.

I'd suggest to introduce another type, call it OnlinePaymentTransactionProcessor or whatever - that type will take the dependencies in its constructor, and have a Complete method that takes an OnlinePaymentTransaction instance.

Kudos for striving to avoid a Service Locator :)


Generating 10-character passwords, with 3-6 digits and 3-6 uppercase letters, in C++

Your passwords will be biased, with more letters appearing towards the front of the password and more digits towards the end.

A better approach would be to determine how many digits you will have in the password. Then, for each character, determine if it should be a digit based on the number of digits you want to have and the number of characters left to fill by checking if (random(characters_left) < digits_left).  Then select either a random digit or letter for that position.

When you declare pass, you don't need to pass it an empty string.  It is default constructed as empty.

std::string pass;


Traversing an array with a pointer to the array

It's not terrible as it stands, but the code could be improved.  Here are some ideas on how to do so.

Minimize work within the loop

If we redefine ptrLastElement to something like this:

const int *end = &arr[SIZE];

Since the pointer starts from the beginning of the array, the for loop could instead look like this:

for (int * curr = arr; curr != end; ++curr)

This matches the use of real iterators in the standard library by having the end of the array be one beyond the last element.

However, modern C++ rarely uses pointers, so I would probably neither use nor advise anyone else to use such a construct.  There's little benefit and the use of pointers can lead to difficult-to-find errors.

Don't use std::endl if '\n' will do

Using std::endl emits a \n and flushes the stream.  Unless you really need the stream flushed, you can improve the performance of the code by simply emitting '\n' instead of using the potentially more computationally costly std::endl.

Omit return 0

When a C or C++ program reaches the end of main the compiler will automatically generate code to return 0, so there is no need to put return 0; explicitly at the end of main.

Note: when I make this suggestion, it's almost invariably followed by one of two kinds of comments:  "I didn't know that." or "That's bad advice!"  My rationale is that it's safe and useful to rely on compiler behavior explicitly supported by the standard.  For C, since C99; see ISO/IEC 9899:1999 section 5.1.2.2.3:

  [...] a return from the initial call to the main function is equivalent to calling the exit function with the value returned by the main function as its argument; reaching the } that terminates the main function returns a value of 0.

For C++, since the first standard in 1998; see ISO/IEC 14882:1998 section 3.6.1:

  If control reaches the end of main without encountering a return statement, the effect is that of executing return 0;

All versions of both standards since then (C99 and C++98) have maintained the same idea.  We rely on automatically generated member functions in C++, and few people write explicit return; statements at the end of a void function.  Reasons against omitting seem to boil down to "it looks weird".  If, like me, you're curious about the rationale for the change to the C standard read this question.  Also note that in the early 1990s this was considered "sloppy practice" because it was undefined behavior (although widely supported) at the time.

So I advocate omitting it; others disagree (often vehemently!)  In any case, if you encounter code that omits it, you'll know that it's explicitly supported by the standard and you'll know what it means.


Verlet integration movement - doubt for on_key_press() methods

def handler(self):
        '''
        Calls Player on_x_key_pressed(), on_x_key_released()
        methods according to event bools set from the listener method.
        '''
        # Moving
        if self.left_key_pressed and self.right_key_pressed:
            self.player.keys_up()

From this point, you know that both keys cannot be pressed at the same time, so you can continue with:

        elif self.left_key_pressed:
            self.player.on_left_key_pressed()
        elif self.right_key_pressed:
            self.player.on_right_key_pressed()
        else:
            self.player.keys_up()

It's up to you to see if you find it more easy to read.

I'm afraid I can't help much with the way you use verlet integration. Last comment: Try to avoid passing the key events around too much, or try to be more 'semantic', eg. send move_left to self.player instead of on_left_key_pressed.


Code style

Always mention what exactly you are importing - never use import * - I don't know these libraries and when I read this for the first time I need to find all possible places where these methods can be from: call, sniff. If you want to write code which other people can read - always import exactly what you are using:

from netaddr import *

Be consistent when you are importing: import argparse is imported in main function which is really unnecessary - import on top with other imports.

Also  - this is picky but it is more readable to have imports in alphabetical order and not mixed with other code like logging.getLogger("scapy.runtime").setLevel(logging.ERROR)

String formatting is awful and not-readable:

Instead of:

str(len(clients))+" "+mac+" ("+macf+") //"+" <--Probing--> "+ssid+"\n"

Use format method:

"{0} mac {1}".format(len(clients), macf)

Or - even more readable:

"{length} mac {macf}".format(
    length=len(clients),
    macf=macf,
 )

Much more readable and is actually a standard.

Formatting (indentations) is a bit off - difficult to understand some pieces.

Performance

You have mach is list and you are constantly searching in it. Search in list is O(n). Make is set or dict - will speed it up - search performance becomes O(1).

mach = []
# this check is expensive
if mac not in mach:

Same comment  - for clients list.

Here - remove condition, just leave else - or maybe your formatting is misleading here and I didn't understand logic correctly:

elif mac not in mach:


Bug

I cannot remove an element from a one-element list. You could easily fix this by adding this after your initial check:

if(head.next == null && n == 0) {
    // return empty list
}

ListNode and the need for a List

The questions starts with Given a linked list. But you don't really have a linked list. You have a node class, but that's it, and that's really not enough. Right now, I would have to take your word that it does what it is supposed to, because there is no way of knowing if it actually does (well, except reading the code :) ).

Creating something like a list, I would have to do this:

    ListNode n = new ListNode(1);
    n.next = new ListNode(2);
    n.next.next = new ListNode(2);

And if I want to write unit tests as @Pimgd suggested (or even just check with a simple example in a main method), I would need to add a whole lot of code. I would suggest that you at least add methods to add elements, to traverse through the list, to check if one list is equal to another, and a toString method.

This would make debugging and testing your code a lot easier, and some of those methods would also help you implement the removeNthFromEnd method.

Handling Exceptions

Don't just accept any input. You are already checking if(head == null || n < 0), which is good, but it would be best to throw an IllegalArgumentException.

If the user tries to remove an element that doesn't exist, don't just let them, throw an IndexOutOfBoundsException instead.

while loop

    while(steps <= n){
        fast = fast.next;
        steps++;
    }

Your first while loop looks better as a for loop:

for (int steps = 1; steps <= n; steps++) {
    fast = fast.next;
}

As to your question, it doesn't really matter if you start at 1 or 0. I would start at 0, but it's just a personal preference.

Naming and Comments

I just want to emphasize what @Pimgd and @shivsky said. You really need better names than fast and slow, and if the program isn't a lot clearer then, also comments (see this code for an example; you don't need that many comments, but something like set p1 and p2 apart by n-1 nodes initially etc is really helpful).

You should also have a method comment. It should state if the list is changed or if a new (changed) list is returned (or what else is returned if not a new list), what arguments are acceptable, etc.

You should also make clear how your indices work. Does nth from last mean that I remove the last node with removeNthFromEnd(0) or removeNthFromEnd(1)?

Misc

declare your function as static to express how it works (it doesn't belong to a specific ListNode and changes that).
use curly brackets even for one-line statements. If you really don't want to, put the line on the same line as the if statement. Otherwise it's easy to introduce bugs.


I am not an angular expert, but this

var piece;
if(returnedData.oIsAvailable == true) {
    piece = 'x';
}
else {
    piece = 'o';
}
$scope.myPiece = {
    val:piece
};

could be

$scope.myPiece = {
    val: returnedData.oIsAvailable ? 'x' : 'o'
};

Also

I know the firebase API calls the object returned by new Firebase a (root) reference. It still irks me since my brain keeps thinking what is ref?? I would probably go against the grain and call it db, but that could be because I am simply not familiar enough with the API
if(checkThisGame.oIsAvailable == true){ should be if(checkThisGame.oIsAvailable){ or if(checkThisGame.oIsAvailable === true){
the game is well documented
JsHint.com finds a number of small things, you should use the site


Your problem is ultimately:

no_noise_text = re.sub(r'[0-9.,:;<>"#!%&/()=?*-+]','',in_text.lower()).strip().split()

There's a lot to dislike about this line. It horribly violates the Law of Demeter. It takes a while to figure out what it is supposed to do. It uses the wrong tool for the job. And it's inefficient.

First, to start with, if the goal is to split a strict by non-alpha characters, then we should just use the correct function: re.split:

words = re.split("[^a-z']+", in_text.lower())
counts = Counter(words)

Next, this is going to be inefficient. We have to walk the entire string once (to make it lower-case), then again to split it. And then we have to keep all of the words in memory. We don't have to do any of that if we really start with the correct function: re.finditer:

re.finditer("([a-zA-Z']+)", in_text)

That will give you an iterator over the words. Which you can stick into a generator expression that you can pass to Counter:

words = re.finditer("([a-zA-Z']+)", in_text)
counts = Counter(m.group(0).lower() for m in words)

And if you're already using Counter, you may as well use defaultdict:

freq_dict = defaultdict(list)
for key, val in counts.iteritems():
    freq_dict[key].append(val)

Note that I'm using iteritems() (which just gives you a generator) rather than items() (which must give you the full list).

Furthermore, why do we need a frequency_dict? We only care about those words which match the given frequency. Why not just keep a list?

frequency_list = []
for key, val in counts.iteritems():
    if val == frequency:
        frequency_list.append(key)

Putting it all together

from collections import Counter
import re

def words_in_text_counter(in_text, frequency=2):
    words = re.finditer("([a-zA-Z']+)", in_text)
    counts = Counter(m.group(0).lower() for m in words)

    frequency_list = []
    for key, val in counts.iteritems():
        if val == frequency:
            frequency_list.append(key)

    if frequency_list:
        print(str(frequency)+' times words in text.')
        return frequency_list
    else:
        return str(frequency)+' times never happens in text.'


My node app with valid syntax is failing silently, but not breaking. Infrastructure problem?

From a once over:

Your console.log statement should be wrapped in a condition so that you can turn off all logging to the console. Node can really slow down because of logging to the console.
If you are going to log to the console, then it would make more sense to log your query and salesData as well.
Opening and closing a connection every time you want to post a sale is (very) bad practice, check out mysql.createPool
Calling connection.end(); will kill a connection gracefully. calling connection.destroy(), not so much..

All in all, I would always be sensitive to performance and established best practice.


Find the largest sum of any slice inside an array

Two minor remarks:

For consistency sake, this

accumulator = maxLastPair > accumulator + addend ? maxLastPair : accumulator + addend;

can be implemented similar to the other statements as

accumulator = Math.Max(maxLastPair, accumulator + addend);

Some comments would be welcome in the loop. What exactly is the idea behind the different variables you keep. It took me a while to convince myself that your method was working as it should.


You should use sorted() for sorting and give your function and variables better names.
Something like this:

import re

find_int = re.compile('(\d+)')

def reorder(sentence):
    def num_part(word):
        return int(find_int.search(word).group(0))

    return ' '.join(sorted(sentence.split(), key=num_part))


You could try to do the whole thing on the fly, like this:

var total = 0;
$("input.tcbx").change(function(){
    if($(this).is(":checked")) {
        total += parseFloat($(this).val(), 10);
    } else {
        total -= parseFloat($(this).val(), 10);
    }
    $('.txt7').val(total.toFixed(2));
});

Just add a class to all your checkboxes, and then either add the value to your total or reduce it as they get selected. If you have preselected values you might want to enter the preselected total already, otherwise you could use an each() to first check all the values.

$(document).ready(function(){
    var total = 0;
    $("input.tcbx").change(function(){
     if($(this).is(":checked")) {
      total += parseFloat($(this).val(), 10);
     } else {
      total -= parseFloat($(this).val(), 10);
     }
     $('#result').val(total.toFixed(2));
    });
});

<input type="checkbox" value="1" id="check-1" class="tcbx" />
1
<input type="checkbox" value="2" id="check-2" class="tcbx" />
2
<input type="checkbox" value="3" id="check-3" class="tcbx" />
3
<input type="text" id="result">0


Remove duplicates from sorted array, in place

I don't see any way to greatly increase performance since your algorithm is already O(n).

However, there is a minor optimization that can be made.  You should try to avoid having a calculation as the limit in a for loop.  This gets calculated on each iteration of the loop.  Initialize a variable with the result and use that instead:

public int removeDuplicates(int[] numbers) {
    if(numbers.length == 0 || numbers.length == 1) {
        return numbers.length;
    }
    int indexForUniqueElements = 0;
    int limit = numbers.length - 1;
    for (int index = 0; index < limit; index++) {
        if (numbers[index] != numbers[index+1]) {
            numbers[indexForUniqueElements++] = numbers[index];
          }
    }
    numbers[indexForUniqueElements++] = numbers[numbers.length - 1];
    return indexForUniqueElements;
  }


I see some things that may help you improve your code.

Simplify your code

You don't need a lambda at all.  Use this instead:

    futures.push_back(std::async(wordsInFile,argv[i]));

Also, instead of if (!file.is_open()) use the more idiomatic if (!file).

Fix the counting

If we have two files which both contain the word fox exactly once then I would expect the result of this program to print fox   2 but in fact, it would falsely claim a count of 1.  The problem is this line:

words[j.first] = j.second;

This overwrites the count instead of accumulating it.  This is what you need instead:

words[j.first] += j.second;

Use reserve to prevent reallocations

This code already knows how many futures are going to be created.  The code can be made a little more efficient by using this:

futures.reserve(argc-1);

With large numbers of files, this eliminates the re-allocation that would be required by dynamically resizing the vector.

Check for exceptions

If any of the file names can't be read.  That is, if it can be opened but not read, as with a directory under Linux, this code will throw an error that isn't caught and so therefore results in a crash.  Since future::get() will throw any error that was stored, it's that part of the code that should check for exceptions.  One alternative would be this:

    Words results;
    try {
        results = i.get();
    }
    catch(std::exception &err)
    {
        std::cerr << "ERROR: " << err.what() << '\n';
    }
    catch(std::string &err)
    {
        std::cerr << "ERROR: " << err << '\n';
    }
    for (const auto& j : results)
    {
        words[j.first] += j.second;
    }

Note that this will simply skip the offending file name but continue to process the rest.

Measure your results

Create a non-parallel version of this code and compare the timing to see if it's faster or slower than this version.  I have found that it's often true that the overhead of the implicit (as in async) or explicit (as in thread) thread creation swamps the savings in time by making things parallel.  The only way to know for sure is to time it.


Naming

You should choose longer variable names. For example, with names as $fil and $fh it's hard to remember which one was which. You could name them $output and $count.

But all other names should be expressive as well. $dat might be $uptime_count, and $myFile -> $output_file_name, $fh -> $output_file, $count_file -> $count_file_name (it't just a string, not a file), $ch -> $curl, etc.

For some names it's obviously more important than for others, but good naming is always an improvement.

Variable Scoping

$fh is only used once, far away from where it was defined, inside the else statement. I would move the opening code here, so you don't have to think about $fh outside the else.

It's closed in the if as well, but that is unnecessary. Don't open up the file if you don't plan on writing to it.

Functions

The code isn't all that long, but I would still extract some functionality to its own function. Especially the counting of previous up and down times, and maybe the connecting to the server.


I haven't tested this but you should be able to do something like:

my_hash = Hash.new { |h, k|
  if k == :total_cash
    h[k] = Hash.new { |h, k| h[k] = Hash.new(0) }
  else
    h[k] = Hash.new(&h.default_proc)
}

Other than that, I think it would be easier to read if you reorder it a bit:

my_hash = {
  total_cash: Hash.new(0), total_credit: Hash.new(0),
  users: Hash.new { |h, k| h[k] = {
    total_cash: Hash.new(0), total_credit: Hash.new(0) },
    showtimes: Hash.new { |h, k| h[k] = {
      total_cash: Hash.new(0), total_credit: Hash.new(0) },
      movies:  Hash.new { |h, k| h[k] = {
        total_cash: Hash.new(0), total_credit: Hash.new(0) },
        payment_methods: Hash.new { |h, k| h[k] = Hash.new(0) }
      }
    }
  }
}


Not your primary concern, but a number of issues are jumping at me; I'll leave the performance aspects to other reviewers.

  Naming

like_Click should be renamed OnLikeButtonClicked or something similar, because it's a method, and method names in C# should be PascalCase (without underscores).

int ID should be renamed id, because it's a local variable, and local variables in C# should be camelCase.

LinkButton like should be renamed likeButton to not look like it's an action or something.

Notice how the syntax formatting gets confused with int ActivityTypeID and int SourceID? That's because PascalCase is for type and method names; these are local variables.

Label numberoflike is alllowercase and that's not helping readability. I'd call it something like likesCountLabel, indicating it's a control and that it's something about "likes count". Again, camelCase.

  Readability

Naming aside, a number of things could be done to improve readability here.

if (ActivityTypeID == 1 || ActivityTypeID == 4)

These values are known as magic numbers - nobody knows what they stand for, they just work. Consider defining an enum ActivityType instead, and refer to the values by their names.

Remove comments that state the obvious, that essentially rephrase what the code is already saying, and those that can eventually (read: will!) turn into blatant lies:

// this code will run a stored procedure

Really? It's an implementation detail of that static/nested class (ew!), that this method doesn't need to care about. Whether it calls a stored procedure, concatenates T-SQL into a string, calls a web service and parses Json or XML to get your results, has no importance whatsoever. This comment is doomed, remove it.

// make an object of like table

Not needed. var myVariable = new MyType() already says everything we need to know.

That one is confusing:

    nl.Save();
      // saves like to database
    numberoflike.Text = BusinessLayer.StatusLike_Table.GetByNumberOfLike(SourceID).ToString();

Either put comments above the line they're commenting, or at the end of it... but not below! It makes it look like numberoflike.Text = ... is going to save something to the database. Yeah, right.

Avoid disemvoweling identifiers. nl means sweet nothing to anyone reading this code, and gvrow would probably be better off as simply row - the fact that it's a GridView isn't really important.

  Gotta catch'em alltm

This is called Pokemon exception handling:

catch (Exception ee)
{
}

Why? Because regardless of what's being thrown, it wil be caught. And then shoved under the carpet without leaving any trace. Not good: you should always catch the most specific exception types, those you can reasonably expect to be thrown, and properly handle them. Exceptions you didn't see coming, or that you can't properly handle, should bubble up the stack, get logged, and perhaps crash the application.

OTOH, you're getting a compiler warning, saying ee is declared but not used. You can eliminate that warning by going all-out on Pokemon handling:

catch (Exception){ }

The BusinessLayer static class smells, but changing it to be a constructor-injected instance would probably require massive structural changes.. I'd approach the refactoring with incremental steps towards dependency-injection, so as to facilitate unit-testing.


I'll start by trying to address your concerns.

Unless in special cases you can't make this function run in less than quadratic time.  Typical linear time solutions in other languages involve generating hashes from values in the filtered array and then using a hash-table to filter out characters whose hash value appears to be the same.  There is no standard way to obtain a hash of an object in JavaScript, however, some objects may be uniquely written to string, which can be used as their hash.  So, there is an O(n) solution for an array of numbers, or an array of strings, but no such solution for an array of functions.  JavaScript makes no guarantees with regard to transforming tail calls into iterations, so, if possible, an iterative solution would be better than recursive one.
Yes, it's certainly better to let one function do one thing.

Now, to the interviewer reaction: Passing an interview and writing good code aren't necessary (and, most unfortunately, aren't in most cases) the same thing.  Interviewers typically want to see the code written in a particular way they believe it should be written.  If your goal is to pass the interview, it is in your best interest to find out the exact requirements.  Sad but true.

Now, assuming I would have to assess your code, here are the things that I'd find to be bad:

The code is too lengthy.  It shouldn't take that much space to write it. (I later give my version of the answer).
Inconsistent spacing and brace positioning.  This is certainly not a crime, but remember that I'm trying to assess your code from small cues.  I wouldn't mind this as much, if, for example, I knew that the code was a result of several people working on the same code base, using different editors etc.
Changing the iterator inside the loop.  Call me superstitions, but I'd make every possible effort to avoid it.
Comparison with Boolean constants.  There is no need to do that (unless for diagnostic purposes).  The language was designed in such a way that you never need to compare anything to true or false.  But if you will continue writing JavaScript code, you will find that a lot of people don't understand this trivial concept.
You've decided to make the algorithm a destructive one.  I.e. the function modifies the argument passed to it.  There are reasons for having both versions.  I, myself, expect the default to be non-destructive.
slice isn't really a way to go here.  Not that it matters a lot on JavaScript, since arrays are typically implemented as hash-tables with integer keys, but, in general, the properties of array data structure make repeated slicing a lot more expensive than maintaining the index of last insertion (example follows).
Using typeof and other kinds of ad hoc typechecking to me always smell bad.  There is a place for it, but in the context of this task it isn't required.

Non-destructive version:

function removeDuplicates(array, comparator) {
    var result = [], i;
    function insert() {
        var match, j;
        if (comparator) {
            for (j = 0; j < i; j++) {
                if (comparator(array[j], array[i])) {
                    match = false;
                    break;
                }
            }
        } else match = result.indexOf(array[i]) == -1;
        return match;
    }

    for (i = 0; i < array.length; i++)
        if (insert()) result.push(array[i]);
    return result;
}

Destructive version:

function deleteDuplicates(array, comparator) {
    var i, k = 1;
    function insert() {
        var match, j;
        if (comparator) {
            for (j = 0; j < k; j++) {
                if (comparator(array[j], array[i])) {
                    match = false;
                    break;
                }
            }
        } else match = array.lastIndexOf(array[i], k) == -1;
        return match;
    }

    for (i = 1; i < array.length; i++)
        if (insert()) array[k++] = array[i];
    array.length = k;
    return array;
}

Note that it would be possible to make it even shorter, if I knew that user-provided comparison function isn't needed (isn't part of the task).  I still chose to include the possibility, even though I wasn't sure about it because it seems like a good thing to include one in the function's interface.  If I had to answer this interview question, I'd probably not bother writing a custom comparator just to save time and spare myself the possibility of making a silly error working on something which wasn't absolutely required.


def typeahead(usernames, queries):
        from bisect import bisect_left
        wordlist=sorted(usernames,key=str.lower)
        l_wordlist=map(str.lower,wordlist)
        for i in queries:
            word_fragment=i.lower()
            x=bisect_left(l_wordlist, word_fragment)
            try:
                if word_fragment in l_wordlist[x]:
                    print wordlist[x]
                else: print -1
            except: print -1

i have increased it performance by

k=wordlist[bisect_left(l_wordlist, word_fragment): bisect_left(l_wordlist, word_fragment[:-1] + chr(ord(word_fragment[-1])+1))][:1]

there are two bisects functions in above by removing one and converting to below

            x=bisect_left(l_wordlist, word_fragment)
            try:
                if word_fragment in l_wordlist[x]:
                    print wordlist[x]
                else: print -1
            except: print -1

by profiling

 100004 function calls in 0.542 seconds

   Ordered by: standard name

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
        1    0.329    0.329    0.542    0.542 reducer.py:21(typeahead)
    50000    0.116    0.000    0.116    0.000 {_bisect.bisect_left}
        1    0.013    0.013    0.013    0.013 {map}
        1    0.000    0.000    0.000    0.000 {method 'disable' of '_lsprof.Profiler' objects}
    50000    0.020    0.000    0.020    0.000 {method 'lower' of 'str' objects}
        1    0.063    0.063    0.063    0.063 {sorted}

reduced the system calls by half from 250005 to 100004 did the magic :)


Coding Critique

public static function makeNumber( $type, $number ) {
    $numObject = null;
    $number = null;

Well, that almost certainly isn't right.  You take a parameter which you then ignore, overwriting it locally.

public static function makeNumber( $type, $number = null ) {
    $numObject = null;

That seems like it might be more useful.  You accept a parameter if offered.  Otherwise, you give it a default value.  That may or may not be what you wanted to do.

        case 'float':
            $numObject = new Float( $number );
            break;

You can make this simpler if you want:

        case 'float':
            return new Float( $number );

That way you can get rid of $numObject entirely.

        default:
            $numObject = new Integer( $number );
            break;

You don't need a break; in the last clause.  You can just let it fall through after a default: clause.

Factory Method

You have this tagged factory-method, but you aren't using a factory method.  The factory method is when you have a method that can return different results depending on the exact class of the object.  So you might have

class CompositeNumberFactory {
    public static function makeNumber( $first, $second );
}

class FractionFactory extends CompositeNumberFactory {
    public static function makeNumber( $first, $second ) {
        return new Fraction($first, $second);
    }
}

class Fraction implements iNumber {
    private $numerator;
    private $denominator;

    function __construct($numerator, $denominator) {
        $this->numerator = $numerator;
        $this->denominator = $denominator;
    }

This gets more interesting if FractionFactory is called something else, e.g. RationalFactory.  Then the caller may not even know that they have a Fraction.  They just know that they have the normal iNumber methods.

Factory Pattern

There is a variant of the factory pattern that does work something like you specify, but it would normally have different types.  E.g.

        case 'decimal':
            return new Double( $number );
        case 'bit64':
            return Long( $number );
        case 'whole':
            return new Unsigned( $number );

This allows the user to not know what the various types are, just how the user would describe them.  Also, this allows the underlying implementation to change without impacting the user.  For example, if we moved from a 32-bit operating system to a 64-bit operating system, the second case might change from Long to Integer.


Genetic algorithm to find the minimum of a three-variable function

Better organisation

You program in its current state is one huge chunk of snippet with all logic inside it. You should consider splitting it into separate smaller functions. The limits on \$ x, y, z \$ are preset. Consider putting them as a GLOBAL_CONSTANT.
Ideal import order (according to PEP8) is

standard library imports
related third party imports
local application/library specific imports

os/collections etc. are standard library, and should be imported first, followed by numpy.
pythonify your code

x = args[0]
y = args[1]
z = args[2]

can be expressed as:

x, y, z = args

similarly

manager.list([np.inf for i in range(child_num)])

can become

manager.list([np.inf] * child_num)

I am not sure about how numpy.argmin works, but I think using None instead of np.inf could be better in the sense that you might extend your program to also find local/global maxima along with minima points.


Why not:

static string Read(istream &stream, uint32_t count)
{
    std::string result(count, ' ');
    stream.read(&result[0], count);

    return result;
}

Though not strictly C++03 compatible that is easily validated. One of the reasons the committee found it easy to add the new constraint in C++11 was that no implementation did not use contiguous memory (Random Access Iterators are a hint).

But a C++03 strict implementation would be:

static string Read(istream &stream, uint32_t count)
{
    std::vector<char> result(count);  // Because vector is guranteed to be contiguous in C++03
    stream.read(&result[0], count);

    return std::string(&result[0], &result[count]);
}


Extract data labels from a chart in PowerPoint to Excel

Would this work instead of all the loops:

For a = 1 To 10
    For x = 1 To z
        xlWorkbook.Sheets(1).Cells(x, a) = chtnow.SeriesCollection(a).DataLabels(x).Text
    Next
Next a

do not use Interger in VBA - https://stackoverflow.com/questions/26409117/why-use-integer-instead-of-long
do not use variable names like a, z, x, but give them some meaningful names. E.g. x can be row or rows, thus the code would be easier to get.


I suppose you don't indent the printf debug lines to make them easier to spot. That's a sensible idea (although not one I've seen before). However, I treat Code Review answers as a "just before checkin" thing, so I'd suggest removing these by this point.

You pre-declare your variables. Don't do this; write them as close to point-of-use as reasonable.

Your NONE op seems to be rather useless - you could just use OR instead. So remove it.

Your

const char zero = '0';
const char _A = 'A';
const char _a = 'a';

variables might seem like a good adherence to "no magic constants", but in reality they have effectively no useful purpose. If you wanted to update your code to support other character sets, you'd want to redesign the whole parser anyway. IMHO, it's simpler to inline these.

You're not required to return 0; in modern C, and it's somewhat typical not to.

Your integer parser should be generic in the base. I suggest extracting this into a function like int parse_digit(char character, int base, int place):

int parse_digit(char character, int base, int place) {
    int value;

    if (character >= 'a') {
        value = character - 'a' + 10;
    }
    else if (character >= 'A') {
        value = character - 'A' + 10;
    }
    else {
        value = character - '0';
    }

    return value * pow(base, place);
}

while (offs < len - 1) {
    optmp += parse_digit(*(nend - offs), base, offs);
    offs++;
}

base can be calculated with a simple lookup on ntype:

int type_to_base(type ntype) {
    switch (ntype) {
    case B02: return 2;
    case B10: return 10;
    case B16: return 16;
    }
}

Although this makes the type enum rightly look silly: it's just an integer after all!

int flag_to_base(char flag) {
    switch (flag) {
    case 'b': return 2;
    case 'd': return 10;
    case 'x': return 16;
    }
}

Note that this is a somewhat silly way to do things, though. First of all, pow takes doubles! I'd rather avoid that if possible. Further, this is more work than needs to be done. Instead, one can do this:

  1*base^4 + 0*base^3 + 1*base^2 + 0*base^1 + 1*base^0  // how you're currently doing it
= (((1 * base + 0) * base + 1) * base + 0) * base + 1          // simpler way to do it

This can be done as:

int parse_digit(char character) {
    if (character >= 'a') {
        return character - 'a' + 10;
    }
    else if (character >= 'A') {
        return character - 'A' + 10;
    }
    else {
        return character - '0';
    }
}

while (offs < len - 1) {
    optmp *= base;
    optmp += parse_digit(*(nend - offs));
    offs++;
}

Parsing integers should be in another function, and can be slightly simplified:

long parse_int(char *start, char *end) {
    int base = flag_to_base(*end);

    long ret = 0;
    for (char *digit = start; digit < end; digit++) {
        ret *= base;
        ret += parse_digit(*digit);
    }

    return ret;
}

I would also put application of optmp into a function, and stop calling it optmp (in general, most variables are temporary - stating this is pointless):

long apply(operator op, long left, long right) {
    switch (op) {
    case AND: return left & right;
    case OR:  return left | right;
    case XOR: return left ^ right;
    }
}

I would then change (generally) long to uint64_t and int to uint32_t (or even uint8_t) - there's no reason to be using variable-length integers in this day and age for these types, and you don't need signed values. When you do want variable-length values, you'd probably rather stick with size_t. (If you find these unwieldy, typedef them to u64, u32 and u8. Just don't use int and long 'cause they're prettier.)

Compiling with warnings, I get

calc.c: In function ‘apply’:
calc.c:18:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
calc.c: In function ‘flag_to_base’:
calc.c:26:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^

This is because the new method allows better case analysis - we've forgotten to check that such operations are actually valid!

#include <math.h>
#include <stdint.h>
#include <stdio.h>
#include <string.h>

typedef enum {
    AND = '&',
    OR = '|',
    XOR = '^',
} operator;

int apply(operator op, uint64_t *left, uint64_t right) {
    switch (op) {
    case AND: *left &= right; break;
    case OR:  *left |= right; break;
    case XOR: *left ^= right; break;
    default: return -1;
    }
    return 0;
}

int flag_to_base(char flag, uint8_t *base) {
    switch (flag) {
    case 'b': *base = 2; break;
    case 'd': *base = 10; break;
    case 'x': *base = 16; break;
    default: return -1;
    }
    return 0;
}

int parse_digit(char digit, uint8_t *value) {
    if ('a' <= digit && digit <= 'z') {
        *value = digit - 'a' + 10;
    }
    else if ('A' <= digit && digit <= 'Z') {
        *value = digit - 'A' + 10;
    }
    else if ('0' <= digit && digit <= '9') {
        *value = digit - '0';
    }
    else {
        return -1;
    }

    return 0;
}

int parse_int(uint64_t *ret, char *start, char *end) {
    uint8_t base;
    if (flag_to_base(*end, &base)) {
        return -1;
    }

    *ret = 0;
    for (char *digit = start; digit < end; digit++) {
        uint8_t value;
        if (parse_digit(*digit, &value) || value >= base) {
            return -1;
        }

        *ret *= base;
        *ret += value;
    }

    return 0;
}

int main(int argc, char **argv) {
    uint64_t result = 0;
    operator op = OR;

    for (int i = 1; i < argc; i++) {
        size_t len = strlen(argv[i]);
        char *end = argv[i] + len - 1;

        if (len == 1) {
            op = *end;
            continue;
        }

        uint64_t operand;
        if (parse_int(&operand, argv[i], end)) {
            printf("Can't parse argument '%s'\n", argv[i]);
            return -1;
        }

        if (apply(op, &result, operand)) {
            printf("Can't parse argument '%c'\n", op);
            return -1;
        }
    }

    printf("%li\n", result);
}

Note that I would be wary of casting to enums where this might result in an "invalid" enum, but at least it is well-defined behaviour.

For the Python, your

_bin = lambda x: bin(x)[2:] + 'b'
_dec = lambda x: str(x) + 'd'
_hex = lambda x: hex(x)[2:] + 'x'

can be just

_bin = "{:b}b".format
_dec = "{:d}d".format
_hex = "{:x}x".format

Your embedded loops can be simplified to

from itertools import product
for args in product(ops, types, range(RANGE), range(RANGE)):
    test(*args)

You don't use time - remove it. Rather than rename subprocess to subp, I would directly import Popen and PIPE.

Use new-style formatting, please! Also, use eval instead of exec - using neither would be best, although its use is mildly understandable in this case (even if still saddening).

Don't call quit - this is a function only for interpreter usage. I actually suggest just asserting in this case.

Your subprocess call only wants output, so drop stdin=PIPE. Further, you can just call check_output:

import subprocess
from itertools import product

def test(op, t, x, y):
    call = ["./calc", t(x), op, t(y)]
    print call
    data = subprocess.check_output(call)
    assert eval("{}{}{}".format(x, op, y)) == int(data)

ops = "&|^"
types = "{:b}b".format, "{:d}d".format, "{:x}x".format
count = 20

for args in product(ops, types, range(count), range(count)):
    test(*args)


After preparing this recent answer it occurred to me that I'd already written something like it.  Here's my review of this code.

Make the class more useful

Right now the only thing we can do with the RockPaperScissors class is to get a new one via random or print the results of a contest.  If we wanted to do something else, such as collect some statistics, there are pieces missing, such as the ability to get a printable version of the class.  Here's one way to add that:

friend std::ostream& operator<<(std::ostream& out, const RockPaperScissors &rps);

Now we can gather some statistics like this:

int main()
{
    std::unordered_map<std::string_view, unsigned> responses;
    std::unordered_map<std::string_view, unsigned> results;
    constexpr unsigned max_trials{300'000'000};
    for (unsigned trials = max_trials; trials; --trials) {
        auto a = RockPaperScissors::random();
        auto b = RockPaperScissors::random();
        ++responses[static_cast<std::string_view>(a)];
        ++responses[static_cast<std::string_view>(b)];
        ++results[a.vs(b)];
    }
    for (const auto& resp: responses) {
        std::cout << std::setw(16) << resp.first << '\t' << resp.second
            << '\t' << static_cast<double>(resp.second)/max_trials/2
            << '\n';
    }
    for (const auto& resp: results) {
        std::cout << std::setw(16) << resp.first << '\t' << resp.second
            << '\t' << static_cast<double>(resp.second)/max_trials
            << '\n';
    }
}

This gives a report something like this:

       Paper    200002179   0.333337
    Scissors    199987250   0.333312
        Rock    200010571   0.333351
         WIN    66662424    0.222208
         TIE    100003629   0.333345
        LOSE    133333947   0.444446

Prefer open functions to class functions

The current code has this member function:

RockPaperScissors RockPaperScissors::random() {
    static std::random_device rd;
    static std::mt19937 gen{rd()};
    static std::uniform_int_distribution<> dis(0, RockPaperScissors::words.size()-1);
    std::size_t which{static_cast<std::size_t>(dis(gen))};
    return RockPaperScissors{which};
}

However, selecting a single item at random from within a collection is a very common need.  A more generally useful way to write this might be this:

template <typename Iter>
Iter random_one(Iter begin, const Iter& end) {
    static std::random_device rd;
    static std::mt19937 gen{rd()};
    static std::uniform_int_distribution<> dis(0, std::distance(begin, end) - 1);
    std::advance(begin, dis(gen));
    return begin;
}

Now we can get an iterator that points to a particular item in any collection.  (To make this template more durable, we'd actually probably want to do some validation of both the Iter type and the values passed, but this conveys the idea.) We could use that with no further modifications as in this:

RockPaperScissors RockPaperScissors::random() {
    auto which = std::distance(words.begin(), random_one(words.begin(), words.end()));
    return RockPaperScissors{which};
}

But that's a bit odd because we are converting an iterator to an ordinal.  That leads to the next suggestion.

Store an iterator within the class

We can simplify some of the code for this implementation by changing the definition of choice within the class to be an iterator.

using iter = decltype(words.begin());
iter choice;

Now the constructor can use the templated function above very simply:

RockPaperScissors::RockPaperScissors() : choice{random_one(words.begin(), words.end())}
{
}

A few other changes are needed, but mostly they simplify the code.

Also note that now the random function is no longer needed.


Efficient string concatenations with DOM

Strings are immutable. It means that when you allocate memory while creating a string, you are not able to reallocate it.
So, this code:

var a = 'a';
a = a + 'bc';

Will allocate a new block of memory for 'a' variable, instead of reusing already allocated.

When you do something like 'a' + 'b' + 'c' + 'd' each concatenation allocates a new block of memory for every temporary string.

When you use something like this:

var a = 'a';
a = [a, 'b', 'c'].join('');

join function calculates memory for result string and allocates it only once for a complete string.

As it is shown in the post that you mentioned, it is easier to handle and to avoid memory leaks in JS interpreter (it seems that IE6 and IE7 just not handle garbage collection for the first variant correctly).

If you are interested for the speed of string concatenation for different browsers you can try and view it here.

UPD: Not advocating join, just trying to explain why it was assumed as an optimized variant. As seen at jsPerf tests new browsers optimize string concatenation, so it is faster.


Testing async method call from constructor

Trying to load the data asynchronously in the constructor is a bad idea and in my opinion goes against what the constructor is for.

Constructors cannot be async, and asynchronous initialization can be seen as an implementation detail.

I suggest separating the initialization and data loading out ...

public class MainVM : ViewModelBase {
    private readonly IRepository<ToDoItem> repository;
    private IEnumerable<ToDoItem> items;
    private bool _dataIsLoaded;
    private bool loading = false;

    public bool DataIsLoaded {
        get { return _dataIsLoaded; }
        set { Set(ref _dataIsLoaded, value, true); }
    }

    public IEnumerable<ToDoItem> ToDoItems {
        get { return items; }
        set { Set(ref items, value); }
    }

    public MainVM(IRepository<ToDoItem> repository) {
        this.repository = repository;
    }

    public async Task LoadData() {
        if (loading) return;

        if (!DataIsLoaded) {
            try {
                loading = true;
                ToDoItems = await repository.GetAsync();
                loading = false;
                DataIsLoaded = true; // task Ran To Completion
            } catch (Exception ex) {
                loading = false;
                //TODO: Log error
                DataIsLoaded = false; // task did not complete
            }
        }
    }
}

... and have whatever is binding/using to the view model (ie the view, another model, a test, etc) load the data after initialization.

This allows the testing to be much simpler and to the point

For example

[TestClass]
public class MainVmTests {
    [TestMethod]
    public async Task DataIsLoaded_is_true_if_loading_task_ran_to_completion() {
        //Arrange
        var repoMock = new Mock<IRepository<ToDoItem>>();
        repoMock.Setup(_ => _.GetAsync()).Returns(Task.FromResult(Enumerable.Empty<ToDoItem>()));
        var subject = new MainVM(repoMock.Object);

        //Act
        await subject.LoadData();

        //Assert
        Assert.IsTrue(subject.DataIsLoaded);
        Assert.IsNotNull(subject.ToDoItems);
    }

    [TestMethod]
    public async Task DataIsLoaded_is_false_if_loading_task_failed() {
        //Arrange
        var repoMock = new Mock<IRepository<ToDoItem>>();
        repoMock.Setup(_ => _.GetAsync()).Throws(new AggregateException());
        var subject = new MainVM(repoMock.Object);

        //Act
        await subject.LoadData();

        //Assert
        Assert.IsFalse(subject.DataIsLoaded);
        Assert.IsNull(subject.ToDoItems);
    }
}


PHP - Importing columns from uploaded CSV file to SQL database (Laravel 5.7)

I defined the fields that I wanted to be required first. Then, I defined a One-to-Many relationship between an account table and a contacts table where the contact id is the account's name in the account's table in the SQL database

        $account = new Account();
                $mapping = $request->$mapping;
                $account->name = $rowProperties[$mapping['account_name']];

                //One-To-Many Eloquent Relationship that links a table of Account Names in the Account's
                //table to contact Account_ID's in the Contact tables
                //$contact->id = $account->id;

                $account->save();

                $contact = new Contact();
                $contact->id = $account->id;
                $contact->contact_id = $rowProperties[$mapping['contact_account_name']];
                $contact->first_name = $rowProperties[$mapping['contact_first_name']];
                $contact->last_name = $rowProperties[$mapping['contact_last_name']];

    account = new Account();
                $account->name = $rowProperties[$mapping['account_name']];
                $this->account->hasMany('contact_account_names');
                //$accountid->contact_account_name
                $account->save();


Sieve of Eratosthenes using Java 8 streams

Implementation

You can invert the values for prime and not prime to avoid the initial rangeClosed(0, _n).forEach(x -> prime[x] = true). The innermost loop can start at x*x instead of x*2:

// (1)
public static List<Integer> sieveOfEratosthenes(int n) {
    boolean[] notPrime = new boolean[n + 1];

    IntStream.rangeClosed(2, (int) Math.sqrt(n))
            .filter(x -> !notPrime[x])
            .forEach(x -> {
                IntStream.iterate(x * x, m -> m <= n, m -> m + x)
                        .forEach(y -> notPrime[y] = true);
            });

    List<Integer> list = new ArrayList<>();
    IntStream.rangeClosed(2, n)
            .filter(x -> !notPrime[x])
            .forEach(x -> list.add(x));
    return list;
}

This implementation still feels like a literal translation of the for-loop approach. Streams offer options to improve readability, i.e. the inner loop can be flattened:

// (2)
public static List<Integer> sieveOfEratosthenes(int n) {
    boolean[] notPrime = new boolean[n + 1];

    rangeClosed(2, (int) Math.sqrt(n))
            .filter(x -> !notPrime[x])
            .flatMap(x -> iterate(x * x, m -> m <= n, m -> m + x))
            .forEach(x -> notPrime[x] = true);

    return rangeClosed(2, n)
            .filter(x -> !notPrime[x])
            .boxed().collect(toList());
}

Stream vs Loop

The main problem I see with a stream based approach is (besides the overhead of the streams), that it is more difficult to optimize compared to a loop approach as the abstraction level is higher. For example converting the implementation to a parallel approach is in my opinion way more complicated with a stream approach.

Algorithm improvements (not directly related to the question)

You can use a BitSet (or directly a long/int/byte array) to ensure that each entry in the sieve consumes only one bit (instead of currently (likely) 8). You are storing all values from 2 to n in the sieve, you can save memory by skipping multiples of 2 (and 3, 5, ...) at the cost of some additional calculations to convert between sieve position and value.

For larger values of n it might be advantageous to divide the sieving process in smaller steps and use a small array instead of a large array for all values and sieving the complete range at once.

The sieving process can be converted to a parallel implementation with nearly linear speedup as two or more values can be sieved simultaneously (sieving process of each value is independent from other values).

Approx. performance for n=100_000_000, included my implementation (which utilizes most of the improvements mentioned above) as reference value:

// 1 Thread
Initial            3406676978ns
Version (1)        2487619662ns
Version (2)        2459434320ns
For loop           2158261344ns
Nevay to List      1436579484ns
Nevay to Sieve     1040829243ns
// 8 Threads
Nevay to List       682085628ns
Nevay to Sieve      172952467ns


get city and state using zip code, scrape in Python

First, a few general comments about the 'main function' in Python. The __name__ == '__main__' idiom is a way to distinguish between simply importing a module and launching one. You may want to simply import the module you just wrote to reuse the checkZips function elsewhere, but you don't want the main function to run when doing that.

Some people simply write:

if __name__ == '__main__':
    main()

to run a function called main() when running this module (eg. with python mymodule.py). This is only a convention, the function could be called whatever you want it to be, or you could simply put all the code in the conditional, as you've done here. TextMate provided you with a main() function with a pass statement to specify that it does nothing and avoid a syntax error. Long story short: remove that main function!

import sys
import os
import csv
from bs4 import BeautifulSoup
import requests

Your module lacks a docstring. It could contain the explanation you gave us. Read more about Python code conventions in PEP 8 and docstring conventions PEP 257.

def main():
    pass

Remove that, as said earlier.

def getCityState(zipCode):
# given zip code, return city and state if available

This should be a docstring.

    zipCode = str(zipCode)

Your zip code is already a string, you don't need to convert it first.

    url = "http://www.city-data.com/zips/" + zipCode + ".html"
    r = requests.get(url)
    data = r.text
    soup = BeautifulSoup(data)

You may not need all those variables, but that's up to you (eg. you could simply write BeautifulSoup(r.text)).

    if soup.findAll(text="City:") ==[]:

Be careful about indentation and follow PEP8 closely (eg. using a PEP 8 checker). Here you need a space after the ==, it makes the whole code easier to read and allows everyone to focus on the important matters.

        cityNeeded = soup.findAll(text="Cities:")
        for t in cityNeeded:
            return t.find_next('a').string

    else:
        cityNeeded = soup.findAll(text="City:")
        for t in cityNeeded:
            return t.find_next('a').string

First, you might want to know about list comprehensions, this can be written as return [t.find_next('a').string for t in cityNeeded]. Second, Don't repeat yourself! Having duplicated code increases the likelihood of errors, eg. when you only fix one part. It is easy here, you can simply move the loop out of the conditional. You can also use a regular expression to remove the conditional entirely (untested):

cityNeeded = soup.findAll(text=re.compile("City:|Cities:"))

Make sure to import re.

def getTableFromCSV(fileName, settings):
    with open(fileName, settings) as fp:
        reader = csv.DictReader(fp, skipinitialspace=True)
        table = [row for row in reader]
        header = reader.fieldnames
    return header,table # header and table are lists of dicts

Nothing much to say here, so I will talk about coding style. :) PEP 8 says that you should write get_table_from_csv. If you don't like it, you don't have to, it's just how most Python coders write code. Second, header,table should be header, table. Third, PEP 8 says you need a double space before inline comments (return header, table  # ...). Fourth, this isn't really a comment, and would be a better fit in the docstring. Fifth, about indentation: don't mix tabs and spaces, and no trailing spaces after your lines. (You didn't have any trailing spaces in this function though).

def printTable(header, table):
# print table in console
    print"begin printTable"
    for row in table:
        print row
    print"end printTable"

print is statement in Python 2, but a function in Python 3. I'd advise you to switch to Python 3 since Python 2 will soon be the past of Python, and at least write things such as print("end printTable") to prepare yourself to write Python 3. There are a few major differences and many small differences between the two version, so the earlier you switch, the easier it will be to get acustomed to Python 3.

Also, remove debug prints when you don't need them anymore.

def addToList(theList, position, newItem):
    theList.insert(position, newItem)
    return theList

def addToDict(theDict, newKey, newItem):
    theDict[newKey] = newItem
    return theDict

Come on, you don't need a function for those!

Overall, don't be overwhelmed by my comments. The code is nice, but there a lots of little things to improve. :)


How would you ruby-fy this non-AR model class for Ruby on Rails app?

For starters:

def initialize(attributes = {})
  attributes.each do |attr, value|
    self.send "#{attr}=", value
  end
end

Would allow you to turn:

r1 = Role.new
r1.id = 1
r1.symbol = :owner
r1.name = "Owner"
r1.has_all_permissions = true
role_list << r1

into:

role_list << Role.new {:id => 1,
                       :symbol => :owner,
                       :name => "Owner",
                       :has_all_permissions => true}

which is much more ActiveRecord-like.

Storing the roles_list in a global variable might be acceptable for your application and would also allow you to have:

def self.create(attributes = {})
  $roles_list << Role.new(attributes)
end

and:

def self.all
  return $roles_list if $roles_list

  # rest of code to init $roles_list with default roles
end

for further ActriveRecord-ness

Then:

def self.find(id)
  roles = Role.all

  roles.each do |role|
    if role.id == id
      return role
    end
  end

  nil
end

could be re-written like so:

def self.find(id)
  Role.all.find{|r| r.id == id}
end

That is because Array has its own #find method. The same refactoring could be applied to self.find_by_symbol.

As a final note, I would move the attr_accessor lines to the top.

Hope I helped :-)


Find numbers that are both Triangle numbers and Square numbers

First, I tried to be mathematically clever (by trying something with the odd and evenness of triangular and square numbers). That resulted in me making a mistake and getting a bogus result.

Then I took a good look at your code. You are calling Math.sqrt for every iteration. That's really slow, so if we could work around that, it'd be a huge performance boost. And we can work around it, by keeping track of the next target to hit.

Additionally, if we're gonna pass every integer anyway, why don't we add them as we go, rather than constantly recalculation triangle? A triangular number consists of all the whole positive integers at or below N, so by simply summing as we go, we can keep a calculated triangular number in memory without the need to recalculate every iteration.

The resulting code...

function main() {
var timer = Date.now();
  var triangleSum = 0;
  var productCounter = 1;
  var product = 1;
  for (var i = 1; i < 100000000; i++) {
    triangleSum += i;
    if(triangleSum === product)
    {
        document.getElementById('log').innerHTML += '
' + Math.sqrt(triangleSum)+ ' and ' + i + ': ' + triangleSum + '
';
    }
    if(triangleSum >= product)
    {
        productCounter++;
        product = productCounter * productCounter;
    }
  }
  document.getElementById('log').innerHTML += '
Time taken: ' + (Date.now() - timer) + ' milliseconds
';
}

Runs in 380 ms compared to the 5346 ms it was giving me before.


I see some things that may help you improve your program.

Use include guards

You probably already know this, but the Ticker.hpp file should have include guards to allow it to be included in multiple files within the same project.  The ordinary form would be something like this:

#ifndef TICKER_HPP
#define TICKER_HPP
// contents of Ticker.hpp go here
#endif //TICKER_HPP

Avoid data races

The tickfunction accesses std::cout but that's a single resource that might simultaneously be used by other threads.  One fix for this is to use a std::mutex like this:

std::mutex cout_mutex;

// wherever cout is used:
some_function() {
    std::lock_guard<std::mutex> lock(cout_mutex);
    std::cout << "Now we can do this safely!\n";
}

Note that std::lock_guard is intended to be used via RAII so that the lock is obtained when the object is created and released when it's destroyed, making it easy to avoid the bug of forgetting to unlock the mutex.

If the actual function called within your code each "tick" uses shared resources, you should apply the same technique.

Use standard durations

To answer your question, one way to use a standard duration would be to specify a particular one.  For example, if you decided that millisecond resolution was enough for this program (and it probably is), the class declaration might look like this:

class Ticker {
private:
    const std::chrono::milliseconds interval;
    std::atomic_bool run;
    std::thread tickthread;
public:
    Ticker(std::chrono::milliseconds intval);
    Ticker(const Ticker& orig) = delete;
    virtual ~Ticker();
    void stop();
private:
    void tickfunction();
};

Use the chrono literals for more readable code

With the code modified to use a standards time interval, we can now use it with std::chrono_literals and make the code even easier to write and to read.

int main()
{
    using namespace std::chrono_literals;
    Ticker t(500ms);
    Ticker t10(10s);
    std::this_thread::sleep_for(15s);
}

Allow for thread cancellation

For various reasons, there is no simple standard way of cancelling a thread.  However, there are almost always ways you can accomplish the equivalent in your own code.  I'll show how this might be done in the next suggestion.

Prefer async to raw threads

C++ has a std::async mechanism that is a bit higher level than the more fundamental thread and mutex mechanism.  The advantage to using it is that you can spend more time thinking about the problem you're trying to solve rather than the details of threads and locks.  Here's one way of refactoring your code that doesn't use thread explicitly:

#include <iostream>
#include <future>
#include <chrono>

static std::mutex cout_mutex;

class Ticker
{
public:
    // create but don't execute yet
    Ticker(std::shared_future<void> f, std::chrono::milliseconds interval)
    : interval{interval}, done{f}
    { }
    // run the thread
    std::future<void> run() {
        return std::async(std::launch::async, &Ticker::tickWrapper, this);
    }
private:
    void tickWrapper() {
        std::future_status status;
        do {
            status = done.wait_for(interval); // waits for the signal from main()
            if (status == std::future_status::timeout) {
                tickfunction();
            }
        } while (status != std::future_status::ready);
    }
    void tickfunction() {
        std::lock_guard<std::mutex> lock(cout_mutex);
        std::cout << "tick (" << std::this_thread::get_id() << ")" << std::endl;
    }

    const std::chrono::milliseconds interval;
    std::shared_future<void> done;
};

Note that I've separated tickWrapper from tickfunction.  The latter is now just a simple function that doesn't contain any references to timers or futures.  This allows us to separate the actual function from the mechanics of a recurring timer implementation.  Here's a main function showing how to create and drive this new Ticker object:

int main()
{
    // for convenience in specifying time durations
    using namespace std::chrono_literals;

    // create a promise and associated shared_future
    std::promise<void> done;
    std::shared_future<void> done_future(done.get_future());

    // create the Ticker objects
    Ticker t1(done_future, 300ms);
    Ticker t2(done_future, 500ms);

    // start the threads
    auto x = t1.run();
    auto y = t2.run();

    // delay for a while
    std::this_thread::sleep_for(3s);

    // tell the other threads to stop
    done.set_value();

    std::cout << "Done\n";
}


Generating string expression from data for filter

Please note that I am not actually reviewing your code, but guiding you towards hopefully better design, so that you can fix the issues yourself
In that same vein, the code snippets I am including do not contain a full solution, but only the hints towards it.

These kinds of transformations are very easy if you start to decompose them.

For example, you have two different rules for dealing with string data type and list data type, so let's create two different entities to deal with them.

(Here I chose functions as top-level entities, you can create classes too)

def convert_string(key, value):
    return f"{key} == '{value}'"   # I am assuming you want value in quotes

Similarly, define one for lists. (Hint: You should define one entity for dict and list function should only call and compose dict function)

How do you choose which entity to call? Simple, call another entity.

def choose_function(key, value):
    # Your logic here
    # returns func

And finally, a composing function, which is also your main function.

def convert(params):
    str_list = []
    for key, value in params.items():
        str_list.append(choose_func(key, value)(key, value))
    return ' and '.join(str_list)}

Why it would be better than your solution?

It is much easier to test. Your problem statement is pretty much easy, but these kinds of problems could become very big with lots of complex rules in the long run, where unit testability gives you the confidence to make changes
You divide the problem statement into individual sub-pieces and another sub-problem deal with how to merge those solutions. That would eliminate lots of your _first variables. It also makes it easier in the long run.
Code readability


This is where the Express Router class comes in.  You could define a router for ‘api/v1/Project’, mount that router to you main app, and then add the individual routes to the router.


Use predefined vs. custom enums to return appropriate message from service layer

I would not recommend using this pattern, personally. What you're effectively trying to do is signal error state, where 0 is null state and therefore success, similar to the way that you might do it with the Win32 API. The designers of c# and the .NET framework explicitly sought to avoid this pattern (and deem it to be an anti pattern). I think it works great for OS level work, and clearly does for both Windows and Linux, but it can be very cumbersome.

The "c#" method of doing this is by using exceptions. For a method called SaveChanges, the return type is usually either void or bool. I personally recommend void, because bool doesn't give any context for what caused the operation to fail, and is really just a slim version using an enumeration.

For the remainder of cases, there is USUALLY a built in exception type that is appropriate. InvalidOperationException can be used to state that you can't save the changes because the entity already exists, for instance. In a Web API, you can return something in one of the error ranges for this case.

If you really look at the exception model,  I believe you'll find that it does provide a decent solution to this problem for you. It liberates you from having to build complex enumerations, and mapping them across layers. Your application is then free to focus on interactions with the layer it communicates with, and you don't need to build extra services across each tier to describe what happened. Ideally, this means easier long term maintenance as well, because changing any part of the implementation does not result in needing to update the ENTIRE implementation.