Token Bucket context manager for rate limiting async calls I've written a class that implements something akin to the token bucket algorithm so that I may rate limit aysnc HTTP requests made from my application.

The code works but I'm still not sure if it feels 'right', maybe just because I am new to async programming in Python.

Specific concerns:

It feels weird/bad how the state (self.last, self.tokens) is managed (or not managed) meaning checks in the _refill method.
Maybe I shouldn't be using a context manager (__aenter__ and __aexit__) for this.
I'm not sure if I should be doing any cleaning-up in __aexit__
Implementation of the algorithm may not be 100% faithful as I am waiting until the bucket is empty before refilling as a batch . I think this may be OK because only one token can be consumed per request.

"""
Typical usage example:
    async def rateLimitedTask(bucket):
        await bucket.removeToken() #wait for token
        #do the task which has now been rate limited
        ...

    async with TokenBucket(2, 1/5) as bucket:
        #make arbitrary async calls to rateLimitedTask()
        ...
"""

import asyncio
import time
from collections import deque

class TokenBucket():
    """Context manager which provides a token bucket.

    Attributes:
        tokens (collections.deque): The tokens (which manifest as timestamps)
                                    in reverse chronological order.
        last (float):               The most recent token consumed.
        rate (float):               See __init__ arg `tokensPerS`.
        capacity (int):             See __init__ arg `capacity`.
        sleepDuration (float):      See __init__ arg `refillSleepS`.

    """

    def __init__(self, capacity=1, tokensPerS=1.0, refillSleepS=0.1):
        """Initialises the TokenBucket context manager.

        Args:
            capacity (int, optional):       The maxiumum tokens the bucket can
                                            hold. Larger is burstier.
                                            Defaults to 1.
            tokensPerS (float, optional):   The average replenishment rate in
                                            tokens per second.
                                            Defaults to 1.
            refillSleepS (float, optional): If not enough time has passed
                                            to refill; how many seconds to
                                            sleep before trying again.
                                            Defaults to 0.1.
        """
        self.capacity = capacity
        self.rate = tokensPerS
        self.last = None
        self.tokens = deque([])
        self.sleepDuration = refillSleepS

    def hasTokens(self):
        return len(self.tokens) > 0

    def isEmpty(self):
        return len(self.tokens) < 1

    async def _refill(self):
        """Refill the empty bucket back up to capacity.

        Is called only when the bucket is empty.
        The tokens themselves are timestamps of when the token was created.
        Uses monotonic time to safeguard against system clock changes.
        """
        if self.last: #prevents unnecessary run on virgin execution
            while self.capacity / (time.monotonic() - self.last) > self.rate:
                await asyncio.sleep(self.sleepDuration)
                if self.hasTokens():
                    return #another call filled the bucket already

        for _ in range(self.capacity):
            self.tokens.appendleft(time.monotonic())

    async def removeToken(self):
        """Removes a token from the bucket and returns void.

        If the bucket is empty it will await its refill before returning.
        """
        if self.isEmpty():
            await self._refill()

        self.last = self.tokens.pop()

    async def __aenter__(self):
        return self

    async def __aexit__(self, exc_type, exc, tb):
        pass

Any help would be much appreciated.

Parent-less class

class TokenBucket():

can drop the parens.

lower_snake_case

These variables:

tokensPerS
refillSleepS
sleepDuration

etc., and all of your methods, should be in lower_snake_case instead of camelCase, by convention.

Type hints

PEP484 allows you to turn this

def hasTokens(self):

into this

def hasTokens(self) -> bool:

and likewise for your other method parameters and returns. Among other things, it will allow you to drop your types from comments like this:

"""
    tokens (collections.deque): The tokens (which manifest as timestamps)
                                in reverse chronological order.
    last (float):               The most recent token consumed.
    rate (float):               See __init__ arg `tokensPerS`.
    capacity (int):             See __init__ arg `capacity`.
    sleepDuration (float):      See __init__ arg `refillSleepS`.
"""

because they'll already be in the signature.


Is this an efficient way to use one event for multiple controls? I've got a NumericUpDown control for each denomination of money and all of the events run the exact same code save for a single line so I'm trying to condense them into 1 event. I was originally going to use a switch but you can't switch on a non-value object so I just did an if tree. Is there a better way to do this?

private void BankInChanged(object sender, EventArgs e)
    {
        var objSender = (NumericUpDown) sender;

        if(objSender == bkI_Ls_Quarters)
        {
            currBoat.inBank.setQuarters(Convert.ToInt32(bkI_Ls_Quarters.Value));
        }
        else if(objSender == bkI_Rl_Quarters)
        {
            currBoat.inBank.setQuarterRolls(Convert.ToInt32(bkI_Rl_Quarters.Value));
        }
        else if(objSender == bkI_Ls_Nickels)
        {
            currBoat.inBank.setNickels(Convert.ToInt32(bkI_Ls_Nickels.Value));
        }
        else if(objSender == bkI_Rl_Nickels)
        {
            currBoat.inBank.setNickelRolls(Convert.ToInt32(bkI_Rl_Nickels.Value));
        }
        else if(objSender == bkI_Ls_Dimes)
        {
            currBoat.inBank.setDimes(Convert.ToInt32(bkI_Ls_Dimes.Value));
        }
        else if(objSender == bkI_Rl_Dimes)
        {
            currBoat.inBank.setDimeRolls(Convert.ToInt32(bkI_Rl_Dimes.Value));
        }
        else if(objSender == bkI_Ls_Pennies)
        {
            currBoat.inBank.setPennies(Convert.ToInt32(bkI_Ls_Pennies.Value));
        }
        else if(objSender == bkI_Rl_Pennies)
        {
            currBoat.inBank.setPennyRolls(Convert.ToInt32(bkI_Rl_Pennies.Value));
        }
        else if(objSender == bkI_Ones)
        {
            currBoat.inBank.setOnes(Convert.ToInt32(bkI_Ones.Value));
        }
        else if(objSender == bkI_Fives)
        {
            currBoat.inBank.setFives(Convert.ToInt32(bkI_Fives.Value));
        }
        else if(objSender == bkI_Tens)
        {
            currBoat.inBank.setTens(Convert.ToInt32(bkI_Tens.Value));
        }
        else if(objSender == bkI_Twenties)
        {
            currBoat.inBank.setTwenties(Convert.ToInt32(bkI_Twenties.Value));
        }
        else if(objSender == bkI_Fifties)
        {
            currBoat.inBank.setFifties(Convert.ToInt32(bkI_Fifties.Value));
        }
        else if(objSender == bkI_Hundreds)
        {
            currBoat.inBank.setHundreds(Convert.ToInt32(bkI_Hundreds.Value));
        }
        currBoat.updateTotals();
        updateStatDisplay();
    }

The problem is not in switching between senders but the fact that each branch calls a different method on inBank. I would suggest creating an enum for the denominations:

enum Denomination {
  Quarters,
  QuarterRolls,
  Nickels,
  NickelRolls,
  ...
}

and having a single setter in inBank:

void set(Denomination denomination, int value);

Then you can create a dictionary of controls to denominations:

IDictionary<NumericUpDown, Denomination> dict = new Dictionary<NumericUpDown, Denomination>();
dict[bkI_Ls_Quarters] = Denomination.Quarters;
dict[bkI_Rl_Quarters] = Denomination.QuarterRolls;
...

and your event handler would simply be:

private void BankInChanged(object sender, EventArgs e)
{
    var objSender = (NumericUpDown) sender;
    Denomination denomination = dict[objSender];

    currBoat.inBank.set(denomination, Convert.ToInt32(objSender.Value));
    currBoat.updateTotals();
    updateStatDisplay();
}


What would be a better way of adding the functionality to "add" a student? I have the following Student class:

class Student {
    public $user_id;
    public $name;

    public function __construct($user_id) {
        $info = $this->studentInfo($user_id);
        $this->name = $info['name'];
        $this->is_instructor = $info['is_instructor'];
        $this->user_id = $info['id'];
    }

    public function studentInfo($id) {
        global $db;

        $u = mysql_fetch_array(mysql_query("SELECT * FROM $db[students] WHERE id='$id'"));
        if($u) {
             return $u;
        }
    }

    public function getCoursesByInstructor() {
        global $db;

        return mysql_query("SELECT courses.*, course_types.name FROM $db[courses] as courses
                            JOIN $db[course_types] as course_types ON courses.course_type_id=course_types.id
                            WHERE instructor_id='$this->user_id'");
    }
}

Ideally, I'd do:

$u = new Student(1);
$courses = $u->getCoursesByInstructor();

Does anyone have any critiques on this class? Also, how would you recommend me adding functionality to "add" a student?

mysql_* functions

Stop using them, immediately! They are essentially deprecated, as they refer to MySQL before version 4.1. They are not removed as to not brake backward compatibility, but there's no point in using them.

The drop in replacement is myslqi_* functions, which are based around a newer MySQL driver that works better with MySQL 5.x. You can easily replace most references to myslq_* functions by just adding the extra i.

But a far better solution is PDO, an object oriented database agnostic interface. Since you are starting out in object oriented PHP you'll definitely appreciate the object orient part, and as for the database agnostic part, it means that you can easily switch databases (with minor modifications).

I would advise against an ORM solution at this point, ORM's are extremely useful but can be easily abused if you aren't 100% certain on what you are doing.

global $db;

Never ever do that again. Forget about the global keyword completely. It has its uses, but in most situations it's an extremely bad habit. The object oriented way to achieve the same effect would be:

class Student {
    private $db;

    public function __construct($db, $user_id) {
        $this->db = $db;

        ...
    }
}

That's a minor example of Dependency Injection. In sort, you should feed your object what it needs, and not depend on something being available in the global namespace. You can't always be absolutely sure of that.

Public properties

Public properties violate the object oriented concept of encapsulation. You really don't want to do that, trust me :)

The most common object oriented workaround are setters and getters:

class Student {
    private $user_id;
    private $name;

    ...

    public function setUserID($user_id) {
        if( !is_int($user_id) ) {
            throw new InvalidArgumentException();
        }

        $this->user_id = $user_id;

        return $this;
    }

    public function setName($name) {
        if( !is_string($name) ) {
            throw new InvalidArgumentException();
        }

        $this->name = $name;

        return $this;
    }

    public function getUserID() {
        return $this->user_id;
    }

    public function getName() {
        return $this->name;
    }

