Improving multiple assignment using an iterator function Often I need to perform multiple assignment from a sequence
(e.g. from the result of a split()) but with some semantics
that, AFAIK, are not available in the language:

Assign N first elements and silently ignore the rest (like Perl's
list assignment).

>>> a, b = 'foo bar is here'.split()
>>> a, b
('foo', 'bar')
>>>

Assign the remaning items to the last variable in the variable list:

>>> a, b, c = 'foo bar is here'.split()
>>> a, b, c
('foo', 'bar', ['is', 'here'])
>>>

If the sequence doesn't have enough values, set the variables to a
default value (None or any other, again similar to Perl):

>>> a, b, c, d, e = 'foo bar is here'.split()
>>> a, b, c, d, e
('foo', 'bar', 'is', 'here', None)
>>>

I have designed an iterator function to help me with this, but I'm
concerned about my solution to this problem from the point of view of
pythonic adherence:

Usefulness: Using an iterator for this kind of job is the right
option, or there is something more appropriate?  Or even worse: my
need to do this kind of assignment in Python is not favoring other
language features?
Clearness: The implementation is solid and clear enough?  Or is
there some other way to implement this that would be cleaner?

In other words, this solution is better discarded or can it be
improved?

from itertools import *

def iterassign(iterable, *, count, use_default=False, default=None, tail=False):
    """Iterattor for multiple assignment.

    :param iterable: The iterable.
    :param int count: The maximum count of iterations.
    :param bool use_default: If set, and iterable ends before max_count,
        continue iteration using :param:`default`.
    :param default: Default value to use when :param:`use_default` is
        set to ``True``.
    """
    it = iter(iterable)
    for idx, element in enumerate(chain(islice(it, 0, count), repeat(default))):
        if idx >= count: break
        yield element
    else:
        raise StopIteration()
    if tail:
        yield list(it)

Usage examples:

>>> ret = 'foo bar is here'.split()
>>> a, b = iterassign(ret, count=2)
>>> a, b
('foo', 'bar')
>>> a, b, c = iterassign(ret, count=2, tail=True)
>>> a, b, c
('foo', 'bar', ['is', 'here'])
>>> a, b, c, d, e = iterassign(ret, count=5, use_default=True)
('foo', 'bar', 'is', 'here', None)
>>>

I'll look at each of your three examples individually.

Ignoring remaining values

In Python 2:

>>> a, b = 'foo bar is here'.split()[:2]
>>> a, b
('foo', 'bar')

In Python 3 (_ used by convention, double underscore, __, is
also appropriate):

>>> a, b, *_ = 'foo bar is here'.split()
>>> a, b
('foo', 'bar')

When ignoring remaining values, using the built-in language features would be the most Pythonic approach.  I think they're clear enough that they do not need to be re-invented.
Capture remaining values

In Python 2:

>>> split_string = 'foo bar is here'.split()
>>> a, b, c = split_string[2:] + [split_string[2:]]
>>> a, b, c
('is', 'here', ['is', 'here'])

In Python 3:

>>> a, b, *c = 'foo bar is here'.split()
>>> a, b, c
('is', 'here', ['is', 'here'])

In Python 3, the * feature is again more Pythonic than your approach.  In Python 2, that approach won't work if you're dealing with a non-indexable type.
Fill remaining values with None.  Here is a somewhat similar question.  That question and the Python 2 solution only work for lists.

Python 2:

>>> split_string = 'foo bar is here'.split()
>>> a, b, c, d, e = split_string + [None] * (5-len(split_string))
>>> a, b, c, d, e
('foo', 'bar', 'is', 'here', None)

Python 3.5 (this is not very clear in my opinion):

>>> a, b, c, d, e, *_ = *'foo bar is here'.split(), *[None]*5
>>> a, b, c, d, e
('foo', 'bar', 'is', 'here', None)

There isn't really a built-in language construct for this so this case might warrant re-inventing.  However, this kind of None-filling seems rare and may not necessarily be clear to other Python programmers.

A review of your actual implementation

Don't use import * in Python.  In Python we value our namespaces highly because namespaces allow us to see what variables came from what modules.

from itertools import *

Do this instead:

from itertools import chain, islice, repeat

You didn't use use_default in your code at all.  It seems like you could use another islice instead of an enumerate to restructure your code like this:

def iterassign(iterable, *, count, default=None, tail=False):
    it = iter(iterable)
    for x in islice(chain(islice(it, 0, count), repeat(default)), 0, count):
        yield x
    if tail:
        yield list(it)

Iterators are the right solution to this problem (assuming we've decided it's one that needs solving).

