Build a MySQL upsert query to insert/update multiple records in Python I'm generating a multi-row insert/update with the MySQLdb/MySQL-python module from lists of data rows and field names. For reference, see the executemany() example in the docs.

You can see some revised code on GitHub.

def upsert(db,table,fields,object_list):
    cursor = db.cursor();
    placeholders = ["%s" for x in fields]
    assignments = ["{x} = VALUES({x})".format(x=x) for x in fields]

    query_string = """INSERT INTO
    {table}
    ({fields})
    VALUES
    ({placeholders})
    ON DUPLICATE KEY UPDATE {assignments}"""

    cursor.executemany(query_string.format(
        table = table,
        fields = ", ".join(fields),
        placeholders = ", ".join(placeholders),
        assignments = ", ".join(assignments)
    ),object_list)
    db.commit()

Should I be quoting or escaping some strings/fields? The data is made safe by parameterized queries, but the table and field names could still cause trouble. I'm less concerned about security than failed queries due to odd characters.

Is there already a library out there for this? The existing solutions for upsert and batch-insert seem more generalized, and therefore slower, than implementing a MySQL-specific solution.

Also, I'm clearly in love with str.format()—is this the best way of handling string composition?

Here's some code to run the function, assuming a database named demo exists and mysql is listening on the localhost socket.

import MySQLdb
db = MySQLdb.connect(host="localhost", user="root", passwd="", db="demo", charset="utf8")
c = db.cursor()

c.execute("""DROP TABLE IF EXISTS upsert_demo""")

c.execute("""CREATE TABLE upsert_demo (
  `id` int(11) unsigned NOT NULL,
  `foo` varchar(100) DEFAULT NULL,
  PRIMARY KEY (`id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8""")

c.execute("""INSERT INTO upsert_demo
  (`id`,`foo`)
  VALUES (1, 'baz')""")

demo_fields = ("id","foo")
demo_objects = [(1,"upserted!"),(2,"new record")]

upsert(db,"upsert_demo",demo_fields,demo_objects)

Should I be quoting or escaping some strings/fields?

Yes, you should definitely escape table and field names. Currently this is trivial to exploit. It should not be possible to produce invalid SQL with any input parameters. If you get to that point there might not be any exploits left.

  The existing solutions for upsert and batch-insert seem more generalized, and therefore slower, than implementing a MySQL-specific solution.

Which options are these? And have you tested them? I would be very surprised if there are no options available which would save you much more time overall than rolling your own.

  Also, I'm clearly in love with str.format()—is this the best way of handling string composition?

str.format() is very nice for relatively simple format strings, but in this code it's hard to see what the actual result will be for some given input. I would pull out variables for the various .join()ed strings, but even then it's a big string to compose.

Some general things:

Use more descriptive variable names. database = MySQLdb.connect(… and cursor = database.cursor() are much more descriptive.
Use transactions by initialising your cursors using with database.cursor() as cursor:
You'll want to run your code through pep8 to improve readability.


Find The Duplicates using binarySearch python I got this interview question from Pramp. You can find it online through their website: https://www.pramp.com/challenge/15oxrQx6LjtQj9JK9XlA

  Find The Duplicates

  Given two sorted arrays arr1 and arr2 of passport numbers, implement a
  function findDuplicates that returns an array of all passport numbers
  that are both in arr1 and arr2. Note that the output array should be
  sorted in an ascending order.

  Let N and M be the lengths of arr1 and arr2, respectively. Solve for
  two cases and analyze the time & space complexities of your solutions:
  M ≈ N - the array lengths are approximately the same M ≫ N - arr2 is
  much bigger than arr1.

  Example:

  input:  arr1 = [1, 2, 3, 5, 6, 7], arr2 = [3, 6, 7, 8, 20]

  output: [3, 6, 7] # since only these three

def find_duplicates(arr1, arr2):
    output = []

    for val in arr1:
        if b_search(arr2, val):
            output.append(val)

    return output

def b_search(A, value):
    lower_ind = 0
    upper_ind = len(A) - 1

    while lower_ind <= upper_ind:
        middle_ind = lower_ind + (upper_ind - lower_ind) // 2

        if A[middle_ind] == value:
            return value
        elif A[middle_ind] > value:
            upper_ind = middle_ind - 1
        elif A[middle_ind] < value:
            lower_ind = middle_ind + 1
    return False

#I also tested it against the test cases, and it passed all of the test cases.

# Input: [1, 2, 3, 5, 6, 7], [3, 6, 7, 8, 20]

print(find_duplicates([1, 2, 3, 5, 6, 7], [3, 6, 7, 8, 20]))
# Output: [3,6,7]

Test Case #1
Input: [11], [11],
Expected: [11],
Actual: [11]
Test Case #2
Input: [1,3,5,9], [2,4,6,10],
Expected: [],
Actual: []
Test Case #3
Input: [1,2,3,5,6,7], [3,6,7,8,20],
Expected: [3,6,7],
Actual: [3, 6, 7]
Test Case #4
Input: [1,2,3,5,6,7], [7,8,9,10,11,12],
Expected: [7],
Actual: [7]
Test Case #5
Input: [10,20,30,40,50,60,70,80], [10,20,30,40,50,60],
Expected: [10,20,30,40,50,60],
Actual: [10, 20, 30, 40, 50, 60]
Test Case #6
Input: [10,20,30,40,50,60,70], [10,20,30,40,50,60,70],
Expected: [10,20,30,40,50,60,70],
Actual: [10, 20, 30, 40, 50, 60, 70]

Given that len(arr1) == n and len(arr2) == m, the bisection method has a \$O(n \log m)\$ time complexity. Notice that it is important which array is scanned, and which is bisected. You really want arr1 to be shorter than arr2 (do you see why?).
That said, in case M ≈ N the bisection method has \$O(n \log n)\$ time complexity. However in this case you can achieve \$O(n)\$. The same linear algorithm applied to the general case yields \$O(n + m)\$. Picking the right one (\$O(n \log m)\$ vs \$O(n + m)\$) is the core of the question as I see it.

I intentionally do not spell the linear algorithm out. It is quite simple.
Returning value from b_search misses an opportunity to optimize further. Notice that both arrays are sorted, therefore you don't need to search the entire arr2 again and again: you are only interested in values to the right of where the last search ended.

Consider instead returning an index. It should not be an index of the element found, but one that partitions arr2 into strictly smaller, and larger or equal than val. A bisect module conveniently provides a bisect_left method:

    from bisect import bisect_left

    def find_duplicates(arr1, arr2):
        output = []
        index = 0
        for val in arr1:
            index = bisect_left(arr2, val, index)
            if arr2[index] == val:
                output.append(val)


Bubble sort visualization: JavaScript I'm very new to the JavaScript, and hence I would like to get a feel on the language by writing some of the basic programs. One of those is the following visualization of the Bubble Sort:

Code:

<!DOCTYPE html>

Bubble sort: visualization

    <button onclick=CreateBarPlot()>Generate data</button>
    <button onclick=bubble_sort()>Sort</button>

To run it, see the JSFiddle

What can be improved?

Regarding

Bubble sort: visualization

  <button onclick=CreateBarPlot()>Generate data</button>
  <button onclick=bubble_sort()>Sort</button>

It's not a good idea to use inline event handlers in modern Javascript, because they require global pollution and can have some very strange behavior. Leave the HTML markup to be the actual content of the site - keep the styling and Javascript to their own self-contained sections instead, preferably. To attach event listeners to an element, use addEventListener.

In the Javascript, you have some places where you assign to different .style properties of an element. Consider adding CSS rules instead. Here, the bars are children of the #bar_plot, so all you need is the #bar_plot > div selector - you don't even need to add a class to the elements. Of course, when the property you want is dynamically calculated in the JS, like div.style.width = (value * 10) + 'px', you need to use JS, but otherwise, better to use CSS rules.

You use div.innerHTML = value;. Unless you're deliberately inserting HTML markup, it's faster, safer, and more predictable if you use the textContent property instead, if you want to put text inside an element. (Even if you're already sure value is plain text, and doesn't contain HTML markup, other readers of the code may not be as sure as you on casual inspection, which may cause worries and double-checks) Same thing for when retrieving element content - unless you need to retrieve HTML markup, use textContent.

You declare a bar element with:

const div = document.createElement("div");

const is ES2015 syntax. ES2015 syntax is great - it often makes code more readable and concise. I'd highly recommend using it everywhere. If you need compatibility with completely obsolete browsers (like IE), the professional thing to do is to use Babel to transpile your clean ES2015+ code into ES5 syntax.

But whatever you decide, make sure to have a consistent style. If you really want to write in ES5 for some reason, then best to do so everywhere - otherwise, use ES2015+. (Don't mix var and const / let).

Either way, also make sure to declare your variables before using them - your numbers variable (and bar1_value, and bar1_width, and more) are not declared, which means that when you assign to it, you're implicitly creating a global variable. Or, if you use strict mode (which you should, it turns potential bugs like these into early errors), an error will be thrown.

Elements with IDs automatically become global variables with the name of the ID. This is weird and can cause bugs and confusion. I'd prefer to avoid IDs when possible, or at least minimize their usage to only absolutely unique elements. Dynamic numeric-indexed IDs definitely should be avoided (especially since they aren't being referenced anywhere else anyway).

The overwhelming majority of professional Javascript uses camelCase for nearly all variable names (including functions). PascalCase is generally reserved for classes and constructor functions, which you aren't using here. You can consider whether you want to conform to the de-facto standard.

You're sometimes putting semicolons at the end of statements, and sometimes you aren't. Omitting semicolons can occasionally result in bugs, especially if you're a beginner - even if you know the rules of ASI, it's best to be consistent with a code style. Consider using a linter like ESLint to automatically prompt you to fix these sorts of inconsistencies and potential bugs.

BARS_DATA is a confusing name. It's declared as an array, and it sounds like it holds data, which I would intuitively think would be the numeric data points, but it actually holds elements. Maybe call it barElements instead. Or, even better, you can avoid the global array there entirely by selecting the rows from the DOM when needed instead.

With bubble sort, after a full initial iteration through the array, the value at the last index will be completely sorted (the final value there). After the second full iteration, the second-last value will be completely sorted, and so on. You can improve your logic by iterating one less element each time, and perhaps make the logic clearer by giving these completely-sorted elements a style change.

Rather than bubbleSort recursively calling itself with index and changed arguments, I think the logic might be clearer if it only ran once instead, and added delays by awaiting a Promise that resolves after a couple seconds.

It's possible to click the sort button more than once, resulting in multiple sorts occurring simultaneously, which doesn't make sense. Perhaps disable the buttons while sorting.

Put it all together, and you get something like:

'use strict';

// you could also put the whole script into an IIFE to avoid globals

function generateNumbers(count = 15) {
  const numbers = [];
  for (let i = 0; i < count; i++) {
    numbers.push(Math.floor(Math.random() * (70 - 1)) + 1);
  }
  return numbers;
}

function createBar(index, value) {
  const div = document.createElement("div");
  div.textContent = value;
  div.style.width = (value * 10) + 'px';
  return div;
}

function createBarPlot() {
  const existingPlot = document.querySelector('.bar-plot');
  if (existingPlot) {
    existingPlot.remove();
  }
  const barPlot = document.body.appendChild(document.createElement("p"));
  barPlot.className = 'bar-plot';
  const numbers = generateNumbers();
  for (let i = 0; i < numbers.length; i++) {
    barPlot.appendChild(createBar(i, numbers[i]));
  }
}

const delay = ms => new Promise(resolve => setTimeout(resolve, ms));
async function bubbleSort() {
  const barElements = [...document.querySelectorAll('.bar-plot > div')];

  // Some helper functions first:
  const getNum = i => Number(barElements[i].textContent);
  const switchBars = (i) => {
    barElements[i].insertAdjacentElement('afterend', barElements[i - 1]);
    // Swap the positions of the bars in the array of elements:
    [barElements[i], barElements[i - 1]] = [barElements[i - 1], barElements[i]];
  };

  for (let cyclesToGo = barElements.length - 1; cyclesToGo >= 0; cyclesToGo--) {
    // Add active class to first element:
    barElements[0].classList.add('active');
    for (let i = 1; i <= cyclesToGo; i++) {
      // Add active class to next element:
      barElements[i].classList.add('active');
      if (getNum(i - 1) > getNum(i)) {
        switchBars(i);
      }
      await delay(50);
      // Remove active class from last element:
      barElements[i - 1].classList.remove('active');
    }
    // Cycle complete; last one iterated over is in its final position
    barElements[cyclesToGo].className = 'done';
  }
  generateBtn.disabled = false;
}

const buttons = document.querySelectorAll('button');
const [generateBtn, sortBtn] = buttons;
const setDisabled = newDisabled => {
  for (const button of buttons) {
    button.disabled = newDisabled;
  }
};
generateBtn.addEventListener('click', () => {
  setDisabled(false);
  createBarPlot();
});
sortBtn.addEventListener('click', () => {
  setDisabled(true);
  bubbleSort();
});
body {
  background: pink;
}
.bar-plot > div {
  color: black;
  text-align: right;
  background: lightblue;
  padding: 2.5px;
  margin: 1px;
}
.bar-plot > div.active {
  background: red;
}
.bar-plot > div.done {
  background: yellow;
}

Bubble sort: visualization

<button>Generate data</button>
<button>Sort</button>


Determine if a string has all unique characters I'm practicing algorithms (for future interviews) while learning about Big O notation and was wondering if there's a faster or more optimal way of writing an algorithm to see if a string has all unique characters (assuming you can't use additional data structures).

- (BOOL)containsUniqueCharacters:(NSString *)string {

    //Make sure that it's case insensitve
    string = [string uppercaseString];

    //Create a buffer for our string
    NSUInteger length = [string length];
    unichar buffer[length];
    [string getCharacters:buffer range:NSMakeRange(0, length)];

    //Iterate through each of our characters
    for (int i = 0; i < length; i++) {

        //Iterate through every other character and compare it with our current character
        for (int n = 0; n < length; n++) {

            //Compare current character with every other character
            if (buffer[i] == buffer[n] && i != n) {
                return NO;
            }
        }
    }

    return YES;
}

If I'm correct, is the time complexity for this \$O(n^2)\$, and the space complexity \$O(1)\$?

I would suggest an algorithmically better O(n) solution by using a buffer or hashtable or other such structure to record whether or not you have "seen" a character.

For example if you are dealing with a basic string containing char, then you can get away with a bitset of size 256. Each character you see marks a bit. If the bit was already marked, return YES.

Furthermore, if the size of the string you're testing is longer than 256, then that is an automatic NO (not all unique chars) as well.

Since you are dealing with unicode for which the possible values a character can take are very many, then you will want to use a hash table. It will be dramatically faster than even any O(n log n) sorting method.


Decorator to return default argument values I'm using Python 3.6. This code, used as a decorator to a python function, will cause the function to return a dict of POSITION_OR_KEYWORD arguments.

I used this to create a standard interface to a factory function.

Inspired by this Stack Overflow post, and implemented using a dictionary-flattening function from this previous post.

Eg.

def example(one=1, two=2):
    return {'one':one, 'two':two}

is equivalent to:

@return_kwargs
def example(one=1, two=2):
    pass

import inspect
from functools import wraps
from itertools import chain, starmap

def flatten_dict(dictionary):
    """Flatten a nested dictionary structure"""

    def unpack(parent_key, parent_value):
        """Unpack one level of nesting in a dictionary"""
        try:
            items = parent_value.items()
        except AttributeError:
            # parent_value was not a dict, no need to flatten
            yield (parent_key, parent_value)
        else:
            for key, value in items:
                yield (key, value)

    # Put each key into a tuple to initiate building a tuple of subkeys
    dictionary = {key: value for key, value in dictionary.items()}

    # Keep unpacking the dictionary until all value's are not dictionary's
    dictionary = dict(chain.from_iterable(starmap(unpack, dictionary.items())))
    return dictionary

def return_kwargs(func):
    """Cause the wrapped function to return a dict of keyword arguments"""
    @wraps(func)
    def wrap(*args, **kwargs):
        bound_arguments = inspect.signature(func).bind(*args, **kwargs)
        bound_arguments.apply_defaults()
        return flatten_dict(dict(bound_arguments.arguments))
    return wrap

Let me know how you would implement the idea, whats ugly, corner cases I have missed.

I am not sure how critical this use case is, but when any of your arguments have a dictionary value, you have that dictionary items used as keyword arguments for the function:

In [1]: @return_kwargs
   ...: def example(arg1):
   ...:     pass
   ...:

In [2]: arg1_value = {'arg3': 'value'}

In [3]: print(example(arg1_value))
{'arg3': 'value'}

In other words, arg3 here is printed as a keyword argument for the function but it was not actually unpacked into keyword arguments to the function at all - it was "sitting" inside the value of an arg1 positional argument.


Digit to words converter This is one of the longest programs I have made with methods and I think I must be doing this rather inefficiently. Any comments to improve would be appreciated.

package numberwords2;
import java.awt.Component;
import javax.swing.JOptionPane;
public class NumberWords2
{
    static String rawInput;
    static String output = "";
    static int hold;

    private static Component frame;

    public static void main(String[] args)
    {

        rawInput = JOptionPane.showInputDialog("Enter a number between 0 and 9999.");
        int input = Integer.parseInt(rawInput);

        if(input == 0)
        {
            output += "Zero";
        }
        else if(input < 10)
        {
            output += ones(0);
        }
        else if(input < 20)
        {
            output += teens(0);
        }
        else if(input < 100)
        {
            output += tens(0);
            output += ones(1);
        }
        else if(input < 1000)
        {
            output += hundreds(0);
            int temp = Integer.parseInt(rawInput.substring(1,3));
            if(temp > 10 && temp < 20)
            {
                output += teens(2);
            }
            else
            {
                output += tens(1);
                output += ones(2);
            }

        }
        else if(input < 10000)
        {
            output += thousands(0);
            output += hundreds(1);
            int temp = Integer.parseInt(rawInput.substring(2,4));
            if(temp > 10 && temp < 20)
            {
                output += teens(3);
            }
            else
            {
                output += tens(2);
                output += ones(3);
            }

        }

        JOptionPane.showMessageDialog(frame, rawInput + " is" + output + ".");
        output = "";
        main(args);
    }

    public static String teens(int hold)
    {
        String[] teens = {"Ten", "Eleven", "Twelve", "Thirteen", "Fourteen",
            "Fifteen", "Sixteen", "Seventeen", "Eighteen", "Nighteen"};
        int x = Character.getNumericValue(rawInput.charAt(hold));
        int y = 0;
        for(int i = 0; i < 10; i++)
        {
            if(i == x)
            {
                y = i;
                break;
            }
        }
        return " " + teens[y];
    }

    private static String tens(int hold)
    {
        String[] tens = {"", "Ten", "Twenty", "Thirty", "Fourty", "Fifty", "Sixty", "Seventy", "Eighty", "Ninety"};
        int x = Character.getNumericValue(rawInput.charAt(hold));
        int y = 0;
        for(int i = 0; i < 10; i++)
        {
            if(i == x)
            {
                y = i;
            }
        }
        return  " " + tens[y];
    }

    private static String ones(int hold)
    {
        String[] ones = {"", "One", "Two", "Three", "Four", "Five", "Six", "Seven", "Eight", "Nine"};
        int x = Character.getNumericValue(rawInput.charAt(hold));
        int y = 0;
        for(int i = 1; i < 10; i++)
        {
            if(i == x)
            {
                y = i;
                break;
            }
        }
        return " " + ones[y];
    }

    private static String hundreds(int hold)
    {
        return ones(hold) + " Hundred";
    }

    private static String thousands(int hold)
    {
        return ones(hold) + " Thousand";
    }
}

int input = Integer.parseInt(rawInput);

This can throw an exception if rawInput isn't a number.

    else if(input < 10)
    {
        output += ones(0);
    }
    else if(input < 20)
    {
        output += teens(0);
    }
    else if(input < 100)
    {
        output += tens(0);
        output += ones(1);
    }
    else if(input < 1000)
    {
        output += hundreds(0);
        int temp = Integer.parseInt(rawInput.substring(1,3));
        if(temp > 10 && temp < 20)
        {
            output += teens(2);
        }
        else
        {
            output += tens(1);
            output += ones(2);
        }

    }

Code (subroutine calls) are duplicated in the above:

If the number is less than 1000 then you call tens, teens, and ones
If the number is greater than 1000 then you call tens, teens, and ones

There would be less code if you processed it from left to right, like you do when you read and say a number:

if (input > 1000)
{
    output += thousands;
    subtract thousands from input;
}
if (input > 100)
{
    output += hundreds;
    subtract hundreds from input;
}
if (10 < input < 20)
{
    output += teens;
}
else
{
    if (input >= 20)
    {
        output += tens;
        subtract tens from input;
    }
    output += ones;
}
if (output is empty)
    output = "Zero";

int temp = Integer.parseInt(rawInput.substring(1,3));

This is a complicated way to remove the thousands from the input.

A simpler way to subtract thousands from the input is:

input = input - ((input / 1000) * 1000);

main(args);

This is a strange way to loop forever.

Better would be:

public static void main(String[] args)
{
    while (true)
        run();
}

static void run()
{
    rawInput = JOptionPane.showInputDialog("Enter a number between 0 and 9999.");
    ... etc ...
}

Perhaps only the first letter should be capitalized; perhaps with hyphens; and the word "and"; for example "Four hundred and forty-three" not "Four Hundred Forty Three".


Python webscraping for lists of company names and details using BeautifulSoup I just started learning Python and made a script that scrapes a corporate directory
https://www.sccci.org.sg/en/directory/corporate/members

So far it gets all the company names and their details under one category page (although i do intend on making it automatically get the details of the other pages too in the future) and writes them into a text file.

However the code uses a lot of nested loops and i am looking to see if there is a better way to code it in terms of efficiency and good practice.

Here is the code:

#Author: James
#Date: 9/11/2017
#nlist stores company names
#detlist stores the details
#finlist stores the href links required to scrape the subsites of SCCCI
import requests
from bs4 import BeautifulSoup
check = False
finlist = []
nlist = []
detlist = []
r = requests.get("https://www.sccci.org.sg/en/directory/corporate/members?ind=150")
soup = BeautifulSoup(r.content, "html.parser")
#finds all the links in the html class "listing" and store them in "finlist"
for items in soup.findAll("div", {"class" : "listing"}):
    for a in items.findAll("ol"):
        for b in a.findAll("a"):
            finlist.append("https://www.sccci.org.sg" + b.get("href"))
#enters each site in finlist and gets the company name found in "member-name"
for record in finlist:
    print("Entering " + record + "...")
    lr = requests.get(record)
    lsoup = BeautifulSoup(lr.content, "html.parser")
    for o in lsoup.findAll(["span"], {"class" : "member-name"}):
        nlist.append(o.get_text("\n",strip=True))
    for o in lsoup.findAll("div", {"class" : "member-info hidden"}):
        detlist.append(o.get_text("\n",strip=True))
    #this loops checks for any additional pages in the link and searches though the additional sites for names and details too
    for j in lsoup.findAll("li", {"class" : "pager-item"}):
        for b in j.findAll("a"):
            print(" Entering https://www.sccci.org.sg" + b.get("href") + "...")
            mR = requests.get("https://www.sccci.org.sg" + b.get("href"))
            mSoup = BeautifulSoup(mR.content, "html.parser")
            for soups in mSoup.findAll("span", {"class" : "member-name"}):
                nlist.append(soups.get_text("\n",strip=True))
            for soups in mSoup.findAll("div", {"class" : "member-info hidden"}):
                detlist.append(soups.get_text("\n",strip=True))

# Request process end -- File dump process start --

print("Start file dump...")
text_file = open("debug.txt", "w", encoding="utf-8")
#combines the namelist and detaillist into the finalList
finalList = [j for i in zip(nlist,detlist) for j in i]
for zippy in finalList:
    zippy = zippy.replace("                                    "," ")
    zipstring = str(zippy)
    text_file.write(zipstring + "\n\n")
text_file.close()
text_file_names = open("cnames.txt", "w", encoding="utf-8")
count = 0
for names in nlist:
    count+= 1
    names = str(names)
    text_file_names.write(str(count) + " | " + names + "\n")
text_file_names.close()
text_file_pnames = open("names.txt", "w", encoding="utf-8")
for pnames in nlist:
    pnames = str(pnames)
    text_file_pnames.write(pnames + "\n")
text_file_pnames.close()
finalitem = len(finalList)/2
print("Done | " + str(finalitem) + " items")
print("Files generated: names.txt | debug.txt | cnames.txt")

It seems like you are creating a kind of a crawler. To improve on performance, you can use a Session object. It will utilize the underlying TCP connection, making the script run faster.

You can also use BeautifulSoup's SoupStrainer to only parse the listing tag. This way, your crawler won't have to go through each and every line of the HTML document.

Additionally on perfomance, you can use the lxml parser which happens to be faster than html.parser.

You seem to be using the base link quite often, move it into a constant and add href tags to it as needed.

As an example, the way you are filling up your finlist can be written as such.

import requests
from bs4 import BeautifulSoup, SoupStrainer

URL = 'https://www.sccci.org.sg'
session = requests.Session()

response = session.get(URL + '/en/directory/corporate/members?ind=150')
strainer = SoupStrainer(class_='listing')
soup = BeautifulSoup(response.content, 'lxml', parse_only=strainer)
listing_links = [URL + link['href'] for link in soup.select('ol[start] a[href]')]

Note that I have used all of the above and CSS selectors resulting in reducing the amount of for loops to just one. Populating finlist takes about 2 seconds on my machine.

Staying on finlist, it's not a particularly descriptive variable name. Perhaps something like listing_links would be better?

I can't go through the rest of the crawler as I am short on time, but for file operations, use the with statement. It will ensure the file is closed, even if an error occurs.
Example usage:

foo = '1234'
with open('bar.txt', 'w') as f:
    f.write(foo)


jQuery tabs improvement I've written a small function which displays a div related to an anchor element using data attributes.
The data attribute is used to match the class of the relevant div which is then displayed whilst the others are hidden.
All very basic, however I'm wondering if there is anyway to improve it? Is there a better solution than using the data attribute? Any best practices?
$(".tabs li a").click(function() {
    var cssClass = "." + $(this).data("class");

    // Add active class to current tab, remove from siblings
    $(this).addClass("active").parent().siblings().find("a").removeClass("active");

    // Find the associated content and show it, hide its siblings
    $(".tabs-content").find(cssClass).show().siblings().hide();

    return false;
});

jsfiddle example here

Edit
This ended up being re-written for production using classes instead of data attributes, like so:
$(".tabs li a").click(function() {
    var cssClass = "." + $(this).attr("class");

    $('.tabs a.active').removeClass("active"); // Find the previously active tab and remove it
    $(this).addClass("active");                // Add active class to current tab
    $(".tabs-content").find('> div').hide();   // Hide the all tab divs
    $(".tabs-content").find(cssClass).show()   // Find the associated div and display it

    return false;
});

Updated jsfiddle

$(".tabs-content").find(cssClass).show().siblings().hide(); is confusing and not easy to read.

Otherwise it seems to be working. As for best practices:

Make sure your content is accessible to non JS users, such as search engines or browsers without JS.
Consider using the Bootstrap tab plugin or jQuery UI tab functionality. Of course if you don't already use one of those, keep your simple jQuery function.


Build a time struct for current time in UTC I'm learning about time structs. Without assigning either in code or input routines I want the current time. Please help me refactor this, if it should be.

This code just tries to make a time struct with the current UTC time. How am I doing?

// better_current_gmtime_struct_builder.c
// compile with :
// gcc -Wall -g better_current_time_struct_builder.c -o better_current_time_struct_builder

#include <time.h>
#include <stdio.h>

/* function declarations */
void print_struct_parts(struct tm* t);

int main()
{

    time_t now;

    /* set the time for now */
    now = time( NULL ); /* NULL is a control character */

    /* set up time ponter */
    struct tm *tp;
    tp = gmtime(&now);

    /* quick check of time expected */
    printf("%s\n", asctime(tp));

    /* assign the current time struct from the time pointer */
    struct tm t = *tp;

    /* print the parts */
    print_struct_parts(&t);
    return 0;
}

/* check the struct parts */
void print_struct_parts(struct tm* t)
{
    printf ("%i\n", t->tm_wday);
    printf ("%i\n", t->tm_mon);
    printf ("%i\n", t->tm_mday);
    printf ("%i\n", t->tm_hour);
    printf ("%i\n", t->tm_min);
    printf ("%i\n", t->tm_sec);
    printf ("%i\n", t->tm_year);
}

Being brutally honest with you: your comments are useless. All of them could be erased. In special, this one is also misleading:

now = time( NULL ); /* NULL is a control character */

Wut? No NULL is not a control character. You can pass a time_t pointer to time() and get the output written to the given variable, or you can pass NULL and get the output from the returned value. It has nothing to do with 'control characters', whatever that means.

Just yesterday I answered this question that had the same problem with noisy comments. I'll quote my own answer:

  Don't write noisy and obvious comments. The overall rule about code
  commenting is to use them to explain why you solved a problem in a
  given way, or to give any extra information (web links, mathematical
  formulas, etc) that will aid the understanding of a piece of code.
  Explaining what the code is doing should not be necessary if the code is well written. Well written code is self explanatory.

Apart from that, I would also give print_struct_parts() a better name that reflects what kind of struct it is printing. print_time() comes to mind...

Maybe also consider taking a FILE stream as parameter so that the caller is not bound to stdout.


Model tournaments between teams In one of my projects I wrote a little tournament platform, but the way I handle participants is a bit messy and I want to clean this up.

So in my case a Tournament can have participants, but either a User or a Team can signup to become a participant. A participant now holds a user_id or a team_id in the database, and depending on which is filled I determine if a User is linked to the participant or if it's a team.

I rewrote the signup part of my Tournament model to accept both a User or a Team as parameter through am interface and was wondering if this is the correct way to go.

ICandidate.php

interface ICandidate
{
    public function getName();
    public function hasCorrectGamesSet($tournamentGame);
}

User.php

class User implements ICandidate
{
    private $game;

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

    public function getName()
    {
        return 'Username';
    }

    public function hasCorrectGamesSet($tournamentGame)
    {
        if($this->game != $tournamentGame){
            throw new Exception('Incorrect name');
        };

        return true;
    }
}

Team.php

class Team implements ICandidate
{
    private $users;

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

    public function getName()
    {
        return 'Teamname';
    }

    public function hasCorrectGamesSet($tournamentGame)
    {
        foreach($this->users as $user){
            $user->hasCorrectGamesSet($tournamentGame);
        }
    }
}

Participant.php

class Participant
{
    private $name;
    private $class;

    public function setName($name)
    {
        $this->name = $name;
    }

    public function setClass($class)
    {
        $this->class = $class;
    }
}

Tournament.php

class Tournament
{
    private $participants;
    private $game;

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

    public function signUp(ICandidate $candidate)
    {
        $candidate->hasCorrectGamesSet($this->game);

        $participant = new Participant();
        $participant->setName($candidate->getName());
        $participant->setClass(get_class($candidate));

        $this->participants[] = $participant;
    }

    public function getParticipants()
    {
        var_dump($this->participants);
    }
}

So with this set, I can signup a User or a Team, do a little check and if that passes, add them as a participant to the tournament.

Logic

$tournament = new Tournament('fifa');

$user1 = new User('fifa');
$user2 = new User('fifa');
$user3 = new User('fifa');
$user4 = new User('fifa');

$team = new Team([$user2, $user3, $user4]);

$tournament->signUp($user1);
$tournament->signUp($team);

$tournament->getParticipants();

Is this a correct way to implement this and make use of the interface?

Your code is actually much better than the average. Congrats.

I would change ICandidate to Candidate. No one needs to know if this is an interface, abstract class, concrete class, etc. Don't break encapsulation.

Also, using types makes your code more readable and less error prone.

I wouldn't also use getters and setters. For instance:

class Participant
{
    private string $name;
    private string $class;

    public function __construct(string $name, string $class)
    {
        $this->name = $name;
        $this->class = $class;
    }

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

    public function class(): string
    {
        return $this->class;
    }
}

But... do you really need the Participant class? From your example, it's not necessary.

In your Tournament class, $participants is an array of Candidates.

Ah, I would also use namespaces so I would be able to implement autoloading (https://www.php-fig.org/psr/psr-4/)


ChessMetric - Topcoder Challenge My solution for the topcoder ChessMetric challenge seems really slow. I feel like there should be a better way, but I'm not sure.

  Challenge Description

  Suppose you had an n by n chess board and a super piece called a
  kingknight. The kingknight can move either one space in any direction
  (vertical, horizontal or diagonally) or can make an 'L' shaped move.
  An 'L' shaped move involves moving 2 spaces horizontally then 1 space
  vertically or 2 spaces vertically then 1 space horizontally.

  Given the size of the board, the start position of the kingknight and
  the end position of the kingknight, your method will return how many
  possible ways there are of getting from start to end in exactly
  numMoves moves. start and finish are int[]s each containing 2
  elements. The first element will be the (0-based) row position and the
  second will be the (0-based) column position. Rows and columns will
  increment down and to the right respectively. The board itself will
  have rows and columns ranging from 0 to size-1 inclusive.

class Board:
    def __init__(self, size):
        self.size = size
        self.board = [[0 for _ in xrange(size)] for _ in xrange(size)]

    def copy(self):
        c = Board(0)
        board_copy = []
        for val in self.board:
            board_copy.append(val[:])
        c.board = board_copy
        c.size = self.size
        return c

    def is_valid(self, space):
        i, j = space
        return 0 <= i < self.size and 0 <= j < self.size

    def set(self, space, val):
        assert self.is_valid(space), "Invalid space: set()"
        i, j = space
        self.board[i][j] = val
        assert self.get(space) == val, "Problems in set"

    def get(self, space):
        assert self.is_valid(space), "Invalid space: get()"
        i, j = space
        return self.board[i][j]

    def get_adj(self, space):
        assert self.is_valid(space), "Invalid space: get()"
        i, j = space

        result = [
            (i+1, j),
            (i-1, j),
            (i, j+1),
            (i, j-1),
            (i+1, j+1),
            (i-1, j-1),
            (i+1, j-1),
            (i-1, j+1),
            (i+1, j+2),
            (i+1, j-2),
            (i-1, j+2),
            (i-1, j-2),
            (i+2, j+1),
            (i-2, j+1),
            (i+2, j-1),
            (i-2, j-1),
        ]
        result = filter(self.is_valid, result)
        return result

def chess_metric(size, start, finish, move_limit):
    board = Board(size)
    board.set(start, 1)
    aux = board.copy()
    for _ in xrange(move_limit):
        print _
        for i in xrange(size):
            for j in xrange(size):
                current_space = (i, j)
                current_val = board.get(current_space)
                if current_val == 0:
                    continue
                for neighbor in board.get_adj(current_space):
                    neighbor_val = aux.get(neighbor)
                    new_val = current_val + neighbor_val
                    aux.set(neighbor, new_val)
        board = aux.copy()
    return aux.get(finish)

Since you have decided to go the way of the specialized object for your board, there are two obvious improvements. One is to use Python magic methods to have a more pythonic notation. The other is to offload the bounds checking to this specialized class. For instance:

class Board(object):
    def __init__(self, rows, cols):
        self.rows = rows
        self.cols = cols
        self.board = [[0] * cols for row in range(rows)]

    def __getitem__(self, index):
        row, col = index
        if 0 <= row < self.rows and 0 <= col < self.cols:
            return self.board[row][col]
        return 0

    def __setitem__(self, index, value):
        row, col = index
        if 0 <= row < self.rows and 0 <= col < self.cols:
            self.board[row][col] = value

This simple class lets you use typical Python indexing with square brackets to access the board, and silently handles out of bounds access.

You may also want to be a little terser about generating your moves. This is one of many options:

def kingknight_moves(start):
    row, col = start
    dest = []
    for dr in range(-2, 3):
        cols = (1, -1) if abs(dr) != 1 else range(-2, 3)
        for dc in cols:
            dest.append((row + dr, col + dc))
    return dest

I'm also going to write a function to advance the board one move:

def advance_board(board):
    new_board = Board(board.rows, board.cols)
    for row in range(board.rows):
        for col in range(board.cols):
            count = board[row, col]
            if count:
                for dest in kingknight_moves((row, col)):
                    new_board[dest] += count
    return new_board

This is pretty much the same you are doing, modulo a nicer syntax.

Now to the meat of the problem... A naive approach like the one you have implemented works:

def chess_metric_naive(size, src, dst, moves):
    board = Board(size, size)
    board[src] = 1

    for move in range(moves):
        board = advance_board(board)

    return board[dst]

But is indeed kind of slow for the largest of the problems posed:

%timeit chess_metric_naive(100, (0, 0), (0, 99), 50)
1 loops, best of 3: 3.7 s per loop

If you were to time each of your iterations advancing the board one move, you would notice that they get progressively slower, as more and more cells are non-zero and contribute to their neighbors. One way of mitigating this effect is to, rather than completing all moves from the source towards the destination, do half of them like this, and the rest from destination to source. If there are \$m\$ ways of getting from the source to a certain cell in half the moves, and \$n\$ ways of getting from the destination to the same cell in the remaining moves, then there are \$mn\$ ways of getting from source to destination. You could implement this like:

def chess_metric(size, src, dst, moves):
    src_board = Board(size, size)
    src_board[src] = 1
    dst_board = Board(size, size)
    dst_board[dst] = 1

    for move in range(moves // 2):
        src_board = advance_board(src_board)
        dst_board = advance_board(dst_board)

    if moves % 2:
        src_board = advance_board(src_board)

    total = 0
    for row in range(size):
        for col in range(size):
            total += src_board[row, col] * dst_board[row, col]

    return total

And it turns out to be substantially (3x) faster:

%timeit chess_metric(100, (0, 0), (0, 99), 50)
1 loops, best of 3: 1.2 s per loop

We can do even better though: an alternative approach is to use something akin to a branch and bound approach. We know that the maximum distance that our made up chess piece can move is \$\sqrt 5\$, so we need not consider cells that are further from the destination more than that value times the number of moves left. A possible approach is to create a board holding minimum number of moves to get to the destination from any other cell:

def moves_lower_bound(rows, cols, dst):
    from math import sqrt, ceil
    dst_row, dst_col = dst
    board = Board(rows, cols)
    for row in range(rows):
        row2 = row - dst_row
        row2 *= row2
        for col in range(cols):
            col2 = col - dst_col
            col2 *= col2
            dist2 = row2 + col2
            # rounded up int division dist2 // 5
            board[row, col] = int(ceil(sqrt(dist2) / sqrt(5)))
    return board

With the aid of this auxiliary board, we can prune the cells that cannot add to the final result whenever we advance the board:

def advance_and_prune_board(board, bounds, moves_left):
    new_board = Board(board.rows, board.cols)
    for row in range(board.rows):
        for col in range(board.cols):
            count = board[row, col]
            if count and bounds[row, col] <= moves_left:
                for dest in kingknight_moves((row, col)):
                    new_board[dest] += count
    return new_board

Putting all this together:

def chess_metric_bb(size, src, dst, moves):
    board = Board(size, size)
    board[src] = 1
    bounds = moves_lower_bound(size, size, dst)

    for move in range(moves):
        board = advance_and_prune_board(board, bounds, moves - move)

    return board[dst]

We get close to a 9x improvement over the simpler approach:

%timeit chess_metric_bb(100, (0, 0), (0, 99), 50)
1 loops, best of 3: 424 ms per loop


Searching for the closest "stop" elements surrounding a start point This code works but is there a simple / better way to do this?

It starts from a particular point (starthere) in the array, looks for first stop value to the left of start point, and the first stop to the right side.

Basically returns the closest values' positions to a start point both sides. (I'll make this a function.)

var arr = ['f','f','stop','stop','f','f','starthere','f','f','f','stop','stop','f','f','f','f'];

var i = arr.indexOf('starthere');
while (i--) {
    if (arr[i] == 'stop') {
        document.write(i + '
');
        break;
    }
}

for (var i = arr.indexOf('starthere'); i < arr.length; i++) {
    if (arr[i] == 'stop') {
        document.write(i);
        break;
    }

}

Returns 3
10

There is an overload of .indexOf that takes a second parameter indicating the starting location for the search.  That can be used to find the stop after starthere. Then you can use .slice to grab the part of the array before starthere with lastIndexOf to find the stop that comes before:

var i = arr.indexOf('starthere');
var stopafter = arr.indexOf('stop', i);
var stopbefore = arr.slice(0, i).lastIndexOf('stop');
document.write(stopbefore + '</br>' + stopafter);


Parsing a HTTP request I'm currently writing a pure ruby webserver, and one of the things that I have to do is parse a HTTP request. The method I've pasted below takes a HTTP request, and puts it in a map keyed by the header field names.

The biggest issue I faced while doing this was dealing with the lack of an EOF from the TCPSocket on requests with bodies (basically every POST request). This meant that I couldn't just keep doing file.gets until I reached the end of the file, because there was no end. What I ended up doing instead, was to create a while true loop that breaks when it finds '\r\n', which are the last characters before the Body, then read the Body separately using the number I get from Content-length.

Is there a more elegant way in ruby to do this? I feel like it's unnecessary to use a infinite loop, but I can't think of anything else that would work.

   # Takes a HTTP request and parses it into a map that's keyed
   # by the title of the heading and the heading itself.
   # Request should always be a TCPSocket object.
   def self.parse_http_request(request)
      headers = {}

      #get the first heading (first line)
      headers['Heading'] = request.gets.gsub /^"|"$/, ''.chomp
      method = headers['Heading'].split(' ')[0]

      #parse the header
      while true
         #do inspect to get the escape characters as literals
         #also remove quotes
         line = request.gets.inspect.gsub /^"|"$/, ''

         #if the line only contains a newline, then the body is about to start
         break if line.eql? '\r\n'

         label = line[0..line.index(':')-1]

         #get rid of the escape characters
         val = line[line.index(':')+1..line.length].tap{|val|val.slice!('\r\n')}.strip
         headers[label] = val
      end

      #If it's a POST, then we need to get the body
      if method.eql?('POST')
         headers['Body'] = request.read(headers['Content-Length'].to_i)
      end

      return headers
   end

I don't know if this is the best method to parse an HTTP request. You probably should have a look into one of the simpler ruby http servers out there to get some ideas. I do know though that you're doing some strange things here:

headers['Heading'] = request.gets.gsub /^"|"$/, ''.chomp

It looks like you're calling the #chomp method on '' here, which doesn't make sense. You really should use parenthesis here:

headers['Heading'] = request.gets.gsub(/^"|"$/, '').chomp

But on the other hand I can't see what the gsub is doing here, you probably can just get rid of it.

method = headers['Heading'].split(' ')[0]

Probably nitpicking but you can make this easier (and a little faster) by using:

method = headers['Heading'][/[^ ]*/]

That translates to: get the substring till the first space character (so no need to create an array here).

line = request.gets.inspect.gsub /^"|"$/, ''

That is really strange, you don't need to "inspect" a string just to parse control characters. You can match control characters with double quoted strings (e.g. "\r\n").

label = line[0..line.index(':')-1]

#get rid of the escape characters
val = line[line.index(':')+1..line.length].tap{|val|val.slice!('\r\n')}.strip

The ruby god weeps. You can make this much simpler (and more efficient) by using a regexp here:

line =~ /(.*?): (.*)/
label = $1
val = $2.strip

Modifying the stream inside #tap is bad style and the #strip removes surrounding whitespace like "\n" and "\r" anyway.

For the loop you can do the most natural thing and just write the break condition in the while condition part:

while (line = request.gets) != "\r\n"
  ...
end


Insertion vs Selection I currently have a selection sorting algorithm, but need some help turning it into a insertion sort, I understand a insertion sort if faster?  The hand array is full, and it needs to sort from lowest to highest rank.

private void sort(PlayingCard[] hand)
    {
        PlayingCard temp;
        for(int i = 0; i < hand.length ; i++)
        {

           for(int j = i+1; j < hand.length ; j++)
           {
               int compareRank = hand[j].getRank().compareTo(hand[i].getRank());

               if(compareRank < 0){
                   temp = hand[i];
                   hand[i] = hand[j];
                   hand[j] = temp;
               }
           }

        }
    }

Here is a sort that I put together after some research:

private void sort(PlayingCard[] hand)
{
    int i, j;
    PlayingCard temp;
    for (i = 1; i < hand.length; i++) {
        temp = hand[i];
        j = i;
        while (j > 0 && hand[j - 1].getRank().compareTo(temp.getRank()) > 0) {
            hand[j] = hand[j - 1];
            j--;
        }
        hand[j] = temp;
    }
}


simple stupid F# async telnet client Did I write this code to correctly be tail call optimized? Am I forcing computations more than I need with !? Did I generally write this asynchronously correctly to allow sends and receives to occur in their own time? Is this using multiple threads needlessly? Other general critiques?

open System
open System.Net.Sockets

let asyncGetInput = async { return BitConverter.GetBytes(Console.Read()) }
let rec asyncSendInput (stream : NetworkStream) = async {
    let! input = asyncGetInput
    stream.WriteByte |> Array.map <| input |> ignore
    do! asyncSendInput stream
    }

let asyncGetResponse (stream : NetworkStream) = async { return Char.ConvertFromUtf32(stream.ReadByte()) }
let rec asyncPrintResponse (stream : NetworkStream) = async {
        let! response = asyncGetResponse stream
        Console.Write(response)
        do! asyncPrintResponse stream
        }

#light
[<EntryPoint>]
let main args =
    let client = new System.Net.Sockets.TcpClient()
    client.Connect(args.[0], Int32.Parse(args.[1]))
    printfn "Connected to %A %A..." args.[0] args.[1]
    let stream = client.GetStream()
    printfn "Got stream, starting two way asynchronous communication."
    Async.Parallel [asyncSendInput stream; asyncPrintResponse stream] |> Async.RunSynchronously |> ignore
    0

It seems you are using asyncGetInput and asyncGetReponse to introduce concurrency, although this is a noble goal it is kind of useless because you are only using those from other workflows which are already running in the thread pool anyway. In that case it would be ok to make a synchronous call from inside those workflows.

The second point is about the do! at the end of asyncPrintResponse, you should use return! in your recursive call otherwise you will get a memory leak. I'm not sure if we can call this a bug because it can probably be considered normal that the definition of Do in the workflow isn't built for that. I've personally been corrected for that same error here: https://stackoverflow.com/questions/6905251/f-async-difference-between-two-structure

[edit]
There's a difference between handling concurrency and introducing concurrency. If we ignore your code for a moment, you have two distinct inherently sequential workflows that you can parallelize and asyncSendInput and asyncPrintResponse is already doing that by itself, as such asyncGetInput and asyncGetResponse are unnecessary and will only create more lightweight thread that will end up being scheduled/deschedule from the thread pool.

Here's a little cleanup:

open System
open System.Net.Sockets

let rec asyncSendInput (stream : NetworkStream) =
    async {
        let input = Console.Read() |> BitConverter.GetBytes
        input |> Array.iter stream.WriteByte
        return! asyncSendInput stream
    }

let rec asyncPrintResponse (stream : NetworkStream) =
    async {
        let response = stream.ReadByte() |> Char.ConvertFromUtf32
        Console.Write(response)
        return! asyncPrintResponse stream
    }

[<EntryPoint>]
let main args =
    let client = new System.Net.Sockets.TcpClient()
    client.Connect(args.[0], Int32.Parse(args.[1]))
    printfn "Connected to %A %A..." args.[0] args.[1]
    let stream = client.GetStream()
    printfn "Got stream, starting two way asynchronous communication."
    asyncSendInput stream |> Async.Start
    asyncPrintResponse stream |> Async.RunSynchronously
    0


DataTables search filter handler I am writing a JavaScript feature where by I need to make a call to a third-party library (pre-1.10 DataTables) depending upon whether or not the user provides an integer as an input to the function during initialization. I am pasting here the snippet. As you can see it has duplicate code in the if/else clauses, the only difference is the method signature for *.fnFilter.

How can I optimize this code further? Also I dont want to test the condition during each key-up.

/*
 * By default this is a global search box events. Global being searchable across all columns.
 * But if an int is provided as an input then it will apply the search only on that column.
 * */
_bindSearchEvents: function(onColumnIndex) {

    var self = this;

    if (Math.ceil(onColumnIndex) === Math.floor(onColumnIndex)) {
        self.settings.search.input.live('keyup', function() {
            self.widget.fnFilter($(this).val(), onColumnIndex);
        });

        self.settings.search.reset.live('click', function() {
            self.settings.search.input.val("");
            self.widget.fnFilter("", onColumnIndex);
        });

    } else {

        self.settings.search.input.live('keyup', function() {
            self.widget.fnFilter($(this).val());
        });

        self.settings.search.reset.live('click', function() {
            self.settings.search.input.val("");
            self.widget.fnFilter("");
        });
    }
},

Since all that is different about the two if/else bodies is the arguments to the fnFilter method, you can construct the difference in one if statement and then use that in one copy of the code rather than two.

/*
 * By default this is a global search box events. Global being searchable across all columns.
 * But if an int is provided as an input then it will apply the search only on that column.
 * */
_bindSearchEvents: function (onColumnIndex) {

    var self = this;
    var args = [];
    if (Math.ceil(onColumnIndex) === Math.floor(onColumnIndex)) {
        args.push(onColumnIndex);
    }
    self.settings.search.input.live('keyup', function () {
        var argsCopy = [].concat(args);           // make copy of args
        argsCopy.unshift($(this).val());      // push item onto front of args
        self.widget.fnFilter.apply(self.widget, argsCopy);
    });

    self.settings.search.reset.live('click', function () {
        self.settings.search.input.val("");
        var argsCopy = [].concat(args);
        argsCopy.unshift("");                 // push item onto front of args
        self.widget.fnFilter.apply(self.widget, argsCopy);
    });
},

If the fnFilter function is written to check to see if the second parameter is null (rather than checking the number of arguments passed), then this shorter version could work also, but we'd have to see what the fnFilter documentation says or how the fnFilter source code actually works to know if this shorter method would work:

/*
 * By default this is a global search box events. Global being searchable across all columns.
 * But if an int is provided as an input then it will apply the search only on that column.
 * */
_bindSearchEvents: function (onColumnIndex) {

    var self = this;
    var index = null;
    if (Math.ceil(onColumnIndex) === Math.floor(onColumnIndex)) {
        index = onColumnIndex;
    }
    self.settings.search.input.live('keyup', function () {
        self.widget.fnFilter($(this).val(), index);
    });

    self.settings.search.reset.live('click', function () {
        self.settings.search.input.val("");
        self.widget.fnFilter("", index);
    });
},


Keep classes anonymous in TicTacToe program My console TicTacToe program

This is my first large project attempt and I want to learn how to keep classes as anonymous as reasonable and as protected/secure as reasonable.

The program works as intended and I think it meets the requirements but can you guys share some pointers?

class TicTacToe
    WINNING_SCORES = [7,56,448,73,146,292,84,273]
    SCORE_HASH = {0 => 1, 1 => 2, 2 => 4,
                             3 => 8, 4 => 16, 5 => 32,
                             6 => 64, 7 => 128, 8 => 256}

    class Player
            attr_reader         :mark
            attr_accessor :score
            attr_accessor :turn

            def initialize(player_mark)
                    @mark = player_mark
                    @score = 0
                    @turn = 0
            end
    end

    def initialize(player1_char, player2_char)
            @player1 = Player.new(player1_char)
            @player2 = Player.new(player2_char)
            set_turn()
            setup_board()
            display_board()
            greet_player()
    end

    def go(spot)
            player_mark = get_player()
            if fill_spot?(spot, player_mark)
                    update_score()
                    next_turn()
                    display_board()
                    greet_player()
            end
    end

    private
    # ----------------------------------------------------------------------------

    def setup_board
            @gameboard = []
            9.times { @gameboard << " " }
    end

    def set_turn()
            @player1.turn = rand(2)
            @player2.turn = 1 unless @player1.turn == 1
    end

    def update_score
            @player1.score = calculate_score(@player1.mark)
            @player2.score = calculate_score(@player2.mark)
            congrats(@player1.mark) if winner?(@player1.score)
            congrats(@player2.mark) if winner?(@player2.score)
    end

    def congrats(player)
            display_board()
            puts "\n\n\t CONGRAGULATIONS PLAYER #{player}, YOU WON!\n\n"
            exit()
    end

    def calculate_score(mark)
            score = 0
            @gameboard.each_with_index do |char, i|
                    score += SCORE_HASH[i] if char == mark
            end
            score
    end

    def winner?(player_score)
            WINNING_SCORES.each do |score|
                    return true if player_score & score == score
            end
            false
    end

    def display_board
            puts "\n\n\t TicTacToe \n\n"
            puts "        1|2|3 \t #{@gameboard[0]}|#{@gameboard[1]}|#{@gameboard[2]}"
            puts "        ------ \t ------"
            puts "        4|5|6 \t #{@gameboard[3]}|#{@gameboard[4]}|#{@gameboard[5]}"
            puts "        ------ \t ------"
            puts "        7|8|9 \t #{@gameboard[6]}|#{@gameboard[7]}|#{@gameboard[8]}"
            puts "\n"
    end

    def greet_player
            player_mark = get_player()
            puts "\t#{player_mark}'s turn!"
    end

    def next_turn
            @player1.turn, @player2.turn = @player2.turn, @player1.turn
    end

    def fill_spot?(spot, player_mark)
            if @gameboard[spot-1] == " " then
                    @gameboard[spot-1] = player_mark
                    return true
            else
                    spot_taken()
                    display_board()
                    greet_player()
                    return false
            end
    end

    def spot_taken
            puts "\n\n\t That spot is already taken!"
    end

    def get_player
            return @player1.mark if @player1.turn == 1
            return @player2.mark if @player2.turn == 1
            raise NoPlayerFound "Neither player's turn is set!"
    end
end

game = TicTacToe.new("X","O")

loop do
    print "Choose spot (1..9): "
    choice = gets.chomp.to_i
    game.go(choice) if choice.between?(1,9)
end

def setup_board
  @gameboard = []
  9.times { @gameboard << " " }
end

Can just be:

@gameboard = Array.new(9){ " " }

It is idiomatic in Ruby to omit parentheses when not needed, e.g. display_board instead of display_board().

def winner?(player_score)
  WINNING_SCORES.each do |score|
    return true if player_score & score == score
  end
  false
end

Instead:

def winner?(player_score)
  WINNING_SCORES.any?{ |score| player_score & score == score }
end

(I'm trusting that the bitwise AND logic you have there is correct.)


Press any login button on any site I'm working on a script that will be able to press the login button on any site for an app I'm working on. I have it working (still a few edge cases to work out such as multiple submit buttons and iFrames and I'm going to rewrite it to make it prettier once I figure them out).

Can you think of any ways I could improve it or make it more robust?

Some notes:

form.submit() doesn't work consistently since many sites don't allow it to be executed via a script.
Three types of login buttons I see consistently are 'a', 'button', and 'input'.

var buttonElements = document.getElementsByTagName('button');
var inputElements = document.getElementsByTagName('input');
var linkElements = document.getElementsByTagName('a');
var loginElement = null;
// For Button/Input
if (buttonElements != null){
    loginElement = getSubmitElement(buttonElements);
}
if (loginElement != null){
    console.log(loginElement);
    loginElement.click();
}
else{
    if (inputElements != null){
        loginElement = getSubmitElement(inputElements);
    }
    if (loginElement != null){
        console.log(loginElement);
        loginElement.click();
    }
    else{
        if (linkElements != null){
            loginElement = getLinkElement(linkElements);
        }
        if (loginElement != null){
            console.log(loginElement);
            loginElement.click();
        }
    }
}
// Used for link elements (<a/>)
function getLinkElement(elements){
    for (i=0;i < elements.length; i++){
        // Is a submit button
        // Check ID class
        if (isLoginElement(elements[i],'id')){
            return elements[i];
        }
    }
    for (i=0;i < elements.length; i++) {
        if(isLoginElement(elements[i], 'name')){
            return elements[i];
        }
        else if(isLoginElement(elements[i],'class')){
            return elements[i];
        }
    }
    return null;
}

// Use for Button or Input - Checks the id/name/class attributes
function getSubmitElement(elements){
    for (i=0;i < elements.length; i++){
        var type = elements[i].getAttribute('type');
        if (type != null){
            if (type.toLowerCase() == 'submit' || type.toLowerCase() == 'image'){
                // Is a submit button
                // Check ID class
                if (isLoginElement(elements[i],'id')){
                    return elements[i];
                }
                else if(isLoginElement(elements[i], 'name')){
                    return elements[i];
                }
                else if(isLoginElement(elements[i],'class')){
                    return elements[i];
                }
            }
        }

    }
    return null;
}

// Checks if a specific attribute value contains our preset login strings
function isLoginElement(element, attribute){
    var attr;
    if (attribute != 'content'){
        attr = element.getAttribute(attribute);
    }
    else{
        attr = element.textContent;
    }
    if (attr != null){
        if (hasLogin(attr)){
            return true;
        }
    }
    return false;
}

// Checks that an attribute's value contains login, sign, or submit
function hasLogin(str){
    var substrings = ['login', 'sign', 'submit', 'continue'];
    if (new RegExp(substrings.join("|")).test(str.toLowerCase())) {
        console.log('Match Found - ' + str);
        return true;
    }
    return false;
}

I have a couple of thoughts on this in terms of coding style,
but not really regarding robustness.
I hope you'll get more in-depth reviews in terms of robustness too!

Simplifying the complex if-else chain of loginElement

In this code:

var loginElement = null;
if (buttonElements != null){
    loginElement = getSubmitElement(buttonElements);
}
if (loginElement != null){
    console.log(loginElement);
    loginElement.click();
}
else{
    // ...
}

Notice that if buttonElements is null, then loginElement will still be null after the first if statement, in which case the second if will be pointless.
This is equivalent to the above but without the wasted step:

var loginElement = null;
if (buttonElements != null){
    loginElement = getSubmitElement(buttonElements);
    if (loginElement != null){
        console.log(loginElement);
        loginElement.click();
    }
}
else{
    // ...
}

On closer look, the entire if-else chain can be simplified to this:

var loginElement = null;

if (buttonElements != null) {
    loginElement = getSubmitElement(buttonElements);
} else if (inputElements != null) {
    loginElement = getSubmitElement(inputElements);
} else if (linkElements != null) {
    loginElement = getLinkElement(linkElements);
}

if (loginElement != null) {
    console.log(loginElement);
    loginElement.click();
}

Cache duplicated operations in a local variable

In this statement,
in the worst case,
the type variable will be lowercased twice:

if (type.toLowerCase() == 'submit' || type.toLowerCase() == 'image'){

It would be better to lowercase only once and store in a local variable:

var normalizedType = type.toLowerCase();
if (normalizedType == 'submit' || normalizedType == 'image') {

Simplify else-if chains using early returns

When you return in the if-else-if branches like this:

if (isLoginElement(elements[i],'id')){
    return elements[i];
}
else if(isLoginElement(elements[i], 'name')){
    return elements[i];
}
else if(isLoginElement(elements[i],'class')){
    return elements[i];
}

You can drop the else, which makes it slightly simpler:

if (isLoginElement(elements[i],'id')) {
    return elements[i];
}
if (isLoginElement(elements[i], 'name')) {
    return elements[i];
}
if (isLoginElement(elements[i],'class')) {
    return elements[i];
}

Other simplification

This code:

if (attr != null){
    if (hasLogin(attr)){
        return true;
    }
}
return false;

Can be written simpler as:

return attr != null && hasLogin(attr);


Clean regex matches with named matches I have a regex pattern that will match some elements from a string and give them a particular name. For example, #^(?<foo>.*)$# will match the whole string and name it foo.

My problem is that the matches also contain the "classic", numbered matches.

For example:

<?php

$pattern = '#^(?<foo>.*)$#';
$str = '123';
$matches = null;
preg_match($pattern, $str, $matches);
print_r($matches);

will print:

Array
(
    [0] => 123
    [foo] => 123
    [1] => 123
)

As all the matches will always be named, I decided to manually remove the numbered indexes from $matches in order to clean things up:

<?php

$pattern = '#^(?<foo>.*)$#';
$str = '123';
$matches = null;
if (preg_match($pattern, $str, $matches))
{
    foreach ($matches as $key => $value)
    {
        if (is_int($key))
            unset($matches[$key]);
    }
}
print_r($matches);

Which prints:

Array
(
    [foo] => 123
)

It works, but I feel that it can be improved. Is there a better way to do this, especially without the foreach loop?

In practice, $pattern and $str can be much more complicated than the example I gave and I want this to be executed as fast as possible.

If you know what the named matches are up front (ie if you know what the pattern looks like, you can simply use array_intersect_key to extract only the values that have a specific key from the $matches array:

$names = ['foo' => null];

$pattern = '#^(?<foo>.*)$#';//changed ^ to $ at the end ;)
$str = '123';
if (preg_match($pattern, $str, $matches))
{//or return here
    $matches = array_intersect_key($matches, $names);
}
return $matches;

Of course, if you are not in control of the names that will be used in the pattern, you'll have to either iterate over the $matches array like you're doing now. However, I'd recommend you don't use unset on the $matches array, but rather copy the relevant values to a new one and return that array instead:

$returnValue = [];//new array
foreach ($matches as $k => $v) {
    if (!is_int($k)) {
        $returnValue[$k] = $v;
    }
}

There are a couple of reasons for this:

It's considered bad practice to change the array you're iterating over inside the loop. It can cause issues in certain cases, and it will definitely bite you if you decide to pick up another language
PHP's memory management and copy-on-write mechanisms work well with code like the loop above: the new array will be assigned a reference to the value in $matches, but once the function returns, $matches is GC'ed. The values not referenced by $returnValue will be GC'ed, the other values are then "owned" by the return array (that's not 100% accurate, but it's true enough for now)
It's probably the most efficient (in terms of readability and execution time) approach.

Time to get silly

Just for the fun of it: you can opt for an inception-style preg_match_all call on the regex you're passing to preg_match (regex matching on a regex... let's be honest, that sounds a tad absurd). It's silly, but it can be done:

$pattern = '#^(?<foo>.*)$#';//changed ^ to $ at the end ;)
$str = '123';
$names = null;
if (preg_match_all('/(?<=\?<)([^>]+)/', $pattern, $matches))
{//create an assoc array containing the match names
    $names = array_fill_keys($matches[0], null);
}

$matches = null;
if (preg_match($pattern, $str, $matches))
{
    if ($names) {
        //gets only the named keys
        $matches = array_intersect_key($matches, $names);
    }
    return $matches;
}
//throw exception, return null, or do something else here

Now, this is not exactly the way to go, but in some cases it might happen that you're processing a string, but what regex you apply to it can change depending on any number of reasons. In that case, array_intersect_key is definitely worth a look, seeing as it only returns the keys that exist in all of the arguments you pass to it:

class Foo
{
    const DOMAIN_PATTERN = '/(?<=@)(?<domain>[^@\.]+)(?=\.)/';//or something
    const EXTENSION = '/\.(?<extension>[a-z]{3,4})$/';

    protected static $names = [
        'domain'     => null,
        'extension'  => null,
    ];

    protected $mode = null;

    public function setValidationOptions(array $options)
    {//based on these options, one or more specific regex's will be applied to the data
        $this->mode = $options;
        return $this;
    }
    public function validateString($string)
    {
        $regex = $this->getPatterns();
        $result = [];
        foreach ($regex as $pattern) {
            if (preg_match($string, $pattern, $matches)) {
                $result = array_merge(
                    $result,
                    array_intersect_keys(
                        $matches,
                        static::$names
                    )
                );
            }
        }
        return $result;
    }
}

This is just a crude example of how you could use array_intersect_key to handle regex matches with named sub-patterns


Barrier implementation in C++ I've implemented a barreir in C++ close to one showed here. In the project code it's used only with two threads and seems to work. A synthetic test didn't expose any flaws too. But I suspect that on some machines it segfaults and/or falls through wait() call.
How do you think, can it fall through wait or is any race scenario possible with it?
Used mainly on Windows machines, if it matters.

class Barrier
{
public:
    explicit Barrier(uint16_t iCount) : maxThreads(iCount), count(iCount) {}

    void wait(std::chrono::milliseconds = std::chrono::milliseconds(0));

private:
    void notifyAll();

private:
    std::mutex mutex;
    std::condition_variable cv;
    uint16_t maxThreads;
    uint16_t count;
    bool generation = true;
};

inline void Barrier::wait(const std::chrono::milliseconds timeWait)
{
    std::unique_lock<std::mutex> ulock(mutex);
    auto gen = generation;
    const auto waitCondition = [this, gen]{ return (gen != generation); };
    if (0 == --count) notifyAll();
    else {
        if (timeWait == std::chrono::milliseconds(0)) cv.wait(ulock, waitCondition);
        else {
            const auto timedOut = !cv.wait_for(ulock, timeWait, waitCondition);
            if (timedOut) ++count;
        }
    }
}

inline void Barrier::notifyAll()
{
    generation = !generation;
    count = maxThreads;
    cv.notify_all();
}

P.S. I know that generation is poor naming for this particular variable. Here it's intended to express similarity to the same name variable from abovementioned SO question code.

Naming nitpicks: The relationship between count and maxThreads is not obvious. I would probably name them something like threads_arrived_ and total_threads_ (using the common convention of postfix-underscore for data members).

You might like to know that there's a std::barrier in C++20. Your wait is what C++20 calls arrive_and_wait.  Your generation is what C++20 calls the "phase."

Why is your notifyAll a separate function, given that it's private and called only in a single place? You should just inline it. As a bonus, you could then drop the mutex lock before calling cv.notify_all() — this is widely quoted as a performance optimization.

You wrote:

void wait(std::chrono::milliseconds = std::chrono::milliseconds(0));

Being as default arguments are the devil, this is a red flag. Personally I would write this as

void arrive_and_wait();
void arrive_and_wait_for(std::chrono::milliseconds timeout);

and continue adding an arrive_and_wait_until as well. I would not use overloading or default arguments to accomplish this, because "wait for 0 milliseconds" and "wait forever" are completely different things. You really really don't want someone doing, like,

barrier.wait(user_configured_timeout);      // wait for 1ms
barrier.wait(user_configured_timeout / 2);  // wait for half that

and getting an infinite wait instead. Follow the STL's lead here: wait, wait_for, and wait_until are all different operations.

if (0 == --count) notifyAll();
else {
    if (timeWait == std::chrono::milliseconds(0)) cv.wait(ulock, waitCondition);
    else {
        const auto timedOut = !cv.wait_for(ulock, timeWait, waitCondition);
        if (timedOut) ++count;
    }
}

This is a ridiculously misindented piece of code. Try this:

if (0 == --count) {
    notifyAll();
} else if (timeWait == std::chrono::milliseconds(0)) {
    // note that our refactored `wait_for` will not contain this branch
    cv.wait(ulock, waitCondition);
} else {
    // note that our refactored `wait` will not contain this branch
    bool timedOut = !cv.wait_for(ulock, timeWait, waitCondition);
    if (timedOut) ++count;
}

That ++count smells like a race condition to me. Sure, we're operating under the mutex lock here; but I suspect it's possible that some other thread might have observed the decremented value of count and acted on it, which might mean that here we're bumping count from 0 to 1, or from maxThreads to maxThreads+1. I'm not sure that this can happen, but I would look at this codepath very closely if I were you.

Consider what happens in code like

Thread A         Thread B         Thread C

Barrier b(2);
b.wait();        b.wait();
                 <notifyAll>
                 b.wait();        b.wait();
                                  <notifyAll>
                 <awaken>
<awaken>
<resume waiting>

If I understand correctly, when thread A finally gets scheduled and belatedly awakens from its cv.wait, it will observe gen == generation because generation has been toggled twice since it went to sleep. So it will remain blocked, even though it should have become unblocked as a result of thread B's first b.wait().

I'm not sure how to fix this, but it might involve thread B being forced to block, itself, until all the other threads in the current phase have indicated that they're unblocked. In fact I'm not sure that that would even fix the issue.

It's worth mentioning that it is physically possible for the programmer to destroy a Barrier while some other thread is still blocked waiting on it. Presumably you're okay with having this be undefined behavior.


Periodically checking for internet connection I have a SwingWorker which checks periodically for an internet connection and updates a jLabel accordingly.

public class InternetCheckWorker extends SwingWorker<Void, String> {

        @Override
        protected Void doInBackground() throws InterruptedException {
            while (true) {
                try (Socket socket = new Socket(HOSTNAME, 80);) {
                    process("on");
                } catch (IOException e) {
                    process("off");
                }
                Thread.sleep(500);
            }
        }

        protected void process(String status) {
            jInternetConnectionLabel.setText("Connection " + status);
        }
    }

    private static final String HOSTNAME = "www.google.com";

Is there maybe a more proper way of doing it or even a library with net tools (along with database management tools or general utility tools) that could do this and have less custom code littering my project?

All in all I am looking for the most minimalistic way.

I know this may sound silly but what exactly do mean by connected to the internet? You're going to have to define what this means to your application.

In your example, if "www.google.com" goes down then the user is no longer connected to the internet and this is not necessarily true. Last time I checked, google isn't the internet (although it is very close).

The problem all boils down to the fact that the user may be connected to some part of the internet but unable to reach other parts due to corporate/national firewalls, broken equipment, wrongly configured hosts files etc. Or just a very zealous ISP who insists every one go through their gateway googlez.net to show you ads on every google search.

It makes more sense to test for an URL that actually means something for your application (I can't tell what this is going to be used for so I can't offer anything more specific). Like a service that you interact with.

As you see it's a tricky question and you need to answer some of these questions before you can come up with a good solution.


Web page downloader that supports redirects I wrote a class that downloads the HTML of a page from the internet. This class also manages redirects. This works well but I'm not very satisfied about time management for slower servers. In fact, if the website is slow, the method GetHtml will return an empty string, so  how can I optimize this class, to prevent such a situation from happening?

CODE

using System;
using System.IO;
using System.Net;
using System.Net.Http;

namespace SWP.Helpers
{
    /// <summary>
    /// Download html from an Internet page.
    /// </summary>
    public class NetworkHelper
    {
        /// <summary>
        /// Store the previous url used in the request.
        /// </summary>
        private Uri _storedUrl = null;

        /// <summary>
        /// Store the previous html downloaded.
        /// </summary>
        private string _storedData = "";

        /// <summary>
        /// Get the html structure of a site.
        /// </summary>
        /// <param name="url">Represents the URL of the page where to download the data.</param>
        /// <returns>Return a string that contains the html of the site.</returns>
        public string GetHtml(Uri url)
        {
            //If the current url is the same of the older, then will returned the previous html structure, in this way will not be wasted any time to perform another connection.
            if (url == _storedUrl)
                return _storedData;
            else
                _storedUrl = url;

            //Web request handler.
            HttpWebRequest webReq = (HttpWebRequest)WebRequest.Create(url);

            try
            {
                //Configure the client manager for perform a page compression.
                HttpClientHandler handler = new HttpClientHandler();
                handler.AutomaticDecompression = DecompressionMethods.Deflate | DecompressionMethods.GZip;

                //Handle the client to manage possible redirect.
                HttpClient httpClient = new HttpClient(handler);

                //Set the web request.
                HttpRequestMessage request = new HttpRequestMessage
                {
                    RequestUri = url,
                    Method = HttpMethod.Get
                };

                //Get the response.
                HttpResponseMessage response = httpClient.SendAsync(request).Result;

                //Return the code of the web request.
                int statusCode = (int)response.StatusCode;

                //If the code is within this range, then the server has executed a redirect.
                if (statusCode >= 300 && statusCode <= 399)
                {
                    //Get the new url.
                    Uri redirectUri = response.Headers.Location;

                    //Sanitize the url.
                    if (!redirectUri.IsAbsoluteUri)
                    {
                        redirectUri = new Uri(request.RequestUri.GetLeftPart(UriPartial.Authority) + redirectUri);
                    }

                    //Call again this method with the correct url.
                    return GetHtml(redirectUri);
                }

                _storedData = response.Content.ReadAsStringAsync().Result;
                return _storedData;
            }
            catch (WebException)
            {
                throw;
            }
        }
    }
}

as you can see, the logic is really simple, and the method is well commented. Are there any problems I haven't taken into account?

HttpClient and associated classes provide most of the desired behavior.

HttpClientHandler already has the ability to follow redirection responses out of the box with AllowAutoRedirect property. You can also specify how many redirections to allow.

That can be extracted out into a function

static HttpMessageHandler CreateHandler() {
    var handler = new HttpClientHandler();
    // if the framework supports redirect configuration
    // set max redirect to the desired amount. The default is 50
    if (handler.SupportsRedirectConfiguration) {
        handler.AllowAutoRedirect = true;
        handler.MaxAutomaticRedirections = 5;
    }
    // if the framework supports automatic decompression
    // set automatic decompression
    if (handler.SupportsAutomaticDecompression) {
        handler.AutomaticDecompression = System.Net.DecompressionMethods.GZip |
            System.Net.DecompressionMethods.Deflate;
    }
    return handler;
}

Creating multiple instances of HttpClient can cause an exhaustion of available sockets, so it is suggested to try keeping one instance for the duration of the application.

Specify a short time out duration for slower responses to help with time management.

static Lazy<HttpClient> httpClient = new Lazy<HttpClient>(() => {
    var handler = CreateHandler();
    return new HttpClient(handler) {
        Timeout = TimeSpan.FromSeconds(3)
    };
});

Timeout can be adjusted to specific needs.

Cancellation tokens can also be used to help with time management.

Seeing as any exception is just being rethrown, then there is really no need for the try/catch. The exception would be handled by the caller.

HttpClient uses an asynchronous API. Its dependents should also be asynchronous all the way through and avoid blocking calls like .Result or .Wait to avoid deadlocks.

The refactored helper now looks like this

/// <summary>
/// Download html from an Internet page.
/// </summary>
public class NetworkHelper {
    static Lazy<HttpClient> httpClient = new Lazy<HttpClient>(() => {
        var handler = CreateHandler();
        return new HttpClient(handler) {
            Timeout = TimeSpan.FromSeconds(3)
        };
    });

    static HttpMessageHandler CreateHandler() {
        var handler = new HttpClientHandler();
        // if the framework supports redirect configuration
        // set max redirect to the desired amount the default is 50
        if (handler.SupportsRedirectConfiguration) {
            handler.AllowAutoRedirect = true;
            handler.MaxAutomaticRedirections = 5;
        }
        // if the framework supports automatic decompression
        // set automatic decompression
        if (handler.SupportsAutomaticDecompression) {
            handler.AutomaticDecompression = System.Net.DecompressionMethods.GZip |
                System.Net.DecompressionMethods.Deflate;
        }
        return handler;
    }

    /// <summary>
    /// Get the html structure of a site.
    /// </summary>
    /// <param name="url">Represents the URL of the page where to download the data.</param>
    /// <returns>Return a string that contains the html of the site.</returns>
    public async Task<string> GetHtmlAsync(Uri url, CancellationToken cancellationToken = default(CancellationToken)) {
        var response = await httpClient.Value.GetAsync(url, cancellationToken);
        var content = await response.Content.ReadAsStringAsync();
        return content;
    }
}

And used

//...

var client = new NetworkHelper();
var html = await clien.GetHtmlAsync(uri);


Using Parsec for lexing&parsing I'm creating list of Tokens from input [Char] stream using Parsec v3. The definition of Token looks like this:

data Token = CharKeyword | OpeningBracket | Identifier String | Natural Int

As result of calling parse lexComb "" inputStream i have [Token]. Everything OK so far.

Now i want to parse this list of tokens using another parse invocation, but i've found myself writing such boilerplate code for every Token constructor:

psOpBracket = tokenPrim s np p
    where
        s _ = "'{'"
        p (OpeningBracket) = Just Nothing
        p _ = Nothing
        np pos _ _  = pos

another example:

psPacketName = tokenPrim s np p
    where
        s x = x
        p (Identifier x) = Just x
        p _ = Nothing
        np pos _ _  = pos

I wonder if it's right to use tokenPrim at all. And if so, is there any way to automatically generate parsers for every token? Maybe Template Haskell?

I've solved it by using Text.ParserCombinators.Parsec.GeneralizedToken from MissingH package:

psOpBracket = tokeng (\x -> case x of TokOpBracket -> Just Nothing ; _ -> Nothing)

I had to use tokenPrim for cases when Token constructor takes arguments, though.


Dynamic programming based range minimum query I am implementing a DP-based range minimum query data structure and algorithm in Python 2.7. Any advice on algorithm time complexity improvements, code bugs or code style issues are appreciated.

More information about range minimum query.

from collections import defaultdict
import random
def build_rmq(numbers):
    rmq_map = defaultdict(int) # key: tuple of (start, end) index, value: min value
    j = 0
    while 2 ** j - 1 < len(numbers):
        j += 1
    j -= 1
    # print j
    for step in range(0,j+1):
        for i in range(0, len(numbers)):
            if step == 0:
                rmq_map[(i,step)] = numbers[i]
                continue
            if i + 2 ** (step-1) < len(numbers):
                rmq_map[(i,step)] = min(rmq_map[(i, step-1)], rmq_map[(i + 2 ** (step-1), step-1)])
            else:
                rmq_map[(i,step)] = rmq_map[(i, step-1)]

    return rmq_map

def rmq_query(start, end, rmq_map):
    j = 0
    while start + 2 ** j - 1 <= end:
        j += 1
    j -= 1
    x = rmq_map[(start, j)]
    y = rmq_map[(end + 1 - 2 ** j, j)]
    return min(x,y)

if __name__ == "__main__":
    numbers = [0,5,2,5,4,3,1,6,3]
    rmq_map = build_rmq(numbers)
    #print rmq_map
    start = random.randint(0, len(numbers)-1)
    end = random.randint(0, len(numbers)-1)
    if start > end:
        start, end = end, start
    print start, end, numbers[start:end+1]
    print rmq_query(start, end, rmq_map)

1. Review

There are no docstrings. What do these functions do? What do they return? How do I use them?
The function build_rmq constructs a persistent data structure which can then be queried by calling the function rmq_query. When you have a persistent data structure and functions that operate on it, this is good candidate for making into a class.
The comment on this line is not quite right:

rmq_map = defaultdict(int) # key: tuple of (start, end) index, value: min value

the second element of the key is a step, not an end index.
There is no need to use a defaultdict here, since the code always fills in the entry before looking it up. An ordinary dictionary would do fine.
This code:

j = 0
while 2 ** j - 1 < len(numbers):
    j += 1

computes the smallest power of 2 that is greater than len(numbers). It would be simpler to use int.bit_length:

j = len(numbers).bit_length()

(In ChatterOne's answer it is suggested that you use int(log(len(numbers), 2)). I think this is a dangerous suggestion, because log makes approximations that can lead to errors. For example,

int(log(2**48 - 1, 2))

evaluates to 48, but the correct result is 47. In order to check that these errors can't actually cause your code to fail, you need to do numerical analysis. This seems complicated and error-prone to me, compared to sticking with integer arithmetic where you know the results are exact.)
Similarly, this loop:

j = 0
while start + 2 ** j - 1 <= end:
    j += 1

becomes:

j = (end - start + 1).bit_length()

This code subtracts 1 from j and then adds it back again, which is not necessary:

j -= 1
for step in range(0,j+1):

range(0, x) can be abbreviated to range(x).
rmq_map[(i,step)] does not need the parentheses: rmq_map[i, step] is the same.
The two loops over step and i can be combined into a single loop using itertools.product
I would avoid the special case if step == 0 by having a separate loop to handle that case:

rmq_map = {(i, 0): m for i, m in enumerate(numbers)}

and then looping over step starting from 1.
In Python it is usual to represent a range in half-open fashion (exclusive at the upper end). It would be convenient if rmq_query took its arguments in the same way.
The top-level code runs a single range query but does not check that the result is correct. It would be easy to check the result of rmq_query(start, end) against min(numbers[start:end+1]). It would then be easy to turn this into a unit test using the features in the unittest module.

2. Revised code

from itertools import product

class RangeMinimum(object):
    "Data structure providing efficient range-minimum queries."

    def __init__(self, numbers):
        "Build a RangeMinimum object for the given sequence of numbers."
        # Mapping from (start, step) to min(numbers[start:start + 2**step])
        self._rmq = rmq = {(i, 0): m for i, m in enumerate(numbers)}
        n = len(numbers)
        for step, i in product(range(1, n.bit_length()), range(n)):
            j = i + 2 ** (step-1)
            if j < n:
                rmq[i, step] = min(rmq[i, step-1], rmq[j, step-1])
            else:
                rmq[i, step] = rmq[i, step-1]

    def query(self, start, stop):
        "Return min(numbers[start:stop])."
        j = (stop - start).bit_length() - 1
        x = self._rmq[start, j]
        y = self._rmq[stop - 2 ** j, j]
        return min(x, y)

import unittest
import random

class TestRangeMinimum(unittest.TestCase):
    def test_range_minimum(self):
        n = 1000
        numbers = [random.randrange(n) for _ in range(n)]
        rmq = RangeMinimum(numbers)
        for _ in range(n):
            start, stop = sorted(random.sample(range(n + 1), 2))
            self.assertEqual(rmq.query(start, stop), min(numbers[start:stop]))

3. Generalization

There's nothing special about the minimum here — exactly the same algorithm can be used for range-maximum queries, or for querying other kinds of statistic on a range (such as the longest common prefix, or the union of sets).

So that suggests this kind of generalization:

from itertools import product

class RangeQuery(object):
    "Data structure providing efficient range queries."

    def __init__(self, items, fn):
        """Build a RangeQuery object for a sequence of items.

        fn -- function taking two items and returning their query
        result, for example "min" to query the range-minimum. It must be
        associative, commutative, and idempotent.

        """
        # Mapping from (start, step) to reduce(fn, items[start:start + 2**step])
        self._rq = rq = {(i, 0): item for i, item in enumerate(items)}
        self._fn = fn
        n = len(items)
        for step, i in product(range(1, n.bit_length()), range(n)):
            j = i + 2 ** (step-1)
            if j < n:
                rq[i, step] = fn(rq[i, step-1], rq[j, step-1])
            else:
                rq[i, step] = rq[i, step-1]

    def query(self, start, stop):
        "Return reduce(fn, items[start:stop])."
        j = (stop - start).bit_length() - 1
        x = self._rq[start, j]
        y = self._rq[stop - 2 ** j, j]
        return self._fn(x, y)

import random
import unittest

class TestRangeQuery(unittest.TestCase):
    def test_range_minmax(self):
        n = 1000
        items = [random.randrange(n) for _ in range(n)]
        rqmin = RangeQuery(items, min)
        rqmax = RangeQuery(items, max)
        for _ in range(n):
            start, stop = sorted(random.sample(range(n + 1), 2))
            self.assertEqual(rqmin.query(start, stop), min(items[start:stop]))
            self.assertEqual(rqmax.query(start, stop), max(items[start:stop]))


Ultra-Beginner Python FizzBuzz ... Am I missing something? I just started programming in Python this morning, and it is (more or less) my first programming language. I've done a bit of programming before, but never really did much except for "Hello World" in a few languages. I searched around for some Python FizzBuzz solutions, and they all seem significantly more complicated then mine, so I think I must be missing something, even though it works correctly. Could you guys point out any errors I've made, or things I can improve?

count = 0
while (count < 101):
    if (count % 5) == 0 and (count % 3) == 0:
        print "FizzBuzz"
        count = count +1
    elif (count % 3) == 0:
        print "Fizz"
        count = count + 1
    elif (count % 5) == 0:
        print "Buzz"
        count = count +1
    else:
        print count
        count = count + 1

Lose the useless brackets

This:

while (count < 101):

can just be:

while count < 101:

Increment out of the ifs

Wouldn't be easier to do:

count = 0
while count < 101:
    if count % 5 == 0 and count % 3 == 0:
        print "FizzBuzz"
    elif count % 3 == 0:
        print "Fizz"
    elif count % 5 == 0:
        print "Buzz"
    else:
        print count

    count = count + 1    # this will get executed every loop

A for loop will be better

for num in xrange(1,101):
    if num % 5 == 0 and num % 3 == 0:
        print "FizzBuzz"
    elif num % 3 == 0:
        print "Fizz"
    elif num % 5 == 0:
        print "Buzz"
    else:
        print num

I've also renamed count to num since it doesn't count much, is just a number between 1 and 100.

Let's use only one print

Why do 4 different print, when what is really changing is the printed message?

for num in xrange(1,101):
    if num % 5 == 0 and num % 3 == 0:
        msg = "FizzBuzz"
    elif num % 3 == 0:
        msg = "Fizz"
    elif num % 5 == 0:
        msg = "Buzz"
    else:
        msg = str(num)
    print msg

Light bulb!

"FizzBuzz" is the same of "Fizz" + "Buzz".

Let's try this one:

for num in xrange(1,101):
    msg = ''
    if num % 3 == 0:
        msg += 'Fizz'
    if num % 5 == 0:       # no more elif
        msg += 'Buzz'
    if not msg:      # check if msg is an empty string
        msg += str(num)
    print msg

Copy and paste this last piece of code here to see what it does.

Python is a very flexible and powerful language, so I'm sure there could be other hundred and one different possible solutions to this problem :)

Edit: Improve more

There's still something "quite not right" with these lines:

if not msg:
    msg += str(num)

IMHO it would be better to do:

for num in xrange(1,101):
    msg = ''
    if num % 3 == 0:
        msg += 'Fizz'
    if num % 5 == 0:
        msg += 'Buzz'
    print msg or num

There! Now with:

print msg or num

is clear that num is the default value to be printed.