    ...

}

Wow! So much more code for such a simple thing! Before you get discouraged, notice that I also validate the input on the setters and I'm throwing exceptions if something is wrong. That's a very good reason to use setters instead of public properties, that way you have the ability of checking what the parameter is, and if it's not the one you are expecting you can deal with it accordingly.

In the instances above I'm throwing an InvalidArgumentException, which is one of the many pre-built SPL extensions. Learn about the SPL, it offers quite a few core object oriented interfaces and classes, such as iterators.

A far better way to validate common datatypes is the filter functions, I'm not using them in the example above to not confuse you any further. And, when you feel confident enough, you can minimize the code via magic Property overloading. Fair warning, don't go there unless you know exactly what you're doing.

But never ever again have public properties, always write getters and setters. And of course, the $db property from my previous example should have its own setter. No need for a getter, unless of course you actually need to pass around the database connection object.

Lastly, notice how I return $this on the setters? This isn't something you have to do, it's a trick to allow method chaining:

$student = new Student( ... );
$student->setUserID(1)->setName("Yannis");

Or, if you prefer a slightly more readable version:

$student->setUserID(1)
        ->setName("Yannis");

Naming

As @jiewmeng writes, studentInfo should be renamed getStudentInfo. You should be very consistent on how you name functions, prefixing functions that return results with get and those that set info in the class with set is a very common practice, and one that will be familiar to most programmers, as it alludes to the concept of getters and setters I've described above.

Always return something

I don't really like it when functions don't return something:

public function studentInfo($id) {
    global $db;

    $u = mysql_fetch_array(mysql_query("SELECT * FROM $db[students] WHERE id='$id'"));
    if($u) {
         return $u;
    }
}

Here, of course you return $u;, but only if($u). For that function, I would probably return false as well:

public function studentInfo($id) {
    global $db;

    $u = mysql_fetch_array(mysql_query("SELECT * FROM $db[students] WHERE id='$id'"));
    if($u) {
         return $u;
    }

    return false;
}

Of course null is returned by default when you don't return something explicitly, but I find it a lot better when I know exactly what I should expect from a function return, and false will tell me that something failed long after I totally forget what goes on in the function. If you are in a situation where you can't think of something obvious to return, that might be a good place to throw an exception instead.

Adding a student

You could do a save() function:

class Student {

    ...

    public function save() {
        if(!this->validate()) {
            return false;
        }

        return $this->store();
    }

    private function validate() {
        // check all properties
        // if the required ones are filled and valid, return true,
        // else return false
    }

    private function store() {
        if($this->userExists()) {
            return $this->insert();
        }

        return $this->update();
    }

    private function userExists() {
        // determine if user already exists in the database
    }

    private function insert() {
        // insert user in the database
    }

    private function update() {
        // update existing user
    }

    ...
}

Of course that's more pseudo code, than code. But more or less, that's the common workflow.  I'm assuming each function returns either true or false. Notice how the return value propagates from insert() or update() to save()?

In conclusion

You need to do a lot of research into the fundamental object orient concepts. The wikipedia article is as a good place to start as any, and most articles on the specific concepts have PHP examples. Your code is clean, and you do quite a few things correctly, but you are starting out in a new programming paradigm, one that is extremely useful and extremely vague. More often than not "best practices" are not obvious, and you'll need quite solid conceptual foundations to write object oriented code.

And please, consider writing phpDoc comments.

Happy new year!


Least Mean Square channel equalizer I've written (and tested) a simple least mean square adaptive filter . The steps of the algorithm are:

$$ For n = 0, 1, 2, \ldots: $$
    $$ x = [u(n), u(n+1), \ldots, n(n-p+1)]^T $$
    $$ e(n) = d(n) - w(n)^Tx(n) $$
    $$ w(n+1) = w(n) + \mu*e(n)^Tx(n) $$

Where \$u\$ is the input signal, \$w\$ are the weights of the filter, \$p\$ is the order (number of taps), \$e\$ is the error signal, and \$d\$ is the desired signal.

import numpy as np
import matplotlib.pyplot as plt
import traceback
import sys

class LMS:
    def __init__(self, input_signal, desired_signal, num_taps, learning_rate):
        self.u = input_signal
        self.d = desired_signal
        self.num_taps = num_taps
        self.mu = learning_rate
        self.num_points = len(self.u)
        self.weights = np.zeros(num_taps)
        self.y = np.zeros(self.num_points)
        self.e = np.zeros(self.num_points)

    def equalize(self):
        for n in xrange(self.num_taps, self.num_points):
            x = self.u[n:n-num_taps:-1]
            self.y[n] = np.dot(x, self.weights)
            self.e[n]= self.d[n] - self.y[n]
            self.weights = self.weights + self.mu*x*self.e[n]

try:
    np.random.seed(1337)
    ulen    = 20000
    coeff   = np.concatenate(([1], np.zeros(10), [-0.9], np.zeros(7), [0.1]))
    u       = np.random.randn(ulen)
    d       = np.convolve(u, coeff)
    num_taps = 20
    step = 0.01

    eq = LMS(u, d, num_taps, step)
    eq.equalize()
    plt.figure()
    plt.semilogy()
    plt.subplot(1,1,1)
    plt.plot(np.abs(eq.e))
    plt.show()
except:
    type, value, tb = sys.exc_info()
    traceback.print_exc()
    pdb.post_mortem(tb)

As written the algorithm converges (I get the following plot of the error vector):

Exceptions
You don't need the sys.exc_info call since pdb.post_mortem without argument uses the exception being currently handled. You don't need pdb either since it serves for development purposes and, at this stage, you should have working code already.
And since all what's left in the except clause is traceback.print_exc(), you could remove the try .. except and let the interpreter print the traceback for you.
Also, it's a very bad habbit to use bare excepts. You should know what kind of exceptions you’re willing to handle. (Even though it can be usefull with pdb to examine various causes of issues.)
Class
Given how you use the class (initialization of attributes, computation and getting one attribute back), I think you could do equally well with a simple function. Just initialize what you need before the for loop and return the computed array:
def LMS(input_signal, desired_signal, num_taps, learning_rate):
    num_points = len(input_signal)
    weights = np.zeros(num_taps)
    equalized = np.zeros(num_points)

    for n in xrange(num_taps, num_points):
        x = input_signal[n:n-num_taps:-1]
        equalized[n]= desired_signal[n] - np.dot(x, weights)
        weights = weights + learning_rate*x*equalized[n]

    return equalized

Note that I kept the meaningfull names to help read the algorithm. I also removed y which was a temporary variable and served no purpose after computation.
main
It is also good practice to avoid keeping code at the top-level of the file. Just in case you want to import your file (for testing purposes in an interactive shell, for instance), you should guard the code in an if __name__ == '__main__': clause.
The whole code becomes:
import numpy as np
import matplotlib.pyplot as plt

def LMS(input_signal, desired_signal, num_taps, learning_rate):
    num_points = len(input_signal)
    weights = np.zeros(num_taps)
    equalized = np.zeros(num_points)

    for n in xrange(num_taps, num_points):
        x = input_signal[n:n-num_taps:-1]
        equalized[n]= desired_signal[n] - np.dot(x, weights)
        weights = weights + learning_rate*x*equalized[n]

    return equalized

if __name__ == '__main__':
    np.random.seed(1337)
    ulen = 20000
    coeff = np.concatenate(([1], np.zeros(10), [-0.9], np.zeros(7), [0.1]))
    u = np.random.randn(ulen)
    d = np.convolve(u, coeff)
    num_taps = 20
    step = 0.01

    eq = LMS(u, d, num_taps, step)

