PHP - avoid many on-demand imports I'm trying to create small PHP framework for my own needs (and likes). However there are questions for which I may need advice of more wise and experienced people before things became too complicated - so I moved it to github from private repo.

Currently I'm curious about organizing imports of modules better. I use the special object, "context" - to hold links to all necessary modules - data access objects, utils etc. I use lazy initialization here, so that when field of this object is required, it is provided by the getter of the same name.

It looks like:

class ProtoContext {

    function __get($name) {
        $methodName = 'get' . ucfirst($name);
        if (!method_exists($this, $methodName)) {
            throw new Exception("No property '$name' in Context!");
        }
        $res = $this->$methodName();
        if (is_object($res)) {
            $res->ctx = $this;
        }
        $this->$name = $res;
        return $res;
    }

    protected function getElems() {
        return Elems::$elems;
    }

    protected function getUtil() {
        module('sys/Util');
        return new Util();
    }

}

main Context class is inherited from this ProtoContext, though it is not significant now. With the grow of application context may look like this:

module('sys/ProtoContext');

class Context extends ProtoContext {

    protected function getAuth() {
        module('MyAuth');
        return new MyAuth();
    }

    protected function getUsersDao() {
        module('dao/MysqlDao');
        return new MysqlDao('users');
    }

    protected function getRolesDao() {
        module('dao/MysqlDao');
        return new MysqlDao('roles');
    }

    /*
     * 5-10 more similar methods 'getSomethingDao'
     * each including MysqlDao via method 'module'
     */

    protected function getLinksCViewDao() {
        module('dao/MysqlDao');
        return new MysqlDao('linksc_view');
    }

}

There is no problem that module method may be called several times, include is performed once only. However it looks annoying that inclusion of MysqlDao is mentioned so many times. On the other hand if I pull it above the class (like import of ProtoContext) it will be imported even if I need it not. For example when addressing $ctx->auth field (which will call getAuth method).

I wonder, whether here is comfortable workaround which will preserve lazy load and lazy initialization - and at the same time will allow to get rid of extra imports?

As N.B. mentioned in their comment - why are you implementing a module method if we already have lazy loading via autoloading? I've even written a class that you can simply attach and not have to worry about loading your classes. Now, there are better autoloaders out there - like Symphony's ClassLoader/autoload.php. Anyways, here's your code, with my LazyLoader class handling the loading of methods (untested code).

/LazyLoader.php

/**
 * LazyLoader
 * A fast, strict lazy loader (time ~ 0.0001)
 *
 * Class name must match File name.
 * If class has namespace, must be called via namespace.
 *
 * @author Juan L. Sanchez <juanleonardosanchez.com>
 * @license MIT
 * @version 1.2.0
 * @internal 06.26.2013
 */

Namespace LazyLoader;

class LazyLoader{
    public static $dirRoot;

    public static function autoload($class_name){
        $file = dirname(__FILE__) .
                (strlen(self::$dirRoot) > 0 ? self::$dirRoot : "") .
                '/' . array_pop(explode("\\", $class_name)) . '.php';

        file_exists($file) ? require_once($file) : "";
    }

    public static function SetBaseDirectory($directory_root){
        self::$dirRoot = substr($directory_root, -1) == "\\" ?
                         substr($directory_root, 0, -1) : "";
    }

    public static function Register(){
        return spl_autoload_register(__NAMESPACE__ .'\LazyLoader::autoload');
    }
}

$LazyLoader = new LazyLoader;
$LazyLoader->SetBaseDirectory("Classes"); # Optional
$LazyLoader->Register();

/Context.php

<?php
module('sys/ProtoContext');

class Context extends ProtoContext {

    protected function getAuth() {
        return new MyAuth();
    }

    protected function getUsersDao() {
        return new MysqlDao('users');
    }

    protected function getRolesDao() {
        return new MysqlDao('roles');
    }

    /*
     * 5-10 more similar methods 'getSomethingDao'
     * each including MysqlDao via method 'module'
     */

    protected function getLinksCViewDao() {
        return new MysqlDao('linksc_view');
    }

}

The directory structure of the above code would look something like:

/LazyLoader.php
/Context.php
/Classes/MyAuth.php
/Classes/ProtoContext.php
/Classes/MysqlDao.php


LeetCode 410: Split Array Largest Sum I'm posting my code for a LeetCode problem copied here. If you would like to review, please do so. Thank you for your time!
Problem

Given an array which consists of non-negative integers and an integer
m, you can split the array into m non-empty continuous subarrays.
Write an algorithm to minimize the largest sum among these m
subarrays.
Note:
If n is the length of array, assume the following constraints are
satisfied:
$$
  1 \le n \le 1000 \\
  1 \le m \le \min(50, n)
  $$
Example:
Input: nums = [7,2,5,10,8] m = 2
Output: 18
Explanation: There are four ways to split nums into two subarrays. The
best way is to split it into [7,2,5] and [10,8], where the largest sum
among the two subarrays is only 18.

Code
#include <vector>

class Solution {
public:
    static int splitArray(const std::vector<int> &nums, const int m) {
        size_t lo = 0;
        size_t hi = 0;

        for (int num : nums) {
            lo = std::max(lo, (size_t) num);
            hi += num;
        }

        while (lo < hi) {
            size_t mid = lo + (hi - lo) / 2;
            if (partitioning_is_valid(nums, m - 1, mid)) {
                hi = mid;
            } else {
                lo = mid + 1;
            }
        }

        return lo;
    }

private:
    static bool partitioning_is_valid(const std::vector<int> &nums, int partitions, size_t partition_max_sum) {
        size_t accumulate_curr_partition = 0;
        for (int num : nums) {
            if (num > partition_max_sum) {
                return false;
            } else if (accumulate_curr_partition + num <= partition_max_sum) {
                accumulate_curr_partition += num;
            } else {
                partitions--;
                accumulate_curr_partition = num;

                if (partitions < 0) {
                    return false;
                }
            }
        }

        return true;
    }

};

Reference
LeetCode has a template for answering questions. There is usually a class named Solution with one or more public functions which we are not allowed to rename. For this question, the template is:
class Solution {
public:
    int splitArray(vector<int>& nums, int m) {

    }
};

Problem

Solution

Discuss

Function signatures
You've made the best out of a slightly silly situation. I think your function signatures, while abiding to the template, have improved since the last post of yours that I reviewed.
I'm not sure whether this will break LeetCode compatibility, but if you want to enforce that no one can instantiate this class, try making a private default constructor.
Mean
A simpler way to express the arithmetic mean than
        size_t mid = lo + (hi - lo) / 2;

is
        size_t mid = (hi + lo) / 2;


Writing a product() function analogous to sum Exercise 1.31

  a.  The sum
  procedure is only the simplest of a
  vast number of similar abstractions
  that can be captured as higher-order
  procedures.51 Write an analogous
  procedure called product that returns
  the product of the values of a
  function at points over a given range.
  Show how to define factorial in terms
  of product. Also use product to
  compute approximations to  using the
  pi / 4 = (2/3) * (4/3) * (4/5) * (6/5)
  * (6/7) * (8/7) ...

  b.  If your product procedure
  generates a recursive process, write
  one that generates an iterative
  process. If it generates an iterative
  process, write one that generates a
  recursive process.

I wrote the following:

Recursive:

(define (product term a next b)
  (cond
    ((> a b) 1)
    (else (* (term a) (product term (next a) next b)))))

Iterative:

(define (i-product term a next b)
  (cond ((> a b) null)
      (else
       (define (iter a result)
         (cond ((> a b) result)
               (else (iter (next a) (* (term a) result)))))
       (iter a 1))))

Multiply-integers [test - does (product ...) work?]

(define (identity x) x)
(define (inc x) (+ 1 x))
(define (multiply-integers a b) (i-product identity a inc b))

Compute pi:

(define (square x) (* x x))
(define (compute-pi steps)
  (define (next n) (+ n 2.0))
  (* 8.0 (* steps 2) (/ (i-product square 4.0 next (* (- steps 1) 2))
              (i-product square 3.0 next (* steps 2)))))

Factorial:

(define (factorial n)
  (define (next n) (+ n 1))
  (i-product identity 1 next n))

What do you think of my solution?

Since your definitions have only two cond clauses, you may replace cond with if.  Your iterative definition is a departure from the example of sum.  It should return 1 (product identity) when a is greater than b, not null.  This makes the outer cond unnecessary.

Your definition of compute-pi suffers from imprecision of float operations (it fails to produce meaningful values for n > 85).  It would be better to convert to float after computing the approximating fraction.

(define (product term a next b)
  (if (> a b)
      1
      (* (term a) (product term (next a) next b))))

(define (i-product term a next b)
  (define (iter a result)
    (if (> a b)
        result
        (iter (next a) (* (term a) result))))
  (iter a 1))

(define (compute-pi steps)
  (define (next n) (+ n 2))
  (* 8.0 (* steps 2) (/ (i-product square 4 next (* (- steps 1) 2))
                        (i-product square 3 next (* steps 2)))))


Rotating an array by position n Input is  1,2,3,4,5,6,7,8

  If rotateOrder = 2

  Output should be  7,8,1,2,3,4,5,6

  If rotateOrder = 3

  Output should be  6,7,8,1,2,3,4,5

Here is my program to do it:

int [] unOrderedArr = {1,2,3,4,5,6,7,8};
        int  orderToRotate =2;

       for(int i = 0; i<orderToRotate; i++){

           for(int j = unOrderedArr.length-1; j>0; j--){
               int temp = unOrderedArr[j];
               unOrderedArr[j] = unOrderedArr[j-1];
               unOrderedArr[j-1] = temp;

           }

        }

       for(int j = 0; j<unOrderedArr.length; j++){
         System.out.println("element is " + unOrderedArr[j]);

           }

How can it be optimized?

Validate your inputs

The current method doesn't check for invalid input:

null array
negative rotation order

I suggest to add this ad the top:

if (arr == null || order < 0) {
    throw new IllegalArgumentException("The array must be non-null and the order must be non-negative");
}

Follow standard formatting

The posted code is messy. Use your IDE to reformat it to the standard:

int[] unOrderedArr = {1, 2, 3, 4, 5, 6, 7, 8};
int orderToRotate = 2;

for (int i = 0; i < orderToRotate; i++) {
    for (int j = unOrderedArr.length - 1; j > 0; j--) {
        int temp = unOrderedArr[j];
        unOrderedArr[j] = unOrderedArr[j - 1];
        unOrderedArr[j - 1] = temp;
    }
}

Now that's a lot easier to read.

On a related note, an easy way to print an array is using Arrays.toString

Optimize

Frist of all,
what will happen if you receive arr to rotate, and the rotate order = arr.length?
The array elements will be rotated pointlessly,
just to return to their original position.
This gets even worse for multiples of arr.length.

Secondly, in this code:

for (int i = 0; i < order; i++) {
    for (int j = arr.length - 1; j > 0; j--) {
        // ...
    }
}

The above code iterates over the entire array order times.

To avoid unnecessary iterations,
you will benefit from using a clone of the original array,
and iterate over the elements only once,
at the expense of using twice as much memory.

Suggested implementation

