Client for many similar kinds of REST requests I have a class that makes calls to a Web API (Using RestSharp), which  works fine but the code is super ugly. What would be the best way to refactor it? I thought of just doing a Facade pattern so all the calls that goes to cart will be in a separate class and order will be in an order class and so on.

All the methods looks very similar maybe I can extract something.

 public class ApiRestClient : IApiRestClient
    {

        private readonly RestClient _client;
        private readonly string _url = ConfigurationManager.AppSettings["webapibaseurl"];

        public ApiRestClient()
        {
            _client = new RestClient(_url);
        }

        public TokenDto Get(Guid id)
        {
            var request = new RestRequest("/cart/{id}", Method.GET) {RequestFormat = DataFormat.Json};
            request.AddParameter("id", id, ParameterType.UrlSegment);

            var response = _client.Execute(request);
            return JsonConvert.DeserializeObject<TokenDto>(response.Content);
        }

        public void SaveOrder(OrderDto order)
        {
            var request = new RestRequest("/order/", Method.POST) {RequestFormat = DataFormat.Json};
            request.AddObject(order);
            _client.Execute<TokenDto>(request);
        }

        public string GetLayout(int? id)
        {
            var request = new RestRequest("/customer/{id}", Method.GET) {RequestFormat = DataFormat.Json};
            request.AddParameter("id", id, ParameterType.UrlSegment);
            var response = _client.Execute(request);
            return JsonConvert.DeserializeObject<CustomerDto>(response.Content).Layout;
        }

        public void UpdateUser(UserDto userMap)
        {
            var request = new RestRequest("/user/", Method.POST) {RequestFormat = DataFormat.Json};
            request.AddObject(userMap);
            _client.Execute<UserDto>(request);
        }

        public IEnumerable<TokenDto> GetInvoices(Guid id)
        {
            var request = new RestRequest("/receipt/{id}", Method.GET) {RequestFormat = DataFormat.Json};
            request.AddParameter("id", id, ParameterType.UrlSegment);

            var response = _client.Execute(request);
            return JsonConvert.DeserializeObject<IEnumerable<TokenDto>>(response.Content);
        }

        public TokenDto GetInvoice(Guid id)
        {
            var request = new RestRequest("/receipt/invoice/{id}", Method.GET) {RequestFormat = DataFormat.Json};
            request.AddParameter("id", id, ParameterType.UrlSegment);

            var response = _client.Execute(request);
            return JsonConvert.DeserializeObject<TokenDto>(response.Content);
        }
    }

I don't see a reason for having a readonly _url in this class. In addition I don't see a reason to have a class level _url variable neither.
I would like to suggest having two constructors, one having a string url argument and one being argumentless. In this way it is easy without changing the AppSettings to have some flexibility.

Applying this would look like so

private readonly RestClient _client;

public ApiRestClient()
    : this(ConfigurationManager.AppSettings["webapibaseurl"])
{ }

public ApiRestClient(string url)
{
    _client = new RestClient(url);
}

It is not obvious from its name what the method public TokenDto Get(Guid id) will get. It seems from the resource that this would be a cart. Having a methodname, which makes it clear what the method is about, would be much better.

The GetLayout() method should be splitted into a GetCustomer() method (which you may need anyway) to only take the Layout of that returned CustomerDto.

The default Method for RestRequest is Method.Get so you could use the constructor which only takes string resource as a parameter.

The creation of this requests could be extracted to a method which takes a string and an object as parameter. Something along this lines

private RestRequest GetGetRequest(string resource, object value, string name)
{
    var request = new RestRequest(resource) {RequestFormat = DataFormat.Json};

    request.AddParameter(name, value, ParameterType.UrlSegment);

    return request;
}

if you by any chance are using C# 6 you could use the nameof operator like so

private RestRequest GetGetRequest(string resource, object id)
{
    var request = new RestRequest(resource) {RequestFormat = DataFormat.Json};

    request.AddParameter(nameof(id), id, ParameterType.UrlSegment);

    return request;
}

In the same way you could implement a GetPostRequest() method like so

private RestRequest GetPostRequest(string resource, object value)
{
    var request = new RestRequest(resource, Method.POST) { RequestFormat = DataFormat.Json };

    request.AddObject(value);

    return request;
}

You could also add a generic method ProcessRequest<T> like so

private T ProcessRequest<T>(RestRequest request)
{
    var response = _client.Execute(request);
    return JsonConvert.DeserializeObject<T>(response.Content);
}