    plt.figure()
    plt.semilogy()
    plt.subplot(1,1,1)
    plt.plot(np.abs(eq))
    plt.show()

(I removed the extra whitespace arround assignements since it is recommended by PEP8)


Bash script to rename subfolder to include name of parent folder Here's the current structure of my directory.

.
├── Show 1
│   ├── Season 1
│   └── Season 2
├── Show 2
    ├── Season 1
    └── Season 2

I want to rename the Season folders to include the name of the Show. The desired structure looks like this :

.
├── Show 1
│   ├── Show 1 - Season 1
│   └── Show 1 - Season 2
├── Show 2
    ├── Show 2 - Season 1
    └── Show 2 - Season 2

Here's the script I wrote :

# Parse through all show folders.
for show in /Users/sanjeetsuhag/Desktop/* ; do
    # Check if it is a folder.
    if [ -d "$show" ]; then
        # Parse through all season folders.
        for season in $show/* ; do
            # Check if it is a folder.
            if [ -d "$season" ]; then
                mv $season "$show/$(basename "$show") - $(basename "$season")"
            fi
        done
    fi
done

This is my first time scripting in Bash. Is there any thing I could improve ?

You should always double-quote variables that are used as filesystem paths.
This is the same script but with variables correctly double-quoted:

# Parse through all show folders.
for show in /Users/sanjeetsuhag/Desktop/*; do
    # Check if it is a folder.
    if [ -d "$show" ]; then
        # Parse through all season folders.
        for season in "$show"/*; do
            # Check if it is a folder.
            if [ -d "$season" ]; then
                mv "$season" "$show/$(basename "$show") - $(basename "$season")"
            fi
        done
    fi
done

If you change the globs to end with /,
they will match only directories,
and so you can skip the directory checks:

for show in /Users/sanjeetsuhag/Desktop/*/; do
    for season in "$show"/*/; do
        mv "$season" "$show/$(basename "$show") - $(basename "$season")"
    done
done

You could further optimize this by using pattern substitution instead of basename:

for show in /Users/sanjeetsuhag/Desktop/*/; do
    for season in "$show"/*/; do
        show=${show%/}
        season=${season%/}
        mv "$season" "$show/${show##*/} - ${season##*/}"
    done
done

The ${show%/} is to shave off the trailing /,
and the ${show##*/} is to delete everything until the last /.

Finally, instead of hardcoding the base path /Users/sanjeetsuhag/Desktop,
the script would be more reusable if you make that a command line parameter of the script.


Javascript querystring to object conversion The below code converts the window.location.search querystring into a JavaScript object. In particular, if a key occurs more than once the resulting value in the object will be an array. Please feel free to comment on both readability and speed of execution. Thanks!

window.location.search.split(/[?&]/g).reduce(function (prev, cur, idx, arr) {
    var segment = cur.split("="),
        key = segment[0],
        val = segment[1];
    if (prev[key]) {
        if (prev[key] instanceof Array) {
            prev[key].push(val);
        } else {
            prev[key] = [prev[key], val];
        }
    } else {
        prev[key] = val;
    }
    return prev;
}, { });

The line if (typeof prev[key] === typeof []) { is... weird. If you're checking for a type specifically, check for it: if (typeof prev[key] === 'object') { - the way you're doing it there causes a new array literal to be instantiated needlessly, as well as a superfluous typeof operation, both inefficient and outright unnecessary.

Making an array out of repeating keys is fallacious - if a key is exactly repeated in a query string, only the right-most instance of that query string value will be received by the server. Further, in cases where the query string contains validly structured arrays (test=1&test2[]=2&test2[]=4), your code ends up with a key like test[], which is a nasty to access: results['test2[]'] is the only way to get at it (no dot syntax allowed).

EDIT
To clarify my remarks about duplicating keys, I purhaps mistakenly restricted my comments to Apache(Linux or Windows)/PHP - certainly other languages and platforms are relevant to the discussion. In the PHP language (what I am most familiar with), the query string test=1&test=2 will result in:

var_export($_GET);
/* array (
  'test' => '2',
) */

... as you can see, only the right-most value is the only one passed into the script. The way PHP expects duplicate query string keys is by use of [] in the query: test[]=1&test[]=2

var_export($_GET);
/*array (
  'test' =>
      array (
        0 => '1',
        1 => '2',
  ),
)*/

This behavior is the same for POST data on PHP - you must use name="myField[]" if multiple fields will use the same name, otherwise, only the last item in the form with a given name will be populated in PHP's $_POST data.

I am not set up to confirm the behavior of this in ASP (or other languages), however research has indicated that my assertion is still somewhat valid: https://stackoverflow.com/questions/6395290/how-may-i-add-integer-list-to-route/6396793#6396793. Please confirm if you are able: from what I have gathered the values would be passed to script as a comma-delimited list, but the language itself still rejects working with duplicate keys and a workaround is required.

If you can add some data about handling of duplicate keys in other server languages, please feel free to edit them into this answer. That said, I feel my criticism of this script's handling of duplicate keys stands, as well as the issue with "properly formed" (PHP) arrays in the query string - result['test[]'] is nasty.


Scraping data from a table in python I'm new to python, and after doing a few tutorials, some about scraping, I been trying some simple scrapping on my own. Using beautifulsoup I manage to get data from webpages where everything has labels, but without them I'm doing a poor job.

I'm trying to get the dollar exchange rate from:
http://www.bancochile.cl/cgi-bin/cgi_mone?pagina=inversiones/mon_tasa/cgi_mone

The value I'm after is highlighted in yellow

After a lot of trial and error, I manage to get the dollar exchange rate, but I think there has to be a better way.

import requests
from bs4 import BeautifulSoup

page = requests.get("http://www.bancochile.cl/cgi-bin /cgi_mone?pagina=inversiones/mon_tasa/cgi_mone")
soup = BeautifulSoup(page.content, 'html.parser')

tables = soup.find_all("table")
dollar = tables[4].find_all("td")

print(dollar[5].string)

Is there a better, or more correct way to do this? Also, I'm not sure if the problem is in the way I coded, or in not being able to better understand the html structure, to navigate to the information in a more efficient way.

The markup is definitely not easy to parse because of the nested table elements with no meaningful attributes. But, you are right that relying on relative index of a table and the desired cell being the 6th in the table is quite a fragile strategy.

Instead, let's use the row title as our "anchor". Then, we'll get the following cell via the .find_next_sibling():

DESIRED_MONEDAS = "DOLAR USA"

label = soup.find(lambda tag: tag.name == "td" and tag.get_text(strip=True) == DESIRED_MONEDAS)
value = label.find_next_sibling("td").get_text(strip=True)
print(value)


Simple Factorial Interview Task I did this code for an interview. Can you tell me how it looks? I didn't get the job, so I'm wondering what I could do better in my code.

/**
 * TASK 1: IMPLEMENT: C - binomial coefficient
 *
 * C(n,k) = n! / [ k! (n - k)! ]
 * Where x! is the factorial of x
 *
 * Use a recursive solution. If needed, you can implement helper functions.
 */

function C(n,k) {
    "use strict";

/**
 * ~~OMID~~
 * The code is not linted, or refactored on purpose.
 * Tested with node 0.10.32.
 */

/**
 * factorial() returns the factorial
 * of any none-negative number.
 * @param {integer}
 */
    function factorial(number) {

        var result = 1;

        if(number < 0){
          return;
        } else if (number <= 1) {
          return  result;
        }
        else {
          result = number * factorial(number-1);
        }
        return result;
      }
    // Testing numerality of parameteres, n and k
    if(isNaN(parseFloat(n)) && isFinite(n) && isNaN(parseFloat(n)) && isFinite(n)) {
        return;
    }
    else {
/**
 * Implementing the formula.
 * C(n,k) = n! / [ k! (n - k)! ]
 */
      return  (factorial(n)/factorial(k)*factorial(n-k));
    }
}

/**
 * TASK 2: IMPLEMENT: Tests for function C
 *
 * Write a set of tests, that validate your binomial coefficient function.
 * Write as many test cases as you feel is needed.
 */

(function testBinomialCoefficient() {

    var tests = {

        example: function() {
            var pass = typeof C === "function";
            return {
                success: pass,
                message: (!pass) ? 'Expected C to be a function' : ''
            };
        },
                normalInteger: function() {
                        var pass = C(5, 4) === 5;
                        return {
                                success: pass,
                                message: (!pass) ? 'C(5,4) Should be equal to 5': ' '
                        };

                },
                negativeInput: function() {
                        var pass = isNaN(C(-5, 4));
                        return {
                                success: pass,
                                message: (!pass) ? 'Negative input must result in NaN': ' '
                        };

                },
                smallerSecondParam: function() {
                        var pass = isNaN(C(10, 19));
                        return {
                                success: pass,
                                message: (!pass) ? 'Second parameter should be smaller': ' '
                        };

                },
    };

    //  ---- You shouldn't change the code below this line  ----

    for (var test in tests) {
        var result = tests[test].apply(undefined);
        if (result.success) {
            console.log("Passed: " + test);
        } else {
            console.log("Failed: " + test + ", Message: " + result.message);
        }
    }

    //  ---- You shouldn't change the code over this line  ----

})();

The code demonstrates some nice effort towards writing good code, so a professional mindset. However, it does have issues, which I'll explain below.

1. Testing input arguments

I believe that you could have done without that, as the point of the exercise is not to check the parameters, but rather to calculate properly the binomial coefficient.

Anyway, your test condition isNaN(parseFloat(n)) && isFinite(n) && isNaN(parseFloat(n)) && isFinite(n) has issues. Firstly, it only tests the value n, and not the value k (this is a typo, but your following tests have not found it, meaning your tests are incomplete). Secondly, you do not check for positive integers. Thirdly, you convert the variables into numbers (fine), but still use the original values thereafter. Finally, if the test fails, then you should probably do something more than return undefined (and document that behavior as well).

2. The factorial function

Your inputs have been checked already, so I think that you should keep this function as simple as possible, e.g.

function factorial(n) {
  if (n <= 1) {
    return 1;
  }
  return n * factorial (n - 1);
}

... or using the ternary operator (less readable in a recursive algorithm, but some people prefer that form), e.g.

function factorial(n) {
   return n <= 1 ? 1 : n * factorial (n - 1);
}

3. The algorithm

This is where, I believe, lies the real point of the exercise. The provided formula C(n,k) = n! / [ k! (n - k)! ], if implemented "as is", has duplications. After all, n! contains both k! and (n - k)!, so you'd be calculating k! or (n - k)! twice, which can be quite expensive. You should therefore remove one of them from n! (the most expensive one), depending on k being superior or inferior to n / 2.

4. The tests

The test named "example" should be removed. Either C is a function, and the following tests are enough, or it is not, and their subsequent execution will lead to a runtime error anyway.

The message property is intended as the name of the test, so there is no need to check the value of the pass variable - and so, there's no need for that variable either.

Finally, I believe that the statement tests[test].apply(undefined) was provided to you by the company. This is... not pretty, and quite unnecessary.


Reading a text file API 15+ Is there a faster, more efficient way to read a text file than this implementation?

Taking into account phones capabilities:

    dictionary = new ArrayList<String>();
    long start = System.currentTimeMillis();
    int count = 0;
    try{
        InputStream inputStream = context.getAssets().open("words.txt");
        InputStreamReader inputStreamReader = new InputStreamReader(inputStream);
        BufferedReader bufferedReader = new BufferedReader(inputStreamReader);
        String word;
        while((word = bufferedReader.readLine()) != null){
            dictionary.add(word);
            count++;
        }
        inputStream.close();
        inputStreamReader.close();
        bufferedReader.close();

    }catch(IOException e){
        e.printStackTrace();
    }
    long end = System.currentTimeMillis();
    double t = ((end - start) / 1000.0);
    System.out.println("Time to read database from file " + count + " items " + t + " seconds");

Output:

  Time to read database from file 272403 items 1.112 seconds

Update:
After taking @rolfl advice into account and doing a little more digging this is what I came up with. Any further advice or a tidy up would be very welcome

    dictionary = new ArrayList<>(300000);
    long start = System.currentTimeMillis();
    InputStream inputStream = null;
    try{
        inputStream = context.getAssets().open("words.txt");
    }catch(IOException e){
        e.printStackTrace();
    }
    ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream();
    byte[] buff = new byte[1048576];
    try{
        for(int i; (i = inputStream.read(buff)) != -1; ){
            byteArrayOutputStream.write(buff, 0, i);
        }
    }catch(IOException ex){
        ex.printStackTrace();
    }
    String[] contents = byteArrayOutputStream.toString().split("\n");
    for(int i = 0; i < contents.length; i++){
        dictionary.add(contents[i]);
    }
    long end = System.currentTimeMillis();
    double t = ((end - start) / 1000.0);
    System.out.println("Time to read database from file " + dictionary.size() + " items " + t + " seconds");

Output:

  Time to read database from file 272403 items 0.708 seconds

Don't do unnecessary work. You have the count variable, but you also have the dictionary which has a size() method. There's no need for the count.
Android supports Java-7 language features, use them. In this case, the try-with-resources would be your friend.
guess the size of the ArrayList that yoy may need. In this case, you should be a little generous, and say, pre-size it at 300,000 entries.
Android now supports (since KitKat) the diamond operator, there should be no need to declare the generic type of the ArrayList as <String>.
I actually like the while loop you have. It is my preferred way of doing line-by-line IO too.

Here's a 'cleaned up' version of your code:

private static final int INITIALSIZE = 300000;

....

long start = System.currentTimeMillis();
dictionary = new ArrayList<>(INITIALSIZE);
try (BufferedReader bufferedReader = new BufferedReader(
          new InputStreamReader(context.getAssets().open("words.txt")));) {

    String word;
    while((word = bufferedReader.readLine()) != null){
        dictionary.add(word);
    }
}catch(IOException e){
    e.printStackTrace();
}
long end = System.currentTimeMillis();
double t = ((end - start) / 1000.0);
System.out.println("Time to read database from file " + dictionary.size()
      + " items " + t + " seconds");

So, that's a "simplified" version, how to make it faster?

Well, there's a few things. First up, nothing can be for sure unless you test it, so, run some experiments. Things I would try:

Specify a buffer-size on the BufferedReader, something large like 1024 * 1024 (a megabyte). This should increase the size of IO's
The pre-sized ArrayList will help
Consider reading the whole data file in to a ByteArrayOutputStream, and then converting that in one go in to a large String, then splitting the string on line-breaks.

In essence, the larger the IO sizes the better, and the larger the cache sizes are, the better.


Function to find the shortest word in an array, where not every element is a string I wrote a solution for a codeCamp exercise:

  Write a function called findShortestWordAmongMixedElements.

  Given an array, findShortestWordAmongMixedElements returns the
  shortest string within the given array.

  Notes:

  If there are ties, it should return the first element to appear in the given array.
  Expect the given array to have values other than strings.
  If the given array is empty, it should return an empty string.
  If the given array contains no strings, it should return an empty string.

Right now I essentially break this function into two parts. The filtering method—to just get the strings—and then reduce to find the shortest one among them.

function findShortestWordAmongMixedElements(arr) {
  var containsStrings = function(arr){
   return arr.every(function(cv){
      return Object.prototype.toString.call(cv) !== '[object String]';
    });
  }, shortestWord;

  if ((!(arr.length)) || ((containsStrings(arr)))) return '';

   arr = arr.filter(function(e, i, a){
     if (typeof e == 'string') {
       return e;
     }
   });

   shortestWord = arr.reduce(function(prev, next) {
     if (prev.length < next.length) {
         return prev;
     } else if (prev.length === next.length){
        return prev;
     } else {
        return next;
     }
   });

   return shortestWord;

}

My take is this is certainly readable which I like and I LOVE the helper function I made for the initial check which uses the every method.

Since (at first) I spent a lot of time trying to do all the operations within the body of the callback for the filter—I feel this take is like I used flat head screw driver for every screw instead of a phillips...

It seems odd to me that you loop through the array twice, once with every and once with filter. Why not just filter it and check if the filtered array has any elements? Also is there a reason why you used two different methods to check if the elements were strings? One more thing, I would have written the filter method as:

arr = arr.filter(function(e) {
  return typeof e == 'string';
});

If you really want to do this using a single loop you could do something like:

    function findShortestWordAmongMixedElements(arr) {
      return arr.reduce( function(shortest, e) {
        return (typeof e == 'string') && (shortest=='' || e.length < shortest.length) ? e : shortest;
      }, '');
    };

    console.log( findShortestWordAmongMixedElements([
       1, 'longer', 2, 'short', 3, 'longest'
    ]) );


Optimizing odds calculator I am trying to setup an odds calculator for a best of 7 series, given independent odds for each event.

The following code works, but I would like to add recursion to simplify the end.

public class Game
{
    public int No { get; set; }
    public List<decimal> Odds;

    public Game(int no, decimal odd1, decimal odd2)
    {
        No = no;

        Odds = new List<decimal>();

        Odds.Add(odd1);
        Odds.Add(odd2);
    }

  }

void Main()
{

    var games = new List<Game>();

    var homeodd = .6m;
    var awayodd = .4m;

    var winningOdds = 0m;

//Add  7 games each with a different odd of winning game
    games.Add(new Game(1,homeodd,awayodd));
    games.Add(new Game(2,homeodd,awayodd));
    games.Add(new Game(3,awayodd,homeodd));
    games.Add(new Game(4,awayodd,homeodd));
    games.Add(new Game(5,awayodd,homeodd));
    games.Add(new Game(6,homeodd,awayodd));
    games.Add(new Game(7,homeodd,awayodd));

//game one has 2 possible outcomes, 0 = win, 1 = loss =>same for all 7 games
    for (int i = 0; i < 2; i++)
    {
        for (int j = 0; j < 2; j++)
        {
            for (int k = 0; k < 2; k++)
            {
                for (int l = 0; l < 2; l++)
                {
                    for (int m = 0; m < 2; m++)
                    {
                        for (int n = 0; n < 2; n++)
                        {
                            for (int o = 0; o < 2; o++)
                            {
                                if ((i+j+k+l+m+n+o)<4)  //if we have loss less than 4 games, we have won the series and we want to add the odds of that possibility to the total odds
                                {
                                    winningOdds += games[0].Odds[i] * games[1].Odds[j] * games[2].Odds[k] * games[3].Odds[l] * games[4].Odds[m] * games[5].Odds[n] * games[6].Odds[o];
                                }
                            }
                        }
                    }
                }
            }
        }
    }

    Console.WriteLine(winningOdds);
}

Basically, what you need is a Cartesian product of 7 sequences, each containing a 0 and a 1. Eric Lippert has an interesting article about doing just that using LINQ (note: I think you could make his code more efficient using immutable collections). If you use his method, your code could look something like:
var product = CartesianProduct(Enumerable.Repeat(new[] { 0, 1 }, games.Count));

foreach (var indexes in product)
{
    if (indexes.Sum() < 4)
    {
        winningOdds += indexes
            .Select((index, game) => games[game].Odds[index])
            .Aggregate((x, y) => x * y);
    }
}


Split download file buffer by any number of threads This is my first attempt at splitting the download buffer into several threads by the number provided.  I want to improve on performance and better implementation approach.

import os
import requests
import threading
import urllib2
import time

url = "http://www.nasa.gov/images/content/607800main_kepler1200_1600-1200.jpg"
# url = 'http://pymotw.com/2/urllib/index.html'
# url = 'http://megaboon.com/common/preview/track/786203.mp3' # size cannot be determined.
# url = 'http://broadcast.lds.org/churchmusic/MP3/1/2/nowords/271.mp3'

def buildRange(value, numsplits):
    lst = []
    for i in range(numsplits):
        if i == 0:
            lst.append('%s-%s' % (i, int(round(1 + i * value/(numsplits*1.0) + value/(numsplits*1.0)-1, 0))))
        else:
            lst.append('%s-%s' % (int(round(1 + i * value/(numsplits*1.0),0)), int(round(1 + i * value/(numsplits*1.0) + value/(numsplits*1.0)-1, 0))))
    return lst

class SplitBufferThreads(threading.Thread):
    """ Splits the buffer to ny number of threads
        thereby, concurrently downloading through
        ny number of threads.
    """
    def __init__(self, url, byteRange):
        super(SplitBufferThreads, self).__init__()
        self.__url = url
        self.__byteRange = byteRange
        self.req = None

    def run(self):
        self.req = urllib2.Request(self.__url,  headers={'Range': 'bytes=%s' % self.__byteRange})

    def getFileData(self):
        return urllib2.urlopen(self.req).read()

def main(url=None, splitBy=3):
    start_time = time.time()
    if not url:
        print "Please Enter some url to begin download."
        return

    fileName = url.split('/')[-1]
    sizeInBytes = requests.head(url, headers={'Accept-Encoding': 'identity'}).headers.get('content-length', None)
    print "%s bytes to download." % sizeInBytes
    if not sizeInBytes:
        print "Size cannot be determined."
        return

    dataLst = []
    for idx in range(splitBy):
        byteRange = buildRange(int(sizeInBytes), splitBy)[idx]
        bufTh = SplitBufferThreads(url, byteRange)
        bufTh.start()
        bufTh.join()
        dataLst.append(bufTh.getFileData())

    content = ''.join(dataLst)

    if dataLst:
        if os.path.exists(fileName):
            os.remove(fileName)
        print "--- %s seconds ---" % str(time.time() - start_time)
        with open(fileName, 'w') as fh:
            fh.write(content)
        print "Finished Writing file %s" % fileName

if __name__ == '__main__':
    main(url)

I hope this is not a challenging one to suggest or comment on improvement.

Utilizing Threads:

You are doing a number of things to slow down and serialize your code while the point of threads is to do things in parallel.

for idx in range(splitBy):
    byteRange = buildRange(int(sizeInBytes), splitBy)[idx]
    bufTh = SplitBufferThreads(url, byteRange)
    bufTh.start()
    bufTh.join()
    dataLst.append(bufTh.getFileData())

In the first line, you generate the full list of byte ranges, but then only use the the range for the specific index you are on. This only needs to be generated once, not every time you start a new chunk.

Then in the next three line, you create the thread, start it, and then immediately wait for it to finish. This means you are never running more than one thread at a time. This implementation will be slower than a single threaded implementation because of the over head involved.

On the last line you get the datafile and append all the bytes into your buffer. This is where the bottle-neck is. Request is just a class that contains the information describing what should be requested. urlopen is the function that actually makes the internet connection. Even if you weren't serializing your thread creation and execution, you still would not have a performance gain because the function that will take time is not part of the run method. You want to execute all the code in the run() method, then just access the resultant data after the thread has terminated.

The body of your main should be like this:

threads = []
byteRanges = buildRange(int(sizeInBytes), splitBy)
for idx in range(splitBy):
    t = SplitBufferThreads(url, byteRange)
    t.start()
    threads.append(t)

# all of the threads are downloading files at this point

for t in threads:
    t.join()

# file prep stuff ...

with open(fileName, 'w') as fh:
    for t in threads:
        fh.write(t.data) #where run() stores the data it read

Edit:
It seems like you need a better understanding of how threads work. When you start a thread (in the main thread), run() is executed in the context of the thread that was started. The main thread can then continue to execute other tasks while the other thread is executing in the background. Once run() terminates, nothing else executes in the thread.

The other key point is that the call to urlopen() is when the download begins to happen. If this is not done in the context of a separate thread, it provides no performance gains. This means it must be called from run() or a method run() calls. Otherwise, it will execute in the main thread.

The thread class should be implemented like this:

class SplitBufferThreads(threading.Thread):
    """ Splits the buffer to ny number of threads
        thereby, concurrently downloading through
        ny number of threads.
    """
    def __init__(self, url, byteRange):
        super(SplitBufferThreads, self).__init__()
        self.__url = url
        self.__byteRange = byteRange

    def run(self):
        req = urllib2.Request(self.__url,  headers={'Range': 'bytes=%s' % self.__byteRange})
        # here is where the file download happens within the context of the thread
        self.data = getFileData(req)

    def getFileData(self, request):
        return urllib2.urlopen(request).read()

buildRange() is going too much on one line.

lst.append('%s-%s' % (i, int(round(1 + i * value/(numsplits*1.0) + value/(numsplits*1.0)-1, 0))))

Adding to a list, formating a string, and two mathematic equations. The equations are non-trivial and it is not immediately apparent what they are actually doing. Both equations should be broken out into separate functions.

Additionally, the if statement is deciding how the first and second values are calculated. It does not effect the string formating or how the list is mutated.

def buildRange(value, numsplits):
    lst = []
    for i in range(numsplits):
        if i == 0:
            first = i
            second = int(round(1 + i * value/(numsplits*1.0) + value/(numsplits*1.0)-1, 0))
        else:
            first = int(round(1 + i * value/(numsplits*1.0),0))
            second = int(round(1 + i * value/(numsplits*1.0) + value/(numsplits*1.0)-1, 0))
        lst.append("{}-{}".format(first, second))
    return lst

Once you do this, it becomes clear that second is not effected by the if statement. Now, we can make the function even simpler. Replacing the equations greatly improves the readability.

def buildRange(value, numsplits):
    lst = []
    for i in range(numsplits):
        first = i if i == 0 else start(i, value, numsplits)
        second = end(i, value, numsplits)
        lst.append("{}-{}".format(first, second))
    return lst

Naming:

lst is a bad variable name. We know it is a list from when the variable is initialized, but we don't know what it is meant to store.

buildRange() returns a list of things, yet the name makes it sound like it only makes one.

In my examples, first and second are also bad names, but I hadn't fully understood what the values represented until the code was cleaner.

SplitBufferThreads is a single thread, it should not be pluralized.

The first argument to buildRange() is value. The name of the variable that is passed in for this argument is sizeInBytes. Which one gives you a better idea about what the code is trying to do?

The Python convention for naming function is underscore_spaced, not camelCase.

You are mixing the meaning of a range and how it is used by the specific library you are using. If you changed the library you are using to make the specific request, you then need to change how you are constructing your ranges or parse out the values from the string.


Bookmarklet to go to YouTube channel RSS feed I've written a simple bookmarklet to go to the YouTube channel RSS feed URL from at least any video or channel page:

javascript:(function () {
    var newLocation = function () {
        var url;
        Array.prototype.slice.call(document.getElementsByTagName('link')).forEach(function (element) {
            if (element.getAttribute('type') === 'application/rss+xml') {
                console.log('Found direct feed link');
                url = element.getAttribute('href');
            }
        });
        if (!url) {
            Array.prototype.slice.call(document.getElementsByTagName('meta')).forEach(function (element) {
                if (element.getAttribute('itemprop') === 'channelId') {
                    console.log('Found channel ID');
                    url = 'https://www.youtube.com/feeds/videos.xml?channel_id=' + element.getAttribute('content');
                }
            });
        }
        return url;
    }();
    if (newLocation === undefined) {
        console.log('Could not find a channel RSS feed from ' + location.href);
    } else {
        location.href = newLocation;
    }
})();

Some questions which come to mind:

Is this the simplest way to find an element with a specific attribute value?
Does this work with recent browsers other than Firefox?
Does it use any non-standard/deprecated features?
Are there any YouTube pages this does not work on, where it should?

Is this the simplest way to find an element with a specific attribute value?

"simple" is subjective. It could mean anything, like lesser lines, fewer keystrokes, lesser complexity. To me, what I consider simple is code that's short, concise and one that I can understand without running the code.

  Does this work with recent browsers other than Firefox?

With regards to the code, I don't see why not. However, I'm not sure how bookmarklets work with browsers these days. That part you might want to check.

  Does it use any non-standard/deprecated features?

Nothing out of the ordinary. You might want to check MDN or Kangax's compatibility table for more info about what APIs are available to which browsers.

  Are there any YouTube pages this does not work on, where it should?

Not something you'd normally ask in Code Review. That's up to you to find out.

You could substitute array.forEach with find. That way, it bails out on the first item found and not go through the entire array.

Next, you can use template literals instead of string concatenation. It's much more concise and lessens the chance that you'll miss a quote or a + somewhere.

Next, your logic has a lot of if statements. You could try to invert the logic and bail out early. You can also split your logic into functions, that way you can maintain extraction methods with clear separation. Then call them in sequence with || between. That way, the script uses the return value of first call that returns a value, or tries the next if the previous fails. Same conditional effect without the use of if and a lot of nested code.

Something along these lines:

javascript:(function(){
  const slice = Array.prototype.slice;

  function getUrlFromRss(){
    const links = slice.call(document.getElementByTagName('link'));
    const link = links.find(l => l.getAttribute('type') === 'application/rss+xml');
    return link ? link.getAttribute('href') : '';
  }

  function getUrlFromMeta(){
    const metas = slice.call(document.getElementsByTagName('meta'));
    const meta = metas.find(m => m.getAttribute('itemprop') === 'channelId');
    const id = meta ? meta.getAttribute('content') : '';
    return id ? `https://www.youtube.com/feeds/videos.xml?channel_id=${id}` : '';
  }

  const url = getUrlFromRss() || getUrlFromMeta() || '';

  if(!url) return console.log(`Could not find a channel RSS feed from ${location.href}`)

  location.href = url;
})();


Currency converter in Python I am looking for some ways to make my code simpler for this currency converter.

currencies = {
    "BRL":"(Brazilian Real)",
    "USD":"(US Dollar)",
    "EUR":"(Euro)",
    "GBP":"(Great Britain Pound)"
}

def real_to_dollar():
    user_value = raw_input("How many reais? ")
    ammount = float(user_value)
    conversion = ammount * 0.26
    print str(user_value) + " reais is equal to " + str(conversion) + " dollars."

def dollar_to_real():
    pass

def dollar_to_euro():
    pass

def euro_to_dollar():
    pass

def euro_to_pound():
    pass

def pound_to_euro():
    pass

def real_to_euro():
    pass

def euro_to_real():
    pass

def real_to_pound():
    pass

def pound_to_real():
    pass

def dollar_to_pound():
    pass

def pound_to_dollar():
    pass

print "Welcome to currency converter."
print "Supported currencies:"
for currency in currencies:
    print currency + " " + currencies[currency]

user_choice1 = raw_input("Convert: ")
user_choice2 = raw_input("To: ")

if user_choice1 == "BRL" and user_choice2 == "USD":
    real_to_dollar()

elif user_choice1 == "USD" and user_choice2 == "BRL":
    dollar_to_real()

You might not have realised it yet, but all your unimplemented methods will very much look like real_to_dollar:

ask the user how much of the original currency he wants converted;
apply a specific conversion rate;
tell the user the result of the conversion in the desired currency.

So that should be a single function, parametrized by the elements that differ between various calls: name of the origin currency, name of the converted currency, exchange rate.

def convert_currency(from, to, rate):
    user_value = raw_input("How many {}? ".format(from))
    ammount = float(user_value)
    conversion = ammount * rate
    print " {} {} is equal to {} {}.".format(user_value, from, conversion, to)

Also note the use of format to simplify building a string with numbers.

You would then call it convert_currency("reais", "dollars", 0.26) or convert_currency(" dollars", "reias", 3.85).

However, having to use a bunch of ifs to know what parameters to use (like you would have used a bunch of ifs to know which function to call) is not that great and can be a burden to maintain and expand. You could use a second dictionnary to map couples of currencies to exchange rates:

RATES = {
    ("BRL", " USD"): 0.26,
    ("USD", "BRL"): 3.85,
    # Expand as necessary
}

And just have to get the rate using RATES[(user_choice1, user_choice2)].


Macro for rounding with variable precision I use the following MY_ROUND macro for rounding:

#define MY_SIGN(inVal) (((inVal)>=0)? 1 : -1)

#define MY_ROUND(inVal,numDig) (((double)(int)((inVal)*pow((double)10,numDig)+MY_SIGN(inVal)*0.5))/pow((double)10,numDig))

Here inVal is the input double to be rounded and numDig is the number of digits (precision) to round for.

Due to the divisions and pow()'s this function is quite slow. Is there any better replacement for this which does not waste that much time?

I'd go with an inline function, as @pacmaninbw said, instead of a macro. It's more readable and it's easier to notice things that you may want to optimize.
A couple of things that may speed this up a bit if you use a function:

It would be easier to store the result of pow((double)10,numDig) in a variable, so you can compute that only once (you're doing it twice, now). While you're at it, you may think about pre-computing a few values, say up to 10^26 and use a lookup table. I'm not really sure that it'd be faster for such a simple operation, though.
The MY_SIGN macro only returns -1 or 1, then you multiply the result by 0.5. These are hard-coded values, so why not having something that returns -0.5 or 0.5 instead ?


Circle Shadow Experiment I'm trying to create a light system, with the midpoint algorithm and Bresenham Line Algorithm. I'm trying to redraw all the time (1 option) or redraw when I need to.  When the circles are too big it becomes a bit laggy.

Example

function Light(x, y, range)
{
    this.CordX = x;
    this.CordY = y;
    this.LuminosityRange = range;
}
   setInterval(function(){
        stats.begin(); // Begin to count FPS
        ctx.clearRect(0, 0, Width, Height); // Clear Canvas

        // STEP 1 -- DRAW THE BACKGROUND (BLACK)
        for(var y = 0; y < Height; y++)
        {
            for(var x = 0; x < Width; x++)
            {
                var index = x + (y * Width),
                    indexColor = index * 4;

                shadowBitMapData[indexColor + 0] = shadowBitMapData[indexColor + 1] = shadowBitMapData[indexColor + 2] = 0;
                shadowBitMapData[indexColor + 3] = 255;
            }
        }

        // SETEP 2 -- DRAW THE LIGHT
        for(var light = 0; light < lamps.length; light++)
        {
            var currentLight = lamps[light];
            var xCL = ~~currentLight.CordX,
                yCL = ~~currentLight.CordY,
                iCL = xCL + (yCL * Width),
                rCL = ~~currentLight.LuminosityRange;

            // MidPoint Algorithm
            var xMP = rCL,
                yMP = 0,
                rEMP = 1 - xMP;

            while(xMP >= yMP)
            {

                var x1 = xMP + xCL,
                    x2 = yMP + xCL,
                    x3 = -xMP + xCL,
                    x4 = -yMP + xCL;

                var y1 = yMP + yCL,
                    y2 = xMP + yCL,
                    y3 = -yMP + yCL,
                    y4 = -xMP + yCL;

                var index1 = x1 + (y1 * Width);
                var index2 = x2 + (y2 * Width);

                var index3 = x3 + (y1 * Width);
                var index4 = x4 + (y2 * Width);

                var index5 = x3 + (y3 * Width);
                var index6 = x4 + (y4 * Width);

                var index7 = x1 + (y3 * Width);
                var index8 = x2 + (y4 * Width);

                // Fill Top
                //shadowBitMapData[index6 * 4 + 3] = 0;
                //shadowBitMapData[index8 * 4 + 3] = 0;
                BresenhamLine(x4, y4, x2, y4, shadowBitMapData, Width);

                // Fill Mid Top
                //shadowBitMapData[index5 * 4 + 3] = 0;
                //shadowBitMapData[index7 * 4 + 3] = 0;
                BresenhamLine(x3, y3, x1, y3, shadowBitMapData, Width);

                // Fill Mid Bottom
                //shadowBitMapData[index1 * 4 + 3] = 0;
                //shadowBitMapData[index3 * 4 + 3] = 0;
                BresenhamLine(x1, y1, x3, y1, shadowBitMapData, Width);

                // Fill Bottom
                //shadowBitMapData[index2 * 4 + 3] = 0;
                //shadowBitMapData[index4 * 4 + 3] = 0;
                BresenhamLine(x2, y2, x4, y2, shadowBitMapData, Width);

                yMP++;

                if (rEMP < 0)
                {
                    rEMP += 2 * yMP + 1;
                }
                else
                {
                    xMP--;
                    rEMP += 2 * (yMP - xMP + 1);
                }
            }
        }

        ctx.putImageData(shadowBitMap, 0, 0); // Draw Shadow
        stats.end(); // End to count FPS
    }, 1000 / 60);

function BresenhamLine(x0, y0, x1, y1, bitmap, Width)
{
    var dx = Math.abs(x0 - x1);
    var dy = Math.abs(y0 - y1);

    var sx = x1 < x0 ? 1 : -1;
    var sy = y1 < y0 ? 1 : -1;

    var err = dx - dy;
    var e2;

    var lm = 0;
    while (true)
    {
        bitmap[(x1 + y1 * Width) * 4 + 3] = 0;

        if (x1 == x0 && y1 == y0) break;

        e2 = 2 * err;
        if (e2 > -dy)
        {
            err = err - dy;
            x1 = x1 + sx;
        }

        if (e2 < dx)
        {
            err = err + dx;
            y1 = y1 + sy;
        }
    }
}

Don't Reinvent the Wheel

In your "STEP 1", you are manually setting each pixel of the background to black. This is expensive and unnecessary. You can simply draw a black rectangle the size of the canvas:

ctx.fillStyle = "rgb(0, 0, 0)";
ctx.fillRect(0, 0, Width, Height);

Next, in "STEP 2" why are you using your own line drawing routines? You could simply call the built-in routines, like:

ctx.beginPath();
ctx.moveTo(x0, y0);
ctx.lineTo(x1, y1);
ctx.stroke();

And to draw circles, you can use the arc() method:

ctx.beginPath();
ctx.arc(centerX, centerY, radius, 0, 2.0 * Math.PI, false);
ctx.fillStyle = "rgb (1, 1, 1);"
ctx.fill();

This will likely be much faster than rolling your own.

Naming

I find your variable names hard to read and understand. Names like xCL, yCL and xMP, yMP are not very descriptive. Maybe centerX, centerY and midPointX and midPointY instead?

Also, iCL seems to be unused. It can be removed.

Abuse of Lambdas

Your entire drawing function (the main purpose of your program) is an anonymous function passed as an argument to another function. It's difficult to follow the flow of it all, and it's odd that the most important thing in your code is a single argument to a function that merely collects stats.

And then you mix in stats collection in the actual function that does the drawing. I'd separate it all out, like this:

function draw()
{
... all the drawing code...
}

function collectDrawStats()
{
    stats.begin();
    draw();
    stats.end();
}

setInterval(collectDrawStats(), 1000 / 60);

Avoid Magic Numbers

What is the 1000 / 60 in the call to setInterval()? Does that have something to do with the refresh rate being 60 fps? You should create a named constant that tells a reader what it means. Is it milliseconds per 60th of a second? If so, what's the significance of that?


Find number matching range I have code that checks the range of a value and sets a variable accordingly.

if ( 169 <= angle && angle < 191 ) {currentOrientation = "1";}
if ( 191 <= angle && angle < 214 ) {currentOrientation = "2";}
if ( 214 <= angle && angle < 236 ) {currentOrientation = "3";}
if ( 236 <= angle && angle < 259 ) {currentOrientation = "4";}
if ( 259 <= angle && angle < 281 ) {currentOrientation = "5";}
if ( 281 <= angle && angle < 304 ) {currentOrientation = "6";}
if ( 304 <= angle && angle < 326 ) {currentOrientation = "7";}
if ( 326 <= angle && angle < 349 ) {currentOrientation = "8";}
if ( 349 <= angle) {currentOrientation = "9";}
if ( angle < 11 ) {currentOrientation = "9";}
if ( 11 <= angle && angle < 34 ) {currentOrientation = "10";}
if ( 34 <= angle && angle < 56 ) {currentOrientation = "11";}
if ( 56 <= angle && angle < 79 ) {currentOrientation = "12";}
if ( 79 <= angle && angle < 101 ) {currentOrientation = "13";}
if ( 101 <= angle && angle < 124 ) {currentOrientation = "14";}
if ( 124 <= angle && angle < 146 ) {currentOrientation = "15";}
if ( 146 <= angle && angle < 169 ) {currentOrientation = "16";}

Each range is 1/16 of 360°. There is a shift so that cardinal values (90,180..) are in the center of the range.

I am curious about how this code can be optimized. Maybe a jQuery function? So far a loop isn't interesting enough if I can't get the values automatically.

This function has very slightly different boundaries, but you may find it does better than what you have right now:

function degreesToOrientation(degrees){
    return 1 + Math.floor(((Number(degrees) + 180 + 11.25) % 360.0) / 22.5);
}

one sixteenth of a circle is 22.5 degrees. centering that is at 11.25 degrees.

So, you add 11.25 degrees to your angle, rotate it a further 180 degrees, Use the Modulo to bring the value back in to range, then divide it by the size of each segment.

This returns a value from 0 through 15 (for the 16 segments). So, add 1 to get your 'orientation'.

It does not care about the degree amount as long as it is greater than -191.25 ... to start with... (the modulo operation is funny with negative numbers...)

Here is a jfiddle


Update single common column in multiple has-many relationships rails I want to update a single column in 4 tables. All relationships are has-many among them.

Private schools has many private classes
Private classes has many lesson plans
Lesson plans has many quizzes
Lesson plans has many documents

The code I am working with is:

@private_school.private_classes.each do |private_class|
            private_class.lesson_plans.each do |lesson_plan|
              lesson_plan.update_attribute("price_type", @private_school.price_type)
              lesson_plan.quizzes.update_attribute("price_type", @private_school.price_type)
              lesson_plan.documents.update_attribute("price_type", @private_school.price_type)
            end
          end

Is this a best approach to follow or is there any more option to tackle this? What are best practices about this scenario?

(Converted a comment into this answer)

If all the records should use @private_school.price_type as their own price_type, then make them fetch it from the school they belong to - don't duplicate the same value across a bunch of tables when you already have it in one place. Keep the database normalized.

Simplest way to do this is to add a price_type method to each of the non-school models. The method simply fetches and returns <belongs_to relation>.price_type. I.e. Document#price_type would call its owner's method, which would be LessonPlan#price_type, which would call it's owner, etc.. until it hits the school's price_type attribute.

It's not the most efficient as it may need to load some records in the chain, but it's a start.


REST-API 3rd party resource model class I'm working on an integration for Magento 2 with a 3rd party. This class will fetch the articledata resource. This resource contains multiple items, with a maximum of 750 per call. The class is designed to be used like do load() while next().

If there are more items than the current 750, there will be a value next_resultset. For example /app/api/v3/articledata/750-1-36/. I only need the last part 750-1-36/ to append it to the resource.

This class works fine, but I'm not happy with how all the logic is being handled. Part of me tells that the API I'm using is not designed so well and another part of me tells that the code could be better regardless.

<?php

class ArticleData
{
    const RESULT_SET_LIMIT = 750;

    /**
     * @var RestClient
     */
    protected $client;

    /**
     * @var ConfigProviderFactory
     */
    protected $configProviderFactory;

    /**
     * @var array
     */
    protected $data = [];

    /**
     * @var \Magento\Framework\Json\Decoder
     */
    protected $jsonDecoder;

    /**
     * ArticleData constructor.
     * @param ConfigProviderFactory $configProviderFactory
     * @param RestClient $client
     * @param \Magento\Framework\Json\Decoder $jsonDecoder
     */
    public function __construct(
        ConfigProviderFactory $configProviderFactory,
        RestClient $client,
        \Magento\Framework\Json\Decoder $jsonDecoder
    ) {
        $this->configProviderFactory = $configProviderFactory;
        $this->client = $client;
        $this->jsonDecoder = $jsonDecoder;
    }

    /**
     * Load first set
     */
    public function load()
    {
        $resultSet = '';
        if ($this->next()) {
            $resultSet = $this->getResultSet();
        }

        $response = $this->client->get('articledata/' . $resultSet);

        $decoded = $this->jsonDecoder->decode($response->getBody());
        $decoded['items'] = $decoded['data'];
        unset($decoded['data']);

        $this->data = $decoded;
    }

    /**
     * @return bool
     */
    public function next(): bool
    {
        return !empty($this->data['results']) && $this->data['results'] >= self::RESULT_SET_LIMIT
            && !empty($this->data['next_resultset']);
    }

    /**
     * @return string
     */
    public function getResultSet(): string
    {
        $matches = [];
        return preg_match('/articledata\/([0-9\-]+\/)/', $this->data['next_resultset'], $matches) ? $matches[1] : '';
    }
}

Where is your namespace?  If your are working in code bases that use namespaces, like Magento, you really should be namespacing your code as well.
The interface presented seems confusing to me.  Is it your expectation that the caller controls pagination through the result set?  If so, then how does the caller actually work with intermediate result sets, given that there is no interface to return the data set to the caller and your data in $this->data are overwritten with every iteration of load()?  How is caller supposed to assemble the results?  Also I would expect method names like getResultSet() to, you know, actually return a result set to the caller, not a string format used for pagination.  Why would this method need a public interface anyway?
I think you need make the determination as to whether this class should abstract away pagination activity from caller or not.  If you want the caller to have full control of pagination (and potentially assembling result sets based on pagination results), then I would think your class would need an interface that a) is more explicit around result set size and page numbers to be used b) actually return result set data to the caller.  This class likely does not need to maintain state the way you do now if you want caller to own pagination control.
You have a data mapping step in your load() function, where you change key for data set from data to items. I would suggest that this mapping function should exist in a separate method. In the class, so you can keep you mapping logic separate from your loading logic.


Significant Figures Formatting.00 I wrote this for an SO question on formatting numbers to specific quantities of significant figures (after all, reinventing-the-wheel), anyway, all critiques welcome.

It's pretty easy to use, FormatSignificantFigures(0.123, 2) should return 0.12; FormatSignificantFigures(0.123, 5) should return 0.12300; FormatSignificantFigures(0.123456, 5) should return 0.12346, etc.

public static string FormatSignificantFigures(double number, int figures)
{
    int e = 0;

    while (number >= 10.0)
    {
        e += 1;
        number /= 10;
    }

    while (number < 1.0)
    {
        e -= 1;
        number *= 10;
    }

    figures--;

    number = Math.Round(number, figures);

    figures += 0 - e;
    while (e > 0)
    {
        number *= 10;
        e -= 1;
    }

    while (e < 0)
    {
        number /= 10;
        e += 1;
    }

    if (figures < 0)
    {
        figures = 0;
    }

    return number.ToString($"f{figures}");
}

int e = 0;

You're initializing e to default(int), which is redundant. This would be enough:

int e;

I'm not quite sure why you're incrementing e like this:

while (number >= 10.0)
{
    e += 1;
    number /= 10;
}

Why not just e++;?

And same thing here:

while (e > 0)
{
    number *= 10;
    e -= 1;
}

There's e--; to decrement.

But you know this already:

figures--;

Why? Why are you using both -= 1 and -- in the same method?

figures += 0 - e;

Looks like a pretty convoluted way to do figures -= e;.

And then we get to the return value...

return number.ToString($"f{figures}");

You're using C# 6.0 string interpolation here. A format string. The SO question you linked to has an accepted answer that uses a format string to do all the work: it seems string.Format("{0:G10}", number) would do the formatting part just right - I'd parameterize the length by making a method that builds a format string from the parameter (something like "{0:G" + digits + "}"), and uses that format string to return the formatted string.

I think it would be easier to read and to follow, than the whole logic you got there.

Or, don't use a format string. IMO using a formatted string here defeats the purpose of reinventing the wheel.


Find Row Pairs that are never both 1 Given sample_size many arrays of length dim. For each index from 1 to dim, i want to give a list of indices, where no sample contained in both indexes 1s. The code is really slow and i want to improve the performance.

# Test set

dim = 10000
sample_size = 1000

from random import *

randBinList = lambda n: [randint(0,1) for b in range(1,n+1)]

samples = [randBinList(dim) for s in xrange(sample_size)]

# Find all variables that are not both 1 in one sample

node_neighbours = {}
for node in xrange(1,dim+1):
    node_neighbours[node] = []
    for node2 in xrange(1,dim+1):
        if node != node2:
            test_failed = False
            for s in samples:
                v1 = s[node-1]
                v2 = s[node2-1]
                if v1 == 1 and v2 == 1:
                    test_failed = True
                    break
            if not test_failed:
                node_neighbours[node].append(node2)
    #print node, ":", node_neighbours[node]

First off, I don't understand why you so much time dealing with off-by-one indices that you created in the first place:

for node in xrange(1,dim+1):
    for node2 in xrange(1,dim+1):
        for s in samples:
            v1 = s[node-1]
            v2 = s[node2-1]
            if v1 == 1 and v2 == 1:

You could simply write xrange(dim) and s[node] and s[node2]. If you want a 1-based indexing when storing the results, you can still use node + 1 and node2 + 1 when appending the results.

You can also take advantage of the fact that node being a neighbour of node2 will imply that node2 will be a neighbour of node and cut down half of your computations (and a test as well):

for node in xrange(dim):
    for node2 in xrange(node + 1, dim):
        test_failed = False
        for s in samples:
            v1 = s[node]
            v2 = s[node2]
            if v1 == 1 and v2 == 1:
                test_failed = True
                break
        if not test_failed:
            node_neighbours[node + 1].append(node2 + 1)
            node_neighbours[node2 + 1].append(node + 1)

Other tiny improvements includes removing the test_failed flag by using the for ... else construct and testing equality with the extended comparisons:

for node in xrange(dim):
    for node2 in xrange(node + 1, dim):
        for s in samples:
            v1 = s[node]
            v2 = s[node2]
            if v1 == v2 == 1:
                break
        else:
            node_neighbours[node + 1].append(node2 + 1)
            node_neighbours[node2 + 1].append(node + 1)

But a bigger improvement can be achieved using the C core of Python instead of these 2 for loops. Because they perform exactly what itertools.combinations does:

for node1, node2 in itertools.combinations(xrange(dim), 2):
    for s in samples:
        if s[node1] == s[node2] == 1:
            break
    else:
        node_neighbours[node1 + 1].append(node2 + 1)
        node_neighbours[node2 + 1].append(node1 + 1)

Also note that all the provided snippets seems to take for granted that lists for each nodes has already been populated in node_neighbours. This is easily achieved using collections.defaultdict as such:

node_neighbours = defaultdict(list)

You should also wrap this code into a function to better separate the "initialization" or generation of test data from the actual computation. And testing this function should be done in an if __name__ == '__main__': clause:

import itertools
from collections import defaultdict

def pairs_never_both_one(samples, dim=None):
    if dim is None:
        dim = len(samples[0])

    node_neighbours = defaultdict(list)
    for node1, node2 in itertools.combinations(xrange(dim), 2):
        for s in samples:
            if s[node1] == s[node2] == 1:
                break
        else:
            node_neighbours[node1 + 1].append(node2 + 1)
            node_neighbours[node2 + 1].append(node1 + 1)

    return node_neighbours

if __name__ == '__main__':
    from random import randint

    dim = 10000
    sample_size = 1000
    samples = [[randint(0, 1) for _ in xrange(dim)] for _ in xrange(sample_size)]
    print pairs_never_both_one(samples, dim)


Library to get entropy of files This is a simple library I just finished writing. The main function is entropy(), which returns the entropy of a file. I'd really appreciate suggestions to improve it.

caos.c:

#include <stdio.h>
#include <math.h>
#include <unistd.h>
#include <sys/stat.h>
#include "caos.h"

static unsigned long long int wc(const char * file_to_size);
static int get_times(double table_of_chars[ASCII], const char * file);
static int calc_frequencies(double table_of_chars[ASCII], const char * file);

double entropy(const char * file)
{
  int iter;
  double entropy = (double)INIT;
  double table_of_chars[ASCII] = {INIT};
  if (get_times(table_of_chars, file) == EREAD)
  {
    return EREAD;
  }

  if (calc_frequencies(table_of_chars, file) == EREAD)
  {
    return EREAD;
  }

  for (iter = INIT; iter < ASCII; iter++)
  {
    if (table_of_chars[iter] != INIT)
    {
      entropy -= (table_of_chars[iter] * log2(table_of_chars[iter]));
    }
  }

  return fabs(entropy);
}

static int get_times(double table_of_chars[ASCII], const char * file)
{
  int getch;
  if (access(file, F_OK) != EREAD)
  {
    FILE * fget = fopen(file, READ);
    while ((getch = getc(fget)) != EOF)
    {
      table_of_chars[getch]++;
    }
    fclose(fget);
    return OK;
  }

  return EREAD;
}

static int calc_frequencies(double table_of_chars[ASCII], const char * file)
{
  int iter;
  unsigned long long int file_size = wc(file);
  if (file_size != (unsigned)EREAD)
 {
    for (iter = INIT; iter < ASCII; iter++)
    {
      if (table_of_chars[iter] != INIT)
      table_of_chars[iter] = (double)(table_of_chars[iter] / file_size);
    }

   return OK;
 }

 return EREAD;
}

static unsigned long long int wc(const char * file)
{
  if (access(file, F_OK) != EREAD)
  {
    struct stat st;
    if (stat(file, &st) == OK)
    {
      unsigned long long int bytes = st.st_size;
      return bytes;
    }
  }

  return EREAD;
}

caos.h:

#define   ASCII         256
#define   INIT           0

#define   EREAD         -1
#define   READ          "r"
#define   OK             0

double entropy(const char * path_to_file);

At first glance, I thought, WOW, Nice library!
Decent variable and function names, decent indentation. Some application of
good software engineering principle such as the Single Responsibility Principle and the KISS principle.

On further inspection I found a few issues:

Error handling Versus Error Reporting
The code does handle errors and return a status of the function, but there
is no error reporting. Standard C library functions such as access(),
fopen() and stat() return the actual error code in the global
variable errno. By including errno.h you have access to the error
codes and can report what is causing the error to the user/programmer
using the entropy library using perror() or stderror(). There is also
a discussion of this on stackoverflow.com. I know that if I was using
this library I would like to know whether a file is not found, or I
don't have read permission on the file.

A common practice when returning status of a function
such as entropy(), get_times() or calc_frequencies()
is to include stdlib.h and return either EXIT_SUCCESS or EXIT_FAILURE
instead of using OK or EREAD.

Direct Addressing Versus Indirect Addressing
One fairly common optimization in C is to use pointers (Direct Addressing)
instead of indexes (Indirect Addressing) when accessing the contents
of arrays. Direct Addressing is faster than Indirect Addressing. In some
cases using the highest level of optimization when compiling will convert
indirect addressing to direct addressing.

In most of the for loops in the library the code is using indirect
addressing:

    for (iter = INIT; iter < ASCII; iter++)
    {
      if (table_of_chars[iter] != INIT)
      table_of_chars[iter] = (double)(table_of_chars[iter] / file_size);
    }

Performance may be improved by:

    double *toc_p = &table_of_chars[0];
    for (iter = INIT; iter < ASCII; iter++, toc_p++)
    {
      if (*toc_p != INIT)
      *toc_p = (double)(*toc_p / file_size);
    }

An enhanced optimization would be:

static int calc_frequencies(double table_of_chars[ASCII], const char * file)
{
  unsigned long long int file_size = wc(file);
  if (file_size != (unsigned)EREAD)
  {
    register int iter;
    register double *toc_p = &table_of_chars[0];
    for (iter = INIT; iter < ASCII; iter++, toc_p++)
    {
      if (*toc_p != INIT)
      *toc_p = (double)(*toc_p / file_size);
    }

   return OK;
  }

  return EREAD;
}

Improve Type Safety
The library as written may insert floating point errors in the following
code:

    while ((getch = getc(fget)) != EOF)
    {
      table_of_chars[getch]++;
    }

It is somewhat rare to see the increment a float or double value in C.

I would suggest that table_of_chars be a table of integers rather than
doubles. A second array of doubles could be returned by calc_frequencies().

Variable and Function Names
In some languages, such as Pascal the name of the function is
assigned the return value. This is not the case in C, I would
differentiate between a local variable name and the function
name such as using entropy_val as the return value for the
entropy() function. Some C compilers may have problems with
the current implementation.

The function get_times() might be better named as
count_all_character_instances()

On the Unix and Linux operating systems, wc stands for word count, and
returns the file size in lines, words and characters, a more appropriate
name for the function wc() might be get_file_size_in_bytes().

Variable and Function Names
In some languages, such as Pascal the name of the function is
assigned the return value. This is not the case in C, I would
differentiate between a local variable name and the function
name such as using entropy_val as the return value for the
entropy() function. Some C compilers may have problems with
the current implementation.

The function get_times() might be better named as
count_all_character_instances()

On the Unix and Linux operating systems, wc stands for word count, and
returns the file size in lines, words and characters, a more appropriate
name for the function wc() might be get_file_size_in_bytes().

DRY Code, or the Don't Repeat Yourself Principle
Another software engineering principle is the Don't Repeat Yourself Principle.
Most times when you have repetitive code create you should create a function
that contains just that code so that the code is reusable.

Two functions in the code get_times() and wc() contain

  if (access(file, F_OK) != EREAD)
  {
  }

this is repetitive code.

In addition to repeating, system calls such as
access(), fopen(), stat() and fclose() are expensive time wise,
quite often programs or libraries will be swapped out during these
calls. In the execution of this library it is unlikely that the status of
access() will change between calls. For this library it would be
advisable to call these functions only once.

Portability
Back in the old days ASCII was the primary character set on Unix and Linux.
Today the ASCII character set does not support some languages such as Chinese
and Japanese which require wider characters. This library will work for most
western cultures, but it will not support all languages. Some languages
require two bytes per character, rather than one. This is something to
think about for later.

Function Complexity
Both the function get_times() and calc_frequencies() are too
complex, because the second parameter is the file name rather than
a file pointer. File access really only needs to be checked once,
and the file only needs to be opened once. The function entropy()
should call the function wc() directly. The input file should only
be opened and read after stat has been performed.


SICP Exercise 1.3: Sum of squares of two largest numbers out of three, Prolog Version The exercise 1.3 of the book Structure and Interpretation of Computer Programs asks the following:

  Exercise 1.3.  Define a procedure that takes three numbers as arguments and returns the sum of the squares of the two larger numbers.

My answer is this:

squareTwoLargest(X, Y, Z, R) :-
    ( X > Y -> A = X, B = Y ; A = Y, B = X ),
    R is A ^ 2 + max(B, Z) ^ 2.

My reason for learning Prolog is to understand the logic programming paradigm. So, how can I solve this exercise in good logic programming style?

The problem, as stated, has a completely straight-forward solution, because the problem statement is limited to numbers:

sicp1(X, Y, Z, SumSquares) :-
        msort([X,Y,Z], [_,A,B]),
        SumSquares is A^2 + B^2.

You can use this predicate every time the three numbers are fully specified, for example:

?- sicp1(1, 2, 3, S).
S = 13.

The problem though is that this code is not as general as it could be. It does not work for example if the arguments are variables:

?- sicp1(X, Y, Z, S).
ERROR: is/2: Arguments are not sufficiently instantiated

From a truly declarative solution, we expect valid answers even if arguments are only partially known.

Enter constraints. I show you a solution with CLP(FD) constraints. This means that the arguments are elements of the set of integers. Check out other constraint libraries like CLP(Q) to get solutions over different domains.

Working solution for SICStus, SWI etc.:

:- use_module(library(clpfd)).

sicp2(X, Y, Z, SumSquares) :-
        Max #= max(max(X, Y), Z),
        Median #= max(min(X, Y), min(max(X, Y), Z)),
        SumSquares #= Max^2 + Median^2.

Now, we can use this just like before:

?- sicp2(1, 2, 3, S).
S = 13.

However, we can also use it in other ways. For example:

?- sicp2(X, 3, 5, S).
...
S in 34..sup.

From this answer, you see that the sum of squares will be at least 34, no matter the eventual value of X.

When giving further constraints, you get more precise answers:

?- sicp2(X, 3, 5, S), X #< 4.
S = 34,
... .

This means that, if it is known that X is less than 4, then the result is certainly 34 in this case.

Thus, the solution using constraints is much more general, and it is just what we would expect from a truly declarative solution that can be queried in all directions and gives useful answers in all cases.


Matrix Multiplication in Typescript I would like some help on making my solution more functional. At present, I rely heavily on using indexes in my map functions. I am also seeking general feedback and advice, thanks.

export const getMatrixColumns = (a: number[][]) => {
  return a[0].length;
};

export const getMatrixRows = (a: number[][]) => {
  return a.length;
};

export const matricesAreValid = (a: number[][], b: number[][]) => {
  return getMatrixColumns(a) === getMatrixRows(b);
};

export const generateEmptyMatrix = (rows: number, cols: number) => {
  return [...Array(rows)].fill(0).map(() => [...Array(cols)].fill(0));
};

export const dotProduct = (a: number[], b: number[]) => {
  return a.map((value, i) => value * b[i]).reduce((acc, val) => acc + val, 0);
};
export const multiplyMatrices = (a: number[][], b: number[][]) => {
  if (matricesAreValid(a, b)) {
    const rows = getMatrixRows(a);
    const cols = getMatrixColumns(b);
    return generateEmptyMatrix(rows, cols).map((resultRow, i) =>
      resultRow.map((element: number, j: number) => {
        const column = b.map(row => row[j]);
        const row = a[i];
        return dotProduct(row, column);
      })
    );
  }
};

Concise Function Body

The function body of an arrow function can be concise.

For instance can getMatrixColumns shorten to

export const getMatrixColumns = (a: number[][]) => a[0].length;

Method Name

The method names getMatrixColumns and getMatrixRows let me expect that the the methods return a number[] instead of a number.

A better fit are getMatrixColumnLength and getMatrixRowLength.

Redundant Method Names

Each method name expect dotProduct contains some how the word matrix.

It is sufficient if the methods are grouped in a Matrix module and have names like:

// in matrix.js

export const columnLenghtOf = (a: number[][]) =>
    a[0].length;

export const rowLengthOf = (a: number[][]) =>
    a.length;

export const areValid = (a: number[][], b: number[][]) =>
    columnLenghtOf(a) === columnLenghtOf(b);

/* ... */

Type Alias

Matrix could be a type alias for number[][]:

// in matrix.js

type Matrix = number[][]

export const columnLenghtOf = (a: Matrix) =>
    a[0].length;
```


JavaScript tournament scheduling (fixture generation) algorithm I have just developed a match scheduling algorithm and had to use some pretty funny modular arithmetic. While it works fine i would like to know if my calculations could be simplified any further.

Though it looks a little complicated it's in fact very simple and seems to work very efficiently.

A little bit explanation on my approach;

So for starters, i have 10 teams to match up. Each team will play 9 matches in a fixture such that all teams play with a separate competition at every round.

Accordingly i have an array of team objects as follows;

var teams = [{id: team_0, matches : []}, ... ,{id: team_9, matches: []}]

I would like to show the general layout of the fixture in the table below;

Where mc is the match count, which is 9 in this example. The items with green background are the ones we calculate by iterating over and the red foreground ones are calculated automatically from the iterated ones. Dark green background is the starting index for insertion of the competitors into the matches array.

So for T0 we start with an empty 9 item matches array to be filled with the competitors. However our first competitor will be the last one and then we will continue from the first one like T9 -> T1 -> T2... -> T8. As I have experienced this odd initial ordering results simpler code but i am open to suggestions.

While we are filling the matches array of T0 with a value such as T9 (T0 line first item with dark green background) we also fill same index of the matches array of T9 with T0. (T9 line first item in red)

When we finish T0's matches and come to T1, we already have one item of T1's matches filled while we were calculating the T0 matches. (row T1 column 1, T0 in red) So we start after that one again with T9 and continue with T2 (not T1, since T1 can not play with itself) up until we reach T8. This is where the modulus arithmetic starts showing itself by assigning T8 at index 0.

So this is why I had to come up with some funny modular arithmetic;

(2*i+j) % mc: Is the calculated index to start inserting results
from T9 on.
(j+mc-i-1) % (mc-i)+i+1: Is the calculated index to pick
the competitor from the teams array.

Again i can not make exactly sure if they can be simplified any further but they seem perfectly functional as they are. This is where i need your help.

So the code is;

function fixture(ts){
  var mc = ts.length-1,  // match count is one less than the total number of teams.
      sp = 0,            // calculated start position to insert matches in the matches array.
      tm = 0;            // competing team's calculated index at the input (ts) array.
  return ts.map((t,i,a) => (a.slice(i+1)
                             .forEach(function(_,j,b){
                                        sp = (2*i+j) % mc;
                                        tm = (j+mc-i-1) % (mc-i)+i+1;
                                        t.matches[sp] = a[tm].id;
                                        a[tm].matches[sp] = t.id;
                                      })
                           ,t));
}

var tc = 10,
 teams = Array(tc).fill().map((_,i) => ({id: "Team_"+i, matches: Array(tc-1)}));
result = fixture(teams);
console.log(teams);
.as-console-wrapper { max-height: 100% !important; top: 0; }

Advises to further simplify this code are most welcome.

I have 10 teams to match up. Each team will play 9 matches in a fixture such that all teams play with a separate competition at every round.

Let's build a schedule for 10 teams. I intuitively came up with the following circulant 10x10 matrix A with entry A[i, j] denoting the opponent of team i in round j:

Each row represents the schedule for a single team. We note that for each round j teams are partitioned into pairs playing against each other - i.e. A[A[i, j], j] = i.

However, there is a flaw: According to above schedule, there is a round j for each team where it has to play against itself (the red squares) - i.e. A[i, j] = i.

That's because a correct schedule for 10 teams can only have 9 rounds, not 10. To fix this, we first slice the main diagonal of A - all matches against team 9 - and end up with:

Now, we re-introduce the matches against team 9 wherever a team would have to play against itself (the red squares). We also have to fix the opponents of team 9 to reflect these changes. This practically means that for each round j we swap A[9, j] with A[i, j] where i denotes the self-matched team (the red square):

This is the resulting correct schedule with all swapped matches highlighted in red. This schedule is identical to the one you compute and pretty simple to generate:

Create a circulant 10 x 9 matrix where each row is a circular shift of [0,1,2,3,4,5,6,7,8].
For each round or column, replace the self-match with a match against team 9 and let team 9 play against the self-matched team.

While there are exactly 945 different ways to partition 10 teams into pairs and thus many more ways to generate a correct schedule, I could not come up with a simpler or more intuitive schedule.

  While it works fine i would like to know if my calculations could be
  simplified any further.

I think that by closely following the above recipe, you can avoid the more obscure arithmetic and come up with a simpler and more intuitive 'generative' algorithm. The scheduling is encapsulated in the round(n, j) function:

// Schedule single round `j` for 'n' teams:
function round(n, j) {
  let m = n - 1;
  let round = Array.from({length: n}, (_, i) => (m + j - i) % m); // circular shift
  round[round[m] = j * (n >> 1) % m] = m; // swapping self-match
  return round;
}

// Schedule matches of 'n' teams:
function fixture(n) {
  let rounds = Array.from({length: n - 1}, (_, j) => round(n, j));
  return Array.from({length: n}, (_, i) => ({
    id: "Team_" + i,
    matches: rounds.map(round => "Team_" + round[i])
  }));
}

// Example:
console.log(fixture(10));