Functional abstraction to find nth root of a number - Newton raphson Below is the solution:

def approx_deriv(fn, x, dx=0.00001):
    return (fn(x + dx) - fn(x))/dx

def improve(update, isdone, guess=1, max_iterations=100):
    def recur_improve(guess, i):
        if i > max_iterations or isdone(guess):
           return guess
        else:
           return recur_improve(update(guess), i + 1)
    return recur_improve(guess, 1)

def newtons_method(fn, guess =1, max_iterations=100):
    def newtons_update(guess):
        return guess - fn(guess) / approx_deriv(fn, guess)
    def newtons_isdone(guess):
        ALLOWED_ERROR_MARGIN = 0.0000001
        return abs(fn(guess)) <= ALLOWED_ERROR_MARGIN
    return improve(newtons_update,
                        newtons_isdone,
                        max_iterations)

def nth_root(rootfunc, x):
    return rootfunc(x)

Test output:

>python -i nth_root.py
>>> def cuberoot(x):
...     return newtons_method(lambda y: y * y * y - x)
...
>>> result = nth_root(cuberoot, 27)
>>> result
3.000000000000141
>>>

My question:

Above is the functional abstraction for finding the \$n_{th}\$ root of a number. Is it possible to improve this abstraction?

First, congratulations for the work you did : one can easily see that you put some thinking into it and as far as I have tested it, it works.

However, I still have a few comments and most of them are about the fact that you might have tried to be too clever.

Indeed, the use of both recursion and nested functions is a bit too much for my brain. Among other things, it took me a while to spot that the guess argument of the newtons_method is not used (and I am not even sure of it, am I right ?). You could make is easier to grasp by using names conveing more meaning.

Here's my attempt to un-nest functions and to change names (I am fully aware that they are not perfect) :

def recur_improve(next_step, isdone, x, i, max_iterations):
    if i > max_iterations or isdone(x):
       return x
    else:
       return recur_improve(next_step, isdone, next_step(x), i + 1, max_iterations)

def improve(next_step, isdone, initval=1, max_iterations=100):
    return recur_improve(next_step, isdone, initval, 1, max_iterations)

def newtons_method(fn, max_iterations=100):
    def newtons_step(x):
        return x - fn(x) / approx_deriv(fn, x)
    def newtons_isdone(x):
        ALLOWED_ERROR_MARGIN = 0.0000001
        return abs(fn(x)) <= ALLOWED_ERROR_MARGIN
    return improve(newtons_step,
                        newtons_isdone,
                        max_iterations)

Then, the recursion can be easily replaced by a while loop :

def improve(next_step, isdone, initval=1, max_iterations=100):
    x = initval
    i = 1
    while i < max_iterations and not isdone(x):
        x = next_step(x)
        i +=1
    return x

Then, one can see that counting iterations is boring. We have builtins to do this in a concise and efficient way.

def improve(next_step, isdone, initval=1, max_iterations=100):
    x = initval
    i = 1
    for i in range(max_iterations):
        if isdone(x):
            break
        x = next_step(x)
    return x