(I don't really know if I like this).

Applying the mentioned points will lead to

public class ApiRestClient : IApiRestClient
{

    private readonly RestClient _client;

    public ApiRestClient()
        : this(ConfigurationManager.AppSettings["webapibaseurl"])
    { }

    public ApiRestClient(string url)
    {
        _client = new RestClient(url);
    }

    private RestRequest GetGetRequest(string resource, object value, string name)
    {
        var request = new RestRequest(resource) { RequestFormat = DataFormat.Json };

        request.AddParameter(name, value, ParameterType.UrlSegment);

        return request;
    }

    private T ProcessRequest<T>(RestRequest request)
    {
        var response = _client.Execute(request);
        return JsonConvert.DeserializeObject<T>(response.Content);
    }

    public TokenDto GetCart(Guid id)
    {
        var request = GetGetRequest("/cart/{id}", id, "id");

        return ProcessRequest<TokenDto>(request);
    }

    public string GetLayout(int? id)
    {
        var customer = GetCustomer(id);

        return customer.Layout;
    }

    public CustomerDto GetCustomer(int? id)
    {
        var request = GetGetRequest("/customer/{id}", id, "id");

        return ProcessRequest<CustomerDto>(request);
    }

    public IEnumerable<TokenDto> GetInvoices(Guid id)
    {
        var request = GetGetRequest("/receipt/{id}", id, "id");

        return ProcessRequest<IEnumerable<TokenDto>>(request);
    }

    public TokenDto GetInvoice(Guid id)
    {
        var request = GetGetRequest("/receipt/invoice/{id}", id, "id");

        return ProcessRequest<TokenDto>(request);
    }

    private RestRequest GetPostRequest(string resource, object value)
    {
        var request = new RestRequest(resource, Method.POST) { RequestFormat = DataFormat.Json };

        request.AddObject(value);

        return request;
    }

    public void SaveOrder(OrderDto order)
    {
        var request = GetPostRequest("/order/", order);

        _client.Execute<TokenDto>(request);
    }

    public void UpdateUser(UserDto userMap)
    {
        var request = GetPostRequest("/user/", userMap);

        _client.Execute<UserDto>(request);
    }

}


String format function string Format(string format_string, T1 p1, T2 p2, ..., TN pn)

The Format() function takes a copy of format_string and replace occurrences of "___" with the remaining parameters to the function (p1, p2,...).  The first occurrence of "___" will be replaced by p1, the next by p2, and so on.  If there are no occurrences of "___" the remaining parameters will be appended to the string delimited by spaces.  If the parameters are not strings they will be converted to strings with operator<<(ostream,x).  Format will return the modified string.

For example:

Format("The ___ is ___ years", "fox", 8, "old")

evaluates to:

  "The fox is 8 years old"

The implementation follows:

struct FormatEmptyStruct {};

const string FormatPlaceholder("___");

template<class T>
inline FormatEmptyStruct OStreamWriteT(ostringstream& os, const std::string& sFormat, string::size_type& iCurrentPos, const T& t)
{
    auto iNextPos = sFormat.find(FormatPlaceholder, iCurrentPos);

    if (iNextPos == std::string::npos)
    {
        os.write(sFormat.data() + iCurrentPos, sFormat.size() - iCurrentPos);
        iCurrentPos = sFormat.size();

        os << " ";

        os << t;
    }
    else
    {
        os.write(sFormat.data() + iCurrentPos, iNextPos - iCurrentPos);

        os << t;

        iCurrentPos = iNextPos + FormatPlaceholder.size();
    }
    return {};
}

struct EmptyStruct {};

inline string Format() { return ""; }

template <class... Args>
inline string Format(const string& sFormat, const Args&... args)
{
    ostringstream os;
    string::size_type iCurrentPos = 0;

    initializer_list<FormatEmptyStruct>{ OStreamWriteT(os, sFormat, iCurrentPos, args)... };

    if (!sFormat.empty())
        os.write(&sFormat.front() + iCurrentPos, sFormat.size() - iCurrentPos);

    return os.str();
}

I can't fault the code (assuming it works). Looks good.

But what I would point is that to improve flexibility you should probably use numbered replacement sites.

The problem with format strings is that for I18N and L10N the format strings will be pulled out of the source code and placed in a separate resource file for translation. Unfortunately not all languages use the same noun verb ordering so for I18N to work efficiently you need a flexible placement strategy (so that you only need to change the string resource not the code).

English:

// "The disk named MyDisk contains 300 files."
"The disk named %1 contains %2 files" % DiskName % FileCount;

Basque:

// "300 fitxategi ditu, izendatutako MyDisk diskoa."
"%2 fitxategi ditu, izendatutako %1 diskoa." % DiskName % FileCount;

Or in your code:

std::cout << Format(stringresource.get("DiskString"), DiskName, FileCount) << std::endl;


Ugly Numbers: A Rags-to-Riches Story I stumbled upon an unaswered question, which looked like a good fit for a functional programming language.

Here is the problem statement from codeeval:

  UGLY NUMBERS

  CHALLENGE DESCRIPTION:

  Credits: This challenge has appeared in a google competition before.

  Once upon a time in a strange situation, people called a number ugly
  if it was divisible by any of the one-digit primes (2, 3, 5 or 7).
  Thus, 14 is ugly, but 13 is fine. 39 is ugly, but 121 is not. Note
  that 0 is ugly. Also note that negative numbers can also be ugly: -14
  and -39 are examples of such numbers.

  One day on your free time, you are gazing at a string of digits,
  something like:

123456

  You are amused by how many possibilities there are if you are
  allowed to insert plus or minus signs between the digits. For example
  you can make:

1 + 234 - 5 + 6 = 236

  which is ugly. Or

123 + 4 - 56 = 71

  which is not ugly.

  It is easy to count the number of different ways you can play with the
  digits: Between each two adjacent digits you may choose put a plus
  sign, a minus sign, or nothing. Therefore, if you start with \$D\$ digits
  there are \$3^{D-1}\$ expressions you can make. Note that it is fine to
  have leading zeros for a number. If the string is 01023, then
  01023, 0+1-02+3 and 01-023 are legal expressions.

  Your task is simple: Among the \$3^{D-1}\$ expressions, count how many of
  them evaluate to an ugly number.

  INPUT SAMPLE:

  Your program should accept as its first argument a path to a filename.
  Each line in this file is one test case. Each test case will be a
  single line containing a non-empty string of decimal digits. The
  string in each test case will be non-empty and will contain only
  characters '0' through '9'. Each string is no more than 13 characters
  long. E.g.

1
9
011
12345

  OUTPUT SAMPLE:

  Print out the number of expressions that evaluate to an ugly number
  for each test case, each one on a new line. E.g.

0
1
6
64

And here is my solution (ignoring the part about reading from a file)

type Expression =
    | Plus of Expression * Expression
    | Minus of Expression * Expression
    | Leaf of int64

let rec eval = function
    | Plus (left, right) -> (eval left) + (eval right)
    | Minus (left, right) -> (eval left) - (eval right)
    | Leaf n -> n

let rec expressions (s : string) =
    seq {
        yield Leaf (Int64.Parse(s))
        let n = s.Length
        for i in 1 .. n - 1 do
            let leaf = Leaf (Int64.Parse(s.Substring(0, i)))
            for e in expressions(s.Substring(i, n - i)) do
                yield Plus (leaf, e)
                yield Minus (leaf, e)
    }

let rec isUgly (n : int64) : bool =
    if n < 0L then
        isUgly -n
    else
        n = 0L || n % 2L = 0L || n % 3L = 0L || n % 5L = 0L || n % 7L = 0L

let countUglyNumbers (s : string) =
    expressions s |> Seq.map eval |> Seq.filter isUgly |> Seq.length

for input in ["1"; "9"; "011"; "12345"; "123456"; "1234566543215"] do
    printfn "%d" <| countUglyNumbers input

What is the purpose of the Expression type? Why doesn't expressions just return all the possible results?

This might be useful in other situations (like printing all the possibilities), but I don't see a reason for it here. Basically, it's an unnecessary level of abstraction.

if n < 0L then
    isUgly -n
else
    n = 0L || n % 2L = 0L || n % 3L = 0L || n % 5L = 0L || n % 7L = 0L

You don't need the special case for negative numbers, % works for them too.


Flooring cost calculator I've made a small program that calculates the cost of flooring for a given area based on the price per sqft and total sqft. I have only been programming for a few days now so I am sure that there are some things I could be doing better.

from decimal import *
import time

print("This program will help you find the cost of floor covering\n")

def program():
    exit_program = ["q", "Q"]

    global move_on
    move_on = ["y", "Y"]

    global more_info
    more_info = ["n", "N"]

    cost_of_tile = Decimal(input("What is the cost of the tile per square foot?"))

    global answer
    answer = input("Do you know how big the room is in sqft? Y/N")

    if answer in move_on:
        global sqft
        sqft = Decimal(input("Please enter the room's size in SQUARE FEET"))
        print("The cost to cover your floor will be $%s" % (sqft * round(cost_of_tile, 2)))
        labor()
        answer = input("Would you like to run the calculator again for another room? Y/N")
        should_i_stay()
    elif answer in more_info:
        answer = input("Do you know the length and the width of the room? Y/N")

        if answer in move_on:
            width = Decimal(input("What is the width of the room?"))
            length = Decimal(input("What is the length of the room?"))
            sqft = width * length
            print("Your room is %s sqft" % sqft)
            print("The cost to cover your room will be $%s" % (sqft * round(cost_of_tile, 2)))
            labor()
        elif answer in more_info:
            print("Sorry we need more information ")

            answer = input("Would you like to run the calculator again? Y/N")
            should_i_stay()
    elif answer in exit_program:
        print("Exiting")
        exit()

def should_i_stay():
    if answer in move_on:
        program()
    else:
        print("Good Bye")
        time.sleep(1)
        exit()

def labor():
    rate = 86
    total_labor = (sqft / 20) * rate
    hours_of_labor = sqft / 20
    if hours_of_labor <= 1:

        hours_of_labor = 1
        total_labor = rate
        print("The labor will be approximately $%s at a rate $%s per "
              "hour for a total of %s hours of labor" % (round(total_labor, 2), rate, hours_of_labor))
    else:
        print("The labor will be approximately $%s at a rate $%s per "
              "hour for a total of %s hours of labor" % (round(total_labor, 2), rate, hours_of_labor))

program()

Separation of concerns

You are mixing user interface and calculations: you should declare function for processing data and doing calculations and other functions to print pretty output. labor() handles both claculating and outputting.

Globals should be avoided

A function should be considered a black box with no knowledge about the outside world, function should never use global variables, because it is hard to mentally keep track of the value of all the global variables. Consider making sqft an argument of labor.

Functions are different from go to

def should_i_stay():
    if answer in move_on:
        program()

So you are basically jumping from one point in the code to an arbitrarly past point in the code. Functions should not be used this way (see above).

User interface

You should strive to offer the best interface possible to the user, your code has several User Interface flaws:

There is no space between the question and the space where you must input, this fells awkard.
Bad input ends the programme immediately or shows a scary error message:

:

This program will help you find the cost of floor covering

What is the cost of the tile per square foot?about 10 dollars
Traceback (most recent call last):
// Omitted for brevity
decimal.InvalidOperation: [<class 'decimal.ConversionSyntax'>]

you should learn about exception handling to make the input more reliable.

Best practices

You are using a magic number 20 in your labor function. A magic number is a number with no explanation whatsoever why you chose it over another.
Do not use from module import *, it will give people (even yourself in the future) problems figuring out where the functions used are taken from and may generate name clashes.
Use an if __name__ == '__main__' guard so that you can import your code in other files.
Global constants should be declared at the start of the file and be ALL CAPS:

:

EXIT_PROGRAM = ["q", "Q"]

MOVE_ON = ["y", "Y"]

MORE_INFO = ["n", "N"]


Scraping a table from Texas Dept. of Criminal Justice website The script scrapes a table from the website mentioned, looks at the last 2 columns, takes that information, and then sorts it (and then returns the largest county, and the set of races and their numbers). But, basically, I just wanted to know in which ways I can make this script parallel more closely with the Zen of Python, and if there are any optimizations I could make to the script to make it a little faster. I also know that Python's goal is not to be fast, but if I can make the script faster, I don't see why not (though the main priority is to make it Pythonic).

#!/usr/bin/env python
"""Scraping information from https://www.tdcj.state.tx.us/death_row/dr_executed_offenders.html"""

import requests, time
from bs4 import BeautifulSoup
from collections import defaultdict

url = "https://www.tdcj.texas.gov/death_row/dr_executed_offenders.html"
county = {}

def counties(countyArg):
    """Fills the `county` variable"""
    f = open("counties.txt")
    for line in f.readlines():
        countyArg[line.replace('\n', '').rstrip()] = 0

def scrape(urlArg, countyArg):
    """Scrape Site Based on HTML"""
    races = defaultdict(int)

    page = requests.get(urlArg)
    html = page.content

    soup = BeautifulSoup(html, 'html.parser')
    table = soup.find('table')
    for row in table.findAll('tr')[1:]:
        if row.findAll('td')[8].text == 'Black':
            races['black'] += 1
        elif row.findAll('td')[8].text == 'White':
            races['white'] += 1
        elif row.findAll('td')[8].text == 'Hispanic':
            races['hispanic'] += 1

        countyArg[row.findAll('td')[9].text.encode('ascii').rstrip()] += 1

    return races, county

def find_largest(countyArg):
    """find largest county"""
    largestCounty = ''
    largestInmates = 0
    for key, value in countyArg.iteritems():
        if largestInmates < value:
            largestInmates = value
            largestCounty = key
    return largestCounty, largestInmates

if __name__ == '__main__':
    counties(county)
    print county
    scrape(url, county)
    print find_largest(county)

In addition to PEP 20, the Python Zen, which you seem to be already familiar with,
another must read (and follow) for Python coders is PEP 8, the style guide.
Many of those recommendations apply to your code,
please read it carefully, I won't repeat them here.

Working with resources

In counties,
you open a file for reading but you never close it.
You should always close the resources, file handles you open.
But actually, in Python, you should use the with syntax, like this:

with open("counties.txt") as fh:
    for line in fh.readlines():
        countyArg[line.replace('\n', '').rstrip()] = 0

In this form, you don't actually need to close:
Python will automatically close the file handle after leaving the with block.

Why out-parameters?

It's not clear why you opted for initializing an empty county dictionary and then filling it in the counties method.
It would be more natural to let counties create the dictionary and return it.

Avoid global variables

county is declared at the outermost scope,
effectively making it a global variable.
Avoid global variables as much as possible.

As hinted earlier,
it would be better to create the dictionary inside a function,
and pass it around to the functions that need it.

For example:

load_counties(path):
    counties = {}
    with open(path) as fh:
        for line in fh.readlines():
            counties[line.replace('\n', '').rstrip()] = 0

    return counties

# ....

def main():
    counties = load_counties("counties.txt")
    scrape(url, counties)
    # ....

Avoid code duplication

Avoid repeated logic like this:

for row in table.findAll('tr')[1:]:
    if row.findAll('td')[8].text == 'Black':
        races['black'] += 1
    elif row.findAll('td')[8].text == 'White':
        races['white'] += 1
    elif row.findAll('td')[8].text == 'Hispanic':
        races['hispanic'] += 1

Not only it is error-prone to repeat row.findAll('td')[8].text many times,
it's also inefficient: beautiful soup has to find all td elements multiple times.

Refactor to do this only once:

for row in table.findAll('tr')[1:]:
    race = row.findAll('td')[8].text
    if race == 'Black':
        races['black'] += 1
    elif race == 'White':
        races['white'] += 1
    elif race == 'Hispanic':
        races['hispanic'] += 1

We can also go one step further:

for row in table.findAll('tr')[1:]:
    race = row.findAll('td')[8].text
    if race in ('Black', 'White', 'Hispanic'):
        races[race.lower()] += 1

Consider using collections.Counter

The collections package has a class that can help you here: Counter.
Instead of doing the counting by yourself,
this handy little utility can do it for you,
it goes something like this:

from collections import Counter

# ...

    races = []
    for row in table.findAll('tr')[1:]:
        race = row.findAll('td')[8].text
        if race in ('Black', 'White', 'Hispanic'):
            races.append(race.lower())

    return Counter(races)

That is,
from a list like ['black', 'white', 'black', 'white', 'black', 'hispanic'] it will create Counter({'black': 3, 'white': 2, 'hispanic': 1})

Use max to find the maximum entry of a dictionary

find_largest can be simplified using max:

def find_largest(counties):
    return max(counties.items(), key=lambda pair: pair[1]))