If we want to rewrite this to only handle case 3 (because we've already decided we can just use Python's built-in language features for case 1 and 2, we could rewrite like this:

def fill_remaining(iterable, *, length, default=None):
    """Return iterable padded to at least ``length`` long."""
    return islice(chain(iterable, repeat(default)), 0, length)

Which would work like this:

>>> a, b, c, d, e = fill_remaining(ret, length=5)
>>> a, b, c, d, e
('foo', 'bar', 'is', 'here', None)


Check if URL contains different strings The code it self is pretty self explanatory, however i do not think it is very efficient at all. Is there a better way to do this?

Its basically so i can track spam sites and save the sites into my honeypot db.

<?php
$site = $_GET['url'];
$con=mysqli_connect("localhost","_","","_");
// Check connection
if (mysqli_connect_errno()) {
  echo "Failed to connect to MySQL: " . mysqli_connect_error();
}

//Gambling
if (strpos($site, "betting") ||strpos($site, "blackjack") ||strpos($site, "casino") ||strpos($site, "craps") ||strpos($site, "football") ||strpos($site, "gamble") ||strpos($site, "gambling") ||strpos($site, "game") ||strpos($site, "gaming") ||strpos($site, "greyhound") ||strpos($site, "highroller") ||strpos($site, "lottery") ||strpos($site, "lotto") ||strpos($site, "odds") ||strpos($site, "poker") ||strpos($site, "racehorse") ||strpos($site, "racing") ||strpos($site, "roulette") ||strpos($site, "royalflush") ||strpos($site, "slotmachine") ||strpos($site, "slots") ||strpos($site, "slotsmachine") ||strpos($site, "sport") ||strpos($site, "wager") ||strpos($site, "paigow") ||strpos($site, "bingo") ||strpos($site, "baccarat"))
{
    //Insert into gambling
    $result = mysqli_query($con,"INSERT INTO `rocket_newsites`.`gambling` (`id`, `url`) VALUES (NULL, '$site');");

}
//Payday
if (strpos($site, "payday") ||strpos($site, "loan") ||strpos($site, "bank") ||strpos($site, "money") ||strpos($site, "wonga") ||strpos($site, "lender") ||strpos($site, "credit") ||strpos($site, "debt") ||strpos($site, "repayment") ||strpos($site, "mortgage"))
{
    //Insert into payday
    $result = mysqli_query($con,"INSERT INTO `rocket_newsites`.`payday` (`id`, `url`) VALUES (NULL, '$site');");
}
//Lawyer
if (strpos($site, "law") ||strpos($site, "solicitor") ||strpos($site, "government") ||strpos($site, "injury") ||strpos($site, "personal") ||strpos($site, "accident") ||strpos($site, "advice") ||strpos($site, "traffic"))
{
    //Insert into lawyer
    $result = mysqli_query($con,"INSERT INTO `rocket_newsites`.`lawyer` (`id`, `url`) VALUES (NULL, '$site');");
}
//Weight Loss
if (strpos($site, "weight") ||strpos($site, "loss") ||strpos($site, "pills") ||strpos($site, "diet") ||strpos($site, "garcinia") ||strpos($site, "cambogia") ||strpos($site, "acai") ||strpos($site, "berry") ||strpos($site, "raspberry") ||strpos($site, "ketone") ||strpos($site, "coffee") ||strpos($site, "tea") ||strpos($site, "health") ||strpos($site, "lose") ||strpos($site, "surgery") ||strpos($site, "fat") ||strpos($site, "dieting") ||strpos($site, "exercise") ||strpos($site, "workout") ||strpos($site, "gym") ||strpos($site, "hypnosis"))
{
    //Insert into weight loss
    $result = mysqli_query($con,"INSERT INTO `rocket_newsites`.`weightloss` (`id`, `url`) VALUES (NULL, '$site');");
}
//Insurance
if (strpos($site, "ppi") ||strpos($site, "insurance") ||strpos($site, "payment") ||strpos($site, "claim") ||strpos($site, "calculator") ||strpos($site, "finance") ||strpos($site, "mis-sold"))
{
    //Insert into insurance
    $result = mysqli_query($con,"INSERT INTO `rocket_newsites`.`insurance` (`id`, `url`) VALUES (NULL, '$site');");
}

mysqli_close($con);
exit();
?>

Why did you create multiple tables for each category?

Surely it would be better to have 1 single table with something like:

id - int
category - enum(lawyer, insurance, etc.)
url - varchar

You might want to have a look at this answer from StackOverflow for a better strpos alternative to your requirements: https://stackoverflow.com/a/9220624/367708


Skip invalid record and avoid execution break for invalid record Here is the pseudo code used inside a Rails controller:

defined_hash_10000_records.each do |data|
    account = Account.where(account_identifier: params[:account_id]).first
    program = Program.where(program_identifier: data[:id]).first
    unless participant_account.present? || utility_program.present?
      # this is just a logger method defined in the second file.
      failure_msg('participant account is #{account.inspect} & utility program is #{program.inspect}')
      next
    end
    sub = Subscription.where(account_id: account.id, program_id: program.id)
end

It is working fine and I would like to know if it can be further improved.

Focus to improve the following line (conditions will help to bypass in case record not found instead of break the execution):

unless participant_account.present? || utility_program.present?
      # this is just a logger method defined in the second file.
      failure_msg('participant account is #{account.inspect} & utility program is #{program.inspect}')
      next
    end

Simplify logic to eliminate next

To improve readability by eliminating next, rearrange your record validity test, so lines 4 to 9 become

if participant_account.present? || utility_program.present?
  sub = Subscription.where(account_id: account.id, program_id: program.id)
else
  # this is just a logger method defined in the second file.
  failure_msg('participant account is #{account.inspect} & utility program is #{program.inspect}')
end

I am not sure this is what you mean. Ask yourself if you really need only one of participant_account or utility_program for the subscription query to succeed.

Don't do things many times that could be done once

To save some time, put line two before the iteration, because value of account does not depend on data. This also helps readability and debugging, as when programmers read the body of an iteration, they  expect only to see lines that need to be evaluated many times.

Only make queries if you need the result

To save a little more time and memory, only calculate the value of program if you know you are going to use it. So put it inside the conditional:

if participant_account.present? && utility_program.present?
  program = Program.where(program_identifier: data[:id]).first
  sub = Subscription.where(account_id: account.id, program_id: program.id)
else # ...

Extract methods

After these changes the conditional is starting to get a bit complicated, so extract some methods, for example:

if participant_account.present? && utility_program.present?
  sub = subscription(account, first_program(data))
else # ...

def subscription(account, program)
  Subscription.where(account_id: account.id, program_id: program.id)
end

def first_program(data)
  Program.where(program_identifier: data[:id]).first
end

Put query logic in models

I don't know if Program is an ActiveRecord model. If it is, then #first_program could be a class method in that model

Don't use #first unless you guarantee the order

For example, if you have ActiveRecord timestamps, you could use

Program.where(program_identifier: data[:id]).order(:created_at).first

Are validation and failure handling cross-cutting concerns?

You might also want to consider patterns that factor out your validation and failure handling code, but whether and how to use them depends on the larger structure of your program. For example, you could yield to your validations and sub = ... line as lambdas from a validate_with_logging method.


Simple Bash log backup script I have developed the following script to zip and then remove all files with "*.log" extension in the file name that were modified yesterday. It's definitely not 100% at the moment. For example, it misses files from yesterday that have a modified time of 00:00.

#!/bin/bash

yest=$(date --date="yesterday" +"%m_%d_%Y")

find /path/to/dir/ -daystart -name "*.log" -type f -mtime 1 -print | zip /path/to/dir/"logbackup-$yest.zip" -@
if [ $? -eq 0 ]; then
    find /path/to/dir/ -daystart -name "*.log" -type f -mtime 1 -exec rm -f {} \;
fi

It seems that using -daystart OR using mtime exclusively are not returning the correct results for me. Only files "c" and "a" should be returned.

[user@computer log]$ ls -lart
drwxr-xr-x 23 user user     4096 Sep  5 22:27 ..
-rw-rw-r--  1 user user        0 Nov 18 14:00 d
-rw-rw-r--  1 user user        0 Nov 19 00:00 c
-rw-rw-r--  1 user user        0 Nov 19 12:30 a
-rw-rw-r--  1 user user        0 Nov 20 12:30 b
drwxr-xr-x  2 user user    16384 Nov 20 12:32 .
[user@computer log]$ find . -mtime 1 -type f | xargs ls -lart
-rw-rw-r-- 1 user user 0 Nov 18 14:00 ./d
-rw-rw-r-- 1 user user 0 Nov 19 00:00 ./c
-rw-rw-r-- 1 user user 0 Nov 19 12:30 ./a
[user@computer log]$ find . -daystart -mtime 1 -type f | xargs ls -lart
-rw-rw-r-- 1 user user 0 Nov 19 12:30 ./a

Put /path/to/dir/ in a variable to avoid writing it repeatedly.

Instead of running the command and checking $? after,
you can put the command inside an if.

The script would become shorter, for example:

path=/path/to/dir

if find $path -daystart -type f -mtime 1 -name "*.log" -print | zip $path/"logbackup-$yest.zip" -@
then
    find $path -daystart -type f -mtime 1 -name "*.log" -exec rm -f {} \;
fi

In my tests, it seems that simply dropping -daystart will have the desired effect of matching the right files. For example:

touch -d yesterday a
touch -d '2014-11-19 00:00:00' b
touch -d '2014-11-18 00:00:00' c
touch -d '2 days ago' d
ls -l

Gives:

-rw-r--r-- 1 janos users 0 Nov 19 12:35 a
-rw-r--r-- 1 janos users 0 Nov 19 00:00 b
-rw-r--r-- 1 janos users 0 Nov 18 00:00 c
-rw-r--r-- 1 janos users 0 Nov 18 12:38 d

And then:

$ find . -daystart -mtime 1 -type f
./a
$ find . -mtime 1 -type f
./a
./b


Dumping fields from Python objects I am writing a small class that has one method: get_state. It is parameterless and returns a dictionary of all the names in self (look in its dict) that are not bound to function objects (test functions by using inspect.isfunction on the objects). When a class inherits from Dumpable, its instances can call get_state() to get a dictionary of all the names in its namespace (that are not functions) and their values.

For example, if we first defined class Modular_Counter(Dumpable) and then mc = Modular_Counter(0,3) and then print(mc.get_state()), it would print the following dictionary: {'_value': 0, '_modulus': 3}.

Here is my current working code for the class:

class Dumpable:
    def get_state(self):
        no_funs = self.__dict__
        no_fun_list = [no_fun_key for no_fun_key in no_funs.keys() if inspect.isfunction(no_funs[no_fun_key])]
        for fun in no_fun_list:
            del no_funs[fun]
        return no_funs

Is there a cleaner way to get the same results? I still need to use the one function in the class. Any help would be appreciated.

You can simply use thee vars() built-in function with a dictionary comprehension here:

def get_state(self):
    return {k: v for k, v in vars(self).items() if not inspect.isfunction(v)}

Note that the methods are usually defined in the class so they actually never show up in instance dictionary, while looking up for them Python can find them from the class dictionary. With that said, in case you defined a functions on the instance:

self.func = lambda: 'func called'

And then tried to run your code then you will notice that your code actually nukes that function itself from the instance when you call get_state, that's because you're deleting them from the instance dictionary. Don't do that, instead create a new dictionary.

class Dumpable:

    def __init__(self):
          self.x = 1
          self.y = 2
          self.func = lambda: 'func called'

    def get_state(self):
        no_funs = self.__dict__
        no_fun_list = [no_fun_key for no_fun_key in no_funs.keys() if inspect.isfunction(no_funs[no_fun_key])]
        for fun in no_fun_list:
            del no_funs[fun]
        return no_funs

d = Dumpable()
print(d.func())
print(d.get_state())
print(d.func())

Output:

func called
{'y': 2, 'x': 1}
Traceback (most recent call last):
  File "/home/ashwini/py/so.py", line 25, in <module>
    print(d.func())
AttributeError: 'Dumpable' object has no attribute 'func'

Other minor nitpicks:

To get the key as well as items from a dict use its .items() method.
To loop over a dict, never call its .keys() method. A dict is an iterable and iterating on it yields one key at a time. The .keys() can be very expensive in Python 2 where it actually creates a list of all keys first.
 -


Elegant way to add a value to a member variable inside a vector I need to add an offset to a time variable inside my class.
The code below works, how do i make it more efficient, and elegant.

#include <array>
#include <iostream>
#include <string>
#include <vector>

class Pressure
{
    public:

    Pressure(int date, int time):m_date(date),m_time(time)
    {

    }

    int GetDate()
    {
        return m_date;
    }

    int GetTime()
    {
        return m_time;
    }

    void AddTime(int val)
    {
        m_time += val;
    }

    private:
    int m_date;
    int m_time;

};

int main()
{
  std::vector<Pressure> p;

  // populate the vector with values.
  for(int i=0; i < 10; i++)
  {
    Pressure x(i,i);
    p.push_back(x);
  }

  int offset = 20000;

  // add the offset
  for(int i=0; i < 10; i++)
  {
      p[i].AddTime(offset);
  }

  // display the data
  for(int i=0; i < 10; i++)
  {
    std::cout << p[i].GetDate() << " "  << p[i].GetTime() << "\n";
  }

}

You could use a range-based for loop and push_back( Pressure(i,i) ).

#include <iostream>
#include <vector>

class Pressure {
public:
    Pressure(int date, int time) : m_date(date), m_time(time) {

    }

    int GetDate() const {
        return m_date;
    }

    int GetTime() const {
        return m_time;
    }

    void AddTime(int val) {
        m_time += val;
    }
private:
    int m_date;
    int m_time;
};

int main() {
    std::vector<Pressure> p;

    // populate the vector with values.
    p.reserve(10); // you can reserve space if you know the size ahead of time to prevent extra reallocations, but it isnt necessary for a vector this small.
    for(int i = 0; i < 10; ++i) {
        p.push_back( Pressure(i,i) );
    }

    for(Pressure& pressure : p) {
        // add offset
        pressure.AddTime(20000);

        // display data
        std::cout << pressure.GetDate() << " " << pressure.GetTime() << "\n";
    }

    return 0;
}


Pomodoro Timer in Python I wrote a simple Pomodoro Timer in Python 3:

from datetime import datetime, timedelta
from gi import require_version
require_version('Notify', '0.7')
from gi.repository import Notify
import sys

class State:
    #length: the length of the state in minutes
    def __init__(self, length):
        if not length == None:
            self.length = length

class Clock:
    work = None
    sBreak = None
    lBreak = None

    numWork = 4
    remWork = numWork #number of work sessions remaining
    remBreaks = numWork - 1 #number of short break sessions remaining
    current = None #the current state
    endtime = None #datetime object for end time of current

    def __init__(self, workTime, sBreakTime, lBreakTime):
        if workTime == None or sBreakTime == None or lBreakTime == None:
            exit(1)
        self.work = State(workTime)
        self.sBreak = State(sBreakTime)
        self.lBreak = State(lBreakTime)
        self.states = [self.work, self.sBreak, self.lBreak]

    def change(self):
        if not self.current == None and not self.current in self.states:
            exit(1)

        title = ''
        timeStr = 'End Time: '

        #current doesn't exist: start the first work period
        if self.current == None:
            self.current = self.work
            title = 'Work Period'

        #in work state now: either short or long break
        elif self.current == self.work:
            self.remWork -= 1
            if self.remBreaks == 0:
                self.current = self.lBreak
                title = 'Long Break'
            else:
                self.current = self.sBreak
                title = 'Short Break'
        #in short break now: start work
        elif self.current == self.sBreak:
            self.remBreaks -= 1
            self.current = self.work
            title = 'Work Period'
        #in long break now: reset number of work and short break periods
        elif self.current == self.lBreak:
            self.current = self.work
            title = 'Work Period'
            self.remWork = self.numWork
            self.remBreaks = self.numWork - 1
        self.endtime = datetime.now() + timedelta(seconds = self.current.length)
        timeStr +=  self.endtime.strftime("%H:%M")
        Notify.init("a")
        notifyme=Notify.Notification.new(title,timeStr)
        notifyme.show()

    def tickTock(self):
        self.change()
        while True:
            if datetime.now() >= self.endtime:
                self.change()

def main():
    try:
        lengths = sys.argv[1:]
        workLen = int(lengths[0])
        sbLen = int(lengths[1])
        lbLen = int(lengths[2])

        clock = Clock(workLen, sbLen, lbLen)
        clock.tickTock()
    except:
        print("One or more arguments were invalid.")
        exit(1)

if __name__ == '__main__':
    main()

First of all, is this a good way to simulate the timer? Also, is there a way to get around using self.variable_name every time I want to use a variable in the Clock class? Finally, what would be a good package to look into if I wanted to extend this into a GUI application?

Just some quick tips, I might append some later. You might disagree with me, it's just my vision.

1) AFAIK, Pomodoro has some "classic" times for work and break sessions. It might be good to create them as default ones so the user wouldn't have to enter them manually via command line each time. I suggest using argparse module for that. A really quick and dirty rewriting of your main function:

def main():
    try:
        argparser = argparse.ArgumentParser()

        argparser.add_argument("--workLen", help="the length of a work session", type=int, default=25)
        argparser.add_argument("--sbLen", help="the length of a short break", type=int, default=5)
        argparser.add_argument("--lbLen", help="the length of a long break", type=int, default=30)

        # parse the arguments. They can be accessed in form args.argument
        args = argparser.parse_args()

        clock = Clock(args.workLen, args.sbLen, args.lbLen)
        clock.tickTock()
    except Exception as e:
        print("One or more arguments were invalid.", str(e))
        exit(1)

2) No need to put your work, sBreak and lBreak (and, probably, others too) variables right after class Clock: because that way you're declaring them as static. When you're declaring them in __init__, you basically declare other variables - the variables for this particular instance. And the static ones just remain None and are useless. So you may safely remove them. More info

3) Not sure if it is a common practice, but I would do if workTime == None or sBreakTime == None or lBreakTime == None: as if None in (workTime,sBreakTime,lBreakTime,):. You're testing for the same value anyway (None). I'm not sure if you want this behaviour, but if you want your program to exit if any of these values is 0 as well, you may use if not all((workTime,sBreakTime,lBreakTime,)):, because 0 is treated as false by Python.

I agree with Matthias on exit. As for naming conventions, I prefer being more liberal: do as you find comfortable, especially if it is your personal project. I would also use the PEP8 convention for variables (small_letters_with_underscore), but when it comes to function names, I'm more used to camelCaseStartingWithSmallLetter ;)

UPD1:
4) Add a sleep(0.1) at the end of your while loop in tickTock, because otherwise it uses up too much CPU.


Remapping Heading Angle to Coordinates to Move By I wrote the following code to take the heading of an object, and determine which direction (x,y) it needs to move to be heading forward in a 2D plane. I have debugged the code and know that it functions as expected, but as far as readability/maintainability is concerned, I feel like it could be better. Thoughts/opinions?

 /*
 *  map($orgLow, $orgHigh, $newLow, $newHigh, $value)
 *  Take existing range of values and remap to new range of values
 *  Requires original high range value, original low range value
 *    new high range value, new low range value, value to be mapped
 *  Returns newly mapped integer value
*/
function map($orgLow, $orgHigh, $newLow, $newHigh, $value) {
    return $newLow + ($value - $orgLow) * ($newHigh - $newLow) / ($orgHigh - $orgLow);
}

/*
 *  coordsToMoveFromDeg($deg)
 *  Take heading and return movement on a (-1 to 1, -1 to 1) plane
 *  Requires heading to be scaled
 *  Optional precision to be given
 *  Returns array consisting of x and y values to change by
*/
function coordsToMoveFromDeg($deg, $precision = false) {

    //Rotate plane 90º CCW by remapping the degree value
    $xdeg = map(0,360,-90,270,$deg);
    if ($xdeg < 0) {
        //When xdeg is negative, add 360 to get proper angle
        $xdeg += 360;
    }
    if ($xdeg > 180) {
        //Remap values larger than 180 to their mirrored
        //value prior to getting final movement values
        $xdeg = map(180,360,180,0,$xdeg);
    }
    $ydeg = $deg;
    if ($ydeg > 180) {
        //Remap values larger than 180 to their mirrored
        //value prior to getting final movement values
        $ydeg = map(180,360,180,0,$deg);
    }
    //Remap x and y to how much to move by, -1 to 1
    $x = map(180,0,-1,1,$xdeg);
    $y = map(180,0,-1,1,$ydeg);

    //Show equal amount of decimal points always for
    //easier readability if precision is set, recommended
    //to only be set when in debug environment
    if ($precision) {
        $x = number_format($x,$precision);
        $y = number_format($y,$precision);
    }

    return array('x'=>$x,'y'=>$y);
}

//Show the mapped values for all degrees value 0 to 359 (360 = 0)
for ($i = 0; $i < 360; $i++) {
    var_dump(coordsToMoveFromDeg($i,4));
}

I'll separate this into two sections: readability and maintainability. I'm not exactly sure of the usage, so I don't feel I can comment on that.
Readability
Personally, I think the largest burden on the cleanliness of this code, is the comments. I think they're too large and there's too many.
One way to improve this, is PHPDocs. Just like JavaDocs, they help organize the functionality of code properties.
Another way to reduce them is to make your variables as meaningful as possible. What exactly is $deg? What kind of degree is it? Same principle for $xdeg and $x.
Breaking apart one large method into several smaller functions can also improve the cleanliness, not to mention the benefits of modularized code and improved organization.
One last note: I see a lot of numbers. What's the decision behind these numbers. Making some of the numbers in map(180,360,180,0 constants or variables at the top can help readers understand the meaning.
Maintainability
You code is very simple (as in it takes care of one thing), and I think this is the greatest tool to avoiding confusion in the future.
Having well documented code is also great, just make sure your wording is clear and concise. Sometimes referencing to other code in the comments can help. Explain why, not how.
I don't think there's too much else you can do to future-proof this!