The above suggestions and some style improvements applied:

private static void rotate(int[] arr, int order) {
    if (arr == null || order < 0) {
        throw new IllegalArgumentException("The array must be non-null and the order must be non-negative");
    }
    int offset = arr.length - order % arr.length;
    if (offset > 0) {
        int[] copy = arr.clone();
        for (int i = 0; i < arr.length; ++i) {
            int j = (i + offset) % arr.length;
            arr[i] = copy[j];
        }
    }
}

Some unit test to verify it works:

@Test
public void testRotateBy2() {
    int[] arr = {1, 2, 3, 4, 5, 6, 7, 8};
    int[] expected = {7, 8, 1, 2, 3, 4, 5, 6};
    rotate(arr, 2);
    assertArrayEquals(expected, arr);
}

@Test
public void testRotateBy3() {
    int[] arr = {1, 2, 3, 4, 5, 6, 7, 8};
    int[] expected = {6, 7, 8, 1, 2, 3, 4, 5};
    rotate(arr, 3);
    assertArrayEquals(expected, arr);
}

@Test
public void testRotateByLength() {
    int[] arr = {1, 2, 3, 4, 5, 6, 7, 8};
    int[] expected = arr.clone();
    rotate(arr, arr.length);
    assertArrayEquals(expected, arr);
    rotate(arr, arr.length * 3);
    assertArrayEquals(expected, arr);
}

@Test
public void testRotateByZero() {
    int[] arr = {1, 2, 3, 4, 5, 6, 7, 8};
    int[] expected = arr.clone();
    rotate(arr, 0);
    assertArrayEquals(expected, arr);
}


Database "design" using Symfony2 and Doctrine2 I'm working on an application, which tracks expenses. I have users, and each user can create his categories for the expenses (like Food, Bills, Transport, Drinks, Clothes) and then create expenses - each expense has one category (OneToMany relationship).

For example, the expense Banana is in the category Food for the user Faery. I also have months for each user. For example, for March 2013, user Faery has a budget of 500.  For March 2013, user SuperCoolUser has a budget of 900 and so on.

Here are my entities:

User.php

/**
 * @ORM\Entity
 * @ORM\Table(name="fos_user")
 */
class User extends BaseUser
{
    /**
     * @ORM\Id
     * @ORM\Column(type="integer")
     * @ORM\GeneratedValue(strategy="AUTO")
     */
    protected $id;

    /**
     * @ORM\OneToMany(targetEntity="Category", mappedBy="category")
     */
    protected $categories;

    /**
     * @ORM\OneToMany(targetEntity="Expense", mappedBy="expense")
     */
    protected $expenses;

    public function __construct()
    {
        parent::__construct();

        $this->categories = new ArrayCollection();
        $this->expenses = new ArrayCollection();
    }
    // ...
}

Category.php

/**
 * @ORM\Table(name="category")
 * @UniqueEntity(fields={"name", "user"})
 */
class Category
{
    /**
     * @ORM\Id
     * @ORM\Column(type="integer")
     * @ORM\GeneratedValue(strategy="AUTO")
     */
    protected $id;

    /**
     * @ORM\OneToMany(targetEntity="Expense", mappedBy="expense")
     */
    protected $expenses;

    public function __construct()
    {
        $this->expenses = new ArrayCollection();
    }

    /**
     * @ORM\Column(type="string", length=100)
    protected $name;

    /**
     * @ORM\ManyToOne(targetEntity="User", inversedBy="fos_user")
     * @ORM\JoinColumn(name="fos_user_id", referencedColumnName="id")
     */
    protected $user;
    // ...
}

Expense.php

/**
 * @ORM\Table(name="expense")
 */
class Expense
{
    /**
     * @ORM\Id
     * @ORM\Column(type="integer")
     * @ORM\GeneratedValue(strategy="AUTO")
     */
    protected $id;

    /**
     * @ORM\ManyToOne(targetEntity="User", inversedBy="fos_user")
     * @ORM\JoinColumn(name="fos_user_id", referencedColumnName="id")
     */
    protected $user;

    /**
     * @ORM\ManyToOne(targetEntity="Category", inversedBy="category")
     * @ORM\JoinColumn(name="category_id", referencedColumnName="id")
     */
    protected $category;

    /**
     * @ORM\Column(type="string", length=100)
     */
    protected $product;

    /**
     * @ORM\Column(type="string")
     */
    protected $description;

    /**
     * @ORM\Column(type="float")
     */
    protected $price;

    /**
     * @ORM\Column(type="date")
     */
    protected $date;
    // ...
}

Month.php

/**
 * @ORM\Table(name="month")
 */
class Month
{
    /**
     * @ORM\Id
     * @ORM\Column(type="integer")
     * @ORM\GeneratedValue(strategy="AUTO")
     */
    protected $id;

    /**
     * @ORM\Column(type="string")
     */
    protected $name;

    /**
     * @ORM\ManyToOne(targetEntity="User", inversedBy="fos_user")
     * @ORM\JoinColumn(name="fos_user_id", referencedColumnName="id")
     */
    protected $user;

    /**
     * @ORM\Column(type="float")
     */
    protected $budget;
    /...

}

All of this looked fine to me, but I noticed that category, month, expense, all of these has a connection to User. I made this for the queries to be more convenient, but isn't it becoming too duplicated?

I have really little experience with databases and with programming, but I want the application to become nice and the code to be of good quality.

I want to know if this code is too problematic and connected and duplicated, or it's allowable. And if it's not please give me some advice or ideas on how to implement this better.

IMO this is great, it's nicely normalised. You're separating things logically into models.

The only thing that's puzzling me is...what is your 'Month' entity for? None of your other Entities reference it?

I don't know if:

/**
 * @ORM\OneToMany(targetEntity="Expense", mappedBy="expense")
 */
protected $months;

Is meant to reference months? I'm guessing it is.

The only thing I can think of is that you're using one model to satisfy two different entities : the categories model.

If I were you I would split it into to things:

User_Categories

and

Expense_Categories

This is something I often do, and I've found it better than having one abstract category entity. Having more tables has almost no downsides from a programatic point of view.

The only other thing I can think of really is, even if (as above) you meant to have a month entity, what is it for? It's actually a budget entity, not a month entity.

Date/time models are already abstracted quite well in doctrine itself (just have a column [or two] specifying the date range), so having a budget entity makes much more sense, then you can specify arbitrary budget time periods instead of the monthly constraint.

Hope this helps!


Analyzing a web access log to obtaining totals for requests This program takes the filename as the input and outputs to a file the number of requests that were more than 1000 bytes and the total bytes.

Log file example:

site.com - - [01/Jul/1995:00:00:01 -0400] "GET /images/launch-logo.gif HTTP/1.0" 200 183

Any suggestions/recommendations on the code or structure?

import re
import os

filename = input()
filename_regex = '^(\S+) (\S+) (\S+) \[([\w:/]+\s[+\-]\d{4})\] "(\S+)\s?(\S+)?\s?(\S+)?" (\d{3}|-) (\d+|-)\s'
nbr_of_requests = 0
sum_of_bytes = 0

def parse_file(filename):
    with open(filename, 'r', encoding='utf-8') as fin:
        print(f"Opening file: {filename}")
        for line in fin.readlines():
            if os.path.exists(filename):
                line = re.findall(filename_regex, line)
                yield line

def save_to_file(filename):
    output_filename = "bytes_" + filename
    with open(output_filename, 'w+') as fout:
        fout.write(str(nbr_of_requests) + '\n' + str(sum_of_bytes))

if __name__ == '__main__':
    for line in parse_file(filename):
        if line and int(line[0][8]) >= 1000:
            nbr_of_requests += 1
            sum_of_bytes = sum_of_bytes + int(line[0][8])

    save_to_file(filename)

If you don't need fields other than size, and have no use for lines with no size, a simpler regex like \s(\d+)$ is going to be appreciably faster.  You could even go with \s(\d{4,})$ and skip the >= 1000 test.

If you do keep the full regex, I'd simplify the date portion because date formats are notoriously unpredictable.  \[[^]]+\] does the job more robustly.

(\S+)? is better written as (\S*).

filename_regex is better named  like log_entry_regex, since that's what it matches.

Overwriting the string line with an array of captures is ugly.  And re.findall is not the best choice, because the regex should never match more than once per line (and your code ignores subsequent matches anyway).  Try something like:

matched = re.search(log_entry_regex, line)
if (matched)
    yield matched.groups()

This eliminates the array-of-arrays in the result and ensures results are always a set of matches.  if line and int(line[0][8]) >= 1000 becomes just if int(line[8]) >= 1000.

The global variables are better placed inside the functions where they're used.  save_to_file would need to take arguments then, but it's three lines and could just be inline in the main function.

When you have an array with many values, where each value means something different, a useful pattern is to create "fake enums" with constant field identifiers.  It makes your uses of that array easier to write and easier to read.  It doesn't make much difference here, where the code is short and only one element is actually used; however it becomes invaluable as line count and complexity increase.  Python doesn't have constants, variables will work too:

HOST = 0
…
SIZE = 8
…
if int( line[SIZE] ) >= 1000:


Gathering statistics on a list of dicts Imagine a large collection of Python dictionaries (a list of dicts just to make it simple). Each element of this list has hundreds of keys (aka attributes). More importantly not all elements share the same attributes. There is considerable overlap though. So this is a collection of semi-structured data.

I want to get a better sense of how this data looks like, so I wrote a small helper class to do what I want. More specifically, some attributes only take a small set of values. I want to know what kind of values a these keys/attributes take, and the count of each unique value. For example, for an attribute called 'color' I want to know that there are 20 reds, 5 greens, 8 blues, and 1 purple. I also want to know how many dictionaries do not have this attribute (i.e., this attribute is not defined for them).

If you are familiar with R, the information I am after is very similar to what you get when calling the built-in function summary() on a dataframe.

Here's my Python class:

class AttributeValueCount:
    '''
    A helper class to collect statistics about the different values that attributes
    (keys in dicts) take. Initialised with a list of attributes we'd like monitored.
    We can then call the update_counts method repeadedly with different dictionaries,
    to update the number of times each attribute takes specific values.
    '''
    def __init__(self, keylist):
        self._keylist = keylist
        self._counts = {key: defaultdict(int) for key in keylist}

    def update_counts(self, single_dict):
        for key in self._keylist:
            if key in single_dict:
                self._counts[key][single_dict[key]] += 1
            else:
                self._counts[key]['Non existent'] += 1

    def __repr__(self):
        s = ''
        for key in self._keylist:
            s += '-- {} --\n'.format(key)
            for value, count in self._counts[key].items():
                s += '\t {} : {}\n'.format(value, count)
        return s

Example Input

data = [
    {'name': 'apple', 'type': 'farm', 'color': 'red', 'volume': 2000},
    {'name': 'orange', 'type': 'farm', 'color': 'orange', 'volume': 5000},
    {'name': 'elderberry', 'type': 'wild', 'color': None},
    {'name': 'pythonfruit', 'type': 'extinct'}
]

value_counts = AttributeValueCount(['type', 'color'])

for fruit in data:
    value_counts.update_counts(fruit)