Walking a directory in Haskell I wrote a function that recursively walks a given directory.

module WalkDir (walkDir) where

import System.Directory (doesDirectoryExist, getDirectoryContents)
import System.FilePath ((</>))

walkDir :: FilePath -> IO [FilePath]
walkDir r = contents >>= fmap concat . traverse helper
    where contents = fmap (r </>) . filter ((&&) . (/=) "." <*> (/=) "..") <$> getDirectoryContents r
          helper x = do e <- doesDirectoryExist x
                        if e then walkDir x else return [x]

However, I have several concerns with this function. For starters, it is slow and it does not print out results until they have all been collected (not as lazy as it should be). My best guess is that this is because of the constant concatenations.

Additionally, the use of do-notation in helper seems clunky. This is where I'd love it if if were just a function because I could just use >>= with no do required. Alternatively if there were a GHC extension equivalent to LambdaCase for if statements that would also work.

As you noticed, this function is slow because it collects all the results before it starts printing them. To circumvent that problem, you need to interleave the collection of information and its printing.

A good way of doing that whilst keeping a compositional approach to solving the problem is to introduce a datatype reifying the structure of walkDir's call graph. Instead of sequencing all IO actions and getting a list of FilePaths back, you'd build a tree describing the computation (RTree for Rose Tree and T for Transformer as it takes an m):

data RTreeT m a = Node a [m (RTreeT m a)]

You can now write walkDir' describing your strategy to explore the directories on your filesystem: return the files present in the current directory immediately and then explore the subdirectories one after the other.

walkDir' :: FilePath -> IO (RTreeT IO [FilePath])
walkDir' r = do
  contents      <- fmap (r </>) . exceptLocal <$> getDirectoryContents r
  (files, dirs) <- filesAndDirs contents
  return $ Node files $ fmap walkDir' dirs

where filesAndDirs partitions a list of FilePath depending on whether they are files or directories (using tagDirectories to perform that test).

  tagDirectories :: [FilePath] -> IO [(FilePath, Bool)]
  tagDirectories = mapM (\ x -> (x,) <$> doesDirectoryExist x)

  filesAndDirs :: [FilePath] -> IO ([FilePath], [FilePath])
  filesAndDirs c = bimap (fmap fst) . partition (not . snd) <$> tagDirectories c
    where bimap f (a, b) = (f a, f b)

and exceptLocal is the filter you had in your original code snippet:

  exceptLocal :: [FilePath] -> [FilePath]
  exceptLocal = filter ((&&) . (/=) "." <*> (/=) "..")

You now have an RTreeT IO [FilePath] and you can described a strategy to print it which will interleave printing some of the content and running some of the remaining IO actions:

printRTreeT :: Show a => RTreeT IO a -> IO ()
printRTreeT (Node a mts) = print a >> mapM_ (printRTreeT =<<) mts

Of course, this is a rather crude printing function (e.g. you will notice quite a few empty lists if you have empty subdirectories) but it gives you an idea of how to proceed from there on.

If this is still slow, you may want to play the same sort of trick on filesAndDirs: rather than sequencing all tests in one go, you could want to have a structure allowing you to only deal with one FilePath at a time.


Method for sorting numbers from user input I have an input with numbers 1, 2, 3, 4 and I want to change the order of this number dynamically according to users input. You can find my code below and also a running example here.

HTML

<input type="text" name="name1" id="1" value="1" class="order">
<input type="text" name="name2" id="2" value="2" class="order">
<input type="text" name="name3" id="3" value="3" class="order">
<input type="text" name="name4" id="4" value="4" class="order">

jQuery

var myArray = new Array();
        $(".order").each(function() {
            myArray[$(this).attr('id')] = $(this).val();

        });//array to use when id and value are equal

    $('.order').live('change', function(){
            previousVal = $(this).val();
            previousId = $(this).attr('id');
            });//on change get this value as static to use later

    $('input.order').on('blur', function() {
    var idofnewvalue = $(this).attr('id');
    var newvalue = $(this).val();
    $(".order").each(function(){
            if (newvalue == $(this).val()) {
                    if(idofnewvalue != $(this).attr('id')){
                    var duplicate = $(this).val();
                        if(+previousVal > +previousId){
                            var plus = +duplicate - 1;
                            $("#"+duplicate).val(plus);

                        }else if(+previousVal < +previousId){
                            var plus = +duplicate + 1;
                            $("#"+duplicate).val(plus);
                        }else if(+previousVal == +previousId){
                            for ( var key in myArray) {

                                if (myArray.hasOwnProperty(key)) {
                                    $("#"+key).val(key);

                                  }
                                }

                        }

                    }
                    $("#"+duplicate).trigger("blur");//triggers blur event untill done
            }
        });
    });

The above code changes the value of the input according to users input.

For example, if the input is 1 2 3 4 and the user changes 4 to be 2, then the result will be 1 3 4 2.  If the user changes 2 to be 4 then the result will be 1 4 2 3.

Please tell me what you think about the above code.

Interesting question,

I cant understand why you would need this functionality. It does not seem to sort at all, but in same cases it will remove dupes.

This code is so bad, you should first use the built in tools in JsFiddle and then come back.

Click TidyUp first, your JavaScript indenting is all over the place.
Then click JsHint, every red dot on the left hand side indicates 1 problem, there are many lines with multiple problems, so you will have to do this exercise multiple times.


Iterations with asynchronous functions I haven't really worked much with asynchronous code so I'm wondering if I am doing something wrong/something could go wrong. Right now of all the test cases that I have done, my code has worked but I'm not sure if I will get side effects or not.

I won't post any of the business logic, just the iteration part with the asynchronous function in it:

function asyncInIteration(skinArray)  {
   request("/someaddresshere", function (err, response, data)  {

      var left = something.length;

      for (var i = 0;i<something.length;i++) (function (i) {
          request("/someotheraddress", function (err, response, data) {
             if (--left === 0)  {
                 console.log("End of for loop");
             }
          })
      })(i);
   });
}

Basically, what I am trying to do with this code is loop through an array (skinArray) and add values to it via sending HTTP requests.

The most important thing for me is that:

Every value of i should only be repeated once.
When I call --left === 0, that has to be called once, and only after all values of i have been gone through.

If someone wants to see the code with the business logic, you can find it here.

Use lodash to make Your iteration "readable".

And use async to handle end of asynchronous iteration to properly respond with "end of loop"

var _ = require('lodash');

var something = ['a','b','c'];
function asyncInIteration(skinArray)  {

   request("/someaddresshere", function (err, response, data)  {
      var functions = []; // array of functions that will be called

      // generating array of functions
      _.times(something.length,
          function() {
              // pushing function to functions list
              functions.push(function(done){ // "next" is necessary to inform async module about finishing call
                  request("/someotheraddress", function (err, response, data) {
                      // doing something
                      console.log("\n--------");
                      console.log("RESPONSE:", data);
                      console.log("--------\n");
                      done();
                  });
              });
          });

      // can change to .series if You want to make it synchronous (not parallel)
      async.parallel(functions, function(){
          console.log('End of loop');
      });
   });
}

P.S. In fact my code looks like not so simple, but there is no another elegant way to control asynchronousity. Maybe Promise object will be easy to work, but I don't think it's suitable for current task.


Iterating over unequal unordered pairs in one collection This code kinda hurts my feelings -- everything else in my code is done in neat one-liners, exploiting algorithms and, sometimes boost::bind, except for this piece. To say nothing about awkward if(b!=a).

Is there a better way to do the task?

#include <iostream>
#include <list>
using namespace std;

int main()
{
    int arr[] = {1,2,3,4,5,6};
    list<int> lst(arr,arr+sizeof(arr)/sizeof(int));

    for(list<int>::iterator a = lst.begin(); a != lst.end();a++)
    {
            for(list<int>::iterator b = a; b != lst.end(); b++)
                    if(b != a) cout << " ("<<*a<<","<<*b<<")";
            cout << "\n";
    }
    return 0;
}

Also not really pretty, but perhaps a start

#include <algorithm>
#include <iostream>
#include <list>
using namespace std;

int main()
{
    list<int> lst = {1,2,3,4,5,6};

    auto a = lst.cbegin();
    int first = *a;
    auto output_pairs = [&first](const int second){ cout << " ("<<first<<','<<second<<')'; };
    while(++a != lst.cend())
    {
        for_each(a, lst.cend(), output_pairs);
        cout << '\n';
        first = *a;
    }
    cout << flush;
}

It's C++0x because I was too lazy to write a functor for that lambda and I don't know the Boost.Lambda syntax off the top of my head.


Reordering OrderedDict Goal: create a new OrderedDict instance's keys in such way that keys in the specified order list appear first. If some keys in order list are not present in OrderedDict, ignore them.

Additionally, reordering should be "stable", that is, if some keys were not specified in order list, they should keep their original order from the old OrderedDict.

In other words:

o = OrderedDict([('a',1), ('b',2), ('d', 5), ('c',3), ('f', 6)])
reorder_ordereddict(o, ['c', 'a', 'x'])

produces:

OrderedDict([('c', 3), ('a', 1), ('b', 2), ('d', 5), ('f', 6)])

Code:

def reorder_ordereddict(od, new_key_order):
    odkeys = od.keys()
    fullorder = [x for x in new_key_order if x in odkeys]
    remaining = [x for x in odkeys if not x in fullorder]
    fullorder.extend(remaining)
    newod = OrderedDict([(k,None) for k in fullorder])
    newod.update(od)
    return newod

Any better ideas how to do this?

You can simplify it to this:

def reorder_ordereddict(od, new_key_order):
    new_od = OrderedDict([(k, None) for k in new_key_order if k in od])
    new_od.update(od)
    return new_od

The first line establishes the required key order, being careful not to introduce keys that don't already exist.

The second line takes care of the rest.  The existing keys not explicitly mentioned should come out in the correct order when update() iterates through the keys of od.


Queue implementation using two stacks Here's my code and I want to know if there's a better method for doing so. While the question has stated two stacks I wonder if we could do only with one as well?

/**
 * Created by mona on 5/26/16.
 */
import java.util.Stack;
//Implement queue using two stacks
public class QueueViaStacks {

    private int size=0;
    private Stack<Integer> firstStack = new Stack<>();
    private Stack<Integer> secondStack = new Stack<>();