(I might have introduced on off-by-one error in the iteration counts but I can't be bothered).

Then, instead of passing functions using your original functions, you could just write conditions directly :

def improve(fn, initval=1, max_iterations=100, epsilon=0.0000001):
    x = initval
    for i in range(max_iterations):
        if abs(fn(x)) <= epsilon:
            break
        x -= fn(x) / approx_deriv(fn, x)
    return x

Then you don't even need to have multiple functions anymore and your code boils down to :

def newtons_method(fn, initval, max_iterations=100, epsilon=0.00000001):
    x = initval
    for i in range(max_iterations):
        if abs(fn(x)) <= epsilon:
            break
        x -= fn(x) / approx_deriv(fn, x)
    return x

It seems much clearer to me.

Also, your def nth_root(rootfunc, x) function doesn't seem really useful to me. You could just have :

def cuberoot(x):
    return newtons_method(lambda y: y * y * y - x, x)

result = cuberoot(27**3)

print(result)

And get the result you expect.

Finally, your function deserves some documentation.

Final code :

def approx_deriv(fn, x, dx=0.00001):
    """ some docstring. """
    return (fn(x + dx) - fn(x))/dx

def newtons_method(fn, initval, max_iterations=100, epsilon=0.00000001):
    """ some docstring. """
    x = initval
    for i in range(max_iterations):
        if abs(fn(x)) <= epsilon:
            break
        x -= fn(x) / approx_deriv(fn, x)
    return x

def cuberoot(x):
    """ some docstring. """
    return newtons_method(lambda y: y * y * y - x, x)

result = cuberoot(27**3)

print(result)


Node.js http retry do while mechanism I have implemented a simple do/while loop to handle a http request to a third-party. I don’t know when the data I receive will have elements in its array so I retry up to 10 times until the array given to me has data which then I stop and store the data in my database.

My question is in terms of performance is my API going to be affected by having this retry mechanism using a simple do/while loop? If so what is a better way to implement a retry mechanism like this??

public MAX_RETRY = 10;

public async processMetrics(sessionId: any, side: any): Promise <any> {

  try {

    let metrics;
    let retryAttempts = 0;

    do {
      await new Promise((done) => setTimeout(done, 2000));
      metrics = await this.getMetrics(session._id, sessionRequest._id);
      retryAttempts++;

    } while (!metrics.body.metrics.length && (retryAttempts < this.MAX_RETRY));

    // Store in DB
  } catch (err) {

  }

}

public async getMetrics(sessionId: any, requestId: any): Promise <any> {

  const url = this.config.backendUrl + "/check/metrics";

  const options = {
    uri: url,
    headers: {
      "X-IDCHECK-SESSION_ID": sessionId,
    },
    body: {},
    json: true,
    resolveWithFullResponse: true,
  };

  const metrics = await request.get(options);

  return metrics;

}

I am calling processMetrics from a different async function.  processMetrics is calling a backend (getMetrics) every 2 seconds, it will retry 10 times to see if the result is ready. If it is I will store something in the database and then return.

In terms of performance this should not be an issue, because async operations don't block the event loop, meaning your server will happily accept incoming requests in between the retries. Once in 2 seconds it will be busy making the request to the 3rd party API, but other than that your will not be blocking the main thread.

(You can verify that by shooting requests to your API while it is in the while loop).


Expand hostnames from a string of hostnames and/or regex This code will be passed a string which will contain one or more hostnames.

A hostname can be

dotted-decimal
plain name (like my_host)
FQDN or even partially qualified
A string containing regex (my_hos[tes])

target_list is an array of known hosts to match any regex against.

Any suggestions on cleaning this up?

sub expand_names {
    my $host = shift;

    my @raw_hosts = split(' ', $host);
    my @hosts;

    my %host_hash;
    foreach my $raw_name (@raw_hosts) {
        $host_hash{$raw_name} = {};
        #if the name might contain regex, let's see what it matched
        if ($raw_name !~ m/^[a-zA-Z0-9-]+$/) {
            foreach my $target (@target_list) {
                if ($target =~ m/^$raw_name$/) {
                    $host_hash{$raw_name}->{$target} = 1;
                }
            }
            #if we didn't match anything, then just use the name
            if (not keys %{$host_hash{$raw_name}} ) {
                $host_hash{$raw_name}->{$raw_name} = 1;
            }
        }
        else {
            # Regular host name or dotted-decimal
            $host_hash{$raw_name}->{$raw_name} = 1;
        }
    }
    # Gather all the hosts we found
    foreach $raw_name  (keys %host_hash) {
        @hosts = (@hosts, (keys %{$host_hash{$raw_name}}));
    }
    return sort @hosts;
}

Your function expects a single argument that should contain hostnames or patterns separated by spaces:

sub expand_names {
    my $host = shift;
    my @raw_hosts = split(' ', $host);
    foreach my $raw_name (@raw_hosts) {

Why not use multiple arguments?

sub expand_names {
    foreach my $raw_name (@_) {

Since @hosts is empty when you reach this code:

# Gather all the hosts we found
foreach $raw_name  (keys %host_hash) {
    @hosts = (@hosts, (keys %{$host_hash{$raw_name}}));
}

You could simplify:

foreach $raw_name  (keys %host_hash) {
    push(@hosts, keys %{$host_hash{$raw_name}});
}

And actually, I don't see the reason for building a map of maps, when a simple flat map will do.
Consider this alternative implementation:

sub expand_names {
    my %hosts;
    foreach my $raw_name (@_) {
        #if the name might contain regex, let's see what it matched
        if ($raw_name !~ m/^[a-zA-Z0-9-]+$/) {
            my $found = 0;
            foreach my $target (@target_list) {
                if ($target =~ m/^$raw_name$/) {
                    $found = $hosts{$target} = 1;
                }
            }
            $hosts{$raw_name} = 1 if not $found;
        } else {
            # Regular host name or dotted-decimal
            $hosts{$raw_name} = 1;
        }
    }
    return sort keys %hosts;
}


My perfect shopping cart class Here is my shopping cart class! I want it to be perfect and never to have to implement it.

What improvements would you do?

New : IComparable

interface IComparable
{
    public function equals($object);
}

New : ICartItem

interface ICartItem extends IComparable
{
    public function setQuantity($quantity);
    public function getQuantity();
}

New : ICart

interface ICart
{
    public function add(ICartItem $item);
    public function remove(ICartItem $item);
    public function getQuantity(ICartItem $item);
    public function setQuantity(ICartItem $item, $quantity);
    public function isEmpty();
    public function getItems();
    public function clear();
}

New : Cart

class SessionCart implements ICart
{
    const IDENTIFIER = '_CART_';
    protected $items;

    public function __construct(&$container = null)
    {
        if (is_null($container)) {
            if (session_id() == '') {
                session_start();
            }
            if (!isset($_SESSION[self::IDENTIFIER])) {
                $_SESSION[self::IDENTIFIER] = array();
            }
            $container = & $_SESSION[self::IDENTIFIER];
        }
        $this->items = & $container;
    }

    public function add(ICartItem $item)
    {
        $index = $this->getIndexOfItem($item);

        if ($index == -1) {
            $this->items[] = $item;

        } else {
            $item = $this->items[$index];
            $item->setQuantity($item->getQuantity() + 1);
        }

        return $item->getQuantity();
    }

    public function remove(ICartItem $item)
    {
        $index = $this->getIndexOfItem($item);

        if ($index == -1) {
            throw new Exception('The item isn\'t inside the cart.');
        }

        $item = $this->items[$index];

        $quantity = $item->getQuantity() - 1;

        if ($quantity > 0) {
            $item->setQuantity($quantity);

        } else {
            unset($this->items[$index]);
        }
        return $quantity;
    }

    public function getQuantity(ICartItem $item)
    {
        $index = $this->getIndexOfItem($item);

        if ($index == -1) {
            return 0;

        } else {
            return $this->items[$index]->getQuantity();
        }
    }

    public function setQuantity(ICartItem $item, $quantity)
    {
        if (($quantity = (int)$quantity) < 1) {
            throw new Exception('A positive quantity is required.');
        }

        $index = $this->getIndexOfItem($item);

        if ($index == -1) {
            $item->setQuantity($quantity);
            $this->items[] = $item;

        } else {
            $item = $this->items[$index];
            $item->setQuantity($quantity);
        }

        return $item->getQuantity();
    }

    public function isEmpty()
    {
        return empty($this->items);
    }

    public function getItems()
    {
        return $this->items;
    }

    public function clear()
    {
        $this->items = array();
    }

    private function getIndexOfItem(ICartItem $item)
    {
        foreach ($this->items as $key => $value) {
            if ($item->equals($value)) {
                return $key;
            }
        }
        return -1;
    }
}

General

You should reconsider the naming of some methods. For example,

interface IComparable
{
    public function equals($obj);
}

is more clear about what it really does (comments are omitted to save space; of course, proper DocBlock comments should always be included with your code). This way, you can easily add other methods like isLessThan, isGreaterOrEqual, and so on. A method named compareTo as in

$a->compareTo($b)

I'd expect to be a valid callback for usort, thus returning -1 on $a < $b, 0 on $a == $b, or 1 on $a > $b.

Cart

One day, you might want to store your cart items in a database, so using $_SESSION directly is not optimal. I'd define a cart interface:

interface ICart
{
    public function add(IComparable $obj);
    public function remove(IComparable $obj);
    public function getQuantity(IComparable $obj);
    public function setQuantity(IComparable $obj, $qty);
    public function isEmpty();
    public function getAll();
    public function clear();
}

class Cart implements ICart
{
    const IDENTIFIER = '_CART_';
    protected $container;

    public function __construct(&$container = null)
    {
        if (is_null($container)) {
            if (session_id() == '') {
                session_start();
            }
            if (!isset($_SESSION[self::IDENTIFIER])) {
                $_SESSION[self::IDENTIFIER] = array();
            }
            $container = &$_SESSION[self::IDENTIFIER];
        }
        $this->container = &$container;
    }
}

With this approach, you can always build other cart implementations, like a database-aware one. The Cart class can be provided with an array, which will be used to store the data. If you want, you can pass in the $_SESSION superglobal directly, or use any other array. If omitted, a kind of namespace within the session variables (self::IDENTIFIER, '_CART_') is used to store the data.

Since the session - if needed - is started within the constructor, you can rely on its existence in the subsequent methods. BTW: the existence of the $_SESSION array does not guarantee that a session has been started! Use session_id() to check that instead.)

Internal Data Structure

Since your data array can be accessed from outside the cart, the current data structure is prone to get out of sync. It is more robust to swap the indices. Then you can replace getIndex() with getEntry(), which makes the handling much easier.

class Cart implements ICart
{
    ... // see above

    public function add(IComparable $obj)
    {
        $entry = &$this->getEntry($obj);
        $entry['quantity']++;

        return $entry['quantity'];
    }

    public function remove(IComparable $obj)
    {
        $entry = &$this->getEntry($obj);
        $entry['quantity'] = max(0, --$entry['quantity']);

        return $entry['quantity'];
    }

    public function getQuantity(IComparable $obj)
    {
        $entry = &$this->getEntry($obj);
        return $entry['quantity'];
    }

    public function setQuantity(IComparable $obj, $qty)
    {
        $entry = &$this->getEntry($obj);
        if ($entry['quantity'] > 0) {
            $entry['quantity'] = (int) $qty;
        }
        return $entry['quantity'];
    }

    public function isEmpty()
    {
        $total = 0;
        foreach ($this->container as $entry) {
            $total += $entry['quantity'];
        }
        return $total <= 0;
    }

    public function getAll()
    {
        $cart = array();
        foreach ($this->container as $entry) {
            if ($entry['quantity'] > 0) {
                $cart[] = $entry;
            }
        }
        return $cart;
    }

    public function clear()
    {
        $this->container = array();
    }

    private function &getEntry(IComparable $obj)
    {
        foreach ($this->container as &$entry) {
            if ($obj->equals($entry['item'])) {
                return $entry;
            }
        }
        $entry = array(
            'item' => $obj,
            'quantity' => 0
        );
        $this->container[] = &$entry;
        return $entry;
    }
}

As you can see, all methods (excl. isEmpty()) become much simpler.


Showing a spinner over content - fade out some elements and disable clicking on some elements I am displaying a loading spinner inside of various elements. There are four potential states an element could be in:

Reduced opacity; unresponsive to pointer events.
Reduced opacity; responsive to pointer events.
Maintain opacity; unresponsive to pointer events.
Maintain opacity; responsive to pointer events.

I'm trying to write some CSS which is elegant, but also allows for enough flexibility to achieve the above goals.

I've attached a code-snippet to this post showing my implementation. Please note that it is intended only for modern browsers.

My concerns are:

I feel like this is the sort of thing which should be done using an overlay element and clever usage of z-index, but I also feel like my scenarios are too dynamic to support this sort of approach.
I am using the * selector with some filters applied to it in order to properly select elements. It is generally frowned upon to use the * selector. However, I don't believe that inverting my logic and applying .is-belowFadeout and .is-unclickableWhenSpinning classes is more appropriate.

How should I be approaching this problem?

.content {
    width: 180px;
    height: 180px;
    position: relative;
    background-color: lightGray;
    padding: 10px;
}

.content-subContent {
    width: 40px;
    height: 40px;
    background-color: red;
    cursor: pointer;
    display: inline-block;
}

@-webkit-keyframes spin {
  0% {
    transform: rotate(0deg);
  }
  100% {
    transform: rotate(359deg);
  }
}

.spinner {
    border: 1px solid #4ea6ea;
    border-left-color: transparent;
    border-top-color: transparent;
    border-radius: 50%;
    -webkit-animation: spin 600ms infinite linear;
    display: none;
    vertical-align: middle;
}

.spinner--large {
    width: 32px;
    height: 32px;
    border-width: 4px;
}

/* TODO: This seems like a poor way of doing this. What would be more robust? */
.is-showingSpinner > *:not(.is-aboveFadeout):not(.spinner) {
    opacity: .2;
}

.is-showingSpinner > *:not(.is-alwaysClickable) {
    pointer-events: none;
}

.is-showingSpinner > .spinner {
    display: inline-block;
}

.u-overlay {
    position: absolute;
    bottom: 0;
    left: 0;
    right: 0;
    top: 0;
}

.u-marginAuto {
    margin: auto;
}

.is-alwaysClickable:hover {
    background-color: #8b0000;
}

First, For the universal selector, the performance diference will be negligible exept if you load 100s or 1000s of those element with into one web page.

This said, I reviewed you code and made several changes :

Used pseudo elements to make the spinner and the overlay for 2 reasons :

one unsemantic html tag removed
this alows to play with z-index so the red elements can be behind or in front of the overlay.

Used z-index instead of opacity to fade red elements out and moved the pointer-events property to the overlay pseudo element so that the .subContent can still be clicked.

.content {
    width: 180px; height: 180px;
    position: relative;
    padding: 10px;
}
.is-showingSpinner:before, .is-showingSpinner:after{
    content:'';
    position:absolute;
    bottom: 0; left: 0; right: 0; top: 0;
    z-index:2;
    pointer-events:none;
}
.is-showingSpinner:before {
    width:100%; height:100%;
    background-color: rgba(211, 211, 211, .8);
}
.is-showingSpinner:after {
    margin:auto;
    width: 32px; height: 32px;
    border: 4px solid #4ea6ea;
    border-left-color: transparent;
    border-top-color: transparent;
    border-radius: 50%;
    -webkit-animation: spin 600ms infinite linear;
    animation: spin 600ms infinite linear;
}
.content-subContent {
    width: 40px; height: 40px;
    background-color: red;
    display: inline-block;
}
@-webkit-keyframes spin {
    0%   { transform: rotate(0deg); }
    100% { transform: rotate(359deg); }
}
/* TODO: This seems like a poor way of doing this. What would be more robust? */
 .is-showingSpinner > .is-aboveFadeout {
    position:relative;
    z-index:2;
}
.is-alwaysClickable:hover {
    cursor: pointer;
    background-color: #8b0000;
}

I also removed the classes you added to the spinner element. If you still need them, you can add them to the .content div to style the spinner ( :after pseudo element) as needed.


Job queue with threading On many articles and blogs, I have read that exceptions should not decide flow of your code.

I have wrote the following code using one thread:

public void run() {
        while (true) {
            if (jobQueue.size() != 0) {
                Job job =
                    (Job) jobQueue.pop();
                try {
                    JobProc jobProc = new JobProc(job);
                    Thread threadJP = new Thread(jobProc);
                    threadJP.start();
                } catch (Exception e) {
                    try {
                        jobQueue.push(job);
                    } catch (StorageFailed | InternalError e1) {
                        try {
                            writeJobToFile(job);
                        } catch (ExecFailed e2) {
                            logger.logFatal("failed to push back job"+ job.getId(), e2);
                        }
                    }
                }
                } else {
                    sleep(5000);
                }
        }
    }

In the first catch exception I can push job back to queue. In the second catch, if it fails to push it back to queue, I am writing that job to file. In third catch, I logging it as FATAL error event.

Am I doing it wrong or is there any better way of doing this?

Exceptions

Exceptions, by their nature, alter the control flow of the program. That's what they're there for. It's just that they should not be used for normal control flow – only in error cases. In your case, a job failing to execute is most likely an error case so I wouldn't object to the usage of exceptions here.

Question yourself, however, what kind of errors you are catching and how you handle them.

It seems that the first (outermost) try / catch block is intended to catch any exception that might be thrown by the job. This looks alright to me. We don't know the job and it might fail for any number of reasons. That should not affect our processing of further jobs. catching Exception also seems appropriate in this case, even though it is normally strongly discouraged. You could even consider catching Throwable but I like Exception better, thinking that if something that is not an Exception gets thrown, the program should probably die immediately.

Well, except that it doesn't.

You are not catching exceptions thrown by the job. Those will be thrown on its own thread and not seen by you. Instead, that thread's uncaught exception handler will deal with them. And the default exception handler – as sad as it is – simply prints a stack trace and otherwise ignores the exception. If you want to change that, you can set a custom uncaught exception handler.

The only case in which your catch gets activated is if creation of the thread fails. And in this case, you're probably in deep trouble anyway and should let your application die, eventually after logging an error. The recovery strategy in the catch is therefore inappropriate here. But let's assume for the following discussion that you've corrected this and the error handling code is in the thread that actually executes the job.

Okay, you have caught an exception from a job that failed for whatever reason. What can you reasonably do now? Your backup strategy is to simply push the job back onto the queue. I'm not sure whether this is a good idea. If some of your jobs can fail for spurious reasons, re-trying a fixed number of times might be a viable strategy. But re-trying forever almost certainly isn't. What if every once in a while you get a job that simply fails unconditionally. Then you keep pushing those jobs in your queue which will slowly but sure enough fill up with garbage jobs that will never succeed but consume precious resources and slow down the processing of jobs that have a real chance to do something useful. If you want to give each job \$n\$ chances, you could manage \$n\$ queues and move failed jobs from queue \$i\$ to queue \$i+1\$ if \$i<n\$ or else drop them onto the floor. Alternatively, you could attach a counter to each job that gets incremented for each failed attempt to execute it.

But your code also handles the case that you cannot push a job back to the queue. I'm not sure whether this is something you should handle. If you're out of memory, let your application die, maybe logging an error. I don't know the nature of your jobs but what good does it do you to have them written to a file? Will you ever re-load them from that file? And if you do, are you sure that this will be a good thing? Imagine that something awkward happens and your application gets flooded with nonsense jobs. Hopefully, it will eventually crash. But now the user re-starts it, hoping that it will behave sane again – only to find that it keeps loading the nonsense jobs from its file log and crashes again.

As a last resort, you log a fatal error. This is not an entirely bad idea. However, if all the things failed that lead up to this situation, I'd rather kill the application and not pretend everything will be fine. You can still log the error but after that, re-throw the exception and die.

I know that unit-testing error handling code can be challenging but I encourage you to do it nevertheless. Feed an intentionally poisoned job to your queue. Make sure that the exception gets caught where you think it should. If you give jobs a second chance, construct a job that fails the first but not the second time. Then construct one that always fails. Construct one that cannot be serialized. If you truly cannot construct a scenario that will execute an error handler, this might be an indicator that the handler is redundant and should be removed from the code.

As a general guideline, don't try to keep your application running no matter what. If things really get bad, giving up is the best thing you can do: „Dead Programs Tell No Lies“.

Readability

The code is not exactly readable. It already got a lot better after you've edited your question to use shorter names instead of the ones your application seems to use. Descriptive names are good but you can also overdo it. If your names simply are that long, I prefer to use auxiliary variables to keep if conditions and function calls reasonable.

What hurts readability more severely, though, is the triple nesting of try-catch blocks.  I suggest you refactor this like so.

boolean handleJob(final Job job) {
    try {
        return firstStrategy(job);
    } catch (final Exception e) {
        // Maybe report the error
    }
    try {
        return secondStrategy(job);
    } catch (final Exception e) {
        // Maybe report the error
    }
    try {
        return thirdStrategy(job);
    } catch (final ThridException e) {
        // Definitely report the error and give up
        return false;
    }
}

Refactoring deeply nested control structures into separate functions and using early returns is a generally useful pattern.

Concurrency

Your code spawns a new thread for each job and immediately detaches from it. This is a bad idea. Creating threads is costly and if you create significantly more than your hardware can execute in parallel, your performance will suffer. Instead of using one thread per job, use a thread pool with a fixed number of threads. This also means that you no longer have to use the threads uncaught exception handler for dealing with exceptions thrown by your jobs but can catch them much more naturally.

I assume that your code runs on Android which already has built-in support for background jobs so you don't even have to roll the code yourself. Just check it out.

The other thing that strikes me is that you simply sleep if your queue is empty. This is not a good strategy. If you set sleepTime1 too low, you'll waste a lot of CPU cycles (and battery) for doing nothing. If you set it too high, your application becomes unresponsive. Instead of sleeping and polling, use proper synchronization techniques. That is, wait() and notify().

1 The question has since been edited to use a hard-coded constant instead of the variable sleepTime.


Better way to calculate double-scattering diffraction using cartesian product of arrays? This script calculates points in reciprocal space for hexagonal 2D lattices, then uses the cartesian product from itertools to add each vector from one lattice to all of the vectors of the other in the line
np.array([a+b for a, b in list(itertools.product(p1.T, p2.T))])

It's slow right now because as written its instantiating millions of tiny numpy arrays.
I'm aware of:

Cartesian product of x and y array points into single array of 2D points
Using numpy to build an array of all combinations of two arrays
cartesian products in numPy

and I suspect there's some way to do this, possibly using np.meshgrid  or np.mgrid that's faster, uses less memory and looks cleaner, but I can not figure out how.
I will use the output in an optimization loop matching these positions to measured positions, so it needs to be callable several hundred times in a row, so reusing large array spaces rather than instantiating and garbage collecting them might have some advantages.
 click for larger
import numpy as np
import matplotlib.pyplot as plt
import itertools

def rotatem(xy, rot):
    r3o2, twopi, to_degs, to_rads = np.sqrt(3)/2., 2*np.pi, 180/np.pi, np.pi/180
    c, s = [f(to_rads*rot) for f in (np.cos, np.sin)]
    x, y = xy
    xr = c*x - s*y
    yr = c*y + s*x
    return np.vstack((xr, yr))

def get_points(a=1.0, nmax=5, rot=0):
    r3o2, twopi, to_degs, to_rads = np.sqrt(3)/2., 2*np.pi, 180/np.pi, np.pi/180
    g = twopi / (r3o2 * a)
    i = np.arange(-nmax, nmax+1)
    I, J = [thing.flatten() for thing in np.meshgrid(i, i)]
    keep = np.abs(I + J) <= nmax
    I, J = [thing[keep] for thing in (I, J)]
    xy = np.vstack((I+0.5*J, r3o2*J))
    return g * rotatem(xy, rot=rot)

r3o2, twopi, to_degs, to_rads = np.sqrt(3)/2., 2*np.pi, 180/np.pi, np.pi/180

a1, a2, rot = 1.0, 2**0.2, 22

p1 = get_points(a=a1, nmax=20)
p2 = get_points(a=a2, nmax=20, rot=rot)
p3 = get_points(a=a2, nmax=20, rot=-rot)

d12 = np.array([a+b for a, b in list(itertools.product(p1.T, p2.T))])
d13 = np.array([a+b for a, b in list(itertools.product(p1.T, p3.T))])
d12, d13 = [d[((d**2).sum(axis=1)<4.)] for d in (d12, d13)]

if True:
    plt.figure()
    for d in (d12, d13):
        plt.plot(*d.T, 'o', ms=2)
    plt.gca().set_aspect('equal')
    plt.show()

You can replace:
d12 = np.array([a+b for a, b in list(itertools.product(p1.T, p2.T))])

with something like:
p1 = p1.T
p2 = p2.T
p3 = p3.T
d12 = p1[:,np.newaxis,:] + p2[np.newaxis,:,:]
d12 = my_d12.reshape((len(p1)*len(p2),2))

I find it most of the times easier to use the first index of an array for point_index and the second index for the dimensions, hence the .T's
With the use of the magic index np.newaxis at the right places you can create numpy array's of shape (M,N) with normal operators acting on arrays of shape (M) and (N).
With the reshape method d12 changed again to the shape in your original solution.


Object key-value map reversal I have implemented functions that will let me reverse a one-to-many value 'Map', that is a plain JavaScript object.

For example,

var map = {'foo': 'bar', 'baz': 'bar', 'moo': 'zoo'};

becomes

var reversedMap = {'zoo': ['moo'], 'bar': ['foo', 'baz']}

and also a function to bring back reversed map to a non-reversed map.

Here's a jsbin to show that my solution is working.

In the following code I have used lo-dash utility toolbelt, but you need not be acquainted with it in order to understand the code. Assume that I trust in performance of lo-dash and what I'm trying to optimize is my own code.

Just for sake of clarity, _.forOwn iterates over object's own properties, and _.forEach iterates over each element of an array.

var identity = function(x) {return x;};
var reverseMapFromMap = function(map, callback) {
    callback = callback || identity; //callback helps to parseInt numeric values, otherwise every object key is a string by default
    var reversedMap = {};
    _.forOwn(map, function(value, key) {
        key = callback(key);
        if (_.isUndefined(reversedMap[value]))
            reversedMap[value] = [key];
        else
            reversedMap[value].push(key);
    });
    return reversedMap;
};

var mapFromReverseMap = function(reverseMap, callback) {
    callback = callback || identity;
    var map = {};
    _.forOwn(reverseMap, function(revMembers, revKey) {
        _.forEach(revMembers, function(revMember) {
            map[revMember] = callback(revKey);
        });
    });
    return map;
};

I'm pretty confident in my solution as it's quite basic, but as it is a frequent part of my code, I'd like to know if you can see any ways to improve performance, or alternatively if there's a better way to solve this problem.

Your code is pretty good as-is. There are other ways to do the same thing, but yours is fine.

One thing I noticed is that you've skipped braces on the if...else in reverseFromMap. My advice is to always use braces, even for one-line expressions.

However, you could also do

reversedMap[value] || (reversedMap[value] = []);
reversedMap[value].push(key);

You can collapse that into 1 line, although it's a little too terse if you ask me

(reversedMap[value] || (reversedMap[value] = [])).push(key);
// or
(reversedMap[value] = reversedMap[value] || []).push(key);

Or just reduce it to a ternary

reversedMap[value] ? reversedMap[value].push(key) : reversedMap[value] = [key];

The _.isUndefined check isn't really required, since reversedMap[key] is either a non-empty array, or it's false'y.

Other small stuff:

I'd prefer the name iterator instead of callback, but lo-dash uses callback in similar situations, so sticking with that convention is fine.
You don't necessarily need the identity function, as you could also do

x = callback ? callback(x) : x;

and just not bother with the identity function. It may or may not be faster - I haven't benchmarked it. In any event, both approaches work just fine, so there's no need to change anything.
If you do need an identity function, lo-dash has one built-in (_.identity). Saves you having to type it out.
Lastly, if you want to be stringent, you might want to check whether callback is a function, instead of just doing "default if false'y". E.g.

callback = _.isFunction(callback) ? callback : _.identity;
// or just plain JS
callback = typeof callback === 'function' ? callback : _.identity;

Personally, I'd skip this, though. If people pass in something that's not a function, it's their bug to deal with. Besides, I'd rather have the code fail loudly than having it silently ignore my (faulty) callback argument and default to something else without telling me. Still, type-checking is an option, so I thought I'd mention it.

As for alternatives to the overall approach, I'd use _.transform (which is basically the same as reduce/foldl but aimed at objects)

var reverseMapFromMap = function(map, callback) {
  callback = callback || _.identity;
  return _.transform(map, function (memo, value, key) {
    key = callback(key);
    memo[value] || (memo[value] = []);
    memo[value].push(key);
  }, {});
}

And again for mapFromReverseMap

var mapFromReverseMap = function(reverseMap, callback) {
  callback = callback || _.identity;
  return _.transform(reverseMap, function (memo, keys, value) {
    _.forEach(keys, function (key) {
      memo[callback(key)] = value;
    });
  }, {});
};


You can use ROW_NUMBER to rank your data according to date for each combination of id and question; then simply select the row with a ROW_NUMBER of 1:
WITH tbl_max_date AS (
  SELECT *,
         ROW_NUMBER() OVER (PARTITION BY id, question ORDER BY date DESC) AS rn
  FROM tests2.o1.mc
)
SELECT *
FROM tbl_max_date
WHERE rn = 1

If you could have more than one row with the same maximum value per group, you can use RANK in place of ROW_NUMBER, as that will give all rows with the same value the same ranking. For example:
WITH tbl_max_date AS (
  SELECT *,
         RANK() OVER (PARTITION BY id, question ORDER BY date DESC) aS rn
  FROM tbl_src
)
SELECT *
FROM tbl_max_date
WHERE rn = 1


Extension method to do Linq Lookups Does this extension method contain unnecessary work? - Thanks for reviewing

Scenario - A series of tables are Key-Value pairs (Int "Id" / nvarChar "Title").  This extension method "dots-off" any integer ("Id") and provide the corresponding string value ("Title") given a target "type"

Used in aspx-code-behind. An extension method was chosen for readable one-liners.  There are many panels with similar controls to be set conditionally.

// aspx.cs
txtParentGender.Text=parent.GenderId.RhGet(typeof(Gender));  //male, female
txtParentEthnicity.Text=parent.EthnicityId.RhGet(typeof(Ethnicity)); //Caucasian, etc.
txtParentMaritalStatus.Text=parent.MaritalStatusId.RhGet(typeof(MaritalStatus)); // married, divorced etc.
txtChildGender.Text=child.GenderId.RhGet(typeof(Gender));
txtChildEthnicity.Text=child.EthnicityId.RhGet(typeof(Ethnicity));
txtChildRelation.Text=child.RelationId.RhGet(typeof(Relation)); //son, daughter etc.

// class.cs
using System.Linq.Dynamic; // nuget package

public static string RhGet(this int id, Type type )
{

   string returnValue = string.Empty;

   // set up data context
   using (MyDataContext dc = new MyDataContext())
   {
       // get table corresponding to this type
       var table = dc.GetTable(type);

       // match and select record using "id"
       var matchingRecord = ((IQueryable)table).Where(string.Format("Id.Equals({0})", id))
                                               .Select("new(Title)").Cast<object>().FirstOrDefault();

       // do a lot of contorting to finally get the string value
       returnValue = matchingRecord.GetType().GetProperty("Title").GetValue(matchingRecord,null).ToString();
    }

    return returnValue;
}

I would not write this extension method for int at all since it assumes everything of type int in your project is referring to the ID of a table.

Also, as a rule of thumb, if you're using strings that contain structural metadata about your objects, it's generally a sign you might want to rethink your approach.

Instead, I would find a way to apply an interface to your tables that have this structure, so you could then make an extension method for that interface, or a method that takes in an instance that implements that interface. That way you don't have to rely on strings at all.

If you are really committed to having a method that finds a value based on an int and type inputs, I would change the method to be a static, non-extension method, passing in the int and type.

I also came up with this alternative solution. Although it doesn't behave the same as your original example, you might benefit by using a type parameter so you get a strongly-typed entity returned by your method, which your code will know has a "Title" property:

public static TEntity GetEntityById<TEntity>(int id)
{

   TEntity entity = null;

   //a using is a try finally. you have stuff you want to execute in a finally so I simplified.
   try
   {
        MyDataContext dc = new MyDataContext();

       // match and select record using "id"
       entity = dc.GetTable<TEntity>().Where(string.Format("Id.Equals({0})", id)).Cast<TEntity>().FirstOrDefault(); //not sure if Cast<TEntity> is redundant or not
    }
    finally
    {
        return entity;
    }
}

//consuming code
string myTitle = GetEntityById<MyEntity>(222).Title;

I also noticed you are instantiating a datacontext in this method--you might want to either pass it in or make this an extension method on your datacontext for testability.


Find two numbers that add up to a given total, from a formatted string The full challenge from codeeval.com can be viewed here.

  Input

  Your program should accept as its first argument a filename. This file
  will contain a comma separated list of sorted numbers and then the sum
  'X', separated by semicolon. Ignore all empty lines. If no pair
  exists, print the string NULL.

  Output

  Print out the pairs of numbers that equal to the sum X. The pairs
  should themselves be printed in sorted order i.e the first number of
  each pair should be in ascending order.

I am writing a program to solve the above conditions. It currently works, however I believe it can be optimized to run better. This challenge is unique, since it requires you to work with a String for input, and to output a specifically formatted String as well. The problem with this is memory efficiency, as I have to convert this String to a String array, and then further convert it to a workable int array.

I am a student developer, and I don't have much experience, so any tips and tricks would be helpful.

Java

import java.util.Arrays;
import java.util.HashMap;
import java.util.Map;

public class SomeClass {
    public static void main(String[] gras) {
        String line = "1,2,3,4,5;6"; //replace string with test case

        //code
        String[] j = line.split(";")[0].split(",");
        int[] input = new int[j.length];
        for (int i = 0; i < j.length; i++) {
            input[i] = Integer.parseInt(j[i]);
        }
        Arrays.sort(input);
        int k = Integer.parseInt(line.split(";")[1]);
        // Modified Algorithm
        Map<Integer, Integer> paers = new HashMap<Integer, Integer>();
        boolean d = false;
        for (int i = input.length - 1; i >= 0; i--) {
            paers.put(k - input[i], input[i]);
        }
        for (int i = 0; i < input.length; i++) {
            if (paers.containsKey(input[i]) && paers.get(input[i]) > input[i]) {
                if (d) {
                    System.out.print(";");
                }
                d = true;
                System.out.print(input[i] + "," + paers.get(input[i]));
            }
        }
        if (!d) System.out.println("NULL"); else System.out.println("");
    }
}

As far as what I've tried, this is my final product. I changed my solution from using a naive \$O(n^2)\$ algorithm to this \$O(3n)\$ algorithm. Again, my terminology might be off, I'm just figuring this stuff out. Also, the code is tested and working.

Improved algorithm

You can significantly reduce the memory requirements of your algorithm by not creating any objects, including avoiding Maps, Sets, and String concatenations (the +s you use in print statements). You only need to use a constant amount of memory, comprised completely of primitives stored on the stack (avoiding garbage collection).

You can significantly reduce the time requirements of your algorithm by using a simple trick that allows you to scan the input list only once by moving toward the middle from the left and the right.

public final class SumPairs {
    public static final void printSumPairs(final String input) {
        final int inputLength = input.length();

        // On the left, move to the very first number.
        int leftStartIndex = 0;
        int leftEndIndex   = input.indexOf(',', 1);
        int leftNumber     = parseIntFromSubstring(input, leftStartIndex, leftEndIndex);

        // On the right, move to the very last number.
        int rightEndIndex   = input.lastIndexOf(';', inputLength - 2);
        int rightStartIndex = input.lastIndexOf(',', rightEndIndex - 2) + 1;
        int rightNumber     = parseIntFromSubstring(input, rightStartIndex, rightEndIndex);

        // Figure out the desired sum.
        final int desiredSum = parseIntFromSubstring(input, rightEndIndex + 1, inputLength);

        boolean noOutputYet = true;

        while (leftStartIndex < rightStartIndex) {
            final int currentSum = leftNumber + rightNumber;

            if (currentSum > desiredSum) {
                // On the right, move to the previous distinct number.
                int oldRightNumber;
                do {
                    oldRightNumber = rightNumber;

                    rightEndIndex   = rightStartIndex - 1;
                    rightStartIndex = input.lastIndexOf(',', rightEndIndex - 2) + 1;
                    rightNumber     = parseIntFromSubstring(input, rightStartIndex, rightEndIndex);
                } while ((rightNumber == oldRightNumber) && (leftStartIndex < rightStartIndex));
            }
            else {
                if (currentSum == desiredSum) {
                    if (noOutputYet) noOutputYet = false;
                    else System.out.print(';');

                    System.out.print(leftNumber);
                    System.out.print(',');
                    System.out.print(rightNumber);
                }

                // On the left, move to the next distinct number.
                int oldLeftNumber;
                do {
                    oldLeftNumber = leftNumber;

                    leftStartIndex = leftEndIndex + 1;
                    leftEndIndex   = input.indexOf(',', leftStartIndex + 1);
                    leftNumber     = parseIntFromSubstring(input, leftStartIndex, leftEndIndex);
                } while ((leftNumber == oldLeftNumber) && (leftStartIndex < rightStartIndex));
            }
        }

        if (noOutputYet) System.out.print("NULL");
        System.out.println();
    }

    // Java 7 and later's String#substring creates a new char[] array just about every time it's used.
    // Since Integer#parseInt requires a full String, we'd have to let String#substring create a lot of char[]s if we used Integer#parseInt.
    // We'd like to avoid that to reduce memory requirements and to eliminate garbage collection.

    // WARNING: This method doesn't actually check whether there is a valid number.
    //          That's your job!
    private static final int parseIntFromSubstring(final String str, int start, final int end) {
        int result = 0;
        final boolean negative = str.charAt(start) == '-';
        if (negative) start++;

        for (; start < end; start++)
            result = 10*result + str.charAt(start) - '0';

        if (negative) return -result;
        else return result;
    }

    public static final void main(String[] args) {
        printSumPairs("1,2,3,4,5,6;6");
    }
}

Critiques

Problem statement

The part about outputting "NULL" should be in the section about output, not the section about input.
The characters in the input are not fully specified. Will there be spaces or other characters to ignore mixed in? Can numbers have a decimal point? Can numbers be negative? Are newlines separators for separate problems to solve?
The CodeEval challenge says not to output duplicate pairs. For example "3,3,3,3;6" should output "3,3", not "3,3;3,3;3,3;...".

Your solution

The problem statement says to read lines from a file, but your algorithm doesn't do that.
You don't need to sort the array of ints. The problem statement says they'll already be sorted.

Time complexity

Your code shouldn't be regarded as \$O(3n)\$ for a few reasons:

Big O notation ignores constant multiples like 3. It should be \$O(n)\$, not \$O(3n)\$.
You use Arrays#sort when you don't need to. While it uses a nicer version of quicksort than normal, it still has a \$O(n^2)\$ worst case time complexity.
If you're limiting your algorithm to a list of unique 32-bit integers, there's only so many of those available to put in your input list. This means that there will be an input that takes the longest amount of time to produce an answer. This means that you have a constant upper bound on the time needed, which makes your algorithm (including your 'naive' one) \$O(1)\$.

If you're not limiting your algorithm to a list of unique 32-bit integers, then you should realize that it takes a lot longer to add together two integers with billions of digits than it does to add 1 and 2. The time complexity of addition is \$O(m)\$, where \$m\$ is the number of digits in the largest number in the input list. This means your complexity will be \$O(mn)\$.


Extension for grouping array into two dimensional array based on element ([] -> [[]]) I'm new at generics, but I've written an array extension to group an array by array element into a two dimensional array:

extension Array {
    func group<U: Hashable>(by key: (Element) -> U) -> [[Element]] {
        //keeps track of what the integer index is per group item
        var indexKeys = [U : Int]()

        var grouped = [[Element]]()
        for element in self {
            let key = key(element)

            if let ind = indexKeys[key] {
                grouped[ind].append(element)
            }
            else {
                grouped.append([element])
                indexKeys[key] = grouped.count - 1
            }
        }
        return grouped
    }
}

For an array of this struct:

struct Thing {
   var category: String
   var name: String
}

I want to be able to group an array of things [Thing] into a two dimensional array [[Thing]] based on Thing.category.

I use the extension like this:

let things = [
   Thing(category: "A", name: "Apple"),
   Thing(category: "B", name: "Boy"),
   Thing(category: "A", name: "Alligator"),
   Thing(category: "B", name: "Ball"),
   Thing(category: "B", name: "Billboard")
]
let groupedThings = things.group { $0.category }

This returns something like:

//groupedThings
[
   [
      Thing(category: "A", name: "Apple"),
      Thing(category: "A", name: "Alligator")
   ],
   [
      Thing(category: "B", name: "Boy"),
      Thing(category: "B", name: "Ball"),
      Thing(category: "B", name: "Billboard")
   ]
]

Please note that I'm not concerned about sort order.

How does my extension look? Since I'm fairly new at this, I'd like to know if I'm over-complicating it. Is there a way to write the extension more concisely? What about speed issues?

That looks like a clean implementation to me. One minor thing: I find it more
natural to assign the new index before the group array is extended,
so that you don't have to subtract one:

indexKeys[key] = grouped.count
grouped.append([element])

The Swift standard library defines many methods for the Sequence protocol and not
for concrete sequences like Array, examples are map, filter, min/max, contains ...

You can do the same with your method to make it more universally applicable:

extension Sequence {
    // ... no other changes necessary ...
}

However: You are essentially reinventing the wheel. In Swift 4 there is a
Dictionary(grouping:by)
method which does almost what your code does, only that a dictionary is returned:

let groupedThings = Dictionary(grouping: things, by: { $0.category } )
// [String : [Thing]]

The keys are the categories, and the values are arrays of corresponding things. With

Array(groupedThings.values)

you get a nested array which (apart from the order) is identical to what your method
returns.


Clojure code for finding prime numbers This is my Clojure code for finding prime numbers.

Note: this is an advanced version which starts eliminating from i*i with step i, instead of filtering all list against mod i == 0 (Sieve of Eratosthenes).

It has better asymptotic runtime: O(n log log n) instead of O(n log n) in typical examples of finding primes in Clojure.

What can be done better? Do I use some slow constructions? Can I make it more concise? Gain more performance? Format it better?

(defn primes [n]
  (let [mark (fn [i di v]
               (if (<= i (count v))
                 (recur (+ i di) di (assoc v (dec i) di))
                 v))
        step (fn [i ps v]
               (if (<= i (count v))
                 (if (= (v (dec i)) 1)
                   (recur (inc i) (conj ps i) (mark (* i i) i v))
                   (recur (inc i) ps v))
                 ps))]
(->> (repeat 1) (take n) vec (step 2 []))))

(defn -main
  [& args]
  (println (primes 50)))

(non-Clojure-specific) Gain performance, sure. Use packed array of odds, where index i represents value 2i+1. Then you don't have to deal with evens, which are all non-prime a priori (except the 2 of course). Then you can increment by 2*p for a prime p to find its odd multiples twice faster.

For a non-marked index i, the prime p is p = 2*i+1, its square is p*p = (2i+1)(2i+1) = 4i^2 + 4i + 1 and its index is (p*p-1)/2 = 2i^2 + 2i = 2i(i+1) = (p-1)(i+1). For the value increment of 2*p, the index increment on 2x-packed array is di = p = 2i+1.


Average of three biggest parameters out of five without using sorting algorithm or vector/array I'm trying to create a function with has 5 input parameters and returns the average of the 3 biggest. However I'm not supposed to use a sorting algorithm, vector/arrays or function libraries and still try to develop the most efficient logic possible.

So far I've come up with the code below. Is it the most efficient possible considering the restrictions?

float avg3of5(float a, float b, float c, float d, float e) {
    float min;
    float min2;

    min = a;
    if (b < min) {
        min2 = min;
        min = b;
    }
    else {
    min2 = b;
    }

    if (c < min) {
        min2 = min;
        min = c;
    }
    else {
        if (c < min2) {
            min2 = c;
        }
    }

    if (d < min) {
        min2 = min;
        min = d;
    }
    else {
        if (d < min2) {
            min2 = d;
        }
    }

    if (e < min) {
        min2 = min;
        min = e;
    }
    else {
        if (e < min2) {
            min2 = e;
        }
    }

    Return = (a + b + c + d + e - min - min2) / 3
}

I would think, that you should try to divide and conquer rather than writing one long function. That means you should split your code into separable and reusable functions that do one thing. For example the individual minima you get by the following functions

float min2(const float& a, const float& b) {
    return a < b ? a : b;
}

float min3(const float& a, const float& b, const float& c) {
    const float temp = min2(a, b);
    return temp < c ? temp : c;
}

float min4(const float& a, const float& b, const float& c, const float& d) {
    const float temp = min3(a, b, c);
    return temp < d ? temp : d;
}

float min5(const float& a, const float& b, const float& c, const float& d, const float& e) {
    const float temp = min4(a, b, c, d);
    return temp < e ? temp : e;
}

You might even put this into variadic templates and overload min.
Now you can get the minimum value and define a switch statement

float average3of3(const float& a, const float& b, const float& c) {
    return (a + b + c)/3;
}

float average3of4(const float& a, const float& b, const float& c, const float& d) {
    const float min = min4(a, b, c, d);
    switch(min) {
        case a:
            return average(b, c, d);
        case b:
            return average(a, c, d);
        case c:
            return average(a, b, d);
        case d:
            return average(a, b, c);
    }
}

float average3of5(const float& a, const float& b, const float& c, const float& d, , const float& e) {
    const float min = min5(a, b, c, d, e);
    switch(min) {
        case a:
            return average3of4(b, c, d, e);
        case b:
            return average3of4(a, c, d, e);
        case c:
            return average3of4(a, b, d, e);
        case d:
            return average3of4(a, b, c, e);
        case e:
            return average3of4(a, b, c, d);
    }
}


Leetcode: Valid parentheses https://leetcode.com/problems/valid-parentheses/

Given a string containing just the characters (, ), {, }, [
and ], determine if the input string is valid.
For an input string to be valid:

Open brackets must be closed by the same type of brackets.
Open brackets must be closed in the correct order.

Note that an empty string is considered valid.
Example 1:
Input: ()
Output: true
Example 2:
Input: ()[]{}
Output: true
Example 3:
Input: (]
Output: false
Example 4:
Input: ([)]
Output: false
Example 5:
Input: {[]}
Output: true

using System;
using System.Collections.Generic;
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace StackQuestions
{
    [TestClass]
    public class ValidParentheses
    {
        [TestMethod]
        public void OpenOpenClosedClosedMixTest()
        {
            string input = "([)]";
            bool result = IsValid(input);
            Assert.IsFalse(result);
        }
        [TestMethod]
        public void OnePairTest()
        {
            string input = "()";
            bool result = IsValid(input);
            Assert.IsTrue(result);
        }

        public bool IsValid(string s)
        {
            Stack<char> myStack = new Stack<char>();
            foreach (var curr in s)
            {
                if (curr == '(')
                {
                    myStack.Push(curr);
                }
                else if (curr == '[')
                {
                    myStack.Push(curr);
                }
                else if (curr == '{')
                {
                    myStack.Push(curr);
                }
                else if (curr == ')')
                {
                    if (myStack.Count > 0)
                    {
                        var top = myStack.Pop();
                        if (top != '(')
                        {
                            return false;
                        }
                    }
                    else
                    {
                        return false;
                    }
                }
                else if (curr == ']')
                {
                    if (myStack.Count > 0)
                    {
                        var top = myStack.Pop();
                        if (top != '[')
                        {
                            return false;
                        }
                    }
                    else
                    {
                        return false;
                    }
                }
                else if (curr == '}')
                {
                    if (myStack.Count > 0)
                    {
                        var top = myStack.Pop();
                        if (top != '{')
                        {
                            return false;
                        }
                    }
                    else
                    {
                        return false;
                    }
                }

            }
            return myStack.Count == 0;
        }

    }
}

Please review coding style as it was a job interview with 30 minutes to code.

You get the job done in 30 minutes and the use of a stack is the way to go, so that's a good start. In my opinion you're writing a little too much (repetitive) code and it could be a lot easier to read if you use a switch-statement instead:

public bool IsValidReview(string s)
{
  Stack<char> endings = new Stack<char>();

  foreach (var curr in s)
  {
    switch (curr)
    {
      case '(':
        endings.Push(')');
        break;
      case '[':
        endings.Push(']');
        break;
      case '{':
        endings.Push('}');
        break;
      case ')':
      case ']':
      case '}':
        if (endings.Count == 0 || endings.Pop() != curr)
          return false;
        break;

    }
  }

  return endings.Count == 0;
}

Here the corresponding ending parenthesis is pushed to the stack instead of the starting one, which makes it easier to check when the ending shows up.

The name myStack doesn't say much, so I have changed it to something more meaningful in the context.


Reverse Polish Notation Calculator in Java I created a highly effective Reverse Polish Notation calculator using Java 8, but am uncertain if there are any better ways to handle the problem. Feedback on any evident taboos and bugs is greatly appreciated!

public final class RPN {

    /**
     * Removes all characters that are not operators, whitespace, or digits
     *
     * @param expr the expression to parse
     * @return the parsed expression
     */
    private static String parseExpression(String expr) {
        String parsedExpr = expr.replaceAll("[^\\^\\*\\+\\-\\d/\\s]", "");
        String trimmedExpr = parsedExpr.replaceAll("\\s+", " ");
        return trimmedExpr;
    }

    /**
     * Computes the outcome of a given expression in Reverse Polish Notation
     *
     * @param expr the expression to compute
     */
    public static void compute(String expr) throws
            ArithmeticException,
            EmptyStackException {
        String validExpr = parseExpression(expr);
        Stack<Double> stack = new Stack<>();

        System.out.println(validExpr);
        System.out.println("Input\tOperation\tStack after");

        for (String token : validExpr.split("\\s")) {
            System.out.print(token + "\t");

            double secondOperand = 0.0;
            double firstOperand = 0.0;

            switch (token) {
                case "+":
                    System.out.print("Operate\t\t");

                    secondOperand = stack.pop();
                    firstOperand = stack.pop();

                    stack.push(firstOperand + secondOperand);
                    break;
                case "-":
                    System.out.print("Operate\t\t");

                    secondOperand = stack.pop();
                    firstOperand = stack.pop();

                    stack.push(firstOperand - secondOperand);
                    break;
                case "*":
                    System.out.print("Operate\t\t");

                    secondOperand = stack.pop();
                    firstOperand = stack.pop();

                    stack.push(firstOperand * secondOperand);
                    break;
                case "/":
                    System.out.print("Operate\t\t");

                    secondOperand = stack.pop();
                    firstOperand = stack.pop();

                    if (secondOperand == 0.0) {
                        throw new ArithmeticException("Cannot divide by zero!");
                    }

                    stack.push(firstOperand / secondOperand);
                    break;
                case "^":
                    System.out.print("Operate\t\t");

                    secondOperand = stack.pop();
                    firstOperand = stack.pop();

                    stack.push(Math.pow(firstOperand, secondOperand));
                    break;
                default:
                    System.out.print("Push\t\t");
                    stack.push(Double.parseDouble(token));
                    break;
            }

            System.out.println(stack);
        }

        System.out.println("Final Answer: " + stack.pop());
    }

    /**
     * What runs the program
     *
     * @param args the command line arguments
     */
    public static void main(String[] args) {
        try {
            String expr = Arrays.toString(args);
            System.out.println(expr);
            compute(expr);
        } catch (Throwable err) {
            System.out.println(err.getMessage());
        }
    }
}

Your parseExpression() function isn't parsing anything.  Rather, it sanitizes the string.  Sanitizing input is a rather risky business.  For example, any decimal points will be discarded!  What if I try to use ! as the factorial operator?  That would be silently ignored as well.  If you can't handle the input in a reasonable way, it should be an error.

It's odd that you convert args from an array to a string, then split it up again.  You could either use args directly, or split just args[0].

Catching Throwable is probably a bad idea.  Anything that is not an Exception is generally not meant to be caught and handled.

It's actually not true that you cannot divide by zero.  That's an error only when performing integer arithmetic.  Why not let Double.POSITIVE_INFINITY be a valid result?

The code would be simpler if you didn't assign to variables firstOperand and secondOperand.  I'd write it more like this:

import java.util.*;

public class RPN {

    /**
     * Computes the outcome of a given expression in Reverse Polish Notation
     *
     * @param expr the expression to compute
     */
    public static void compute(String expr) throws
            ArithmeticException,
            EmptyStackException {
        Stack<Double> stack = new Stack<>();

        System.out.println(expr);
        System.out.println("Input\tOperation\tStack after");

        for (String token : expr.split("\\s+")) {
            System.out.print(token + "\t");
            switch (token) {
                case "+":
                    System.out.print("Operate\t\t");
                    stack.push(stack.pop() + stack.pop());
                    break;
                case "-":
                    System.out.print("Operate\t\t");
                    stack.push(-stack.pop() + stack.pop());
                    break;
                case "*":
                    System.out.print("Operate\t\t");
                    stack.push(stack.pop() * stack.pop());
                    break;
                case "/":
                    System.out.print("Operate\t\t");
                    double divisor = stack.pop();
                    stack.push(stack.pop() / divisor);
                    break;
                case "^":
                    System.out.print("Operate\t\t");
                    double exponent = stack.pop();
                    stack.push(Math.pow(stack.pop(), exponent));
                    break;
                default:
                    System.out.print("Push\t\t");
                    stack.push(Double.parseDouble(token));
                    break;
            }

            System.out.println(stack);
        }

        System.out.println("Final Answer: " + stack.pop());
    }

    /**
     * Runs the calculation for the RPN expression in args[0].
     */
    public static void main(String[] args) {
        try {
            compute(args[0]);
        } catch (Exception err) {
            System.out.println(err.getMessage());
        }
    }
}

Sample run:

$ java RPN '3 5 +'
3 5 +
Input   Operation   Stack after
3   Push        [3.0]
5   Push        [3.0, 5.0]
+   Operate     [8.0]
Final Answer: 8.0


Using keywords async/await in database queries (Windows Phone 8) I have a local database in Windows Phone 8 app. The app includes a lot of queries to the database and I don't want bad effect on the responsiveness of UI.

For example I have a table of users and method to get a user from database by id.

Current variant

public class CacheDataContext : DataContext
{
    public static string DBConnectionString = "Data Source=isostore:/Cache.sdf";

    public CacheDataContext(string connectionString)
            : base(connectionString) { }

    public static AutoResetEvent OperationOnDatabaseUser = new AutoResetEvent(true);

    public Table<User> UserItems;
}

public class CacheDataContextUser : CacheDataContext
{
    public CacheDataContextUser(string connectionString)
        : base(connectionString) { }

    public User GetUser(string id)
    {
        try
        {
            OperationOnDatabaseUser.WaitOne();
            using (CacheDataContext context = new CacheDataContext(DBConnectionString))
            {
                //find user in the data base and return
            }
        }
        finally
        {
            OperationOnDatabaseUser.Set();
        }
    }
}

I need ensure safety of the data if at the same time on database wallow different requests to add, modify, delete data. For this I use AutoResetEvent. Not sure what I'm doing it right, but so far no problems.

I can get user from the database:

using (DataBaseUser = new CacheDataContextFriends(ConnectionString))
{
   var user = DataBaseUser.GetUser(id);
}

Async/await

But I want work with the database using keywords async/await.

public class CacheDataContextUser : CacheDataContext
{
    public CacheDataContextUser(string connectionString)
        : base(connectionString) { }

    private object threadLock = new object();

    public Task<User> GetUser(string id)
    {
        using (CacheDataContext context = new CacheDataContext(DBConnectionString))
        {
            var result = await Task<User>.Factory.StartNew(() =>
                {
                    lock (threadLock)
                    {
                       //find user in the data base and return
                    }
                });
            return result;
        }
    }
}

I'm afraid to rewrite the method as described above, because I'm not sure it's right. Please tell me what the problem may be. My main goal is to improve the responsiveness of the app.

Before even addressing the actual question, have you actually proven that database queries are harming your responsiveness? Since the queries are to local storage it's possible that they are fast enough to not require using asynchrony, particularly if they are simple queries or the database is small.

Next, I notice you've assumed that the database itself is not threadsafe, i.e. that you must only allow access from one thread at a time. Are you sure that's actually true? Many (most?) database handle concurrency themselves, so you may be adding an unnecessary layer of synchronization. I looked around a bit, but could not find anything specifically documenting concurrent access to isolated storage databases. I would start by researching that, or possibly asking a question on StackOverflow. If the database does allow concurrent access then you just need to worry about update conflicts, which you could hopefully avoid in a single-user phone application.

What I'm getting at here is that multi-threading and locking is hard. Don't do it unless you're sure you have a good reason to do it.

If you really must to multi-threading, then the C# lock keyword is a good place to start. Unfortunately, your example probably will not work properly because each CacheDataUserContext instance will have it's own lock object - so if you create more than one instance they could conflict with each other.

You "Current Variant" actually gets this more right, because your AutoResetEvent is a static variable, so there is only a single instance of it across the system. However, as I understand DataContext it lets you use Linq statements against the database, which will not know anything about your lock and hence will not be synchronized.

I think you'd have to create a separate application layer to wrap the DataContext and expose just the certain operations that your application needs. This is generally called the "Repository Pattern". Inside the repository you could create a single lock object, wrap a lock around all accesses to a DataContext, and use Task.Factory.StartNew inside each of the repository methods to make them asynchronous.


Download pictures from Reddit using System;
using System.Linq;
using RedditSharp;
using System.Net;
using RedditSharp.Things;

namespace reSharp
{
    class Program
    {
        public static void Main()
        {
            string sub = "/r/";
            string saveDir = @"SAVE_DIRECTORY";
            Console.WriteLine("Subreddit:");
            sub += Console.ReadLine();

            Console.WriteLine("Amount:");
            int amount = Convert.ToInt32(Console.ReadLine());
            amount += 1;

            Console.WriteLine("Time Period");
            string timePer = Console.ReadLine();

            Reddit reddit = new Reddit();
            var subreddit = reddit.GetSubreddit(sub);
            if (timePer == "all")
            {
                foreach (var post in subreddit.GetTop(FromTime.All).Take(amount))
                {
                    if (post.IsStickied || post.IsSelfPost || Convert.ToString(post.Url).Contains("reddituploads")) continue;
                    string postURL = Convert.ToString(post.Url);
                    DownloadImages(postURL, saveDir);

                }
            }
            else
            {
                if (timePer == "hot")
                {
                    foreach (var post in subreddit.Hot.Take(amount))
                    {
                        if (post.IsStickied || post.IsSelfPost || Convert.ToString(post.Url).Contains("reddituploads")) continue;
                        string postURL = Convert.ToString(post.Url);
                        DownloadImages(postURL, saveDir);

                    }
                }
            }

        }

        public static void DownloadImages(string imageURL, string userDir)
        {

            if (imageURL.Contains("gfycat.com"))
            {
                imageURL = imageURL.Replace("gfycat.com", "zippy.gfycat.com") + ".mp4";
            }

            if (imageURL.Contains(".gifv"))
            {
                imageURL = imageURL.Replace(".gifv", ".mp4");
            }

            Console.WriteLine("Downloading {0}", imageURL);
            string[] splitURL = imageURL.Split('/');
            int index = splitURL.Length - 1;
            string fileName = splitURL[index];
            WebClient client = new WebClient();
            try
            {
                client.DownloadFile(imageURL, userDir + fileName);
            }
            catch (Exception)
            {
                Console.WriteLine("[INFO] ERROR DOWNLOADING FILE");
            }
        }
    }
}

The program effectively just downloads pictures from a given reddit subreddit. How could this be tided? Thanks.

There are few things you can do. Start by encapsulating these lines

if (imageURL.Contains("gfycat.com"))
{
    imageURL = imageURL.Replace("gfycat.com", "zippy.gfycat.com") + ".mp4";
}

if (imageURL.Contains(".gifv"))
{
    imageURL = imageURL.Replace(".gifv", ".mp4");
}

in their own method. Let's call it FixImageUrl (this is C# 7)

public static string FixImageUrl(string imageUrl)
{
    switch (imageUrl)
    {
        case string url when url.Contains("gfycat.com"):
            return imageUrl.Replace("gfycat.com", "zippy.gfycat.com") + ".mp4";

        case string url when url.Contains(".gifv"):
            return imageUrl.Replace(".gifv", ".mp4");

        default: return imageUrl;
    }
}

string[] splitURL = imageURL.Split('/');
int index = splitURL.Length - 1;
string fileName = splitURL[index];

Then you can simplify this a little bit by using the Last extension:

var fileName = imageURL.Split('/').Last();

It probably doesn't matter here but it's a good habit to always dispose objects that are disposable. This means the WebClient requires special care.

You also want to know what went wrong so don't swallow the exception. Print either the message or log the entire exception.

When working with paths you should use the Path.Combine method to avoid any invalid paths like ones with missing \ etc.

try
{
    using (var client = new WebClient())
    {
        client.DownloadFile(imageURL, Path.Combine(userDir, fileName));
    }
}
catch (Exception ex)
{
    Console.WriteLine($"[INFO] ERROR DOWNLOADING FILE: {ex}");
}


Check if a list of ints is pairwise sorted In class we created our own ArrayList class; the code I wrote is meant to interact with the ArrayIntList class. I'm looking for general feedback to help with the next time I write code. I'm also wondering if it's "safe" to change the value of the size variable in my method (size represents the current size of the ArrayIntList.)

  Write a method isPairwiseSorted that returns whether or not a list of integers is pairwise sorted (true if it is, false otherwise). A list is considered pairwise sorted if each successive pair of numbers is in sorted (non-decreasing) order. For example, if a variable called list stores the following sequence of values:

[3, 8, 2, 5, 19, 24, -3, 0, 4, 4, 8, 205, 42]

  Then the call of list.isPairwiseSorted() should return true because the successive pairs of this list are all sorted: (3, 8), (2, 5), (19, 24), (-3, 0), (4, 4), (8, 205). Notice that the extra value 42 at the end had no effect on the result because it is not part of a pair. If the list had instead stored the following:

[1, 9, 3, 17, 4, 28, -5, -3, 0, 42, 308, 409, 19, 17, 2, 4]

  Then the method should return false because the pair (19, 17) is not in sorted order. If a list is so short that it has no pairs, then it is considered to be pairwise sorted. If a list is of odd length, the final element should be ignored since it has no pair. In other words, if the rest of the list is pairwise sorted until that last unpaired element, your method should return true.

  Assume you are adding to the ArrayIntList class with following fields:

public class ArrayIntList {
    private int[] elementData;
    private int size;

   // your code goes here
}

And here's my code:

public boolean isPairwiseSorted() {
    if (size <= 1) {
        // Pairwise sorted for lists smaller than two elements
        return true;
    }
    boolean reIncrementSize = false;
    if (size % 2 != 0) {
        reIncrementSize = true;
        size--;
    }
    for(int i = 0; i < size; i+=2) {
        // Pairwise sorted if elements are in increasing order
        if (elementData[i] > elementData[i + 1]) {
            return false;
        }
    }
    if (reIncrementSize) {
        size++;
    }
    return true;
}

if (size <= 1) {
        // Pairwise sorted for lists smaller than two elements
        return true;
    }

Special cases are sometimes useful, but KISS: does this simplify the code or make it more complex for no benefit?

    boolean reIncrementSize = false;
    if (size % 2 != 0) {
        reIncrementSize = true;
        size--;
    }

  ....

    if (reIncrementSize) {
        size++;
    }

Yikes! It really shouldn't be necessary to make temporary changes to the state of the class. But if you really absolutely have to do it, you should use try ... finally to guarantee that they are undone. Here the temporary changes introduce a bug: if we hit the return false then the size is permanently decremented.

    for(int i = 0; i < size; i+=2) {
        // Pairwise sorted if elements are in increasing order
        if (elementData[i] > elementData[i + 1]) {
            return false;
        }
    }

Why was size temporarily decremented? I presume it's to avoid an out of bounds array access in elementData[i + 1]. But you can do that just as well by making that be the guard condition:

    for(int i = 0; i + 1 < size; i+=2) {


Turn XML into Object I've got a working program that turns XML into a XmlObject. The object can then be accessed using getName(), getValue(), and getChild(). The code works but I'm looking for recommendations. I couldn't find anything built into c# that creates an object from XML. I'm currently learning by doing games in Unity so hopefully the code isn't too awful.

The static class for creating an XmlObject

namespace XmlToObject
{
    using System.Collections.Generic;
    using System.Xml;
    using System.IO;
    using UnityEngine;

    /// XmlObjectCreator turns xml into an XmlObject
    public static class XmlObjectCreator
    {

        /// Generates the xml object.
        public static XmlObject generateXmlObject (TextAsset xmlTextFile, string nameOfFirstTag)
        {
            //load the xml document
            XmlDocument xmlDoc = new XmlDocument ();
            xmlDoc.Load (new StringReader (xmlTextFile.text));

            //Navigate to the first tag
            XmlNode firstNode = xmlDoc.SelectSingleNode ("//" + nameOfFirstTag);

            //Return the built object
            return recursiveBuildXmlObject (firstNode);
        }

        /// Recursivly builds an xml object.
        private static XmlObject recursiveBuildXmlObject (XmlNode startNode)
        {
            //Create the buildObject
            XmlObject buildObject = new XmlObject ();

            //Set the name and value
            buildObject.setName (startNode.Name);
            buildObject.setValue (startNode.InnerText);

            //Add child nodes to subList if they exist
            foreach (XmlNode child in startNode.ChildNodes) {
                buildObject.add (recursiveBuildXmlObject (child));
            }

            //Once children are done
            return buildObject;
        }
    }
}

The XmlObject is what is actually used for accessing the data taken from the XML document.

namespace XmlToObject
{
    using System.Collections.Generic;

    /// Xml object is the actual object used for navigating the data.
    public sealed class XmlObject
    {
        private string name;
        private string value;
        private List<XmlObject> subList;

        /// Initializes a new instance of the XmlObject class.
        public XmlObject ()
        {
            subList = new List<XmlObject> ();
            value = null;
        }

        /// Add the specified inputXmlObject.
        public void add (XmlObject inputXmlObject)
        {
            subList.Add (inputXmlObject);
        }

        /// Gets a xml child by name.
        public XmlObject getChild (string name)
        {
            foreach (XmlObject child in subList) {
                if (name.Equals (child.getName ())) {
                    return child;
                }
            }

            throw new System.ArgumentException ("Child with name: " + name + " does not exist.");
        }

        //Setters and getters for fields: name, value

        public string getName ()
        {
            return name;
        }

        public void setName (string newName)
        {
            name = newName;
        }

        public string getValue ()
        {
            return value;
        }

        public void setValue (string newValue)
        {
            value = newValue;
        }
    }
}

I've seen lots of code that deals with known XML structures, but few examples where the XML structure is not predefined/known. I'm sure that there is a better way to do what this code does, I'm just not sure what it is.

It is a good Idea to create a generic object model that may represent an XML document. Therefore, the .Net Framework already contains such models: XmlDocument and XDocument.

I would really suggest to use them.

Some comments to your code:

Methods and properties usually start with a capital letter in C#.
The method getChild throws an exception if no element was found. Consider to return null instead because it is not an exceptional use case and returning null is more flexible.
The field subList could be read-only.
C# has the concept of Properties which should be used instead of get* and set* methods

buildObject.setName(startNode.Name);
buildObject.setValue(startNode.InnerText);

The code above does not produce the expected result because InnerText is not just the inner text of the current node but the text of the current and all sub nodes. The following XML for example:

<xml>
    <test>
        <blub>foo</blub>
        <blub>bar</blub>
    </test>
</xml>

Produces something like:

XmlObject(Name: xml, Value: foobar)
    XmlObject(Name: test, Value foobar)
        XmlObject(Name: blub, Value foo)
        XmlObject(Name: blub, Value bar)


Returns an array of all inputs with specified name I've made a nice little function to return a NodeList array containing all the inputs that have a specified name attribute:

function GiveElementsByName(name) {
var finalOutput = [], times = document.getElementsByTagName('input').length, inOutput = 0, inAll = 0; //For use later.
while(times--)
{
if (document.getElementsByTagName('input')[inAll].name == name) //Apply to all inputs with specified name.
 {
 finalOutput[inOutput] = document.getElementsByTagName('input')[inAll]; //Array with all the matching inputs.
 inOutput++;
 }
inAll++;
}
return finalOutput;
}
function MyFunc() {
  GiveElementsByName("Lol1")[1].value = "I don't like potatos anymore";
}
<input type="text" name="Lol1" value="I like potatos">
<input type="text" name="Lol1" value="I like potatos">
<input type="text" name="Lol1" value="I like potatos">
<input type="text" name="Lol1" value="I like potatos">

<input type="text" name="Lol2" value="I like potatos more">
<input type="text" name="Lol2" value="I like potatos more">
<input type="text" name="Lol2" value="I like potatos more">
<input type="text" name="Lol2" value="I like potatos more">

<input type="button" value="Click me!" onClick="MyFunc()">

Could you help me shrink down the function and make it quicker?

The GiveElementsByName function is poorly indented.
And the variable declarations should not be only a single extremely long var line,
but each variable should be on its own line.
It's better like this:

function GiveElementsByName(name) {
    var finalOutput = [],
        times = document.getElementsByTagName('input').length,
        inOutput = 0,
        inAll = 0;
    while (times--) {
        if (document.getElementsByTagName('input')[inAll].name == name) {
            finalOutput[inOutput] = document.getElementsByTagName('input')[inAll];
            inOutput++;
        }
        inAll++;
    }
    return finalOutput;
}

It seems to me that there is an easier way to do this using document.querySelectorAll, which should be supported by modern browsers:

function GiveElementsByName(name) {
    return document.querySelectorAll('input[name=' + name + ']');
}


Finding anagrams in a word list using Python ##open file and create a dictionary of {"word":"sorted word"}

word_file = open("word_list(1).txt","r")
word_list = {}
for text in word_file:
    simple_text = ''.join(sorted(text.lower().strip()))
    word_list.update({text.lower().strip():simple_text})

##lowercase and sort the user input word
def simplify_entered_word():
    lower_word = ''.join(sorted(input_word.lower().strip())) #Creates a list with the lowercase, sorted letters.
    return lower_word

#Compare the sorted user input with the values of the dictionary to find the anagrams.
def find_in_dict(entered_word):
    for key,val in word_list.items():
        if val == entered_word:
            print(key,input_word)

##main loop

while True:
    input_word = input("Input the word for anagram checking: ")
    find_in_dict(simplify_entered_word())

Good that you tagged with Python 3, as it kinda works in Python 2.7
with a bit different behaviour.

All the time reminder to format according to
PEP8; here I'd note that
there should be spaces between names, i.e. key, val and
print(key, input_word).

Firstly, it's good that there are functions to split the logic up.
However part of that is negated by using globals.  In general using
globals is a bad pattern - here in particular it makes more sense to
pass the word list through to the individual functions.

Edit: And I forgot to update it to not use globals, shame on me ...

Most of the comments can also be docstrings instead.

Now, for the globals.  Just pass them through to the functions:

def simplify_entered_word(input_word):
    "Lowercase and sort the user input word."
    word = input_word.lower().strip()
    return word, "".join(sorted(word))

This function can now also be reused easily by returning two results,
the lowercased word and the sorted result.

Next I'd move the word list reading its own function:

def read_word_list(filename):
    """Open file and create a dictionary of {"word":"sorted word"}."""
    with open(filename, "r") as word_file:
        word_list = {}
        for text in word_file:
            word = text.lower().strip()
            simple_text = "".join(sorted(word))
            word_list.update({word: simple_text})
        return word_list

However this can also be much easier in terms of simplify_entered_word:

def read_word_list(filename):
    """Open file and create a dictionary of {"word":"sorted word"}."""
    with open(filename, "r") as word_file:
        return {simple[0]: simple[1]
                for simple in map(simplify_entered_word, word_file)}

N.b. there might be a better way to write that dictionary
comprehension.  Note also that I used with as to not leak the file
handle from open.

And then call that with the filename:

if __name__ == "__main__":
    word_list = read_word_list("word_list(1).txt")
    while True:
        input_word = input("Input the word for anagram checking: ")
        find_in_dict(word_list, simplify_entered_word(input_word)[1])

Just in case this will only run as a script and not if the file is
imported somewhere else, which is good practice.

One problem that still remains is the iteration in find_in_dict.  It'd
be much simpler if the data structure is slightly changed by collecting
all words for one anagram into a list:

from collections import defaultdict

def read_word_list(filename):
    """Open file and create a dictionary of {"word":"sorted word"}."""
    with open(filename, "r") as word_file:
        word_list = defaultdict(list)
        for word, simplified in map(simplify_entered_word, word_file):
            word_list[simplified].append(word)
        return word_list

def find_in_dict(word_list, entered_word):
    """Compare the sorted user input with the values of the dictionary to find
    the anagrams."""
    for name in word_list.get(entered_word, []):
        print(name, entered_word)

Note the use of defaultdict to simplify the automatic creation of the
list.


Efficiently filter a large (100gb+) csv file (v1) EDIT: This question was followed up by this question, with improved code.

I have a set of pretty large files (>2 GB, over 30m rows) containing intraday data in the following format:

TIC, Date, Time, Bid, Offer
AAPL, 20090901, 09:45, 145, 145.5
AAPL, 20090902, 09:45, 145, 145.5
AAPL, 20090903, 09:45, 145, 145.5

I also have a file that contains exactly those dates that I'm interested in, for each firm, like so:

AAPL, 20090902
AAPL, 20090903

Next I want to extract a subset of the large intra-day files, to only keep those firm-days that occur in the dates of interest file. I solved this by creating a dictionary (one for each Ticker), that contains a list of dates as its values. I iterate over each line, and identify whether the current line's date exists in the dictionary.

Is there a faster way of accomplishing this, such as loading in multiple rows per iteration? Would it help to load it directly into a PostgreSQL server and querying it there? It currently runs through ~ 2 GB of data in 180 seconds. I would also appreciate any other feedback.

ROOT_DIR = "C:/Files"
os.chdir(ROOT_DIR)

with open("firm_dates_of_interest.csv") as f:
    d = {}
    lines = f.readlines()
    for line in lines:
        firm = line.split(",")[1]
        d[firm] = []
    for line in lines:
        firm = line.split(",")[1]
        date = line.split(",")[2]
        d[firm].append(date.strip())

for root, dir, files in os.walk(os.path.join(ROOT_DIR, 'Srcfiles'):
    for file in files:
        basename = os.path.splitext(file)[0]
        filename = file

        start = datetime.datetime.now()
        tokeep = []
        for i, line in enumerate(open(os.path.join(root, filename))):
            line = line.split(",")
            if line[0] in d.keys():
                if line[1] in d[line[0]]:
                    tokeep.append(line)
        elapsed = datetime.datetime.now() - start
        print("This file took me " + str(elapsed.seconds) + ' seconds...')

        with open(os.path.join(root, basename+' EXTRACT' + '.csv'), 'w') as f:
            for i, line in enumerate(tokeep):
                f.write(",".join(line))
        tokeep = []

Write code that does the conversion with a small portion of your data defined in the python module. That makes you focus on the data structures and if they fit to the problem. If you remove the code for file handling, logging - all that is not specific to your conversion problem - only a couple lines of code remain. That is what needs your attention and ours attention.

If that simple conversion work, add code that reads the data from external sources to populate the functions for your conversion code. Instead of splitting strings prefer code that is made for this common format: use built-in csv module. Separate file handling code from any algorithms.

Next create code that writes your intermediate output to external sources - again use csv module.

Separate any part/idea into their own place and give them names that makes readers understand their purpose. This is actually the main problem for any reader of your code.

What will make your code faster?

use generators to keep memory usage low and list comprehension for beauty and awesome filtering

What will make you solve it right?

If you split the problem into many parts, you can test any part itself. What makes you think your conversion is done properly at a scale of 2GB input files and large output files? Did you verified every line in output?
Apply testing with smaller data, gives you confidence for larger data.

What will make you giant leaps?

Go get IPython notebook. Write smaller parts and test execution time.

If you have understood your problem, use python libraries made for your problem and write the necessary glue code.

csvkit, pandas

Learn from similar questions/answers on CodeReview

Find the peak stock price for each company from CSV data