print(value_counts)

Example Output

-- type --
     farm : 2
     wild : 1
     extinct : 1
-- color --
     red : 1
     orange : 1
     None : 1
     Non existent : 1

Does this implementation look pythonic to you? What other ways could I achieve my result? I am mostly interested about major design decisions and  not so much about style or minor enhancements. For example, a minor enhancement would be to allow update_counts to take lists of dicts, or any kind of iterable. A major decision would be: I should not be defining a class at all, maybe a function factory would be better (idk, I am just throwing around terms). But I do not want to deter you, any feedback/observation is useful. I just realised for instance that I don't do much argument validation.

Update

The answer by Mathias provided some good points, and I also realised I should have included a bit more information: As mentioned the dictionaries contain hundreds of attributes, and I am only interested in some of them. To put the "some" in perspective, I only need to know the counts for at most 10-15 of the attributes. I also have thousands of dictionaries. For this reason I believe that flowA: specifying the attributes you want and then feed in the data, is preferable to flowB: giving all the data to the class and then ask questions on specific attributes. To my mind FlowB should just be a function definition count_values(data, key) not a class definition. Moreover, the added advantage of flowA is that you do not have to have all data present, you can increment the count incrementally.

Minor improvements

I think defining value_counts = AttributeValueCount('type', 'color') would be more user-friendly than having to pass in a list, so you could just define the constructor as

class AttributeValueCount:
    def __init__(self, *keylist):
        self._keylist = keylist
        self._counts = {key: defaultdict(int) for key in keylist}

and remove that need for an iterable. And if a user still has an iterable to pass to the constructor, they can still unpack it:

keys = ['type', 'color']
value_counts = AttributeValueCount(*keys)

I would also use add and update methods as in a dictionary or a set: add is for a single element and update is for several:

    def update(self, iterable):
        for element in iterable:
            self.add(element)

    def add(self, element):
        for key in self._keylist:
            if key in element:
                self._counts[key][element[key]] += 1
            else:
                self._counts[key]['Non existent'] += 1

You can also simplify this add method using dict.get():

    def add(self, element):
        for key in self._keylist:
            category = element.get(key, 'Non existent')
            self._counts[key][category] += 1

Lastly, the __repr__ method is meant to return an "official" representation of the object, something that could potentialy be passed to eval to reconstruct it. I’d rather use __str__ here, or even something completely different, like the summary you’re talking about:

    def summary(self):
        return '\n'.join(
                '-- {} --\n{}'.format(
                    key,
                    '\n'.join(
                        '\t {}: {}'.format(value, count)
                        for value, count in self._counts[key].items()
                    ))
                for key in self._keylist
        )

Note that I used str.join instead of string concatenation as it is usually faster and more memory efficient.

Major improvements

The first thing I would add is the ability to specify the name of the "missing" category. I have a tendency to consider None in such cases, rather than any arbitrary string, but seeing at your data, it seems like None could be a valid entry. So I'd say: let the user decide based on their data.

There is still a question as where to specify this default: in __init__ or at each update/add call? Both might be valid.

An other change I would make is to completely invert the flow of things. Instead of specifying the keys you're interested in beforehand, you just store the data. And you provide methods to retrieve the counts associated to each key latter, this will allow data exploration to be more dynamic. This will give a session like:

>>> counts = AttributeValueCount(data, missing='Non existent')
>>> print(counts['type'])
farm: 2
wild: 1
extinct: 1
>>> print(counts['color'])
red: 1
orange: 1
None: 1
Non existent: 1
>>> print(counts['volume'])
2000: 1
5000: 1
Non existent: 2

Which seems easier than having to recreate an AttributeValueCount('volume') afterwards.

There, also, you have 2 possibilities:

only store values using __init__, add and update and compute the counts later (\$\mathcal{O}(n)\$ at each __getitem__);
compute the counts upfront (\$\mathcal{O}(n\times{}m)\$) but have a constant time __getitem__.

I would go for the second solution as it is interesting how to deal with missing keys when you first encounter a new category:

from collections import Counter

class AttributeValueCount:
    def __init__(self, iterable, *, missing=None):
        self._missing = missing
        self.length = 0
        self._counts = {}
        self.update(iterable)

    def update(self, iterable):
        categories = set(self._counts)
        for length, element in enumerate(iterable, self.length):
            categories.update(element)
            for category in categories:
                try:
                    counter = self._counts[category]
                except KeyError:
                    self._counts[category] = counter = Counter({self._missing: length})
                counter[element.get(category, self._missing)] += 1
        self.length = length + 1

    def add(self, element):
        self.update([element])

    def __getitem__(self, key):
        return self._counts[key]

    def summary(self, key=None):
        if key is None:
            return '\n'.join(self.summary(key) for key in self._counts)

        return '-- {} --\n{}'.format(key, '\n'.join(
                '\t {}: {}'.format(value, count)
                for value, count in self._counts[key].items()
        ))

Usage being:

>>> from count_attributes import AttributeValueCount
>>> data = [
...     {'name': 'apple', 'type': 'farm', 'color': 'red', 'volume': 2000},
...     {'name': 'orange', 'type': 'farm', 'color': 'orange', 'volume': 5000},
...     {'name': 'elderberry', 'type': 'wild', 'color': None},
...     {'name': 'pythonfruit', 'type': 'extinct'}
... ]
>>> c = AttributeValueCount(data, missing='Non existent')
>>> print(c.summary())
-- type --
     Non existent: 0
     farm: 2
     wild: 1
     extinct: 1
-- name --
     Non existent: 0
     apple: 1
     orange: 1
     elderberry: 1
     pythonfruit: 1
-- volume --
     Non existent: 2
     2000: 1
     5000: 1
-- color --
     Non existent: 1
     red: 1
     orange: 1
     None: 1
>>> c['volume']
Counter({'Non existent': 2, 2000: 1, 5000: 1})
>>> c.add({'foo': 'bar'})
>>> print(c.summary())
-- name --
     Non existent: 1
     apple: 1
     orange: 1
     elderberry: 1
     pythonfruit: 1
-- type --
     Non existent: 1
     farm: 2
     wild: 1
     extinct: 1
-- volume --
     Non existent: 3
     2000: 1
     5000: 1
-- color --
     Non existent: 2
     red: 1
     orange: 1
     None: 1
-- foo --
     Non existent: 4
     bar: 1


Javascript Placeholder Polyfill I was wondering if you could review the following code I've written as a polyfill for the placeholder attribute in HTML5?

The object of the polyfill is to replicate the functionality of the placeholder attribute. In effect, the polyfill is supposed to do the following:

Default text to be shown.
On Focus: Erase the default text.
On Focus Out: Put in default text if nothing has been entered

Dependancies:

jQuery

var Utils = Utils || {};

Utils = (function (window, $) {

    var _c, _private, _public;

    _private = {

        /**
         * @event
         * @param e The event object.
         */
        'handleFocusIn': function (e) {
            var $target = $(e.currentTarget);
            if($target.val() === $target.attr('placeholder')) {
                $target.val('');
            }
            $target
                .unbind('focusin')
                .bind('focusout', _private.handleFocusOut);
        },

        /**
         * @event
         * @param e The event object.
         */
        'handleFocusOut': function (e) {
            var $target = $(e.currentTarget);
            if($target.val() === '') {
                $target.val($target.attr('placeholder'));
            }
            $target
                .unbind('focusout')
                .bind('focusin', _private.handleFocusIn);
        }
    };

    _public = {
        /**
         * @function
         * @description
         * A poly-fill for the placeholder attribute.
         */
        'supportsPlaceholderAttr': function (enableFallback) {
            var placeHolderSupported = false;

            //If enableFallback is either undefined or not a Boolean, set it to false
            if((typeof enableFallback === 'undefined') || !(enableFallback instanceof Boolean)) {
                enableFallback = false;
            }

            //if placeholder is supported, no reason to continue on
            if ('placeholder' in document.createElement("input")) {
                return placeHolderSupported = true;
            }

            if (!placeHolderSupported && !!enableFallback) {
                var $inputEle;
                $('input')
                    .each( function(index, ele) {
                        $inputEle = $(ele);
                        if($inputEle.attr('placeholder')) {
                            $inputEle
                                .val($inputEle.attr('placeholder'))
                                .bind('focusin', _private.handleFocusIn);
                        }
                    });
            }
        }
    };

    return _public;

}(window, jQuery));

Note: Regarding the OP's comment on the "nature of IE7 mobile", I suspect it has more to do with jQuery's implementation of focusin and focusout. This may not be the case, but I have written my review with that assumption in mind.

There are a few things about your code that are, to put it bluntly, pointless.

First, checking whether Utils exists is a good thing, but you then immediately clobber it with the return of your function:

var Utils = Utils || {};

Utils = (function(window, $) {
    /* ... */
    return _public;
})(window, jQuery);

Something like the following would most likely be better:

var Utils = Utils || (function(window, $) {
    /* ... */
    return _public;
})(window, jQuery);

Second, why pass window to the function at all? Browsers will not allow overwriting of the window object, and your parameter name is window anyway, so it doesn't save you anything.

Third, _private is unnecessary. Any variables (including functions) that you don't include in whatever object you return will be hidden by the local scope of your self-executing function.

var handleFocusIn = function(e) { /*...*/ },
    handleFocusOut = function(e) { /*...*/ };

_public is also unnecessary, though not as much as _private. If it were me, I would simply return the object literal, rather than bothering with the _public variable.

Lastly, given that your elements will always be input elements, there is no need to use focusin and focusout; Simply use focus and blur - this will avoid any possible bubbling issues, and then, most importantly, allow you to dispense with the unbinding/rebinding that happens in your handlers.

@Raynos also makes some good points on your implementation of supportsPlaceHolderAttr, regarding variables and conditionals.

//if placeholder is supported, no reason to continue on
if ('placeholder' in document.createElement("input")) {
    // useless variable, just return true
    return placeHolderSupported = true;
}