    public void add(int item) {
        while (!firstStack.isEmpty()) {
            secondStack.push(firstStack.pop());
        }
        secondStack.push(item);
        while (!secondStack.isEmpty()) {
            firstStack.push(secondStack.pop());
        }
        size++;
    }

    public int remove() {
        size--;
        return firstStack.pop();

    }

    public int size() {
        return size;
    }

    public static void main(String[] args) {
        QueueViaStacks queue = new QueueViaStacks();
        queue.add(1);
        queue.add(2);
        queue.add(3);
        queue.add(4);
        queue.add(5);
        System.out.println(queue.remove());
        System.out.println(queue.size());
        System.out.println(queue.remove());
        System.out.println(queue.size());
    }

}

Your solution is kind of crappy time complexity wise, as adding elements without removing any takes quadratic time, as you reverse the full stack twice for every value added.  The better algorithm (which I think is amortized constant time) for this problem is to have one stack for input, and another for output. add() simply pushes to the inputStack, while remove() tries to pop from the outputStack, except if it is empty, in which case it first reverses the inputStack onto the outputStack:

public class QueueViaStacks {
    private Stack<Integer> inputStack = new Stack<>();
    private Stack<Integer> outputStack = new Stack<>();

    public void add(int item) {
        inputStack.push(item);
    }

    public int remove() {
        if (outputStack.isEmpty()) {
            while (!inputStack.isEmpty()) {
                outputStack.push(inputStack.pop());
            }
        }
        return outputStack.pop();
    }

    public int size() {
        return inputStack.size() + outputStack.size();
    }
}


Directory that lists employee information - follow-up Yesterday I posted a question involving multiple nested queries.  The queries pulled information from the database and created a directory listing of all employees.  There are two many-to-many relationships involving 5 tables.  For each employee they can have multiple job titles and multiple departments.