Maintaining state while walking a tree structure The following code creates a simple tree structure and then walks through the tree looking for node with the value 9 that appears after we have seen a node with the value 1 and a node with the value 4 (order of the 1 and 4 don't matter).
type N = Value of int  | Children of N list
type S = {oneFound:bool; fourFound:bool}

let walk () =
    let mutable state = {oneFound = false; fourFound = false}
    let rec walkImpl curNode =
        match curNode with
        | Value n ->    match n with
                        | 1 -> state <- {state with oneFound = true }
                        | 4 -> state <- {state with fourFound = true }
                        | 9 -> if state.oneFound && state.fourFound then
                                printfn "Found a good 9"
                               else
                                printfn "Found a bad 9"
                        | _ -> ()
                        printfn "Node value %d" n
        | Children c -> c |> List.iter (fun n -> walkImpl n)
    walkImpl (Children [Value 1; Value 2 ; Children [Value 9; Children [Value 4; Value 5]; Value 9]])

walk ()

When I run it produces the correct output:
Node value 1
Node value 2
Found a bad 9
Node value 9
Node value 4
Node value 5
Found a good 9
Node value 9

Is there a method to do this that threads the state through the recursive function calls rather than making it a mutable variable?  The mutability is confined to my function but I keep thinking there must be a way to structure this that makes it unnecessary.
In real life the tree structure will be much larger, several thousand nodes and 10 levels deep, while the  code has not run into any recursion limits yet are there improvements that would make this less likely?

At first step I've removed hardcoded into function input data, so it's become testable.
   ...
   walkImpl (Children [Value 1; Value 2 ; Children [Value 9; Children [Value 4; Value 5]; Value 9]])

walk ()

let walk n =
    ...
    walkImpl n

walk (Children ...)

Than get rid of type which exists only for one function and replaced it with anonymous record.
    let initialState = {| oneFound = false; fourFound = false |}

Afterwards comes hardest part of refactoring: remove mutability. In order to use mutable state in functional way we have to use recursion and pass state as parameter to function which must return updated state, so signature should be State -> State instead of () -> () (curNode omitted for clarity).
      | Value n ->
            let patched =
                match n with
                | 1 -> {| state with oneFound = true |}
                | 4 -> {| state with fourFound = true |}
                | 9 ->
                    if state.oneFound && state.fourFound then
                        printfn "Found a good 9"
                    else
                        printfn "Found a bad 9"
                    state
                | _ -> state
            printfn "Node value %d" n
            patched

Now we have part of function that matches over curNode and transforms state if Value branch is evaluated. We also must transfer state when walking across different children while iterating list. List.fold is function that iterates over list while keep tracking of state changes. All list function may be implemented with fold, such a powerful abstraction.
        | Children c -> c |> List.fold walkImpl state

Now we have complete working example without mutations
type N = Value of int  | Children of N list

let walk n =
    let rec walkImpl (state : {| oneFound: bool; fourFound: bool |}) curNode = // explicit type required here
        match curNode with
        | Value n ->
            let patched =
                match n with
                | 1 -> {| state with oneFound = true |}
                | 4 -> {| state with fourFound = true |}
                | 9 ->
                    if state.oneFound && state.fourFound then
                        printfn "Found a good 9"
                    else
                        printfn "Found a bad 9"
                    state
                | _ -> state
            printfn "Node value %d" n
            patched
        | Children c ->
            c |> List.fold walkImpl state

    let initialState = {| oneFound = false; fourFound = false |}
    walkImpl initialState n
    |> ignore // don't care about state, because it's for internal use only

[<EntryPoint>]
let main argv =
    walk (Children [Value 1; Value 2 ; Children [Value 9; Children [Value 4; Value 5]; Value 9]])
    0


Reverse words in Java Short and Spicy question is that If input is how are you and output should beyou are how.

public class ReverseWord {
    public static void main(String[] args) throws IOException{
        String input;
        BufferedReader br = new BufferedReader(new InputStreamReader(System.in));
        System.out.println("Enter the input string");
        input = br.readLine();
        System.out.println("Enterd String:"+input);
        System.out.println("Reversed:"+reverse(input));
    }

    public static String reverse(String input){
        String delimiter = " ";
        String[] splits = input.split(delimiter);
        String reverse = new String("");
        for(int i=splits.length-1; i>=0; i--){
            //System.out.println(splits[i]);
            reverse += splits[i];
            reverse = reverse.concat(delimiter);
        }
        System.out.println(reverse);
        return reverse;
    }
}

How can I improve my code? And Is there any other way to do it with less time and space complexity?

The odd behaviour is that the result gets printed twice: once in reverse(), and again in main().  reverse() should be fixed so that it returns the result without printing it.

The output will also include an extra space at the end.  It may be invisible, but I would consider it technically incorrect.

Your code, with those issues fixed, and a few tweaks:

public static String reverse(String input) {
    String delimiter = " ";
    String[] splits = input.split(delimiter);
    String reverse = "";                            // Same as new String("")
    for (int i = splits.length - 1; i > 0; i--) {   // Be more generous with spaces
        reverse += splits[i] + delimiter;           // Get rid of .concat()
    }
    if (splits.length > 0) {
        reverse += splits[0];
    }
    return reverse;
}

However, repeated string concatenation is inefficient: every += involves allocating a new buffer and copying everything.  You should really be using a StringBuilder, like this.  (Other suggested solutions in that question are also quite good.  You should study those answers and form your own opinion.)


Parsing JSON in one go using state machine solution I need to parse a simple JSON string (flat JSON, no hierarchy) for keys and values, and there is a system constraint that I cannot use any built-in JSON library and can only read a string once due to latency requirements. I need to use Python 2.7.x series and cannot use a higher version.

Note: This question is version 3 of my code to solve this problem.

Version 1 - answered by @holroy
Version 2 - answered by @Tersosauros

This is a new version of code based on guidance from @Tersosauros' post of the previous version. Note that I still do not handle some corner cases. I do not want to use two state variables as with @Tersosauros' code, but rewrote using only one state variable. I am not 100% confident if my code is correct logic and your advice is highly appreciated.

READY = 0
COLON = 1
COMMA = 2
STRING = 3
NUMBER = 4

numbers='0123456789.'
NoOps=' {'

def parse(strJSON):
    state = READY
    lastKey=''
    lastValue=''
    temp=''
    result={}
    for c in strJSON:
        if state == READY:
            if c in NoOps:
                continue
            elif c == '"':
                state = STRING
            elif c == ':':
                lastKey=temp
                temp=''
                state = READY
            elif c == ',' or c=='}':
                lastValue=temp
                temp=''
                if lastKey:
                    result[lastKey]=lastValue
                    lastValue=''
                    lastKey=''
                    state=READY
            elif c in numbers:
                state=NUMBER
        elif state == STRING:
            if c == '"':
                state = READY
            else:
                temp += c
        if state == NUMBER:
            if c in NoOps or c==',' or c=='}':
                lastValue=temp
                temp=''
                if lastKey:
                    result[lastKey]=lastValue
                    lastValue=''
                    lastKey=''
                state=READY
            else:
                temp+=c

    if lastKey:
            result[lastKey]=lastValue

    return result

if __name__ == "__main__":
    #JSONString = '{ "id": 1, "name": "A green door", "price": 12.50, "tags": ["home", "green"]}'
    JSONString1 = '{ "id": "1", "name": "A green door", "price": "12.50", "tags": "home green"}'
    JSONString2 = '{ "id": 1, "name": "A green door", "price": 12.50, "tags": "home green"}'
    print parse(JSONString1)
    print parse(JSONString2)

Variable Names

Apart from the odd (non PEP8 compliant) variable casing, you also have a temp variable.  Names like temp are frowned upon as they convey nothing about what that variable represents.

Repeated Tests

You do seem to repeat the tests c==',' or c=='}': a couple of times (lines 26 and 42. I think this could be refactored into using a test string, similar to your NoOps variable.

For example, on line 42, instead of:

        if c in NoOps or c==',' or c=='}':

You could just do

        if c in NoOps or c in CloseOps:

Assuming you had a variable CloseOps defined in the preamble, like this:

 CloseOps = ",}"

The same thing would apply for the repeated use of this test condition on line 26.

Repeated Logic

The following code (lines 45 - 49 inclusive):

        if lastKey:
                result[lastKey]=lastValue
                lastValue=''
                lastKey=''
        state=READY

Appears to be an (almost) verbatim repeat of the logic contained upon lines 29 - 33 (inclusive):

            if lastKey:
                result[lastKey]=lastValue
                lastValue=''
                lastKey=''
                state=READY

Functional Decomposition would dictate that such repeated logic be refactored into it's own function.

Numbers aren't Strings!

There exists a flaw in your code (which was not present in the answer code to your last question), where-by JSON-encoded numbers are turned into strings in your resulting dict.

If you (using your parse function implementation from this question) do:

 >>> type(parse('{"foo":"1"}')['foo'])

This yields:

 <type 'str'>

(Which is fine, since here "1" should be a string.  However:

 >>> type(parse('{"foo":1}')['foo'])

Also yields:

 <type 'str'>

Which indicates an error, as this should be <type 'int'> if the JSON were decoded properly.


Insane template metaprogramming madness Given any callable, deduces the first parameter in the parameter list of the function. Be ware, not for light hearted.

template <typename ... Args>
struct pick_first;

template <typename T, typename ... Args>
struct pick_first<T, Args ...>
{
    using type = T;
};

template <typename ... Args>
using pick_first_t = typename pick_first<Args...>::type;

template <typename Callable>
struct deduce_first_parameter
{
private:
    template <typename Func, template <typename ...> typename Operation>
    struct extract_function;

    template <typename R, typename ... Args, template <typename ...> typename Operation>
    struct extract_function<R(Args...), Operation>
    {
        using type = typename Operation<Args...>::type;
    };

    template <typename C, typename R, typename ... Args, template <typename ...> typename Operation>
    struct extract_function<R (C::*)(Args...), Operation>
    {
        using type = typename Operation<Args...>::type;
    };

    template <typename Func, template <typename ...> typename Operation>
    using extract_function_t = typename extract_function<Func, Operation>::type;

public:
    using type = extract_function_t<decltype(&Callable::operator()), pick_first>;
};

template <typename R, typename ... InputArgs>
struct deduce_first_parameter<R(InputArgs...)>
{
    using type = pick_first_t<InputArgs...>;
};

Some tests:

#include <vector>
#include <map>

struct dummy
{
    void operator()(std::vector<int>&) {}
    using correct_answer = std::vector<int>&;
};

struct not_so_dummy
{
    using correct_answer = const std::vector<std::map<int, char>>&;
    void operator()(const std::vector<std::map<int, char>>&) {}

};

void f(char, int);

int main()
{
    static_assert(std::is_same<deduce_first_parameter<decltype(f)>::type, char>::value);
    static_assert(std::is_same<deduce_first_parameter<dummy>::type, dummy::correct_answer >::value);
    static_assert(std::is_same<deduce_first_parameter<not_so_dummy>::type, not_so_dummy::correct_answer>::value);
}

After about two weeks of intense Java, I started missing C++. Especially templates.

Motivation:

I'm writing generic sliding window, which is complete, but I don't want to show it yet (I figured that it is possible to do stunning things with it). To supplement it, I need preprocessing iterator, which applies a user supplied functor before assigning to underlying iterator. To make the preprocessing iterator easy to use I need to deduce the first parameter's type to put into std::iterator<> (I know its deprecated, but some compilers still use it to detect iterators), because binding users to what underlying iterator can accept is awkward.

How to compile:

I'm afraid not that many compilers can handle this. I've tested it on clang++-3.9. There might be some warnings, they should be due to using C++17 feature to make template template parameters declared by typename possible (good luck on understanding this sentence).

I'm interested in any critique, but especially about reducing instantiation depth and count. Also fitting statements in one readable line would be great.

You can figure out the first parameter of a non-overloaded function pointer pretty easily; I'm not sure but your code looks more complicated than it ought to be.

Unfortunately, your code completely falls down in the presence of overloads and/or templates. Here's the error message:

prog.cc:36:46: error: reference to overloaded function could not be resolved; did you mean to call it?
    using type = extract_function_t<decltype(&Callable::operator()), pick_first>;
                                             ^~~~~~~~~~~~~~~~~~~~~

and here are some types that trigger it:

struct oops1 {
    void operator()(int);
    void operator()(double);
};

struct oops2 {
    void operator()(int);
    void operator()(int) const;
};

void oops3(int);
void oops3(double);

struct oops4
{
    using correct_answer = const std::vector<std::map<int, char>>&;
    template<class T, std::enable_if_t<std::is_same_v<T, const std::vector<std::map<int, char>>>, int> = 0>
        void operator()(T&) {}
};

And here's another error message that I get:

prog.cc:11:1: error: implicit instantiation of undefined template 'pick_first<>'
using pick_first_t = typename pick_first<Args...>::type;
^

with this input:

void oops5(void);

If I were tackling this problem, I would first ask myself what "the type of the first parameter" even means in C++, given that we have function overloading and templates. But then I might try to detect the appropriate type by messing around with

struct ConvertibleToAnything {
    template<class T> operator T() const;
};

and then inspecting f(ConvertibleToAnything{}) and trying to figure out which operator T was getting instantiated.

In the cases where overloading and/or templates rendered the question meaningless, I would try to at least give a nice static_assert error message, if not actually produce a tag type so that is_same_v<deduce_first_parameter_t<oops1>, signature_is_ambiguous_t>.


Long running in-process Publish/Subscribe class I have a Pub/Sub class that will call Notify() when published and that Notify() could take around 10 seconds, so I need to make sure that this does not block the UI.

public static class NotificationService
{
    private static Dictionary<string, INotification> Observers { get; }

    static NotificationService()
    {
        Observers = new Dictionary<string, INotification>();
    }

    public static void Publish(string title, string requester)
    {
        foreach (var observer in Observers)
        {
            var notification = observer.Value;

            new Thread(() =>
            {
                Thread.CurrentThread.IsBackground = true;
                notification.Notify(title, requester);
            }).Start();

        }
    }

    public static void Subscribe(INotification notification)
    {
        INotification notificationValue;
        if (Observers.TryGetValue(notification.NotificationName, out notificationValue))
        {
            // Observer already exists
            return;
        }

        Observers[notification.NotificationName] = notification;
    }

    public static void UnSubscribe(INotification notification)
    {
        INotification notificationValue;
        if (!Observers.TryGetValue(notification.NotificationName, out notificationValue))
        {
            // Observer doesn't exists
            return;
        }
        Observers.Remove(notification.NotificationName);
    }
}

INotification

public interface INotification
{
    string NotificationName { get; }

    bool Notify(string title, string requester);
}

Example implementation of INotification

public class EmailMessageNotification : INotification
{
    public EmailMessageNotification(EmailNotificationSettingsService settings)
    {
        EmailNotificationSettings = settings;
    }

    private EmailNotificationSettingsService EmailNotificationSettings { get; }
    public string NotificationName => "EmailMessageNotification";
    public bool Notify(string title, string requester)
    {
        var configuration = GetConfiguration();
        if (!ValidateConfiguration(configuration))
        {
            return false;
        }

        var message = new MailMessage
        {
            IsBodyHtml = true,
            To = { new MailAddress(configuration.RecipientEmail) },
            Body = $"User {requester} has requested {title}!",
            From = new MailAddress(configuration.EmailUsername),
            Subject = $"New Request for {title}!"
        };

        try
        {
            using (var smtp = new SmtpClient(configuration.EmailHost, configuration.EmailPort))
            {
                smtp.Credentials = new NetworkCredential(configuration.EmailUsername, configuration.EmailPassword);
                smtp.EnableSsl = configuration.Ssl;
                smtp.Send(message);
                return true;
            }
        }
        catch (SmtpException smtp)
        {
            Log.Fatal(smtp);
        }
        catch (Exception e)
        {
            Log.Fatal(e);
        }
        return false;
    }

    private EmailNotificationSettings GetConfiguration()
    {
        // Gets and Returns Config
    }

    private bool ValidateConfiguration(EmailNotificationSettings settings)
    {
        // Do some validation
    }

Can anyone see anything wrong with this?

It seems that you have a global static logger methods. It is not a good practice, as you're loosing the possibility to tune logging levels for different areas of your application (e.g. setting log level for EmailMessageNotification to DEBUG while keeping INFO level everywhere else. I recommend to follow the logging patterns suggested for your logging framework (see Creating loggers for NLog for example).
The code is not thread-safe, so you may get issues if your application publishes, subscribes or unsubscribes in different threads. Use either locking around Observers collection or ConcurrentDictionary.
Fatal logging level should be used only when the application encounters a critical error after which it usually cannot continue to run. In your case I would suggest to use Error log level when email cannot be sent.
NotificationService is static, which means that you will have a strong dependencies in your code on this class. I suggest to introduce IoC framework (check out Autofac or StructureMap).
The response of INotification.Notify(string, string) is not used, so it can be void.
Your current code requires spinning up threads per each observer per each notification. I suggest to switch to asynchronous processing model, changing the return type of INotification.Notify(string, string) to Task.
Most likely you would want to wrap the notification of each listener in try..catch to avoid side effects when buggy INotification subscribes to your service

As a result:

INotification:

public interface INotification
{
    string NotificationName { get; }

    Task NotifyAsync(string title, string requester);
}

EmailMessageNotification:

public class EmailMessageNotification : INotification
{
    private static Logger logger = LogManager.GetCurrentClassLogger();

    public EmailMessageNotification(EmailNotificationSettingsService settings)
    {
        EmailNotificationSettings = settings;
    }

    private EmailNotificationSettingsService EmailNotificationSettings { get; }

    public string NotificationName => "EmailMessageNotification";

    public async Task NotifyAsync(string title, string requester)
    {
        var configuration = GetConfiguration();
        if (!ValidateConfiguration(configuration))
        {
            return;
        }

        var message = new MailMessage
        {
            IsBodyHtml = true,
            To = { new MailAddress(configuration.RecipientEmail) },
            Body = $"User {requester} has requested {title}!",
            From = new MailAddress(configuration.EmailUsername),
            Subject = $"New Request for {title}!"
        };

        try
        {
            using (var smtp = new SmtpClient(configuration.EmailHost, configuration.EmailPort))
            {
                smtp.Credentials = new NetworkCredential(configuration.EmailUsername, configuration.EmailPassword);
                smtp.EnableSsl = configuration.Ssl;
                await smtp.SendMailAsync(message).ConfigureAwait(false);
            }
        }
        catch (SmtpException smtp)
        {
            logger.Error(smtp);
        }
        catch (Exception e)
        {
            logger.Error(e);
        }
    }

    private EmailNotificationSettings GetConfiguration()
    {
        throw new NotImplementedException();
    }

    private bool ValidateConfiguration(EmailNotificationSettings settings)
    {
        throw new NotImplementedException();
    }
}

NotificationService:

public interface INotificationService
{
    void Publish(string title, string requester);
    void Subscribe(INotification notification);
    void UnSubscribe(INotification notification);
}

public class NotificationService : INotificationService
{
    private static Logger logger = LogManager.GetCurrentClassLogger();
    private ConcurrentDictionary<string, INotification> Observers { get; } = new ConcurrentDictionary<string, INotification>();

    private static async Task NotifyAsync(INotification notification, string title, string requester)
    {
        try
        {
            await notification.NotifyAsync(title, requester).ConfigureAwait(false);
        }
        catch (Exception ex)
        {
            logger.Error(ex, $"Notification '{notification.NotificationName}' failed with exception");
        }
    }

    public async void Publish(string title, string requester)
    {
        IEnumerable<Task> notificationTasks = Observers.Values.Select(notification => NotifyAsync(notification, title, requester));

        await Task.WhenAll(notificationTasks).ConfigureAwait(false);
    }

    public void Subscribe(INotification notification)
    {
        Observers.TryAdd(notification.NotificationName, notification);
    }

    public void UnSubscribe(INotification notification)
    {
        Observers.TryRemove(notification.NotificationName, out notification);
    }
}


optimize nested for loops within nested if statements I am trying to optimize this code. The only optimization I can think of is to ?¿? a return or break statement after applyOfferChanges(…) inside the second if condition which does not work either because there could be multiple matches. I cannot either use a map for favoriteMerchantsList because getFavoriteMerchantsList() return type is a list (of favorited merchants). Any ideas?

void applyFavoriteChangesToMerchantStore(){

    List<Merchant> favoriteMerchantsList = FavoriteMerchantStore.getInstance().getFavoriteMerchantsList();
    if(favoriteMerchantsList != null && !favoriteMerchantsList.isEmpty()) {
        List<Merchant> storeMerchantList = MerchantStore.getInstance().getMerchantList();
        for (Merchant storeMerchant : storeMerchantList) {
            for (Merchant favoriteMerchant: favoriteMerchantsList){
                if(TextUtils.equals(storeMerchant.getId(), favoriteMerchant.getId())){
                    //merchant match found
                    //set merchant favorite status

                   storeMerchant.setFavoriteMerchant(favoriteMerchant.getFavoriteMerchant());
                    //set offer favorite status
                    applyOfferChanges(favoriteMerchant.getOffferList(),
                            storeMerchant.getOffferList());
                }
            }
        }
    }
}

You are implementing an inner join as a nested loop join. There are several other ways to implement joins, such as hash-based or sort-merge sort. Which one is better depends on the length of the lists.

I will sketch a hash join as that usually works most of the time.

void applyFavoriteChangesToMerchantStore(){

    List<Merchant> favoriteMerchantsList = FavoriteMerchantStore.getInstance().getFavoriteMerchantsList();
    if(favoriteMerchantsList != null && !favoriteMerchantsList.isEmpty()) {
        // build the hash table
        Map<String, Merchant> favoriteMerchantById = favoriteMerchantsList.stream().collect(Collectors.toMap(Merchant::getId, Function.identity()));
        List<Merchant> storeMerchantList = MerchantStore.getInstance().getMerchantList();
        for (Merchant storeMerchant : storeMerchantList) {
            // probe the hash table
            Merchant favoriteMerchant = favoriteMerchantById.get(storeMerchant.getId())
            if(favoriteMerchant != null){
                //merchant match found
                //set merchant favorite status

               storeMerchant.setFavoriteMerchant(favoriteMerchant.getFavoriteMerchant());
                //set offer favorite status
                applyOfferChanges(favoriteMerchant.getOffferList(),
                        storeMerchant.getOffferList());
            }
        }
    }
}

I assumed that TextUtils.equals is equal to String.equals. If it's more lenient (e.g., case-insensitive), you need to normalize the lookup key (e.g., String.toLowerCase()).


HackerRank - Candies Here is the solution I've written for Hacker Rank Candies challenge.
It uses dynamic programming (memoization in a boxed array).
I'm interested on what could be done to improve this code in term of style and performance (although it passes all the testcases of the challenge)
For example, I wonder if the bunch of guards I used could be made prettier

import System.IO
import Data.Array

main = do
    contents <- getContents
    print $ runTest contents

runTest :: String -> Int
runTest = sum . candies . map read . tail . lines

candies :: [Int] -> [Int]
candies rs = elems candies where
    go i
        | n ==1 = 1
        | i == 1 = if (ri > riplus) then candies ! (2) + 1 else 1
        | i == n = if (ri > riminus) then candies ! (n - 1) + 1 else 1
        | ri > riminus  && ri > riplus = max (candies! (i-1)) (candies! (i+1)) + 1
        | ri > riminus = candies! (i-1) + 1
        | ri > riplus = candies! (i+1) + 1
        | otherwise = 1
        where
            ri = ratings!i
            riplus = ratings!(i+1)
            riminus = ratings!(i-1)
    ratings = listArray (1,n) rs
    candies = listArray (1,n) [go i | i <- [1..n]]
    n = length rs

You might consider using the trick of extending your array by one on both ends, ie. to include indices 0 and n+1, and assign:

ratings[0] = ratings[n+1] = 0
candies[0] = candies[n+1] = 0

and then go 1 and go n will be handled by the other clauses.

Also, you can eliminate the boundary assignments in go with:

candies = listArray (0,n+1) $ [0] ++ [go i | i <- [1..n] ] ++ [0]

Finally you can use these definitions to make the guards more succinct:

go i | incL && incR  = 1 + max cL cR
     | incL          = 1 + cL
     | incR          = 1 + cR
     | otherwise     = 1
  where incL = (ratings!(i-1)) < (ratings!i)
        incR = (ratings!i) > (ratings(i+1))
        cL   = candies!(i-1)
        cR   = candies!(i+1)


Reduce duplicate sequences I am writing a copy paste detector using esprima.js and Hmm.

This particular function checks a number of sequences and reduces them.
Due to the parsing I might have 2 sequences with the same piece of text, if so I need to merge the 2 sequences and add the start/ends ( both are source token indexes ) and occurrences ( contains location of the dupes in the original source ).

Obviously I could apply some DRY to this code, but I feel that every time I start I actually make the code less readable. Also I am not too happy with the naming, is there some good vocabulary out there for comparing sequences what is a good name for sequences[inner] and sequences[outer].

Anyway, all feedback is welcome, though I know that I am on a lonely crusade that ~ is so superior to -1 that I am sticking it to it indefinitely.

  Tokens.prototype.reduceSequences = function()
  {
    var sequences = this.sequences,
        tokens = this.raw();

    if( sequences.length < 2 )
      return;

    for( var outer = 1 ; outer < sequences.length ; outer++)
    {
      for( var inner = 0 ; inner < outer ; inner ++)
      {
        if( sequences[inner].content == sequences[outer].content )
        {

          if( !~sequences[inner].starts.indexOf( sequences[outer].starts[0] ) )
          {
            sequences[inner].starts.push( sequences[outer].starts[0] );
            sequences[inner].ends.push( sequences[outer].starts[0] + sequences[inner].tokenCount );
            sequences[inner].occurences.push( sequences[outer].occurences[0] );
          }

          if( !~sequences[inner].starts.indexOf( sequences[outer].starts[1] ) )
          {
            sequences[inner].starts.push( sequences[outer].starts[1] );
            sequences[inner].ends.push( sequences[outer].starts[1] + sequences[inner].tokenCount );
            sequences[inner].occurences.push( sequences[outer].occurences[1] );
          }

          sequences.splice(outer, 1);
          outer = 0;
          break;
        }
        if( ~sequences[inner].content.indexOf( sequences[outer].content )  )
        {
          sequences.splice(outer, 1);
          outer = 0;
          break;
        }
      }
    }
  }

A use case would be having 5 sequences ( sequences come in sorted by length )

[ 'I'm a sequence'(found on line 1&7) ,
  'I'm a sequence'(found on line 1&9) ,
  'I'm a sequence'(found on line 7&9) ,
  'I'm also a sequence'(found on line 2&3)
  'a sequence'(found on line 1&7) ,
  'a sequence'(found on line 1&9) ,
  'a sequence'(found on line 2&3) ,
]

The outcome would be

[ 'I'm a sequence'(found on line 1&7&9) ,   <-- Note how 9 got added
  'I'm also a sequence'(found on line 2&3)
   <- No more 'a sequence' , it is a substring of the first sequence
]

The algorithm starts taken sequence 1 till the end, and compares to all the prior sequences for dupes and larger sequences.

It looks like you could use reduce to your advantage, and extracting a function to merge two sequences. (Edit: messed up 1st version; this should be better)

Tokens.prototype.reduceSequences = function () {
  // source and destination seem like slightly better names
  function merge(source, destination) {
    // can't merge, so return false
    if( !~destination.content.indexOf( source.content ) )
      return false;

    // no need to actually merge (source included in destination),
    // so just return true
    if( destination.content != source.content )
      return true;

    // otherwise, merge
    for( var i = 0, l = source.starts.length ; i < l ; i++ ) {
      if( ~destination.starts.indexOf(source.starts[i]) ) continue;
      destination.starts.push( source.starts[i] );
      destination.ends.push( source.starts[i] + destination.tokenCount );
      destination.occurences.push( source.occurences[i] );
    }

    return true;
  }

  if(this.sequences.length < 2) return;

  this.sequences = this.sequences.reduce(function (sequences, current) {
    var merged, i, l;

    for( i = 0, l = sequences.length ; i < l && !merged ; i++ )
      merged = merge(current, sequences[i]);

    if( !merged )
      sequences.push(current);

    return sequences;
  }, []);
}

I think that should work.

Oh, and yes, I've undone your brace-on-new-line style - sorry, couldn't help myself. But I've left out braces I'd usually include, so I've met you halfway! :)

If reduce is not an option, I can't really come up with anything besides extracting a merge function (basically a "lighter" refactoring). Like so

Tokens.prototype.reduceSequences = function () {
  // source and destination seem like slightly better names to use *here*
  // but they don't make as much sense in the loops below, IMO
  function merge(source, destination) {
    for( var i = 0, l = source.starts.length ; i < l ; i++ ) {
      if( ~destination.starts.indexOf(source.starts[i]) ) continue;
      destination.starts.push( source.starts[i] );
      destination.ends.push( source.starts[i] + destination.tokenCount );
      destination.occurences.push( source.occurences[i] );
    }
  }

  var sequences = this.sequences;

  for( var outer = 1 ; outer < sequences.length ; outer++) {
    for( var inner = 0 ; inner < outer ; inner ++) {
      if( sequences[inner].content == sequences[outer].content ) {
        merge(sequences[outer], sequences[inner]);
        sequences.splice(outer, 1);
        outer = 0;
        break;
      }

      if( ~sequences[inner].content.indexOf( sequences[outer].content )  ) {
        sequences.splice(outer, 1);
        outer = 0;
        break;
      }
    }
  }
}

It's not brilliant, but either way it feels a tiny bit cleaner to encapsulate the "merge" step in a function.

By the way, var tokens = this.raw() didn't seem to be used for anything.

It might be more beneficial to simply add the merge function to your sequence objects, though. It'd clean up this particular code nicely. But you know better than I if something like that would make sense in your project.


Using static for class default properties I have made a class for generating html form elements:

class input
{
  public $tag = 'input', $type = 'text', $customClass, $name;

  public function __construct(array $cfg){
    if ( isset($cfg['name']) ){
      if ( isset($cfg['tag']){
        $this->tag = strtolower($cfg['tag']);
      }
      if ( $this->tag === 'input' && isset($cfg['type']){
        $this->type = $cfg['type'];
      }
      if ( isset($cfg['customClass']){
        $this->customClass= $cfg['customClass'];
      }
      //etc...
    }
  }
}

I would like to be able to set a custom default config for all my fields - without changing the class. For example I want all the fields in a given script to have 'myClass' as customClass. For me, the solution would be to make these default values static, and make a function to change them like this:

class input
{
  static $tag = 'input', $type = 'text', $customClass, $name;

  public static function setDefault(array $cfg){
    if ( isset($cfg['tag']) ){
      self::tag = strtolower($cfg['tag']);
    }
    if ( isset($cfg['type']) ){
      self::type = $cfg['type'];
    }
    if ( isset($cfg['customClass']) ){
      self::customClass= $cfg['customClass'];
    }
    //etc...
  }

  public function __construct(array $cfg){
    if ( isset($cfg['name']) ){
      $this->tag = isset($cfg['tag']) ? strtolower($cfg['tag']) : self::tag;
      $this->type = isset($cfg['type']) ? strtolower($cfg['type']) : self::type;
      $this->customClass = isset($cfg['customClass']) ? $cfg['customClass'] : self::customClass;
      //etc...
    }
  }
}

The problem is that I read everywhere - and particulary here - that static are evil. What other solution would be suitable in the present case? Or can someone tell me "yeah, that's the perfect case to use statics!"? :)

You could either a configuration object or a builder for this. As you probably can see yourself, using static gets complicated quite fast.

Using a builder:

class InputBuilder
{
    private $_defaults = array(
        'tag' => null,
        'type' => null,
    );

    private $_current;

    public function __construct(array $defaults = null)
    {
        foreach ($defaults as $key => $value) {
            if (array_key_exists($this->_defaults[$key])) {
                $this->_defaults[$key] = $value;
            }
        }

        $this->reset();
    }

    public function reset()
    {
        $this->_current = array();
        foreach ($this->_defaults as $key => $value) {
            $this->_current[$key] = $value;
        }
    }

    public function setTag($tag)
    {
        if (!is_string($tag)) {
            throw new InvalidArgumentException('...');
        }
        $this->_current['tag'] = $tag;
    }

    public function setType($type)
    {
        if (!is_string($type)) {
            throw new InvalidArgumentException('...');
        }
        $this->_current['type'] = $type;
    }

    public function getInput()
    {
        return new Input($this->_current);
    }
}

Usage would be:

$builder->setType('text')
        ->build();  // tag: input, type: text
$builder->setType('password')
        ->build(); // tag: input, type: password
$builder->reset(); // reset everything to defaults
$builder->setTag('textarea') // tag: textarea: type: text
        ->build();

The builder pattern is extremely strong if you want to build the object programmatically. (SQL) Query-builder are a very common use case for this.

Using a configuration object:

class InputConfiguration
{
        private $_init = array(
            'tag' => null,
            'type' => null,
        );

        private $_current;

        public function __construct(array $defaults = null)
        {
            foreach ($defaults as $key => $value) {
                if (array_key_exists($this->_defaults[$key])) {
                    $this->_defaults[$key] = $value;
                }
            }
        }

        public function configure(Input $element)
        {
            $config = $this->_init['tag'];

            if (null !== $config['tag'] && null !== $element->getTag()) {
                $element->setTag($config['tag']);
            }

            if (null !== $config['type'] && null !== $element->getType()) {
                $element->setType($config['type']);
            }
        }
}

class Input
{
     public function construct(InputConfiguration $config = null)
     {
           if (null !== $config) {
               $config->configure($this);
           }
     }

     // ...
}

Usage would be:

$configuration = new InputConfiguration(
    'tag' => 'input',
    'type' => 'text',
);

$newObject = new Input($configuration); // has defaults applied

$newObject2 = new Input(); // we don't want to use defaults
$configuration->configure($newObject2); // or decide to apply them later on

The configuration object decouples the configuration from your object. Furthermore it makes one configuration reusable and optional.


Dual-mode JavaScript textbox watermark I was just trying to make an alternate Ajax watermark from "insert date" (first mode) to "mm/dd/yyyy" (second mode). After some trial and error, I successfully implemented a solution using JavaScript.

How can I improve this script functionality?

JavaScript:

var original = "insert date";
var alternate = "mmddyyyy";
var countalerted = 0;
var txt = "";
var intird = 0;
if (intird == 0) {
    intrId = setInterval(function () { alterWaterMarkForDateTBX() }, 1000);
}
function SetIntervalTBX() {
    setInterval(function () { alterWaterMarkForDateTBX() }, 1000);
}

function alterWaterMarkForDateTBX() {
    var d = new Date();
    var t = d.toLocaleTimeString();

    if (countalerted < 4) {
        if (countalerted % 2 == 0) {
            document.forms["form1"].elements["TBXinsertDate"].value = original;
        }
        else {
            document.forms["form1"].elements["TBXinsertDate"].value = alternate;
        }
        countalerted++;
    }
    else {
        window.status = intrId;
        clearInterval(intrId);
//        document.forms["form1"].elements["TBXinsertDate"].value = original;
    }
}

var wasFocused = false;var wasBlur = false;
function Focus(objname, waterMarkText) {
    wasFocused = true;
    if (wasBlur == false) {
        obj = document.getElementById(objname);
        if (obj.value == waterMarkText) {
            obj.value = "";
            obj.className = "NormalTextBox";
            if (obj.value == original || obj.value == alternate || obj.value == "" || obj.value == null) {
                obj.style.color = "black";
            }
        }
    }
}

//i added "__/__/____" for the value of the ajax MaskEdit

function Blur(objname, waterMarkText) {
    wasBlur = true;
    var alternateWM1 = "insert date";
    var alternateWM2 = "mm/dd/yyyy";
    obj = document.getElementById(objname);
    if (obj.value == "" || obj.value == "__/__/____") {
        obj.value = waterMarkText;
        obj.className = "WaterMarkedTextBox";

    }
    else {
        obj.className = "NormalTextBox";
    }

    if (obj.value == original || obj.value == alternate || obj.value == "" || obj.value == null || obj.value == "__/__/____") {
        obj.style.color = "gray";
        if (wasFocused == true) {
            SetIntervalTBX();
        }
    }
}

ASPX / HTML

<form id="form1" runat="server">
    <asp:TextBox ID="TBXinsertDate" runat="server"
     ToolTip="date Box"  Width="79px"
     onfocus="Focus(this.id, this.value)"
     onblur="Blur(this.id, this.value)" >
    </asp:TextBox>
 </form>

Some notes:

Think about reusability.  Your code integrates the field names directly into the functions.  Same goes for some of your choices of function names (alterWaterMarkForDateTBX). This makes it difficult to reuse next time you need this function.
You appear to be mixing ideas.  You set a class name sometimes, set the text color other times.  I'm not sure if this was intentional but I just changed it all to class names.
You had some dead variables.  Stuff like this just make it harder to edit in the future.  Remove them.
var d = new Date(); var t = d.toLocaleTimeString();
You are checking to make sure the field had focus before it blurred (and vice versa).  And then Not sure what the purpose of that is.
Your focus and blur events pass in the textbox name and the textbox value (as watermarkText.  Then you check the value against a couple of other values and if it matches set the textbox back to watermarkText.  This will never accomplish anything and is confusing.  Just pass in the textbox itself.
Think about non-intrusive javascript.  It is easier to maintain.  Notice how I added the blur and focus events to the field through javascript instead of directly in the html.
You should try to avoid global objects.  Look into putting this entire code block into an object.  The problem you will face is what if you have two fields on the same form that need this functionality.  The changes I have made below will go a long way in helping but the global interval and global counter will conflict with one another and it will not work.

EDIT:

I was interested in this problem. So I extended my code.

I moved the main work into a function object.  This can now be called using a constructor which passes in the text box.  This also allows for multiple textboxes using this even on the same page.
I added an options object to the parameters.  This will allow you to change the functionality of the script without changing the actual code.  This allows for reusability without needing to modify the code (open-closed principle).  This is also a common technique used in many of the JS frameworks (jQuery, mooTools, etc..)

Styles:

.NormalTextBox {
color = "black";
}
.WaterMarkedTextBox {
color = "grey";
}

Script:

    var MultiWatermark = function (obj, options) {

    var defaultOptions = {
        // These variables can be changed to alter functionality using the passed in options object
        watermarkVals: ["insert date", "mmddyyyy"], // all watermark values.
        totalIterations: 2, // how many times to go through watermark Array.
        watermarkClassName: "WaterMarkedTextBox",
        normalClassName: "NormalTextBox",
        changeDelay: 1000, // in milliseconds
        extraValsToCheck: ['', null]//
    }

    //merge options with defaultOptions
    for (var property in defaultOptions) {
        if (!options.hasOwnProperty(property)) {
            options[property] = defaultOptions[property];
        }
    }

    // local variables (do not change)
    var textBox = obj;
    var iterationCounter = 0;
    var itemCounter = 0;
    var intrId;

    var startWatermark = function () {
        itemCounter = 0; iterationCounter = 0; // resets for after blur
        intrId = setInterval(function () { alterWaterMark() }, options.changeDelay);
    }

    var stopWatermark = function () {
        clearInterval(intrId);
    }

    var alterWaterMark = function () {

        if (iterationCounter < options.totalIterations) {

            textBox.value = options.watermarkVals[itemCounter];
            itemCounter += 1;
            if (itemCounter == options.watermarkVals.length) {
                itemCounter = 0;
                iterationCounter += 1;
            }
        }
        else {
            stopWatermark();
        }

    }

    var isWatermarkValue = function (val) {

        var i = options.watermarkVals.length;
        while (i--) {
            if (options.watermarkVals[i] === val) {
                return true;
            }
        }
        // value is not in watermark array, so check secondary values.
        i = options.extraValsToCheck.length;
        while (i--) {
            if (options.extraValsToCheck[i] === val) {
                return true;
            }
        }
    }

    var setClass = function () {
        textBox.className = isWatermarkValue(textBox.value) ? options.watermarkClassName : options.normalClassName;
    }

    var focusEvent = function () {

        stopWatermark();
        if (isWatermarkValue(textBox.value)) {
            textBox.value = "";
        }
        setClass()
    }

    var blurEvent = function () {

        if (isWatermarkValue(textBox.value)) {
            startWatermark();
        }
        setClass();

    }

    var initWatermark = function () {
        setClass();
        textBox.onfocus = focusEvent;
        textBox.onblur = blurEvent;
        startWatermark();
    }

    initWatermark();
}

Body:

<form id="form1" runat="server">
    <input ID="TBXinsertDate" ToolTip="date Box"  Width="79px" />
    <input ID="Text1" ToolTip="date Box"  Width="79px" />
 </form>

<script>

var dateOptions = {
    // These variables alter functionality, the main script will use the defaults of any options not included here
    watermarkVals: ["insert date", "mmddyyyy", "mm/dd/yyyy"], // all watermark values.
    totalIterations: 4,
    changeDelay: 500, // in milliseconds
    extraValsToCheck: ['', null, '__/__/____', 'test']//
}
new MultiWatermark(document.forms["form1"].elements["TBXinsertDate"], dateOptions);
new MultiWatermark(document.forms["form1"].elements["Text1"], dateOptions);
</script>
</body>
</html>


Saving a webpage to text file I'm trying to save a webpage as text file, using the following function:

def html_to_txt():
    import urllib.request
    url = str(input('Enter URL: '))
    page = urllib.request.urlopen(url)
    with open(str(input('Enter filename: ')), "w") as f:
        for x in page:
            f.write(str(x).replace('\\n','\n'))
    s= 'Done'
    return s

It gives a correct output, that means I can save a given webpage. But, the final text file contains a b' before the very first line, and a 'b' before each following line.

Here's a sample output of my function (taken from: http://www.uniprot.org/uniprot/P57052.txt)

$ cat P57052.txt
b'ID   RBM11_HUMAN             Reviewed;         281 AA.
'b'AC   P57052; Q6YNC2; Q8NBA1; Q8NFF6;
'b'DT   01-DEC-2000, integrated into UniProtKB/Swiss-Prot.
'b'DT   01-DEC-2000, sequence version 1.
'b'DT   09-JUL-2014, entry version 111.
'b'DE   RecName: Full=Splicing regulator RBM11;
'b'DE   AltName: Full=RNA-binding motif protein 11;
'b'GN   Name=RBM11;

I wonder if it has something to do with binary file "default", I mean, if the webpage is  automatically saved with binary features by my python script.

Thanks in advance for your review.

As a review, the code needs some work with regard separation of concerns. A single function shouldn't get user input, get a URL and write to a file. If you try to write unit tests for it, you'll quickly see it's better to split the functionality.

With regard to the problem, try something as follows:

import urllib2

url = 'http://www.uniprot.org/uniprot/P57052.txt'
response = urllib2.urlopen(url)
with open('output.txt', 'w') as f:
    f.write(response.read())

I hope this helps.


Pretty print with unit and physical suffix I have a solution for pretty printing physical values with the correct suffix (kilo, mega etc.).

I'd like to discuss my solution with you.

# -*- coding: utf-8 -*-

import math
def eng_format(x, unit=''):
    e =(math.floor(math.log10(x))//3)*3
    sL = ['f', 'p', 'n', 'u', 'm', '', 'k', 'M', 'G']

    idx = int(e/3) + sL.index('')

    if idx<0 or idx>=len(sL):
        s = '*10^%d '%e
    else:
        s = sL[idx]

    nx = x * 10**(-e)
    return '%.2f%s%s'%(nx, s, unit)

if __name__ == '__main__':
    print eng_format(0.002, 'V')
    print eng_format(0.000003221, 'A')
    print eng_format(30589, 'A')
    print eng_format(60000000, 'W')

Screen output:

2.00mV
3.22uA
30.59kA
60.00MW

Use better variable naming, with proper vocabulary.  In particular, "significand" is the term to use instead of nx.
You multiplied some intermediate result by 3 to obtain e.  Then you immediately do idx = int(e/3) + …, which represents wasted work.  More importantly, clarity suffers.  I suggest replacing e with power_of_1000 = int(math.floor(math.log10(x) // 3)), whose intent is much clearer.
You can take advantage of the fact that negative indexes count backwards from the end of a list.
There should be a Space in front of each prefix for consistency with the '*10^%d ' format.

import math

def eng_format(x, unit=''):
    # U+03BC is Greek lowercase mu
    UNITS = [' ', ' k', ' M', ' G'] + \
            ([None] * 10) + \
            [' f', ' p', ' n', u' \u03bc', ' m']

    power_of_1000 = int(math.floor(math.log10(x) // 3))
    exponent = 3 * power_of_1000
    prefix = UNITS[power_of_1000]
    if prefix is None:
        prefix = '*10^%d ' % exponent

    significand = x * 10**(-exponent)
    return '%.2f%s%s' % (significand, prefix, unit)


Callback notation in a Mediator I'm implementing a Mediator and I'd like to simplify the callback notation a bit.

My mediator has Register and Notify methods :

public class Mediator {
    private Dictionary<Messages, Action<object>> registeredCallback;

    public void Register(Messages message, Action callback) {
        registeredCallback[message] = callback;
    }

    public void Notify(Messages message, object parameter) {
        callback(parameter);
    }

    public Mediator() {
        registeredCallback = new Dictionary<Messages, Action<object>>();
    }
}

It uses a Messages enumerator to sanitize events between registered objects, for example :

public enum Messages {
    UserCreated,
    UserDeleted,
    UserChangedPassword
};

So far, so good. However, when I use this mediator I'm making ugly callback definitions :

mediator.Register(Messages.UserCreated, (object user) => GreetUser(((User)user).name));

with the following User:

public class User {
    public string name;
}

Type casting? Lambda expressions? This looks obviously filthy. How can I clean this up? Is my architecture at fault?

What about making the Register method generic?

public class Mediator
{
    private Dictionary<Messages, Action<object>> registeredCallback =
        new Dictionary<Messages, Action<object>>();

    public void Register<TParam>(Messages message, Action<TParam> callback)
    {
        registeredCallback[message] = p => callback((TParam)p);
    }

    public void Notify(Messages message, object parameter)
    {
        callback(parameter);
    }
}

The typecasting is done inside the Register method, making it invisible to any class using it.

You can also change signature of the GreetUser method so that it takes a User instead of a string as a parameter. By doing so you can call Register like this:

mediator.Register<User>(Messages.UserCreated, GreetUser);


Insert data to PostgreSQL Im inserting data to a PostgreSQL database using the below function.

Im not very happy with the solution for the IF statements to handle the adaptation of Python None to PostgreSQL NULL. Would there be a more elegant solution I would be happy yo know.

Also, I believe I am not being very performant using .execute() method .
Where could i start to convert my function to make usage of .executemany() perhaps.

def insert_into_table(data):
    # print json.dumps(data, indent=4)

    conn = psycopg2.connect(database='testdb', user='postgres', password='password', host='localhost')
    cursor = conn.cursor()
    for i in data:
        # print json.dumps(i, indent=4)
        iso_code = i["iso_code"]

        if i["l_postcode"] is None:
            l_postcode = 'NULL'
        else:
            l_postcode = i["l_postcode"]

        if i["r_postcode"] is None:
            r_postcode = 'NULL'
        else:
            r_postcode = i["r_postcode"]
        link_id = i["link_id"]
        geom = i["geometry"]

        # sql = 'INSERT into data_load VALUES (\'{}\', {}, {},{} , st_geomfromgeojson(\'{}\'))'.format(iso_code, l_postcode, r_postcode, link_id, json.dumps(geom))
        sql = """INSERT into data_load (iso_code, l_postcode, r_postcode, link_id, geom) VALUES ('{}',{},{},{},st_geomfromgeojson(\'{}\'));""".format(iso_code, l_postcode, r_postcode, link_id, json.dumps(geom))
        print sql

        cursor.execute(sql, )
        print sql

    cursor.close()
    conn.commit()

    conn.close()

A json fragment example of the load im trying to load , this example has only 3 objects for the sake of simplicity .

[
    {
        "geometry": {
            "type": "LineString",
            "coordinates": [
                [
                    -91.98979,
                    15.644559999999998
                ],
                [
                    -91.98971,
                    15.645249999999999
                ]
            ]
        },
        "iso_code": "MEX",
        "l_postcode": null,
        "r_postcode": null,
        "link_id": 1186786776
    },
    {
        "geometry": {
            "type": "LineString",
            "coordinates": [
                [
                    -106.77742,
                    28.390159999999998
                ],
                [
                    -106.77806,
                    28.39076
                ]
            ]
        },
        "iso_code": "MEX",
        "l_postcode": null,
        "r_postcode": null,
        "link_id": 818231403
    },
    {
        "geometry": {
            "type": "LineString",
            "coordinates": [
                [
                    -98.89940999999999,
                    18.90605
                ],
                [
                    -98.89926,
                    18.906689999999998
                ]
            ]
        },
        "iso_code": "MEX",
        "l_postcode": null,
        "r_postcode": null,
        "link_id": 1130886811
    }
]

Do not use string formatting to pass parameters to SQL queries.

This way you are not only getting type conversion and quotes balancing/escaping issues, but also making your code vulnerable to SQL injections. Mandatory XKCD:

(source)

Instead, parameterize your query - put placeholders into the query and let your database driver worry about the Python to database type conversions.

In this case, you should be able to pass your data list of dictionaries directly to executemany() specifying dictionary-based query placeholders:

import psycopg2
from psycopg2.extras import Json

def insert_into_table(data):
    # preparing geometry json data for insertion
    for item in data:
        item['geom'] = Json(item['geometry'])

    with psycopg2.connect(database='testdb', user='postgres', password='password', host='localhost') as conn:
        with conn.cursor() as cursor:
            query = """
                INSERT into
                    data_load
                    (iso_code, l_postcode, r_postcode, link_id, geom)
                VALUES
                    (%(iso_code)s, %(l_postcode)s, %(r_postcode)s, %(link_id)s, st_geomfromgeojson(%(geom)s));
            """
            cursor.executemany(query, data)

        conn.commit()

Also note how I'm using cursor and conn as context managers.


Given a binary tree, find the maximum path sum This is a leetcode.com problem.

  For this problem, a path is defined as any sequence of nodes from some
  starting node to any node in the tree along the parent-child
  connections. The path must contain at least one node and does not need
  to go through the root.

  For example: Given the below binary tree,

   1
  / \
 2   3 Return 6.

Following is my solution that is accepted.

As you can see I have used a magic number in place of -inf. This is not only inelegant, it will actually produce wrong answer if the input node has values in that range. I can think of a few ways to solve it, but I am open to suggestions about how you would do it. I can perhaps use special value like None but that introduces extra if logic that's not that beautiful. Are there better ideas?
Do you have suggestions for better names for variables and methods? Please ignore the name of class (and the fact that it does use class) since this is due to how Leetcode's wants it to be written.

To better understand the code, the intuition is, for any given subtree rooted at node n, I calculate the maximum path that includes n as one endpoint and ends within that subtree. I also calculate the maximum path that is between any two node in that subtree (to do that I take the max of few things). The global solution is then computed at the root of the whole tree.

# Definition for a binary tree node.
# class TreeNode(object):
#     def __init__(self, x):
#         self.val = x
#         self.left = None
#         self.right = None

class Solution(object):
    def maxPathSum(self, root):
        """
        :type root: TreeNode
        :rtype: int
        """
        _, max_path = self.max_path(root)
        return max_path

    def max_path(self, node):
        if node == None:
            return 0, -9999999

        max_path_from_left, max_path_in_left = self.max_path(node.left)
        max_path_from_right, max_path_in_right = self.max_path(node.right)

        max_path_from_current = max(\
            max_path_from_left + node.val,\
            node.val, \
            max_path_from_right + node.val)

        max_path = max(max_path_in_left, max_path_in_right, \
                  max_path_from_left + node.val + max_path_from_right, \
                  max_path_from_current)
        return max_path_from_current, max_path

If you want \$−∞\$, just write float('-inf').
The names could be improved by removing the common prefix max_path_, using to instead of from, and node instead of current. (The replacements are shorter than the originals, so reduce the size of the code without reducing readability.)
The computation of max_path_from_current (which becomes to_node after my renaming), can be simplified by pulling the addition of node.val out of the call to max:

to_node = node.val + max(0, to_left, to_right)

Since max_path is only called from maxPathSum it makes sense for it to be a local function there. Then you could avoid the unnecessary use of self.

Revised code:

def maxPathSum(self, root):
    def max_path(node):
        if node is None:
            return 0, float('-inf')
        to_left, in_left = max_path(node.left)
        to_right, in_right = max_path(node.right)
        to_node = node.val + max(0, to_left, to_right)
        in_node = max(in_left, in_right, to_left + node.val + to_right, to_node)
        return to_node, in_node
    return max_path(root)[1]