// useless placeHoldSupported check, always false. Useless enableFallback conversion always a bool
if (!placeHolderSupported && !!enableFallback) {

It's a good attempt at writing unobtrusive code, but it falls short of good modularity and namespacing. Still, like I said, a good start.


Change URL on select change I've made a method, which inserts a new query parameter, when the value of a select changes, then reloads the page. I've had enough of URL jQuery hax, so I figured it would be a great idea to put the query parameters in an array, and manipulate it that way. I'm a little unsure whether or not this is the best way of doing it, so some input would be appreciated.

HTML

<select id="selCurrency">
   <option value="">Choose currency &hellip;</option>
   <option value="DKK">Danske kroner</option>
   <option value="NOK">Norske kroner</option>
   <option value="SEK">Svenske kronor</option>
</select>

JS

$("#selCurrency").change(function(){
     var val = $(this).val(); // This should be the new CurrencyCode, extracted from a select box
     var pag = window.location.pathname;
     var url = window.location.search;
         url = url.replace("?", "").split("&"); // Clean up the URL, and create an array with each query parameter

     var n = 0;
     for (var count = 0; count < url.length; count++) {
         if (!url[count].indexOf("CurrencyCode")) { // Figure out if if/where the Currency is set in the array, then break out
             n = count;
             break;
         }
     }

     if (n !=0) {
        url.splice(n,1); // If the Currency was set, remove it from the array
     }

     var len = url.length;
     var newUrl = url.join("&"); // Restringify the array

     if (len > 0) { // Check whether or not the currency is the only parameter, then build new URL with ? or &
        newUrl = pag + "?" + newUrl + "&CurrencyCode=" + val;
     } else {
        newUrl = pag + newUrl + "?CurrencyCode=" + val;
     }

     window.location.href = newUrl; // Finished, let's go!
  });

You will notice, that I've used the jQuery selector engine, while the rest is native JS. Would it be smarter/faster to use native JS to check for the onchange event?

document.getElementById("selCurrency").onchange( /* My function right here, mayn */ );

if you already use jQuery for something else in your page, it doesn't bother if you use jQuery or not.

if it's the only JQuery in your page, I suggest to change it to "vanilla" javascript, because you won't need to load the jquery library.

Some may say that often the jquery library is already in the browser cache, but still there's no need to add a library/framework for just one call.


Number guessing game in Python3 Is there any room for improvement?

import random

def play_again():
    while True:
        replay = input('Play again? Yes or No?\n')
        if replay.lower() == 'yes':
            return True
        elif replay.lower() == 'no':
            return False
        print('Invalid input.')

def main():
    print('Welcome to the number guess game!', end='\n\n')

    random_number = random.choice(range(1, 51)) #Genarate a number between 1 and 50 inclusive

    print("I'm thinking of a number which is between 1 and 50 (inclusive). Can you guess it?")

    guesses, guess = 0, 0

    while guess != random_number:
        try:
            guess = int(input("Enter a number: "))
        except:
            print('Invalid input.')
            continue
        if guess > random_number:
            print('Too high. Guess lower.')
        elif guess < random_number:
            print('Too low. Guess higher.')
        guesses+=1
    print('Congratz. You guessed my number {0} in {1} guesses'.format(random_number, guesses))

    if play_again():
        main()
    else:
        print('Bye!')

main()

main()
You should use if __name__ == "__main__":, like this:
if __name__ == "__main__":
    main()

This will prevent your code from automatically running if you ever import this module into another module.
Methods
I like how you split play_again() into a separate method.  However, you should use more methods, including a unique method to play the game, and perhaps a method to input a guess from the user.
Recursion
Recursion here is not good.  If the user plays the game enough times, you could have a stack overflow exception:
if play_again():
    main()
else:
    print('Bye!')

If you split the game up into multiple methods that you can call, each method could finish and get removed from the stack before it returns control to the caller, so you never have a stack overflow.
An example of this would be to have something like this:
def play_again():
    # code here

def game():
    # play game here

def main():
    print('Welcome to the number guess game!', end='\n\n')

    play_game = True
    while play_game:
        game()
        play_game = play_again()

if __name__ == "__main__":
    main()


My first Qt application - a multiple-state window with a menu This is my first Qt app and also some of my first non-trivial C++ code. The application displays a menu offering two options. Clicking either option takes you to a sub-page with some text and a back button, taking you back to the menu.

Feedback I'm especially interested in:

Am I doing anything weird, culturally, either in C++ culture or Qt culture?
Am I using the wrong tool for the job at any point, for instance in my use of QStackedLayout?
Is the design robust? For instance, adding buttons to the menu happens separately to adding pages to the StackedLayout. Should I create a MultiPageEnvironment class or something to handle this automatically?
Any major C++ or Qt no-nos?

main.cpp

#include <QApplication>

#include "MainWindow.h"

int main(int argc, char **argv)
{
    QApplication *app = new QApplication(argc, argv);
    MainWindow *mw = new MainWindow();
    mw -> show();
    return app -> exec();
}

MainWindow.cpp

#include <stdio.h>
#include <QWidget>
#include <QPushButton>
#include <QStackedLayout>

#include "MainWindow.h"
#include "MenuPage.h"
#include "SubPage.h"

MainWindow::MainWindow(QWidget *parent) : QWidget(parent)
{
    setWindowTitle("Application");
    layout = new QStackedLayout();
    MenuPage *menuPage = new MenuPage(this);
    SubPage  *page1    = new SubPage("This is page one.", this);
    SubPage  *page2    = new SubPage("This is page two.", this);

    layout -> addWidget(menuPage);
    layout -> addWidget(page1);
    layout -> addWidget(page2);
    layout -> setCurrentIndex(0);
    this   -> setLayout(layout);

    menuPage -> addButton();
    menuPage -> addButton();
}

void MainWindow::setPage(int index)
{
    layout -> setCurrentIndex(index);
}

void MainWindow::goToMenu()
{
    layout -> setCurrentIndex(0);
}

MenuPage.cpp

#include <stdio.h>
#include <QWidget>
#include <QPushButton>
#include <QSignalMapper>

#include "MenuPage.h"

MenuPage::MenuPage(MainWindow *parent) : QWidget(parent)
{
    signalMapper = new QSignalMapper(this);
    connect(signalMapper, SIGNAL(mapped(int)), parent, SLOT(setPage(int)));

    layout = new QVBoxLayout();
    setLayout(layout);

    nButtons = 0;
}

void MenuPage::addButton()
{
    nButtons += 1;

    QString label = (new QString("Page %1")) -> arg(nButtons);
    QPushButton *button = new QPushButton(label, this);
    layout -> addWidget(button);

    signalMapper -> setMapping(button, nButtons);
    connect(button, SIGNAL(clicked()), signalMapper, SLOT(map()));
}

SubPage.cpp

#include <stdio.h>
#include <QWidget>
#include <QPushButton>
#include <QVBoxLayout>

#include "SubPage.h"
#include "MainWindow.h"

SubPage::SubPage(const char messageString[], MainWindow *parent) : QWidget(parent)
{
    message = new QLabel(messageString, this);
    backButton = new QPushButton("Back", this);

    QVBoxLayout *layout = new QVBoxLayout();
    layout -> addWidget(message);
    layout -> addWidget(backButton);
    setLayout(layout);

    connect(backButton, SIGNAL(clicked()), parent, SLOT(goToMenu()));
}

That's a good first Qt program.  Most of my comments are preferences rather than rules, but I'll try to provide justifications.

In main(), no need to new

I would write

int main(int argc, char **argv)
{
    QApplication app(argc, argv);
    MainWindow mw;
    mw.show();
    return app.exec();
}

That way, you don't need to remember to delete the objects app and mw.  (You can argue that there's no need to clean up when you're exiting, but I find it helps to avoid false-positives when I'm checking memory correctness with Valgrind).

In constructors, prefer to use initializers

I can write