$stmt = $mysqli->prepare("SELECT employeeId, firstName, middleName, lastName, suffix, profilePhoto, GROUP_CONCAT(DISTINCT jobName ORDER BY jobTitleId), GROUP_CONCAT(DISTINCT departmentName ORDER BY departmentId), GROUP_CONCAT(DISTINCT departmentURL ORDER BY departmentId) FROM employee
INNER JOIN employee_has_jobTitle ON employeeId = employee_has_jobTitle.employee_employeeId
INNER JOIN jobTitle ON employee_has_jobTitle.jobTitle_jobTitleId = jobTitleId
INNER JOIN employee_has_department ON employee.employeeId = employee_has_department.employee_employeeId
INNER JOIN department ON employee_has_department.department_departmentId = departmentId
GROUP BY employeeId ORDER By lastName, firstName");

$stmt->execute();
$stmt->store_result();
$stmt->bind_result($employeeId, $firstName, $middleName, $lastName, $suffix, $profilePhoto, $jobName, $departmentName, $departmentURL);

//set up the list
echo "
";

while($stmt->fetch()){

    echo "

";

    $deptName = explode(",", $departmentName);
    $deptURL = explode(",", $departmentURL);
    $jobTitle = explode(",", $jobName);
    echo "$firstName $lastName
";

    $counter = 0;
    foreach($deptName as $name) {
        //echo a preceding comma if not the first department
        if($counter > 0) echo ", ";

        echo "$name";
        $counter++;
    }

    echo "
";

    foreach($jobTitle as $job) {
        echo "$job
";
    }

    echo "

";
}

//close the list
echo "

";

If you check my other question, you can see that this is an improvement.  However, I don't think the query and code are as elegant as they should be.  Is there an obvious way to simplify the code and enhance performance that I'm not seeing?

Screenshot of the ERD for reference:

in your while statement you are listing all sorts of information inside of one list item tag, I am not so sure this is intentional,  it looks like it is going to be really messy. I think you should look into creating sub-lists there especially for the multiple Department names and Job Names.  you want something like an XML Date File here,  maybe I am overstepping my reviewer line and telling you to change the structure, but this is going to look really messy on a website I think.

something like this would be more convenient.

       $firstName $lastName

Department1

Department1

Job1

Job2

I don't know exactly what you want your finished product to look like, but this structure is far more solid than what your PHP is going to output,  you should look into making it output something like this,  because this is going a lot easier to work with for Styling and manipulating with PHP.

all of your image tags () need to be closed(), so does your break tags (
,
)

From what you are saying it sounds like you want something more like this

            $firstName $lastName

Department1, Department1

Job1, Job2

I removed the Div tag and some of the other stuff just so we could visualize this better.  this is a little more organized than what you are going to get with your current code.  it will still be easier to deal with.

As Far as whether or not to do another list for the Department Names and Job Titles,  I would go with yes, even if they only have a single item, it will be easier to navigate the ones that do have them, and easier to manipulate the data,  it is more maintainable I think.

I mentioned still using nested lists for the Department Names and Job Names,  you should still have a title for the list item that is holding them, so you know what they are, but I will leave that up to you

            $firstName $lastName

Department1

Department1

Job1

Job2

so here is what that would look like with out the titles for the sub lists


Exclusive access to objects Take a look at following code:

/*abstract*/ class Capturable
{
    object _lock = new object();
    public bool IsCaptured { get; private set; }

    public bool TryCapture()
    {
        if (!IsCaptured)
            lock (_lock)
                if (!IsCaptured)
                    return IsCaptured = true;
        return false;
    }

    public void Release()
    {
        lock (_lock)
            IsCaptured = false;
    }
}

The idea is to inherit from Capturable when I need an object with exclusive access. Object will be used from different places (not necessarily different threads). If this object is needed, then the caller should first call TryCapture() (so theoretically there could be many calls when object is occupied, therefore double check locking is used to quickly return false).

Expected scenarios of usage:

periodically do something (e.g. by timer);
get exclusive access for a duration (e.g. if there is a long operation during which nobody should change state of object);

Both cases are covered with something like:

var obj = Manager.Get<SomeType>();
...
if(obj.TryCapture())
    try
    {
        ...
        obj.SomeMethod();
        ...
    }
    finally
    {
        obj.Release();
    }

The questions:

Is this a good approach to get exclusive access? Code-wise and architecturally? E.g. I can use Monitor.TryEnter instead of double check locking, but not sure how to return IsCaptured then.
Is there a possibility to avoid if+TryCapture+try+finally everywhere? Similar to how clean lock looks like (it covers Monitor calls) or how nice using is to ensure Dispose() call. The problem here comes when multiple of such object has to be captured at the beginning of method, then it will looks like dozens of nested if/try/finally, which I don't like.

What happens if this is called by something that hasn't captured the object?

public void Release()
{
    lock (_lock)
        IsCaptured = false;
}

Honestly, I don't see what this is giving you that you don't get from:

Monitor.TryEnter(lockObj)

and

Monitor.Exit(lockObj);

Unless you want to release the capture from a different thread, in which case it seems like you're more likely to run into the 'Released by wrong thing` problem.

Why do you need to know if it's captured?  Try to capture the lock and if it fails, you know that it's captured... What purpose does the property really serve?

  The problem here comes when multiple of such object has to be captured at the beginning of method, then it will looks like dozens of nested if/try/finally

If you're going to be capturing groups of objects then you're going to need to be very careful about the order that you capture those objects in order to prevent deadlocks where one method performs:

Capture A, B, C

And another method performs

Capture C, A

One of the things you've indicated is that you don't like the idea of the nested try/finally blocks to ensure that all captured items are released.  You could consider writing some utility functions to help make this cleaner.  Something like this:

static bool TryCaptureAll(params Capturable[] capturables)
{
    List<Capturable> capturedItems = new List<Capturable>();

    foreach(var capturable in capturables)
    {
        if(!capturable.TryCapture())
        {
            // Failed to capture one of the items, release the ones
            // we did capture and return failure.
            foreach(var captured in capturedItems)
            {
                captured.Release();
            }
            return false;
        }

        capturedItems.Add(capturable);
    }

    return true;
}
static void ReleaseAll(params Capturable[] capturables)
{
    foreach (var capturable in capturables)
    {
        capturable.Release();
    }
}

Would allow you to lock and release groups of objects like this:

if (CaptureList.TryCaptureAll(a, b, c))
{
    try
    {
        a.SomeMethod();
        b.SomeMethod();
        c.SomeMethod();
    }
    finally
    {
        CaptureList.ReleaseAll(a, b, c);
    }
}


Double-ended queue for Embedded Systems Following my previous question about a FIFO for embedded systems and the very detailed answer I got, I made some modifications to convert a simple FIFO to double-ended queue for Embedded Systems.
It is using a circular buffer.
How it works: When an element is added and the queue is not empty, the corresponding index is increased or decreased according to the side the element is added. When the queue is empty, and an element is added, neither index is changed so that the element can be accessed from either side.
The size of the queue is constant and it must be defined at compile time.
The data pointer must point to a static array that is also defined at compile time with the same size.
So here it is:
queue.h
#ifndef QUEUE_H
#define QUEUE_H

#include <inttypes.h>

typedef uint16_t QueueDataType_t;

struct queue
{
    QueueDataType_t * data;
    QueueDataType_t front_idx;
    QueueDataType_t back_idx;
    const QueueDataType_t size;
    QueueDataType_t elements;
};

#endif

queue.c
/**
 * \file queue.c
 *
 * \brief   A double-ended queue (deque). Elements can be added or removed from
 *          either the front or the back side.
 * \warning The current implementation is NOT interrupt safe. Make sure interrupts
 *          are disabled before access the QUEUE otherwise the program might yield
 *          unexpected results.
*/

#include "queue.h"
#include <stdbool.h>

/**
 * Initializes - resets the queue.
*/
void queue_init(struct queue * queue)
{
    memset(queue->data, 0, queue->size);
    queue->back_idx = 0;
    queue->front_idx = 0;
    queue->elements = 0;
}

/**
 * Checks if queue is full.
 *
 * \returns true if queue is full.
*/
bool queue_is_full(struct queue * queue)
{
    return (queue->elements == queue->size);
}

/**
 * Checks if queue is empty
 *
 * \returns true if queue is empty.
*/
bool queue_is_empty(struct queue * queue)
{
    return (queue->elements == 0);
}

/**
 * Adds one byte to the front of the queue.
 *
 * \returns false if the queue is full.
*/
bool queue_add_front(struct queue * queue,
                     QueueDataType_t data)
{
    if (queue_is_full(queue))
    {
        return 0;
    }

    if (queue_is_empty(queue) == 0)
    {
        queue->front_idx = (queue->front_idx + 1) >= queue->size ? 0 : (queue->front_idx + 1);
    }

    queue->data[queue->front_idx] = data;
    queue->elements++;
    return 1;
}

/**
 * Adds one byte to the back of the queue.
 *
 * \returns false if the queue is full.
*/
bool queue_add_back(struct queue * queue,
                    QueueDataType_t data)
{
    if (queue_is_full(queue))
    {
        return 0;
    }

    if (queue_is_empty(queue) == 0)
    {
        queue->back_idx = (queue->back_idx == 0) ? (queue->size - 1) : (queue->back_idx - 1);
    }

    queue->data[queue->back_idx] = data;
    queue->elements++;
    return 1;
}

/**
 * Reads one byte from the front of the queue.
 *
 * \returns false if the queue is empty.
*/
bool queue_get_front(struct queue * queue,
                     QueueDataType_t * data)
{
    if (queue_is_empty(queue))
    {
        return 0;
    }

    *data = queue->data[queue->front_idx];
    queue->front_idx = (queue->front_idx == 0) ? (queue->size - 1) : (queue->front_idx - 1);
    queue->elements--;
    return 1;
}

/**
 * Reads one byte from the back of the queue.
 *
 * \returns false if the queue is empty.
*/
bool queue_get_back(struct queue * queue,
                    QueueDataType_t * data)
{
    if (queue_is_empty(queue))
    {
        return 0;
    }

    *data = queue->data[queue->back_idx];
    queue->back_idx = (queue->back_idx + 1) >= queue->size ? 0 : (queue->back_idx + 1);
    queue->elements--;
    return 1;
}

Initializing the queue
#define MY_QUEUE_DATA_SIZE  50
static QueueDataType_t q_data[MY_QUEUE_DATA_SIZE];
static struct queue my_queue =
{
    .data = q_data,
    .size = MY_QUEUE_DATA_SIZE,
};

queue_init(&my_queue);

For consistency with your use of the <stdbool.h> header and the bool macro, your functions that return a bool should use return true; or return false; instead of returning 0 or 1. There will be no difference in the code but it is easier for a person reading the code to recognize what the return is (and this would also align better with your function documentation where you say that it "returns false").
Also related to comments, the add and get functions say that they add or read "one byte" but, because QueueDataType_t is a 16 bit type, they add or read two bytes.
If supported in your tool chain, queue_is_full and queue_is_empty could take their parameters as const struct queue * queue since they do not make any chances to the queue.
There is a subtle bug when removing the last element from the queue. When a queue is initialized, and when the first element is added, the front_idx and back_idx values both refer to the same slot in the data array. However, when you remove this single element, you still change the idx variable, so the resulting empty queue has the front and back indexes referring to different slots. Then when you add a new element, the indexes are not changed so that this element isn't pointed to by both indexes. This can result in an incorrect value being returned when this elements are removed from the queue. The fix is to not change the idx value when removing the last element.
You can create a function to adjust the indexes forward or backward to avoid having similar and duplicated code. This could then use the % operator (instead of a conditional branch) if appropriate on your embedded system.


Beginner jQuery – Coding Fundamentals / Performance Empty Textboxes, class="loc" display "(City, State \ Country)" using italics and silver.  Of course, Textboxes that do have data are displayed according to page defaults.

I noticed chaining and caching could be used.  There are inheritances issues, so I am not bothering with using .addClass instead of targeting with .css.

    $(function () {
        var loc = $('.loc');

        function desc() {
            var tar = $(this);
            if (tar.val().length == 0) {
                tar.val("(City, State \\ Country)").css({
                    "color": "silver",
                    "fontStyle": "italic"
                });
            };
        }

        function data() {
            var tar = $(this);
            if (tar.val() == "(City, State \\ Country)") {
                tar.val("").css({
                    "color": "black",
                    "fontStyle": "normal"
                });
            };
        }

        loc.each(desc).blur(desc).click(data);
    });

How far off base am I on understanding proper formatting in jQuery?

A couple of things:

Anytime you compare to 0 (zero) you want to make sure you use the exactly equal operator === instead of ==.  What would make your code even more expressive is doing something like if(tar.val().length){} .  This way your code is saying is "if the value of tar is blank then do this"
I'd also bring the last line to the top of your function and place it under the var loc statement.  The functions will be hoisted above all that so they'll be there even if they are defined after the calls.
this could prob be re-factored into one function with a simple if/else statement checking the value of the field.
Instead of using the each to call the data function ,assuming you don't have any other events attached to blur event you could call blur explicitly at the end of your chain to invoke that functionality ...click(data).blur().
I would look into ANeves suggestions as well.


Inefficient code inside of getter? I have two lists of the same type and I want to be able view a list of both lists:

    public List<Page> HeaderPages;
    public List<Page> SurveyPages;

    public IReadOnlyList<Page> AllPages
    {
        get
        {
            List<Page> allPages = new List<Page>();
            allPages.AddRange(this.HeaderPages);
            allPages.AddRange(this.SurveyPages);
            return allPages;
        }
    }

I think it is very inefficient to keep creating a new list object each time I want to get AllPages, so I thought I could store a private allPages list.  But then I thought if I am in the class, I would have to remember to use the public AllPages rather than the private allPages - which may not be up-to-date.

Any elegant solutions?

If you want to keep the two lists (otherwise check Hackworth's answer), the best solution would be to use LINQ extensions

public IEnumerable<Page> AllPages
{
    get
    {
        return HeaderPages.Concat(SurveyPages);
    }
}

This way you do not create any unnecessary collections. It forces you to use IEnumerable<Page> though, but in my opinion it is a good thing.


Script to download images from Reddit I wrote this script to download images from Reddit. I would like to hear from others on how can I improve this script.

import requests as _requests
import os as _os

class Redpy:

    def __init__(self, user):
        """Enter a user agent"""
        print("hello")
        self.user = user

    def download(self, subreddit, number=5, sort_option=None):
        """Downloads images from subreddit.
            subreddit="Name of subreddit"
            number=Number of images to be downloaded
            sort_option=new/hot/top
        """
        subreddit.strip('/')
        if sort_option == None:
            sort_option = ''

        self.url = 'https://www.reddit.com/r/' + subreddit + '/'+  sort_option  +  '.json'

        self.user = {'user-agent':self.user}
        res = _requests.get(self.url, headers=self.user)

        if res.status_code != 200:
            print("Could not download")
            print(res.status_code)
            return

        self._DownloadFiles(res.json(), number)

    def _DownloadFiles(self, jsonfile, number_of_files):
        image_links = self._getImages(jsonfile, number_of_files)

        if not self.createFolder():
            print("Error creating folder")
            return

        index = 0 #used to name the files
        for image_link in image_links:
            image_link = image_link.replace('amp;', '')
            f = _requests.get(image_link)

            if f.status_code==200:
                media_file = open(f'{_os.getcwd()}/red_media/{index}.jpg', 'wb')

                for chunk in f.iter_content(100000):
                    media_file.write(chunk)
                media_file.close()
                print("Downloaded")
                index+=1
        print("Download complete")
        global flag
        flag=1

    def _getImages(self, jsonfile, number_of_files):

        images = [] #contains links of images
        for index in range(number_of_files):
            try:
                images.append(jsonfile['data']['children'][index]['data']['preview']['images'][0]['source']['url'])
            except Exception as e:
                print(e)
        return images

    @staticmethod
    def createFolder():
        try:
            if not _os.path.exists(f'{_os.getcwd()}\\red_media'):
                _os.mkdir(f'{_os.getcwd()}\\red_media')
                return True
            return True
        except Exception as e:
            print(e)
            return False

Things I would like to know:

Is there anything that I can do to improve the performance of the code
What can I do to improve the code styling. I have tried to follow the PEP standards as much as I could.
Anything else to improve the code.

A few suggestions for general code quality:

If your class only does one thing and it doesn't store any values, it should be a function. Redpy only downloads images from reddit and stores values to achieve exactly this, which you could do in a function. Using a class can have unforeseen consequences.
Choose descriptive names for variables and functions. _getImages does not actually get the images, it returns a list of links of images. In this method, you have images = [] #contains links of images. The comment could have been avoided if you would have chosen image_links as name.
If you split your code up into methods or functions, everything belonging to one task should be inside it. The removal of 'amp;' in every image_link does not belong in _DownloadFiles, it should be in _getImages. download gets unnecessarily separated into _DownloadFiles and _DownloadFiles doesn't generally download files, but it could if some of its functionality got relocated elsewhere.
Clean up your code: there are unnecessary line breaks after _DownloadFiles and a redundant return True in createFolder.
Don't catch general Exceptions, be more specific. In _getImages, you should just look out for KeyErrors. Exceptions in request.get on the other hand are not handled although they possibly should be.
The pattern of looping over a list with a counter (index in your code) in _DownloadFiles can be simplified with enumerate.
When working with files, it is more elegant to use a context manager.

Possible bugs:

subreddit.strip('/') just returns a new string that you would have to assign to a new variable. In your code, the value of subreddit remains unchanged.
self.user gets updated every time download is called. If this happens multiple  times, self.user becomes a dict encapsulating a dict encapsulated a dict...
If something goes wrong when extracting links in _getImages, less links than expected get returned.
If your folder already contains images, they will be overwritten.

Concerning PEP8:

A few of your lines are longer than 80 characters. Try to split them up, either by implementing the same logic over multiple lines or by breaking the line up.
In PEP8, functions and methods are in snake_case.

Nit-picky stuff:

You could just use an empty string as default argument for sort_option. Strings are immutable, so you don't have the problem of mutable default arguments.
I don't see why you would import requests as _requests and os as _os
There is no need to construct an absolute file path. f'{_os.getcwd()}/red_media/{index}.jpg' could become f'red_media/{index}.jpg'

Here is my attempt at solving this problem:

import requests
import os

def get_image_links(json, N):
    '''returs a list of the first <N> links to reddit images found in <json>'''
    try:
        children = json['data']['children']
    except KeyError:
        return []

    # append links from children until N are found
    image_links = []
    for child in children:
        try:
            image_link = child['data']['preview']['images'][0]['source']['url']
        except KeyError:
            continue

        image_link = image_link.replace('amp;', '')
        image_links.append(image_link)

        if len(image_links)==N:
            break

    return image_links

def download_files(file_links, folder_name='data', file_extension='jpeg'):
    '''downloads files from <file_links> into subfolder ./<folder_name>/'''

    # create subfolder if it does not exist
    if not os.path.exists(folder_name):
        os.mkdir(folder_name)

    # download files
    for i, file_link in enumerate(file_links):
        try:
            res = requests.get(file_link)
        except requests.exceptions.RequestException:
            print(f"Unable to download {file_link}")
            continue
        if not res.ok:
            print(f"Error {res.status_code} when requesting {file_link}")
            continue

        file_path = os.path.join(folder_name, f'{i}.{file_extension}')
        with open(file_path, 'wb') as file:
            for chunk in res.iter_content(100000):
                file.write(chunk)

def download_reddit_images(user, subreddit, N=5, sort_by='',
                           folder_name='red_media'):
    '''
    downloads the first <N> images of <subreddit> sorted by <sort_by>
    (''/'new'/'hot'/'top') into subfolder ./<folder_name>/
    '''

    json_url = ('https://www.reddit.com/r/' + subreddit.strip('/') + '/'
                + sort_by + '.json')

    try:
        res = requests.get(json_url, headers={'user-agent':user})
    except requests.exceptions.RequestException:
        print(f"Unable to get {json_url}")
        return

    if not res.ok:
        print(f"Error {res.status_code} when requesting {json_url}")
        return

    image_links = get_image_links(res.json(), N)
    if not len(image_links):
        print("Unable to find any images")
        return

    download_files(image_links, folder_name=folder_name, file_extension='jpeg')

if __name__=='__main__':
    download_reddit_images('', 'all')

The problem of overwriting existing images persists. A solution would be to use the original filename from reddit that is included in the url.


Review on my multiple JQuery Slider on a page I have successfully placed multiple jquery ui slider on a page and its transalation text too. you can check it live on:
http://outsourcingnepal.com/projects/jQuery%20slider/

I have my jquery code as :

var arrLabel1 = new Array('Not Good', 'OK', 'Average', 'Above Average', 'Excellent', 'Outstanding');
$(function() {
    $( ".slider_control" ).slider({
        value:0,
        min: 0,
        max: 5,
        step: 1,
        slide: function( event, ui ) {
            $( "#slider_" + $(this).attr('rel') ).val( ui.value );
            $( "#label_" + $(this).attr('rel') ).text( arrLabel1[ ui.value ] );
        }
    }).each(function(index){
        $( "#slider_" + (index + 1) ).val( $( this ).slider( "value" ) );
        $( "#label_" + (index + 1) ).text( arrLabel1[ $(this).slider( "value" ) ] );
    });
});

Now i would like to optimize it so that i can be independent of rel attribute.

MY HTML:

Overall:

        <input type="hidden" name="overall_review" id="slider_1" />

Cleanliness:

        <input type="hidden" name="overall_review" id="slider_2" />

Facilities:

        <input type="hidden" name="overall_review" id="slider_3" />

Location:

        <input type="hidden" name="overall_review" id="slider_4" />

Quality of Service:

        <input type="hidden" name="overall_review" id="slider_5" />

Room:

        <input type="hidden" name="overall_review" id="slider_6" />

Value of Money:

        <input type="hidden" name="overall_review" id="slider_7" />

In my option your code has two problems:

You unnecessarily fill your HTML with empty elements which you could just create in your script. Changing that would remove the necessity for the rel attribute.
more importantly (even if there are people that may disagree), you have unnecessarily made your form dependent on JavaScript. It would be much better to have select elements in your HTML, which you can replace with sliders. That way even users without JavaScript can use your form unrestricted.

I've written an example how I would do it:

$(".ui_slider select").each(function() {
    var select = $(this); // Cache a refernce to the current select
    var sliderDiv = $("

"); // create a div for the slider
    var displayLabel = $(""); // create a span to display current selection

    if (select[0].selectedIndex < 0) // Make sure that an item is selected
        select[0].selectedIndex = 0;

    select
        .hide() // hide the select
        .before( // Insert display label before the select
            displayLabel
               .text(select.find("option:selected").text()) // and set it's default text
        )
        .after(
           sliderDiv // Insert the silder div after the select
               .data("select", select) // store a reference to the select
               .data("label", displayLabel) // store a reference to the display label
               .slider({
                    max: select.find("option").length - 1, // set to number of items in select
                    slide: function(event, ui) {
                        var select = $(this).data("select");
                        select[0].selectedIndex = ui.value; // update the select
                        $(this).data("label").text( // Update the display label
                            select.find("option:selected").text()
                        );
                    }
               })
        );
});

HTML (repeat as needed):

    Overall:
    <select name="overall" id="overall">
        <option value="0">Not Good</option>
        <option value="1">OK</option>
        <option value="2">Average</option>
        <option value="3">Above Average</option>
        <option value="4">Excellent</option>
        <option value="5">Outstanding</option>
    </select>

Working sample: http://jsfiddle.net/YfqCx/2/


Writing a static Log class for PHP I wanted to write a simple Log class for PHP, I use ajax calls with AngularJS and often return the log in an array

Example:

$return['data'] = $returnedDataArray;
$return['log'] = $logDataArray;
$return['status'] = 'success';
echo json_encode($return);

I was hoping to implement a logging system in my other classes, like my DB wrapper, etc.

Example:

Log::put('sql', $sql, 'DB');
Log::put('fields', $fieldsArray, 'DB');

And than pass the log.

Example:

$return['log'] = Log::getLog();

Here is my Log class:

class Log {
    private static $_loggingOn = true, $_log = array();

    public static function put ($key, $value, $className = null, $functionName = null) {
        if ($className) {
            if ($functionName) {
                self::$_log[$className][$functionName][$key] = $value;
            } else {
                self::$_log[$className][$key] = $value;
            }
        } else {
            self::$_log[$key] = $value;
        }
    }

    public static function getLog () {
        if (self::loggingOn()) {
            return self::$_log;
        }
        return array('Logging is turned off.');
    }

    public static function loggingOn () {
        return self::$_loggingOn;
    }
}

I am wondering if using the static class would be a recommended approach?

No, using static methods is not a recommand approach. With static methods, your class always knows which class it calls. That means you can never switch to another logger class (e.g. one that writes the logs in a file).

A class should be independent of other classes as much as possible. In this case, you should just create a LoggerInterface (or use the one from PSR-3) and base on that interface. Inject it in the classes that needs a logger and then use that object. This way, you can always change from loggers (as long as they implement the correct interface) and your class doesn't know which classes it uses.

Example:

interface LoggerInterface
{
    public function put($key, $value, $class = null, $function = null);
    public function getMessages();
}

class Logger
{
    protected $messages;

    public function put($key, $value, $class = null, $function = null)
    {
        // ...
    }

    public function getMessages()
    {
        // ...
    }
}

class DataBase
{
    protected $logger;

    public function __construct(LoggerInterface $logger)
    {
        $this->logger = $logger;
    }

    public function someDbFunction(...)
    {
        // ... do something

        $this->logger->put(...); // log something
    }
}

This approach is called Dependency Injection. To make live easier for you, you can sue a Dependency Injection Container (also called a Service Container) to manage which classes depends on which other classes and creating those objects. A simple example is Pimple

There are also more disadvantages of using static methods. You can for instance never have 2 different instances of the logger, as there are no instances. That means that all messages are put in the same instance. You may want to internally use a logger in the Database section of your lib and another logger in the Form section of your lib and then one logger for your complete lib.


JavaScript image-Carousel with using IIFE I have to implement an image-Carousel.

Because I'll reuse the snippet within other projects which might contain all kinds of external libraries I used an immediately invoked function expression.

Could my code be improved?

All Hints appreciated.

(function (imgPaths, imgTag, prevButton, nextButton) {
  var length = imgPaths.length;
  var i = 0;

  function setImageSrc () {
    // Using alt-attribute for simplicity.
    // In production the src-attribute is used.
    imgTag.setAttribute('alt', imgPaths[i]);
  }

  setImageSrc();

  prevButton.addEventListener('click', function() {
    i = ((i - 1) + length) % length;
    setImageSrc();
  });

  nextButton.addEventListener('click', function() {
    i = (i + 1) % length;
    setImageSrc();
  });
})( [ 'imgs/first', 'imgs/second', 'imgs/third',
     'imgs/fourth', 'imgs/fifth' ],
   document.querySelector('.display img'),
   document.querySelector('.prev'),
   document.querySelector('.next') );

  <button class="prev"> << </button>
  <button class="next"> >> </button>

A carousel is an enhancement, a different way of displaying the same data. Without the carousel effect, a carousel is just a list of images. If JS was turned off or for some reason the script blew up, one would still expect to see the images, albeit in a broken layout.

In this case, I would format it to become a list.

    <button type="button" class="my-carousel__control my-carousel__control--prev"> << </button>
    <button type="button" class="my-carousel__control my-carousel__control--next"> >> </button>

In addition, I just popped in a few classes for styling. You can, by default, hide the buttons and show all images. When JS runs, it shows the controls and hides all but one image.

.my-carousel{...}                   # Widget style
.my-carousel__deck{...}             # Deck
.my-carousel__slide{...}            # Default slide style
.my-carousel__slide--hidden{...}    # Styles when a slide is hidden
.my-carousel__controls{...}         # Default controls styles
.my-carousel__controls--active{...} # Controls when active

Your carousel looks simple, just showing one image, no sliding effect etc. We can improve that even further. Since we moved out the image sources away from the JS, we can focus on just the effect.

;(function(images, controls, prevButton, nextButton){

  const imageHiddenClass    = 'my-carousel__slide--hidden';
  const controlsActiveClass = 'my-carousel__controls--active';
  let currentSlide = 0;

  function setActiveSlide(index){
    // Hide all images
    images.forEach(image => image.classList.add(imageHiddenClass));
    // But then show the one we want shown
    images[index].classList.remove(imageHiddenClass);
  }

  prevButton.addEventListener('click', function() {
    currentSlide = (currentSlide - 1) % images.length;
    setActiveSlide(currentSlide);
  });

  nextButton.addEventListener('click', function() {
    currentSlide = (currentSlide + 1) % images.length;
    setActiveSlide(currentSlide);
  });

  // Show the controls that are hidden by default
  controls.classList.add(controlsActiveClass);

  // Set the first slide as active
  setActiveSlide(currentSlide);

})(
   Array.prototype.slice.call(document.querySelectorAll('.my-carousel__slide')),
   document.querySelector('.carousel__controls'),
   document.querySelector('.carousel__control--prev'),
   document.querySelector('.carousel__control--next')
);

In the above, the script is a bit longer but it takes an alternate approach. The major difference is that instead of changing srcs of the image, since the images are all in the markup, what the script does now is toggle the my-carousel__slide--hidden class, adding it to hide images not in focus.


Here are some advices regarding the D3 part of the code:

Use scales!

Your if... else logic for filling the rectangles is cumbersome and luckily unnecessary: you can use a scale, in that case a threshold scale:

var color = d3.scaleThreshold()
    .range(["#F8B195", "#F67280", "#C06C84", "#6C5B7B", "#355C7D"])
    .domain([0.5, 0.55, 0.59, 0.62]);

Then, you can do just this in the attr method:

.attr("fill", function(d) {
    return d[1] === 0 ? "#2A363B" : color(d[1])
});

Do the same for the positions. Right now, you're using a lot of magic numbers everywhere: for translating the groups, positioning the rectangles and setting their width/height.

Instead of that, set up band scales:

var horizontalGrid = d3.scaleBand()
    .range([100, 400])
    .domain(d3.range(cols))
    .padding(0.1);

var verticalGrid = d3.scaleBand()
    .range([0, 300])
    .domain(d3.range(rows))
    .padding(0.1);

Using them for translating the groups...

.attr("transform", function(_, i) {
    return "translate(" + horizontalGrid(i) + ")"
})

...and positioning the rectangles, as well as setting the dimensions:

.attr("y", function(d, i) {
    return verticalGrid(i);
})
.attr("width", horizontalGrid.bandwidth())
.attr("height", verticalGrid.bandwidth());

Finally, two important advices:

Always name your selections

That's very important when the visualisation becomes more and more complex. For instance, you should do:

var groups = svg.selectAll("g")
    //etc...

Break your selections

By breaking your selections not only you can easily reference the correct selection, but you can also avoid things (silently) breaking. And it's way easy to maintain. For instance, in your code:

var groups = svg.selectAll("g")
    //etc...

var rects = groups.selectAll("rect")
    //etc...

Now you have two selections, groups (which refers to the containing <g> elements) and rects (which refers to the rectangles).

Select nothing

You read it right. If you don't plan to have an update selection here, just select nothing in your enter selections, or for a cleaner code select null. So, since you're not selecting anything, you have an enter selection which always contains all the data points, and a way faster code.

So, the move snippet becomes:

var groups = svg.selectAll(null)
    //etc...

var rects = groups.selectAll(null)
    //etc...

If you want to read more about selectAll(null) read my explanation here: Selecting null: what is the reason behind 'selectAll(null)' in D3.js?.

Demo

Here is your code with those changes:

var data = [
  [
    ["Arsenal", 0.0],
    ["Chelsea", 0.6014876082652767],
    ["Liverpool", 0.5204181171517794],
    ["ManchesterCity", 0.549210189254557],
    ["ManchesterUnited", 0.5440890632512689],
    ["Tottenham", 0.6304670189118691]
  ],
  [
    ["Arsenal", 0.6014876082652767],
    ["Chelsea", 0.0],
    ["Liverpool", 0.5507313736526684],
    ["ManchesterCity", 0.5559069243804156],
    ["MancheserUnited", 0.5231358671618266],
    ["Tottenham", 0.6508134781353688]
  ],
  [
    ["Arsenal", 0.5204181171517794],
    ["Chelsea", 0.5507313736526684],
    ["Liverpool", 0.0],
    ["ManchesterCity", 0.49759390310994533],
    ["MancheserUnited", 0.4787550034617063],
    ["Tottenham", 0.5749363562907429]
  ],
  [
    ["Arsenal", 0.549210189254557],
    ["Chelsea", 0.5559069243804156],
    ["Liverpool", 0.49759390310994533],
    ["ManchesterCity", 0.0, ],
    ["MancheserUnited", 0.50215325905151],
    ["Tottenham", 0.5802928689025063]
  ],
  [
    ["Arsenal", 0.5440890632512689],
    ["Chelsea", 0.5231358671618266],
    ["Liverpool", 0.4787550034617063],
    ["ManchesterCity", 0.50215325905151],
    ["MancheserUnited", 0.0],
    ["Tottenham", 0.5497016431211542]
  ],
  [
    ["Arsenal", 0.6304670189118691],
    ["Chelsea", 0.6508134781353688],
    ["Liverpool", 0.5749363562907429],
    ["ManchesterCity", 0.5802928689025063],
    ["MancheserUnited", 0.5497016431211542],
    ["Tottenham", 0.0]
  ]
];

var teams = ["ARS", "CHE", "LIV", "MCI", "MUN", "TOT"];

var width = 600,
  height = 500;

var cols = data.length;
var rows = data.length;
var cellSize = 55;

var horizontalGrid = d3.scaleBand()
  .range([100, 400])
  .domain(d3.range(cols))
  .padding(0.1);

var verticalGrid = d3.scaleBand()
  .range([0, 300])
  .domain(d3.range(rows))
  .padding(0.1);

var color = d3.scaleThreshold()
  .range(["#F8B195", "#F67280", "#C06C84", "#6C5B7B", "#355C7D"])
  .domain([0.5, 0.55, 0.59, 0.62])

var svg = d3.select("body")
  .append("svg")
  .attr("width", width)
  .attr("height", height)

var groups = svg.selectAll(null)
  .data(data)
  .enter()
  .append("g")
  .attr("transform", function(_, i) {
    return "translate(" + horizontalGrid(i) + ")"
  });

var rects = groups.selectAll(null)
  .data(function(d) {
    return d;
  })
  .enter()
  .append("rect")
  .attr("fill", function(d) {
    return d[1] === 0 ? "#2A363B" : color(d[1])
  })
  .attr("y", function(d, i) {
    return verticalGrid(i);
  })
  .attr("width", horizontalGrid.bandwidth())
  .attr("height", verticalGrid.bandwidth());
<script src="https://d3js.org/d3.v5.min.js"></script>


Searching for a specific object in an ArrayList Is this an efficient way to search for a specific object in an ArrayList?

Product is the superclasS. Mobo, CPU, RAM, GPU... are the subclasses.
ArrayList(called stock) stores Product objects. I consider that they key i am searching is found when class and the attribute "model"(you can see the getter of the attribute in my code) are matching.

public static boolean checkAvailability(ArrayList<Product> stock, Product product) {
    boolean found = false;
    for(Product p: stock) {
        if(p instanceof Mobo && (product.getModel() == p.getModel())) {
            found = true;
            break;
        };
        if(p instanceof CPU && (product.getModel() == p.getModel())) {
            found = true;
            break;
        };
        if(p instanceof GPU && (product.getModel() == p.getModel())) {
            found = true;
            break;
        };
        if(p instanceof RAM && (product.getModel() == p.getModel())) {
            found = true;
            break;
        };
        if(p instanceof Monitor && (product.getModel() == p.getModel())) {
            found = true;
            break;
        };
        if(p instanceof Keyboard && (product.getModel() == p.getModel())) {
            found = true;
            break;
        };
        if(p instanceof Mouse && (product.getModel() == p.getModel())) {
            found = true;
            break;
        };
        if(p instanceof Printer && (product.getModel() == p.getModel())) {
            found = true;
            break;
        };
    };
    return found;
}

Your code is very wet (the opposite of DRY). It repeats a lot. Let's start with what you're trying to achieve:

You have an ArrayList of Products.
You want checkAvailability to return true if any Product within the given ArrayList has the same model and type of the given Product.

The way you've implemented this, with instanceof checks, is IMO not ideal. Instead, let's modify the Product class to have a custom equals() method to compare equality of Products; we can store the equality logic inside of the Product class so it's reusable.

The equals() method on an Object, by default, will act similar to == - that is, it will compare referential equality, but it's a common idiom to override this method to compare logical equality instead.

class Product {
    ...

    @Override()
    public boolean equals(Object other) {
        // This is unavoidable, since equals() must accept an Object and not something more derived
        if (other instanceof Product) {
            // Note that I use equals() here too, otherwise, again, we will check for referential equality.
            // Using equals() here allows the Model class to implement it's own version of equality, rather than
            // us always checking for referential equality.
            Product otherProduct = (Product) other;
            return otherProduct.getModel().equals(this.getModel());
        }

        return false;
    }
}

Of course, this does lose the instanceof checking in your checkAvailability method, but you can easily reintroduce something similar by overriding equals() in the subclasses of Product if you wanted to reintroduce this check (Spoiler: You probably don't! Don't compare types unless you have to). At the very least, try to move equality logic to an equals() method on a class rather than comparing equality outside of the class. Encapsulation and reusability is always nice!

With this override, we can simplify checkAvailability to the following:

public static boolean checkAvailability(ArrayList<Product> stock, Product product) {
    for (Product p : stock) {
        if (p.equals(product)) {
            return true;
        }
    }

    return false;
}

Which can be simplified to use Java streams if you'd like.

public static boolean checkAvailability(ArrayList<Product> stock, Product product) {
    return stock.stream()
                .anyMatch(p -> p.equals(product));
}

Or, even better, we can make the required type less specific:

public static boolean checkAvailability(Collection<Product> stock, Product product) {
    return stock.stream()
                .anyMatch(p -> p.equals(product));
}

As an aside: it is generally good practice to reimplement hashCode() if you reimplement equals(). Without having more knowledge of your solution, it's difficult for me to tell you how to do this, but I would recommend you do it if you end up using my suggestions.

Another side note: If you find yourself making lots of comprehensions like this, I would recommend having checkAvailability accept a Stream<Product> instead of Collection<Product> so as to avoid creating multiple streams.


Make an Array Reactive (Observable) I have written a simple JavaScript function to make an array reactive (i.e. observable). Is there anything I could improve? What the code does is take an array, a callback and a this value as arguments, then whenever the array gets updated or a value gets added to it, it runs the callback with the specified this value. It works similar to the obsolete Array.observe().

function arrayObserve(array, callback, thisArg){
    var result = [];
    result.push.apply(result, array);
    function getterAndSetter() {
        if (Array.isArray(array))
            var currentArray = result;
        else
            var currentArray = Object.keys(result);
        console.log(result);
        currentArray.forEach( function(prop){
            if (Array.isArray(result))
                var currentProp = result.lastIndexOf(prop);
            else
                var currentProp = prop;

            Object.defineProperty(array, currentProp, {
                configurable: true,
                enumerable: true,
                set: function(val) {
                    result[currentProp] = val;
                    callback.call(thisArg, result);
                },
                get: function() {
                    return result[currentProp];
                }
            });
            //}
            if (typeof result[currentProp] == 'object') {
                arrayObserve(result[currentProp], callback, result);
            }
        });
    }
    getterAndSetter();
    ['pop','push','reverse','shift','unshift','splice','sort','map','filter'].forEach( function(method){
        Object.defineProperty(array, method, {
            configurable: false,
            enumerable: false,
            writable: true,
            value: function () {
                var noReturnMethods = ['push', 'pop', 'reverse', 'shift', 'unshift'];
                if (noReturnMethods.indexOf(method) > -1) {
                    Array.prototype[method].apply(result, arguments);
                } else {
                    var results = Array.prototype[method].apply(result, arguments);
                    result = results;
                }
                callback.call(thisArg, result);
                getterAndSetter();
                return result;
            }
        });
    });
    return result;
}

Some minor nit-picks about your code.

Object.defineProperty(array, method, {
    configurable: false,
    enumerable: false,
    writable: true,
    value: function () {
        // ...

Here, configurable and enumerable default to false anyway. You needn't set them explicitly.

if (noReturnMethods.indexOf(method) > -1) {
    Array.prototype[method].apply(result, arguments);
}

With ES6, you can write the if condition more concisely as noReturnMethods.includes(method).

You could improve readability by storing ['pop','push','reverse','shift','unshift','splice','sort','map','filter'] in a variable. You could probably do

var noReturnMethods = ['push', 'pop', 'reverse', 'shift', 'unshift'];
var returnMethods = ['splice','sort','map','filter'];
var methods = [];
methods = methods.push.apply(noReturnMethods, returnMethods);

... to avoid duplication of the names of methods.

With ES6, you could do methods = [...noReturnMethods, ...returnMethods].

Assuming your environment supports ES6, you should probably make use of arrow functions and the let and const keywords.


Fastest way to determine if a number is perfect square I'm looking for the fastest way to determine if a long value is a perfect square (i.e. its square root is another integer). I was asked this in an interview recently.

Here is the very simple and straightforward way I'm doing it now:

  private static boolean isSquare(long n) {
    int i = 1;
    for (;;) {
      if (n < 0)
        return false;
      if (n == 0)
        return true;
      n -= i;
      i += 2;
    }
  }

Is there any better way by which we can do this or optimize this more?

It is indeed very simple and straightforward. The time complexity is however \$(O(\sqrt n)\$. A binary search variation will do it in \$O(\log n)\$ time. Along the lines of the pseudocode:

    while lo < hi
        mid = lo + (hi - lo)/2
        square = mid * mid
        if square == n
            return true
        if square < mid
            lo = mid
        else
            hi = mid
    return false

Depending on the expected magnitude of \$n\$ it may or may not worth the effort. In the interview setting (no restrictions on \$n\$) I would go for the binary search.

if inside a tight loop usually hurts performance. Consider

    while (n > 0) {
        n -= i;
        i += 2;
    }
    return n == 0;


The first thing that stands out from your code is the big table giving the morse code: it's lengthy, hard to read, and hard to check. (And consequently, it contains two mistakes.) I would write something like this:

from itertools import izip

def pairwise(s):
    """
    Generate pairs from the sequence `s`.

    >>> list(pairwise('abcd'))
    [('a', 'b'), ('c', 'd')]
    """
    it = iter(s)
    return izip(it,it)

morse_encoding = dict(pairwise('''
    a .-    b -...  c -.-.  d -..   e .     f ..-.
    g --.   h ....  i ..    j .---  k -.-   l .-..
    m --    n -.    o ---   p .--.  q --.-  r .-.
    s ...   t -     u ..-   v ...-  w .--   x -..-
    y -.--  z --..

    1 .----   2 ..---   3 ...--   4 ....-   5 .....
    6 -....   7 --...   8 ---..   9 ----.   0 -----

    , --..--  . .-.-.-  ? ..--..  / -..-.   - -....-
    ( -.--.   ) -.--.-  ' .----.  ! -.-.--  & .-...
    : ---...  ; -.-.-.  = -...-   + .-.-.   _ ..--.-
    " .-..-.  $ ...-..- @ .--.-.
'''.split()))

morse_decoding = {v:k for k,v in morse_encoding.iteritems()}

You rebuild the decoding table every time you start a new translation. This seems wasteful: I suggest building it only once, as above.
Not everything needs to be a class. A class is a data abstraction whose instances represent some thing together with methods for operating on that thing. You don't actually have any persistent data here: all you are using the class for is to store the temporary state for a single translation). It makes more sense to structure this code as functions.
Your classes and methods have no docstrings and no doctests. Even a small doctest or two might have caught the mistakes in your Morse code table.
In your determine_lang method, you use a regular expression that matches strings that are not Morse code. It would be simpler to do the test the other way round: to determine that the string is Morse code (because Morse code contains only dots, dashes and spaces). Python's sets provide a clear and concise way to express this test:

def is_morse_code(s):
    """
    Return True if the string `s` resembles Morse code (contains only
    dots, dashes and spaces), False otherwise.

    >>> is_morse_code('... --- ...')
    True
    >>> is_morse_code('abc')
    False
    """
    return set(s) <= set('.- ')

The original challenge may have referred to "English", but there's nothing actually English-specific about Morse code. It translates any language that can be written in the Latin alphabet. So I would prefer names that didn't presume that we were translating English.
When a character cannot be encoded or decoded, you print a message to inform the user. The usual approach in Python is to raise an exception.
Your translation code seems very complex for what it is doing. All you need to do is split the string into words, split each word into letters, translate each letter, join the translated letters into words, and then join the translated words into the result. This can be done neatly and concisely using split and join and a couple of loops:

def morse_encode(s):
    """
    Encode the string `s` in Morse code and return the result.
    Raise KeyError if it cannot be encoded.

    >>> morse_encode('morse code')
    '-- --- .-. ... .  -.-. --- -.. .'
    """
    return '  '.join(' '.join(morse_encoding[l] for l in word)
                    for word in s.lower().split(' '))

def morse_decode(s):
    """
    Decode the string `s` from Morse code and return the result.
    Raise KeyError if it cannot be decoded.

    >>> morse_decode('-- --- .-. ... .  -.-. --- -.. .')
    'morse code'
    """
    return ' '.join(''.join(morse_decoding[l] for l in word.split(' '))
                    for word in s.split('  '))


Functional Game of Life implementation in Ruby We ran a Code Retreat day at Hacker Paradise this Saturday, and I was pretty happy with my eventual solution. It's written in Ruby, but would work reasonably well in JS/CS and is (hopefully) reasonably readable across languages.

module Enumerable
  def bag
    # [1,1,1,1,2,2,3] => {1 => 4, 2 => 2, 3 => 1}
    # similar to python's Counter
    Hash[group_by { |x| x }.map {|k, v| [k, v.count] }]
  end
end

def neighbors_of(location)
  x_range = (location.first-1).upto(location.first+1).to_a
  y_range = (location.last-1).upto(location.last+1).to_a
  x_range.product(y_range) - [location]
end

def will_be_alive(was_alive, neighbors_count)
  if was_alive
    [2,3].include? neighbors_count
  else
    neighbors_count == 3
  end
end

def iterate(old_grid)
  old_grid.flat_map do |loc, _|
    neighbors_of(loc)
  end.bag.select do |loc, alive_near_me|
    will_be_alive(old_grid[loc], alive_near_me)
  end
end

def print(grid)
  system('clear')
  min_x, max_x = grid.keys.map(&:first).minmax
  min_y, max_y = grid.keys.map(&:last).minmax

  result = min_y.upto(max_y).map do |y|
     min_x.upto(max_x).map do |x|
      grid[[x,y]] ? 'X' : ' '
    end.to_a.join("")
  end.to_a.join("\n")

  puts result
end

def new_grid()
  glider = [[1,0], [1, 2], [0,2], [2,2], [2,1]]
  box = [[-3,3], [-3,4], [-4,3], [-4,4]]
  alive_start = glider + box
  Hash[alive_start.map { |k| [k, true] }]
end

grid = new_grid()

until grid.empty? do
  print(grid)
  grid = iterate(grid)
  sleep 1
end

I'm using this as my style guide here.

Don't use parentheses when there aren't parameters. In this case, it applies to new_grid's definition and uses -- rather than new_grid(), it should be just new_grid.

In new_grid, I'd recommend adding two things:

A comment just before the last line to explain why you're not using nested (i.e. 2D) arrays. I suspect it's for memory reasons, but you may well have your own reasons for this somewhat counterintuitive choice.
A hash.default= call to set the default to false (as "not here"), just for consistency with true as "here". Note that this doesn't change anything about how the code actually runs, but it's more consistent to use true/false than true/[falsey value].

In your actual run loop, you should separate the iterations by something, so they don't blend together. In the console (which is how I ran this), this is what it looks like:

It's like this because system('clear') doesn't work on Windows. If you wanted to support Windows, you'd use system('cls') instead. (Before we go arguing about which OS is best, it doesn't matter. This is a code review. I'm finding bugs.)

However, since trying to make console commands work on every system is hard, I'd recommend simply putting some divider between the boards. For example, immediately after your current last line, you could put this:

puts '-' * (max_x - min_x + 1)

which will change the output to something more like this (I forgot where I originally took the picture, so it's not exactly the same area):

Or, alternatively, combine them! Try both clear and cls, and if neither succeed, puts a divider. Just get rid of system('clear') and toss this if before puts result to get that:

if system('clear')
  #Empty intentionally
elsif system('cls')
  #Empty intentionally
else
  puts '-' * (max_x - min_x + 1)
end

It works on my Bash emulator, my Ubuntu server, my Windows 8 laptop, and with some dark magicks I confirmed that if it can't run any of the commands it successfully outputs the dividers.

Testing with RubyMine confirms that RubyMine is weird. cls returns true, but does nothing. This is a (now reported) bug with RubyMine.

Now, while we're nitpicking print, have another: Rather than joining the lines with \n then outputting all at once, which may cause some errors with things that try to autoindent code (cough my stuff that puts spaces in front so it's easier to take pictures cough), replace it with .each { |line| puts line }.

Note: If you use both this and the last tip, you'll have to move the if block(s) up above it.

You've got a couple of magic numbers in will_be_alive, but it's Conway's Game of Life and the function is called will_be_alive, so it's pretty obvious what they are, so that's not that big a deal.

Aside from that, it looks good!

I like how you commented in bag to explain what it does, since it took me a good four minutes to get what product does.

The actual algorithm is good, though it took me a minute to figure out how iterate works when it seems like it should just give 8 every time, at least at first.

I quite like how you pass the grid as an argument to each function, rather than using a global variable or the like.

As I can tell, the majority of the code is good! Well done.