MainWindow::MainWindow(QWidget *parent)
    : QWidget(parent),
      layout(new QStackedLayout(this))
{

This allows us to declare layout as a const pointer, for increased safety:

QStackedLayout *const layout;

Note that I've passed this to the QStackedLayout constructor: that ensures that it always has an owner that's responsible for deleting it, and it installs it as the layout manager for the MainWindow widget (saving us a line of code).

Use new-style connect() when you can

Unless you don't know the complete type of the objects being connected, you can connect by pointer-to-member-function.  This allows more errors to be caught at compilation time (a Good Thing):

MenuPage::MenuPage(MainWindow *parent)
    : QWidget(parent),
      layout(new QVBoxLayout(this)),
      signalMapper(new QSignalMapper(this))
{
    void(QSignalMapper::*signal)(int) = &QSignalMapper::mapped;
    connect(signalMapper, signal, parent, &MainWindow::setPage);
}

However, I'd move the signal mapper and connect() call up into MainWindow, so that MenuPage doesn't need to know about its parent.

Reduce coupling

Both MainWindow and MenuPage need to know about MainWindow in their constructors.  Although we can simply pass a QObject, and connect by name, there's still coupling because they impose requirements on the slots exposed, so they can't be used more generally.

The way to fix this coupling is for the pages to emit a signal to indicate user actions; the MainWindow (which knows about its contents) can then connect those signals to the relevant actions.

MenuPage then looks like

public:
    explicit MenuPage(QWidget *parent);
signals:
    void buttonPressed(int button_number);

and its constructor is now

MenuPage::MenuPage(QWidget *parent)
    : QWidget(parent),
      layout(new QVBoxLayout(this)),
      signalMapper(new QSignalMapper(this))
{
    void(QSignalMapper::*signal)(int) = &QSignalMapper::mapped;
    connect(signalMapper, signal, this, &MenuPage::buttonPressed);
}

(The temporary variable signal is to avoid the problem of connecting overloaded signals and slots in Qt 5).

In MainWindow, we connect it directly:

connect(menuPage, &MenuPage::buttonPressed, layout, &QStackedLayout::setCurrentIndex);

We can do something similar in SubPage, but this is a little simpler, as we don't have an overloaded signal.  So just create a signal (call it done(), perhaps) and make the page's constructor connect the button's clicked() to the page's done().

A memory leak

Here, we allocate a new QString, which is never released:

QString label = (new QString("Page %1")) -> arg(nButtons);

It's easy to fix this - QString objects are fine to pass by value:

QString label = QString(tr("Page %1")).arg(nButtons);

Allow for translation

If you get into the habit of using tr() on all your user-facing strings, you've made a good start on internationalizing your application

Full worked example

I've inlined the methods just to make this smaller - I don't expect that in a real, multiple-source-file program.

#include <QWidget>
#include <QPushButton>
#include <QStackedLayout>
#include <QVBoxLayout>
#include <QSignalMapper>
#include <QLabel>

class MenuPage : public QWidget
{
    Q_OBJECT

public:
    explicit MenuPage(QWidget *parent)
        : QWidget(parent),
          layout(new QVBoxLayout(this)),
          signalMapper(new QSignalMapper(this))
    {
        void(QSignalMapper::*signal)(int) = &QSignalMapper::mapped;
        connect(signalMapper, signal, this, &MenuPage::buttonPressed);
    }

    void addButton()
    {
        ++nButtons;

        QString label = QString(tr("Page %1")).arg(nButtons);
        QPushButton *button = new QPushButton(label, this);
        layout->addWidget(button);

        signalMapper->setMapping(button, nButtons);
        connect(button, SIGNAL(clicked()), signalMapper, SLOT(map()));
    }

signals:
    void buttonPressed(int button_number);

private:
    QVBoxLayout *const layout;
    QSignalMapper *const signalMapper;
    int nButtons = 0;
};

class SubPage : public QWidget
{
    Q_OBJECT

public:
    explicit SubPage(const QString& messageText, QWidget *parent)
        : QWidget(parent),
          message(new QLabel(messageText, this)),
          backButton(new QPushButton(tr("Back"), this))
    {
        QVBoxLayout *const layout = new QVBoxLayout(this);
        layout->addWidget(message);
        layout->addWidget(backButton);
        connect(backButton, &QPushButton::clicked, this, &SubPage::done);
    }

signals:
    void done();

private:
    QLabel *const message;
    QPushButton *const backButton;
};

class MainWindow : public QWidget
{
    Q_OBJECT

public:
    explicit MainWindow(QWidget *parent = 0)
        : QWidget(parent),
          layout(new QStackedLayout(this))
    {
        setWindowTitle(tr("Application"));
        MenuPage *const menuPage = new MenuPage(this);
        SubPage  *const page1    = new SubPage(tr("This is page one."), this);
        SubPage  *const page2    = new SubPage(tr("This is page two."), this);

        layout->addWidget(menuPage);
        layout->addWidget(page1);
        layout->addWidget(page2);
        layout->setCurrentIndex(0);

        menuPage->addButton();
        menuPage->addButton();
        connect(menuPage, &MenuPage::buttonPressed, layout, &QStackedLayout::setCurrentIndex);
        connect(page1, &SubPage::done, this, &MainWindow::goToMenu);
        connect(page2, &SubPage::done, this, &MainWindow::goToMenu);
    }

private:
    QStackedLayout *const layout;
public slots:
    void setPage(int index)
    {
        layout->setCurrentIndex(index);
    }

    void goToMenu()
    {
        layout->setCurrentIndex(0);
    }
};

#include <QApplication>

int main(int argc, char **argv)
{
    QApplication app(argc, argv);
    MainWindow mw;
    mw.show();
    return app.exec();
}

#include "165088_moc.cpp"


Fastest way searching specific files I got thousands of files with a specific file extension in thousands of sub folders. Now, what is the fastest way to search with a pattern? I tried the method DirectoryInfo.GetFiles(rootfolder) (~8 minutes) and a recursive custom method (~5 minutes).

 private void WalkDirectoryTree(DirectoryInfo dr, string searchname)
    {
        System.IO.FileInfo[] files = null;
        System.IO.DirectoryInfo[] subDirs = null;
        try
        {
            files = dr.GetFiles(searchname + ".*");
        }
        catch (Exception ex)
        {
            MessageBox.Show(ex.Message);
        }

        if (files != null)
        {
            foreach (FileInfo fi in files)
            {
                allFiles.Add(fi);
            }
            subDirs = dr.GetDirectories();

            foreach (DirectoryInfo di in subDirs)
            {
                WalkDirectoryTree(di, searchname);
            }
        }
    }

Is there any faster way to do it?

Try parallelization. Instead of:

foreach (DirectoryInfo di in subDirs)
{
    WalkDirectoryTree(di, searchname);
}

Do

Parallel.ForEach(subDirs, dir => WalkDirectoryTree(dir, searchname));

Notice that by doing this allFiles will be accessed concurrently so change your collection to a ConcurrentBag.


Find an equation that uses all 9 digits (1-9) I used Python to write the following program:

"""
A desired equation includes a 4-digit integer multiplied with a single-digit integer
to give another 4-digit integer and all 9 digits(1-9) are used in the equation.
"""

import random

found=False

while not found:
    full_range=range(1,10)

    four_digit_num=""
    multiple=""
    for i in range(4):
        four_digit_num+=str(full_range.pop(random.randint(1,len(full_range)-1)))

    single_digit_num=full_range.pop(random.randint(1,len(full_range)-1))

    for i in range(4):
        multiple+=str(full_range.pop(random.randint(1,len(full_range)-1)))

    if int(four_digit_num)*single_digit_num==int(multiple):
        found=True

#output the equation
print "The desired equation is:"
print four_digit_num+"*"+str(single_digit_num)+"="+multiple

Is there any better way to perform this task?

It is extremely inefficient to do this randomly; you could end up checking the same numbers again and again. For a more structured approach, you could use itertools.permutations:

from itertools import permutations

for nums in permutations("123456789"):
    a = int("".join(nums[:4]))
    b = int(nums[4])
    c = int("".join(nums[5:]))
    if a * b == c:
        print "{0} * {1} == {2}".format(a, b, c)
        break # optional - there's more than one answer!

Also, note that:

found=False
while not found:
    ...
    if int(four_digit_num)*single_digit_num==int(multiple):
        found=True

would be neater as:

while True:
    ...
    if int(four_digit_num) * single_digit_num == int(multiple):
        break


Find the longest word in set D that is a subsequence of string S Given a string S and a set of words D, find the longest word in D that is a subsequence of S.

  Word W is a subsequence of S if some number of characters, possibly zero, can be deleted from S to form W, without reordering the remaining characters.

  Note: D can appear in any format (list, hash table, prefix tree, etc.)

  For example, given the input of S = "abppplee" and D = {"able", "ale", "apple", "bale", "kangaroo"} the correct output would be "apple".

  The words "able" and "ale" are both subsequences of S, but they are shorter than "apple".
  The word "bale" is not a subsequence of S because even though S has all the right letters, they are not in the right order.
  The word "kangaroo" is the longest word in D, but it isn't a subsequence of S.

I am currently trying to learn how to become a better programmer.  I am taking the Google Tech Dev Guide recommended path and this is the first problem.  I came up with my own solution, and read Google's first recommended answer is using brute force, and a slight optimization is to at least ensure the dictionary is represented by a hash table or prefix tree so that lookups are efficient.

This is my first time using a dictionary, and I also don't feel confident about using objects and classes.  If you could review and recommend better ways to improve my code, maybe by using more (or better) objects, I would appreciate it.

class Main:
    def create_dictionary(string):
        dictionary = {}
        index = 0

        for letter in string:
            if letter in dictionary:
                dictionary[letter].append(index)
            else:
                dictionary[letter] = [index]
            index += 1
        return(dictionary)

    def get_word_is_substring(word, dictionary):
        index_of_last_letter_found = None
        for letter in word:
            if letter in dictionary and (index_of_last_letter_found is None or dictionary[letter][-1] > index_of_last_letter_found):
                index = 0
                while index < len(dictionary[letter]):
                    if index_of_last_letter_found is None or index_of_last_letter_found < dictionary[letter][index]:
                        index_of_last_letter_found = dictionary[letter][index]
                        break
                    else:
                        index += 1

            else:
                return False
        return True

    def replace_word_if_necessary(word, longest_word):
        if (longest_word is None) or (len(word) > len(longest_word)):
            longest_word = word
        return longest_word

    def get_longest_word(s, d):
        dictionary = Main.create_dictionary(s)
        longest_word = None

        for word in d:
            word_is_substring = Main.get_word_is_substring(word, dictionary)
            if word_is_substring:
                longest_word = Main.replace_word_if_necessary(word, longest_word)
        print(longest_word)

Main.get_longest_word("abppplee", {"ale", "bale", "able", "apple", "kangaroo"})

First, the good news: your code looks pretty good. It's indented properly, spelled properly, spaced properly, capitalized properly, and uses the right Python style. Good work!

Here are a couple of nits:

This is a "program." (As opposed to just a module, or package, or library.) So use the standard python idiom for programs at the bottom:

if __name__ == '__main__':
    Main.get_longest_word("abppplee", {"ale", "bale", "able", "apple", "kangaroo"})

This mechanism is to allow your code to be loaded (via import myprogram) without having the main entry point automatically get invoked. That lets you load it up in the interpreter and browse around before you call it.
Use docstrings! Write down the problem you are trying to solve, and any notes about inputs or outputs for functions. Especially data formats.

For coding puzzle type problems, the docstring is a great place to copy the puzzle specification. This lets you refer back to it, lets you remember what you were doing when you wrote this code, and lets you paste it up at CodeReview with ease!

#!/usr/bin/env python3
""" Given a string S and a set of words D, find the longest word in D that is a subsequence of S.

    Word W is a subsequence of S if some number of characters, possibly zero, can be deleted from S to form W, without reordering the remaining characters.

    Note: D can appear in any format (list, hash table, prefix tree, etc.)

    For example, given the input of S = "abppplee" and D = {"able", "ale", "apple", "bale", "kangaroo"} the correct output would be "apple".

      - The words "able" and "ale" are both subsequences of S, but they are shorter than "apple".
      - The word "bale" is not a subsequence of S because even though S has all the right letters, they are not in the right order.
      - The word "kangaroo" is the longest word in D, but it isn't a subsequence of S.
"""

Now here are some possible improvements:

In create_dictionary

def create_dictionary(string):
    dictionary = {}
    index = 0

    for letter in string:
        if letter in dictionary:
            dictionary[letter].append(index)
        else:
            dictionary[letter] = [index]
        index += 1
    return(dictionary)

First, congratulations! You have written the hand-coded version of how to manage a dictionary whose values are lists. You got it right, your code works, and you should never do that again because it's boring. Instead, use collections.defaultdict(list). A defaultdict remembers a factory function that it will call whenever it is asked about a key but doesn't have a corresponding value.

The word list is not just the name of a Python data type, it's also the function to call when you want to construct one! (Just like you use the name of every class as it's constructor:  my_obj = MyClass(1, 2, "hello")) So when you want a dictionary that looks up lists, it's much easier to assume that there will always be a list in the dictionary, but it might be empty. That's what defaultdict(list) will get you:

import collections  # somewhere at top of file

def create_dictionary(s):
    dictionary = collections.defaultdict(list)
    index = 0

    for letter in string:
        dictionary[letter].append(index)    # defaultdict FTW!
        index += 1
    return(dictionary)

Next, give up your love affair with integers! Ned Batchelder gave a nice talk on the subject, so here's a link to that: https://www.youtube.com/watch?v=EnSu9hHGq5o

The idea is that many (most?) python loops don't need to use integers to index over things. And when you do need an integer index (and in this case you do!) there are better ways than maintaining your own integer index variable. Here's one such way: the enumerate() built-in function.

With enumerate, you can write a loop that iterates over the values from some iterable along with an automatically-associated integer:

    # No index=0 here
    for index, letter in enumerate(string):
        dictionary[letter].append(index)
        # No index+=1 here!

The index, string pair is called a tuple, and it is a built-in type just like list. The act of assigning or iterating using multiple target variables that take their values from a single tuple is called tuple unpacking. (Remember that phrase, you'll need it when you want to ask for help on the subject.)

In get_longest_word

The issue I have with this function is not one of Python, but rather of design. You are given some words, d. You want to find the longest word that is a subsequence of the string s. How do you do it?

In your case, the answer is "look at every single word in d, ignore the ones that are not subsequences of s, pick the longest one that remains."

There are a couple of better (read: faster, more efficient) ways of doing that job. Let me make one simple suggestion: sort the words!

In Python, there are Iterables and Sequences. An iterable is something you can iterate. A sequence is something you can access using s[i]. It's possible to have an infinite iterable, by writing an infinite generator function. It's not possible to have an infinite sequence, since you'll run out of memory trying to store it.

In this particular case it seems okay to assume that d is going to be a sequence: a finite list or tuple. So the fastest way to find the "longest word" is to start by looking at long words first. Because once you find a long word that is a subsequence, you can stop - there are no shorter ones!

The way to sort things in Python is the sorted built-in function. (Yes, it takes an iterable. No, it won't sort an infinite one. Yeesh!) By default, it sorts things by comparing the items using the "native" comparison. You can specify a key function, however, to sort using some different mechanism. Let's use the length of the words, which is the len function
(function len(x) -> int returns the length of the list/string/whatever). And let's reverse the order, so that the big ones come first:

d = sorted(d, key=len, reverse=True)

Now, instead of needing to check the length and update the longest-word-so-far variable, you can just return immediately once you find a subsequence.

In get_word_is_substring

Let's talk about default values. You say:

index_of_last_letter_found = None

But :! grep letter_found % in Vim tells me:

    index_of_last_letter_found = None
        if letter in dictionary and (index_of_last_letter_found is None or dictionary[letter][-1] > index_of_last_letter_found):
                if index_of_last_letter_found is None or index_of_last_letter_found < dictionary[letter][index]:
                    index_of_last_letter_found = dictionary[letter][index]

You're spending a lot of keystrokes checking for None. And all you do is compare using <, and assign new values. Why not just set the default value to some value that you know is going to be "too low"? Since string index values start at 0, maybe -1 would make sense:

index_of_last_letter_found = -1

While were at it, shorten that variable name. Names should be as long as they need to be, and no longer!

def get_word_is_substring(word, dictionary):
    last_index = -1  # Index of last letter found

    for letter in word:
        if letter in dictionary and dictionary[letter][-1] > last_index:
            index = 0
            while index < len(dictionary[letter]):
                if last_index < dictionary[letter][index]:
                    last_index = dictionary[letter][index]
                    break
                else:
                    index += 1
        else:
            return False
    return True

That's a lot more readable, since there are fewer tests and fewer characters.

Next, let's go back and work on your fetish for simple integer arithmetic. Now that you've watched Ned Batchelder's talk, you can see that index += 1 is not the way!

for index in dictionary[letter]:
    if last_index < index:
        last_index = index
        break

(There are some other ways to find the first element in an iterable that matches a condition. See here for many of them. But this is nice and clear, and works.)

Some words on "style"

The above code may be somewhat more difficult to understand because I changed the meaning of index. In your original code, index was an index into the list of indices that pointed to where letters occur in the master sequence. In my updated version index means the actual indices from the list. I have removed one level of indirection.

This is an example of why "too short" variable names are actually a good thing. You'll find a lot of people use very small names, like ch or i, to represent loop variables. This is because most of the time, loop variables are not a "concept" or a "noun". Instead, they are a subscript. We write a[i] because the original Teletype devices hand-carved out of wood by our pioneer forbears would not allow writing ᵢ.

But we don't really care about i, or about a[i]. We care about the thing stored there, and its properties. So, I encourage you to use short little 1-letter and 2-letter names when you are indexing a sequence, or when you're stuck in some other language that doesn't provide the nice looping options that Python does. Then you could say:

for i ...
    index = s_dict[letter][i]

And there would be no confusion between index and that-loop-variable.

Next, the structuring of your escape clauses: I see a lot of people being taught to code in a way that puts exit conditions last. You're doing that in your loop:

    for letter in word:
        if letter in dictionary and dictionary[letter][-1] > last_index:
            # much code here
        else:
            return False

But moving the exit condition to the top (by reversing the sense of the if statement) makes it clear what the alternatives are (if this is true, leave, otherwise ...) and reduces the indentation level of your code (by removing the else indent). Some people would have you use two-space tabs to address the indentation problem. Those people are all young with good eyesight. They're also quite wrong. :-)

    for letter in word:
        if letter not in dictionary or dictionary[letter][-1] <= last_index:
            return False

        # much code here

Lastly, there's the name. You called it get_word_is_substring, but we want subsequences not substring, and there's no reason to say get_ at the beginning. In fact, the word can be assumed, since you're passing it in, so:

def is_subsequence(word, s_dict):
    last_index = -1  # Index (in s) of last letter found

    for letter in word:
        if letter not in s_dict or s_dict[letter][-1] <= last_index:
            return False

        for index in s_dict[letter]:
            if last_index < index:
                last_index = index
                break

    return True

(Note: Because I suggested s_dict be a defaultdict, you could eliminate the if test entirely and rely on the for ... else construct, which is rather obscure. I don't recommend it, simply because it's non-intuitive, hard to understand, and because spelling out the failure conditions is better than leaving them to the language. ("explicit is better than implicit"))

The class

Finally, let's talk about the class:

class Main:

But first! Here's another video for you to watch, Jack Diederich's talk at Pycon 2012 entitled: "Stop Writing Classes!": https://www.youtube.com/watch?v=o9pEzgHorH0

As Jack says, "The signature of 'this shouldn't be a class' is that it has two methods, one of which is __init__."

In this case, you've created a class, but you don't store any data and you only call one method from the outside. It should be a function.


TicTacToe solver in JavaScript I come more from a frontend background and have been reading up on more backend approaches and algorithms while solving a TicTacToe grid to see if there are any wins normally, I'd probably use something like this

for (let i = 0; i < rows; i++) {
    if (arr[i][0] === arr[i][1] && arr[i][0] === arr[i][2]) {

but with what I've been studying, I thought it'd be better to use a map and iterate through them instead

'use strict';

const board = [
  ['x','x','x'],
  ['x','o','o'],
  ['x','x','x']
]

const matches = [
  [1,4,7],
  [2,5,8],
  [3,6,9],
  [1,2,3],
  [4,5,6],
  [7,8,9],
  [1,5,9],
  [3,5,7]
]

const solver = (board, matches) => {
  // build map
  let counter = 1;
  const map = {};

  // convert array to object
  for (let row of board) {
    for (let char of row) {
      map[counter] = char;
      counter++;
    }
  }

  for (let match of matches) {
    // create a new array with the values
    const arr = [...map[match[0]],map[match[1]],map[match[2]]];
    // check for duplicates
    const arr2 = [...new Set(arr)].join("");
    // console log if length is 1
    if (arr2.length === 1) console.log(`match found at set: ${match}`)
  }
}

solver(board, matches)

Is this efficient? Is there a better way of doing this?

It looks a lot cleaner than

if (arr[i][0] === arr[i][1] && arr[i][0] === arr[i][2])

for sure, but then again, that itself could have probably been designed more efficiently.

Is there a better way to generate the coordinates dynamically?

you can simplify this block a little:

for (let row of board) {
    for (let char of row) {
         map[counter] = char;
         counter++;
    }
}

into:

const map = board.flat();

You can generate matches pseudo-automatically which simultaneously improve readability

function generateMatches() {
     function simpleRange(upTo) {
          return new Array(upTo).fill(true).map((_, index) => index);
     }

     function customRange(start, step, length) {
          return simpleRange(length).map(index => start + index * step);
     }

     function horizontalRow(top) {
          return customRange(3 * top, 1, 3);
     }

     function verticalRow(left) {
          return customRange(left, 3, 3);
     }

     function progressingDiagonal() {
          return customRange(0, 4, 3);
     }

     function regressingDiagonal() {
          return customRange(2, 2, 3);
     }

     const matches = [];
     for (let top = 0; top < 3; ++top) {
          matches.push(horizontalRow(top));
     }
     for (let left = 0; left < 3; ++left) {
          matches.push(verticalRow(left));
     }
     matches.push(progressingDiagonal());
     matches.push(regressingDiagonal());
     return matches;
}

  Warning! I assumed starting at 0 index

Don't use console.log(...); in solve function. Try to return list of matches instead and use user interface API outside of it. It will allow you to write some automatic (for instance unit) tests for your code (using Motcha, Jasmine, etc... depending on your target technology)
General advices:

Use better names for variables: map -> flatBoard, arr -> row, arr2 -> reducedRow
Split your code into smaller functions for each step, for instance:

const arr2 = [...new Set(arr)].join("");

into:

function reduceRow(array) {
     return [...new Set(array)].join("");
}
const reducedRow = reduceRow(row);

Your original question is about performance. Let just start it now:

IMHO you don't need to convert Set into array into string to test its size. Use something like this instead:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set/size

function rowDiversity(array) {
     return new Set(array).size;
}
const rowDiversity = rowDiversity(row);

Not sure if destructuring operator is needed at all at previous line too (most likely its performance impact is already neutral) But I would write this instead anyway:

function fetchRow(flattenBoard, match) {
     return match.map(index => flattenBoard[index]);
}
const row = fetchRow(flattenBoard, match);


Most basic HTML website - naming and CSS rules I'm trying to accomplish something very simple as seen here.

HTML

Link

Link

Link

CSS

.container {
    padding-left: 80px;
    padding-right: 80px;
}
#header {
    height: 50px;
    line-height: 50px;
    background-color: blue;
}
#header_nav {
    float:right;
}
#header_nav li {
    display: inline-block;
}

Questions:

Am I using the IDs and classes correctly?  For example, should header be an ID or class?
Is my naming convention ok?
Should I be using nested rules instead? E.g #header ul li { ...  }
Is this the best way to achieve this layout?

Any advice at this stage would be good, so I can take these practices forward when doing the next steps of the structure.

You should be using HTML 5 elements such as header and nav making some/all classes and IDs unnecessary:

Link

Link

Link


Football Elo Rankings I have created the following code:

class Club(object):
    def __init__(self, name, points=1000):
        self.name = name
        self.points = points

    def __eq__(self, other):
        return self.name == other.name

    def __gt__(self, other):
        return self.points > other.points

    def compute_points(self, opponent, is_home, result):
        rating_diff = self.points - opponent.points + (100 if is_home else -100)
        expected = 1 / (1 + 10**(-rating_diff/400))
        return 32 * (result-expected)

def play_match(home, away, result):
    home_change = int(home.compute_points(away, True, result))
    away_change = int(away.compute_points(home, False, 1-result))
    home.points += home_change
    away.points += away_change

def club_index(club_to_find, clubs):
    index = 0
    while index < len(clubs):
        club = clubs[index]
        if club.name == club_to_find:
            return index
        index += 1
    return -1

def main():
    clubs = []
    with open("matches.txt") as file:
        matches = file.readlines()

    for match in matches:
        (home_str, away_str, result) = match.split(" ")

        index = club_index(home_str, clubs)
        if index == -1:
            home = Club(home_str)
            clubs.append(home)
        else:
            home = clubs[index]

        index = club_index(away_str, clubs)
        if index == -1:
            away = Club(away_str)
            clubs.append(away)
        else:
            away = clubs[index]

        play_match(home, away, float(result))

    clubs = sorted(clubs)
    clubs.reverse()
    with open("ranking.txt", "w") as file:
        for club in clubs:
            file.write(club.name + ": " + str(club.points) + "\n")

if __name__ == "__main__":
    main()

which takes a file containing match results, and creates a file containing a ranking based on the Elo Ranking System. However, I feel like my main() method could've been implemented in a better way, so I came here to see for some pointers as to how I can better implement it or pick up any other good practices.

EDIT: Here's an example of the input file:

Swansea ManUtd 0
WestHam Cardiff 1
WestBrom Southampton 0
Sunderland Fulham 0
Norwich Everton 0.5
Arsenal AstonVilla 0
Liverpool Stoke 1
Chelsea Hull 1

In play_match, the Club class should be responsible for updating its own score. You could define an update_points method taking the same parameter as the computes_point one (and calling it). Also, shouldn't compute_points be responsible for the conversion to int ?

I find it a bit awkward to have the __eq__ and the __gt__ base their logic on completely different fields.

In a club_index : as a general rule, you do not iterate that way over object in Python.
The pythonic way to write

def club_index(club_to_find, clubs):
    index = 0
    while index < len(clubs):
        club = clubs[index]
        if club.name == club_to_find:
            return i
        index += 1
    return -1

is

def club_index(club_to_find, clubs):
    for i,club in enumerate(clubs)
        if club.name == club_to_find:
            return index
    return -1

Now, if I was to write such a program, I wouldn't even create a class ( you can have a look at http://www.youtube.com/watch?v=o9pEzgHorH0 ). A dictionnary to map club names to score is pretty much everything you need but that's a different way to see the problem I guess.

Edit : Actual tested code added.

def get_new_scores(home_score,away_score,result):
    rating_diff = home_score - away_score + 100
    home_expected = 1 / (1 + 10**(-rating_diff/400))
    away_expected = 1 / (1 + 10**(+rating_diff/400))
    home_return = int(32 * ((result  )-home_expected))
    away_return = int(32 * ((1-result)-away_expected))
    return home_score+home_return,away_score+away_return

def main():
    with open("club.txt") as file:
        matches = file.readlines()

    clubs = dict()
    for match in matches:
        (home_str, away_str, result) = match.split(" ")
        clubs[home_str],clubs[away_str] = get_new_scores(clubs.setdefault(home_str,1000), clubs.setdefault(away_str,1000), float(result))

    for club in sorted(clubs, key=clubs.get, reverse=True):
        print(club + ": " + str(clubs[club]) + "\n")

if __name__ == "__main__":
    main()


Determining the value of a position on an array based on its nearest datapoint known value I was wondering what would be the efficient way to determine the value of a position on an array based on its nearest datapoint known value in Python?

  Problem description:

  There are datapoints in an array which are 1000 units apart and their values are known (see the figure). However,
  there are datapoints which fall between the known datapoints and their
  values should be determined based on their nearest known datapoint.

  For instance, following the figure, the value of the datapoint at
  position 124200 should be set to 435 and the value of the datapoint at
  position 124850 should be set to 380.

At the moment, I am doing it like this:

# 'values' is an array which has the values for the known datapoints
# 'position' is an array that has the datapoints which their values should be determined
# 'step' keeps the number of units in between two datapoints with known values
# 'n' is a list which keeps the extracted values

for pos in positions:
    if pos % step <= 500:
        idx = (( pos - (pos % step)) / step) - 1
        n.append(values[idx])
    else:       #pos % step > 500:
        idx = (( pos + (step - (pos % step))) / step)  - 1
       n.append(values[idx])

I was also considering implementing the logic with addition and subtraction operations instead of modulo and division operations, but after some discussion with a friend, we came up with the conclusion that modern processors in doing divisions and multiplication are as fast as doing addition and subtraction, so I ruled that option out.

This is an application of rounding.

idx = int(round(float(pos) / step)) - 1

Note, however, that the result in case pos is exactly halfway into a step is different from your code, and also differs between Python versions.


Normalizing an apartment rental database We are to normalize a database, though it is mostly all theoretical, we aren't actually plugging in the SQL. This is the Data we have normalized to these tables..

and here is our SQL for normalizing it. I'd just like to know if anyone spots any glaring issues.

insert into bldg_table (bldg)
Select distinct bldg from apartmentmaster order by bldg;

----------------------------------------------------------

insert into id_rec (id, firstName, lastName)
Select id, tenantfirstname, tenantlastname from apartmentmaster where apt not in (603, 602);

----------------------------------------------------------

insert into apt_bldg_table(aptNumber, baseRent, bldg)
select apartmentmaster.apt, apartmentmaster.baseRent, apartmentmaster.bldg
from apartmentmaster;

---------------------------------------------------------

insert into tenant_apt_table (id, aptbldg, begdate, baseRent)
select apartmentmaster.id, aptbldg_table.rec_no, "2015-01-01", aptbldg_table.baseRent
from apartmentmaster
join apt_table
on apartmentmaster.apt = aptbldg_table.aptNumber and
apartmentmaster.bldg = aptbldg_table.bldg;

----------------------------------------------------------

insert into rent_rec (tenant_apt_id, paid_date, paid_amt)
select
    tenant_apt_table.rec_no, apartmentmaster.janPaid, apartmentmaster.janRent
from apartmentmaster
join apt_bldg_table
on apartmentmaster.Apt = apt_bldg_table.aptNumber and apartmentmaster.Bldg = apt_bldg_table.bldg
join
    tenant_apt_table
on tenant_apt_table.aptBldg = apt_bldg_table.rec_no

/*and we repeat this query five more times for February, March, etc.*
----------------------------------------------------------

I have added some comments to your code since, as comments have stated, the relational design is really hard to comment on since we don't know all the business requirements.

There is no need to have an order by with an insert statement since when querying,
the order isn't deterministic unless you specify an ORDER BY. Thus, attempting to insert
data in an order isn't really necessary

insert into bldg_table (bldg)
Select distinct bldg from apartmentmaster order by bldg;

----------------------------------------------------------

insert into id_rec (id, firstName, lastName)
Select id, tenantfirstname, tenantlastname
from apartmentmaster where apt not in (603, 602);

----------------------------------------------------------

In the first two queries you didn't fully qualify your column names as there was no reason to. In this segment, and the following ones, you used a full qualifier of table.column. Since you are selecting from a single table, this is unnecessary. If you do want to fully qualify it, I would use table aliases. See below for the change.

insert into apt_bldg_table(aptNumber, baseRent, bldg)
select aptmstr.apt, aptmstr.baseRent, aptmstr.bldg
from apartmentmaster as aptmstr;

---------------------------------------------------------

Instead of repeating this for Feburary, March, etc as you stated below,
you could create a dates table variable and use CROSS JOIN to eliminate the need to run this five times. See this link for a quick demo. Also, if you want to hard code the date as you have done, and want to use double quotes
you need to SET QUOTED_IDENTIFIER OFF.

insert into tenant_apt_table (id, aptbldg, begdate, baseRent)
select apartmentmaster.id, aptbldg_table.rec_no, "2015-01-01", aptbldg_table.baseRent
from apartmentmaster
join apt_table
on apartmentmaster.apt = aptbldg_table.aptNumber and
apartmentmaster.bldg = aptbldg_table.bldg;

----------------------------------------------------------

insert into rent_rec (tenant_apt_id, paid_date, paid_amt)
select
    tenant_apt_table.rec_no, apartmentmaster.janPaid, apartmentmaster.janRent
from apartmentmaster
join apt_bldg_table
on apartmentmaster.Apt = apt_bldg_table.aptNumber and apartmentmaster.Bldg = apt_bldg_table.bldg
join
    tenant_apt_table
on tenant_apt_table.aptBldg = apt_bldg_table.rec_no

/*and we repeat this query five more times for February, March, etc.*
----------------------------------------------------------


High-Low Guessing Game The program is asking you to guess a number between 1-100 and if you guessed too low it says that in Swedish and the same goes for if it's too high and if it's the correct answer it says congratz you guessed right in Swedish.

I'd like comments about any aspects of my code.

public class Kidsprogram {
    public static void main(String[] args) {
        Scanner input = new Scanner(System.in);
        Random rand = new Random();
        int number = rand.nextInt(100) + 1;
        System.out.println("Gissa på ett nr mellan 1 och 100");
        for (int i = 1; i > -1; i++)
        {
            int guess;
            guess = input.nextInt();
            if (guess < number)
            {
                System.out.println("Du gissade för lågt");
            }
            else if(guess > number)
            {
                System.out.println("Du gissade för högt");
            }
            else if(guess == number)
            {
                System.out.println("Antal gissningar: " + i);
                System.out.println("Grattis du gissade rätt!");
                break;
            }
        }
    }
}

Since the end condition of the loop is always true, you can just omit it:

    for (int i = 1; ; i++)

The condition in the final else if(guess == number) is always true,
so you can use simply else.

There's no need to separate the declaration and initialization of the guess variable,
you can do int guess = input.nextInt() which is shorter.

The convention in Java is to put opening braces on the same line as the statement,
like this:

    for (int i = 1; ; i++) {
        int guess = input.nextInt();
        if (guess < number) {
            System.out.println("Du gissade för lågt");
        } else if(guess > number) {
            System.out.println("Du gissade för högt");
        } else {
            System.out.println("Antal gissningar: " + i);
            System.out.println("Grattis du gissade rätt!");
            break;
        }
    }


Regex to parse URLs for their correctness according to RFC 3986 I recently came to write a regex to parse URLs. Now I wonder, did I miss something? Did I make a mistake or could I have written it cleaner? That's why I'm here.

In order to write the regex, I took this paper as a reference  (together with some Wikipedia articles about URIs/URLs).

//According to http://www.ietf.org/rfc/rfc3986.txt
private static final String URL_UNRESERVED_CHARS = "0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ";
private static final String URL_UNRESERVED_SPECIAL_CHARS = "-._~";
private static final String URL_UNRESERVED = URL_UNRESERVED_CHARS + URL_UNRESERVED_SPECIAL_CHARS;
private static final String URL_RESERVED_GEN_DELIMS = ":/?#[]@";
private static final String URL_RESERVED_SUB_DELIMS = "!$&'()*+,;=";
private static final String URL_CHAR_ENCODING_SIGN = "%";

public static final String URL_ALLOWED_CHARS = URL_UNRESERVED + URL_RESERVED_GEN_DELIMS + URL_RESERVED_SUB_DELIMS + URL_CHAR_ENCODING_SIGN;

private static final String REGEX_SCHEME = "[A-Za-z][A-Za-z0-9+.-]*:"; //Also called 'protocol'
private static final String REGEX_AUTHORATIVE_DECLARATION = "/{2}";
private static final String REGEX_USERINFO = "(?:[A-Za-z0-9-._~]|%[A-Fa-f0-9]{2})+(?::(?:[A-Za-z0-9-._~]|%[A-Fa-f0-9]{2})+)?@";
private static final String REGEX_HOST = "(?:[A-Za-z0-9](?:[A-Za-z0-9-]*[A-Za-z0-9])?\\.){1,126}[A-Za-z0-9](?:[A-Za-z0-9-]*[A-Za-z0-9])?";
private static final String REGEX_PORT = ":[0-9]+";
private static final String REGEX_PATH = "/(?:[A-Za-z0-9-._~]|%[A-Fa-f0-9]{2})*";
private static final String REGEX_QUERY = "\\?(?:[A-Za-z0-9-._~]+(?:=(?:[A-Za-z0-9-._~+]|%[A-Fa-f0-9]{2})+)?)(?:[&|;][A-Za-z0-9-._~]+(?:=(?:[A-Za-z0-9-._~+]|%[A-Fa-f0-9]{2})+)?)*";
//FRAGMENTs don't need to be parsed as they won't be sent to the server anyways

public static final String REGEX_URL = "(?:" + REGEX_SCHEME + REGEX_AUTHORATIVE_DECLARATION + ")?(?:" + REGEX_USERINFO + ")?" + REGEX_HOST + "(?:" + REGEX_PORT + ")?(?:" + REGEX_PATH + ")*(?:" + REGEX_QUERY + ")?";

The above code snippet is written in Java (which explains for the \\ as they need to be escaped in Strings in order to be taken as literals). I split the regex up into multiple parts in order to increase readability and maintainability (although it technically shouldn't require any).

That would be the regex in its true form as when encountered in the wild:

(?:[A-Za-z][A-Za-z0-9+.-]*:/{2})?(?:(?:[A-Za-z0-9-._~]|%[A-Fa-f0-9]{2})+(?::(?:[A-Za-z0-9-._~]|[%][A-Fa-f0-9]{2})+)?@)?(?:[A-Za-z0-9](?:[A-Za-z0-9-]*[A-Za-z0-9])?\.){1,126}[A-Za-z0-9](?:[A-Za-z0-9-]*[A-Za-z0-9])?(?::[0-9]+)?(?:/(?:[A-Za-z0-9-._~]|%[A-Fa-f0-9]{2})*)*(?:\?(?:[A-Za-z0-9-._~]+(?:=(?:[A-Za-z0-9-._~+]|%[A-Fa-f0-9]{2})+)?)(?:&|;[A-Za-z0-9-._~]+(?:=(?:[A-Za-z0-9-._~+]|%[A-Fa-f0-9]{2})+)?)*)?

TL;DR Regex above does validate a URL against the defined rules here. I couldn't break it yet, but is it breakable? Can some of it be rewritten in a cleaner fashion?

Holy regex Batman!

Without testing, you should be able to replace all occurrences of:

[0-9] with \d
[A-Za-z0-9] with [\w^_]
[A-Fa-f0-9] with \p{XDigit}
[A-Za-z0-9-._~] with [\w.~-]

and so forth. See documentation for Pattern.

As for your capturing groups I'm quite sure you can do some clever back referencing there but I will save my sanity and not try to parse the regexp. :)


Storing the API endpoints in a Pythonic way I am currently trying to create an API-wrapper. For that, I connect to the endpoint (for example https://host.example.com/api/accounts/open) with the request module. Since there are a bunch of endpoints, I need a way to sort them. Currently I have a constants file, where most of the endpoints are created like this:

HOST = 'https://wallet.shiftnrg.org'
START = ''.join([HOST, '/api'])

ACCOUNTS = ''.join([START, '/accounts'])
ACCOUNTS_OPEN = ''.join([ACCOUNTS, '/open'])
...
LOADER = ''.join([START, '/loader'])
LOADER_GET_STATUS = ''.join([LOADER, '/status'])

And I access them like this (one example):

def accounts_get_balance(address):
    payload = {'address': address}

response = requests.get(constants.ACCOUNTS_GET_BALANCE,
                        params=payload,
                        timeout=constants.TIMEOUT
)

While this is working, I was wondering if there is a more efficient, pythonic way of doing this, since I am just a beginner (especially the constants.py, but if you find anything else, feel free to tell me as well!)

A tip on the second part, is looking to what Brendan Rhodes has to say about Clean Architecture and bring your IO out as much as possible. This will make testing everything a lot easier, and limit the number of God-methods that do everything from constructing the query, getting the response to parsing and persisting the result

For the first part, I would work with a dict of destinations, that you can persist via json or so

settings = {
    'host': 'https://wallet.shiftnrg.org',
    'entry-points':{
        'start': ('api',),
        'accounts': ('_start', 'accounts'),
        'accounts_open': ('_accounts', 'open'),
        'loader': ('_start', 'loader')
    }

}

that you can dump to and load from json

For the entry-points, I would parse this somewhat like this:

def parse_entry_points(entry_points):
    for name, path in entry_points.items():
        yield name, list(expand_path(entry_points, name))

def expand_path(entry_points, item):
    path = entry_points[item]
    for item in path:
        if item[0] != '_':
            yield item
        else:
            yield from expand_path(entry_points, item[1:])

ENTRY_POINTS = dict(parse_entry_points(entry_points=settings['entry-points']))

Off course you can/should replace the _-placeholder with a unicode symbol that will not figure in the eventual URLs

{'accounts': ['api', 'accounts'],
 'accounts_open': ['api', 'accounts', 'open'],
 'loader': ['api', 'loader'],
 'start': ['api']}

assembling the URL can then be as simple as

from urllib.parse import urljoin
def assemble_url(enty_point):
    parts = constants.ENTRY_POINTS[entry_point]
    return urljoin(constants.HOST, parts)


Matchmaking system - sorting musicians by skills, genres, and location I'm making a matchmaking system using PHP and MySQL which allows users to search through a list of musicians so that they can find people with the right skills and interests who are near them. It works but it's extremely messy, and I'd like to learn how I can clean it up and eliminate as many bad habits as possible. I've been trying to learn OOP but it feels more like I'm just writing procedural code and putting it into methods.

The search form:

The database:

When the form is submitted, the find_users method is called from my database class.

class DB
{
protected $conn;

public function __construct($config)
{
    try {
        $conn = new PDO('mysql:host='. $config['host'] . ';
            dbname=' . $config['db'],
            $config['username'],
            $config['password']);

        $conn->setAttribute(\PDO::ATTR_ERRMODE, \PDO::ERRMODE_EXCEPTION);

        $this->conn = $conn;
    } catch (Exception $e) {
        return false;
    }
}

public function query($query, $bindings = [])
{
    try {
        $stmt = $this->conn->prepare($query);
        $stmt->execute($bindings);
        return ($stmt->rowCount() > 0)
        ? $stmt :
        false;
    } catch (Exception $e) {
        var_dump($e);
    }
}

public function get($tableName, $limit = 24)
{
    try {
        $result = $this->conn->query("SELECT * FROM $tableName LIMIT $limit");
        return ( $result->rowCount() > 0)
        ? $result->fetchAll(PDO::FETCH_OBJ)
        : false;
    } catch (Exception $e) {
        die($e);
    }
}

public function get_user_by_username($username)
{
    $result = $this->query("SELECT * FROM users where username = :username LIMIT 1", array(
        "username" => $username
        ));

    return $result->fetch(PDO::FETCH_OBJ);
}

public function find_users($username = '', $skill = '', $tag = '', $lat = 1, $lng = 1, $distance)
{

    $sql = "SELECT DISTINCT
    users.username, ( 3959 * acos( cos( radians($lat) ) * cos( radians( latitude ) )
            * cos( radians( longitude ) - radians($lng) ) + sin( radians($lat) ) * sin(radians(latitude)) ) ) AS distance
    FROM
    users
    INNER JOIN
    user_skills ON users.user_id = user_skills.user_id
    INNER JOIN
    skills ON user_skills.skill_id = skills.skill_id
    INNER JOIN
    user_tags ON users.user_id = user_tags.user_id
    INNER JOIN
    tags ON user_tags.tag_id = tags.tag_id
    WHERE users.availability = 'Yes'";

    $bindings = array();

    if ($username) {
        $sql .= " AND users.username LIKE :username";
        $bindings['username'] = "%" . $username . "%";
    }

    if ($skill) {
        $sql .= " AND skills.skill_name = :skill";
        $bindings['skill'] = $skill;
    }

    if ($tag) {
        $sql .= " AND tags.tag_name = :tag";
        $bindings['tag'] = $tag;
    }

    $sql .= " HAVING distance < $distance ";
    $sql .= " ORDER BY distance";

    $result = $this->query($sql, $bindings);

    return ( $result )
    ? $result->fetchAll(PDO::FETCH_OBJ)
    : false;
}
}

Like I mentioned earlier, it's seriously messy but I'm not sure how to tidy it up. Accepting 6 parameters feels really wrong but I need them so that I can sort users using the specified criteria.

OOP

Why have a get method, if your not going to use it (eg in get_user_by_username)?
It's not clear what the responsibility of your class is. DB suggests that it's some kind of generic wrapper around PDO (with methods such as query and get fitting into it), but then it's also the user dao? That's a bit much for one class. If you separate it into DbWrapper (query and get) and UserDAO (get_by_username and find_user) you should be on the right track.
It's not ok to just dump an exception in a class (the calling code cannot do anything to recover, you have ugly output to the enduser), and it's also not ok to just die inside a class (again, no possibility to recover for the calling class). If you can't recover in the class, just throw the exception upwards.

Misc

be consistent with your SQL: all keywords should be all uppercase (see eg where).
Why do you return false for no results? I think an empty array works just as well (and it's what is returned by PDO, so it means less code).


python Repeat the Sequence This is a game in which a sequence of 4 characters is displayed to the player, one at a time with a delay between them. Then the player must input that sequence as it was displayed.
Sequences are displayed until input is incorrect.
The characters composing the sequences are chosen at random and they are all lines.
__all__ = []

import random
import time

from colorama import deinit, init

MSG_ASK = 'What was the sequence? '
MSG_CORRECT = '\033[32mCorrect!\033[0m'
MSG_INCORRECT = '\033[31mIncorrect\033[0m'

def main():
    k = 4

    lines = r'|\-/'

    seconds = 1

    init()

    while True:
        s = ''

        sequence = random.choices(lines, k=k)
        sequence = ''.join(sequence)

        for i in range(k):
            if not i:
                s = ''
            elif i == 1:
                s = ' '
            else:
                s = i * ' '

            print(s, sequence[i], end='\r')
            time.sleep(seconds)

        print(f'{s}  ')

        if input(MSG_ASK) == sequence:
            print(MSG_CORRECT)
        else:
            print(MSG_INCORRECT)
            break

    deinit()

if __name__ == '__main__':
    main()

Using __all__ = [] is mostly useless in the main program, and should probably be removed.

This is hard to read:
MSG_CORRECT = '\033[32mCorrect!\033[0m'
MSG_INCORRECT = '\033[31mIncorrect\033[0m'

This is much easier:
from colorama import Fore

MSG_CORRECT = Fore.GREEN + 'Correct!' + Fore.RESET
MSG_INCORRECT = Fore.RED + 'Incorrect' + Fore.RESET

Note: Style.RESET_ALL would be an exact translation of \033[0m.  Adjust as desired.

You could pull your loop body out into separate functions:
def generate_random_sequence(length: int, characters: str) -> str:
    """
    Generate a random sequence of characters.
    """

    return ''.join(random.choices(characters, k=length))

def show_sequence(sequence: str, seconds_per_character: float) -> None:
    """
    Display a sequence to the user, one character at a time, on one line,
    using the given delay between characters.

    After the last character has been shown, clear the line.
    """

    for i, ch in enumerate(sequence):
        print(' ' * i, ch, end='\r')
        time.sleep(seconds_per_character)

    print(' ' * len(sequence), ' ')

I've done several things here:

type-hints and a """docstring""" have been added
sequence is computed in one step
removed the s variable, since it can be directly computed as ' ' * i
looped directly over the sequence, using enumerate to keep track of the index.
speed, length and characters are all arguments to the functions, so the challenge could become programmatically harder (ie, faster or longer)

With the above function, the main loop becomes simpler:
def main():
    k = 4
    lines = r'|\-/'
    seconds = 1

    init()

    while True:
        sequence = generate_random_sequence(k, lines)
        show_sequence(sequence, seconds)

        guess = input(MSG_ASK)
        if guess == sequence:
            print(MSG_CORRECT)
        else:
            print(MSG_INCORRECT)
            break

    deinit()