Dependency Container I'm currently creating my own microframework for learning purpose and I want to make it more powerful by providing a way to register classes and instantiate them (once) only when needed. So here is my try :

<?php

namespace Framework\Core;

use Exception;
use ReflectionClass;

/**
 * Class Container
 *
 * @package App\Core
 *
 * Depandancies injection and lazy loading, I guess ? (not sure how to call it for now)
 *
 */
class Container
{

    /**
     * @var array Contains all classes instance.
     */
    private static $instances = array();

    /**
     * @var array Contains all fully qualified classes name.
     */
    private static $classes = array();

    /**
     * @var string Should contain the name of the last registered class name.
     */
    private static $lastRegistered;

    /**
     * Container constructor.
     */
    private function __construct() {}

    /**
     * Prevents cloning.
     */
    private function __clone() {}

    /**
     * Prevents deserialization.
     */
    private function __wakeup() {}

    /**
     * Gets a class instance.
     *
     * @param $name
     * @param null $renew
     * @return mixed
     * @throws Exception
     */
    public static function get($name = NULL) {
        $name = (!$name) ? self::$lastRegistered : $name;
        if (self::$classes[$name]) {
            if (self::$instances[$name]) {
                return self::$instances[$name];
            } else {
                self::$instances[$name] = new self::$classes[$name];
                return self::$instances[$name];
            }
        } else {
            throw new Exception('The following class name could not be found: ' . $name);
        }
    }

    /**
     * Registers a class.
     *
     * @param $name
     * @param $class
     * @return $this|null
     * @throws Exception
     */
    public static function register($name, $class) {
        $reflection = new ReflectionClass($class);
        if (!$reflection->isInstantiable()) {
            throw new Exception('The provided class is not instantiable: ' . $class);
        }
        if (!in_array($name, self::$classes)) {
            self::$classes[$name] = $class;
        }
        self::$lastRegistered = $name;
        return __CLASS__;
    }

    /**
     * Returns an array of all registered classes.
     *
     * @return array
     */
    public static function getRegisteredClasses() {
        return self::$classes;
    }

    /**
     * Returns an array of all instantiated classes.
     *
     * @return array
     */
    public static function getInstantiatedClasses() {
        return self::$instances;
    }

}

So basically, what it does it that it allows you to register a class name as follow :

Container::register('config', \Framework\Core\Config::class);

And then you can get the class instance like this :

Container::get('config');

You can also register() and immediately get() the class instance like this :

Container::register('config', \Framework\Core\Config::class)::get();

I added this "feature" into my microframework in order to prevent instantiating a class more than one time.

The interesting part is that it instantiates the class only when you call the get() method, not when you register it. It allows me to register all framework dependancies at once when the app starts and then get their instances later.

Obvisouly, this class needs improvement in terms of "options". Because, what if I need to register() and get() a class that needs arguments to be passed into its constructor ? I'm aware about that and before making this class more "flexible" I just wanted to know if it's useful to have a "feature" like this in my microframework.

My question is :
How is this called ? Singleton ? Container ? Dependency injection ?
Also, can my microframework benefits from having this "feature" ?

Before addressing the main question, I would like to compare two of the primary methods: register and get. The register method is written well in terms of returning early.

  public static function register($name, $class) {
        $reflection = new ReflectionClass($class);
        if (!$reflection->isInstantiable()) {
            throw new Exception('The provided class is not instantiable: ' . $class);
        }
        if (!in_array($name, self::$classes)) {
            self::$classes[$name] = $class;
        }
        self::$lastRegistered = $name;
        return __CLASS__;
    }

With this method the indentation is kept to a single level. Because of this it is simple to read.
The get method could be re-worked to decrease indentation levels.

  public static function get($name = NULL)   {
          $name = (!$name) ? self::$lastRegistered : $name;
          if (self::$classes[$name]) {
              if (self::$instances[$name]) {
                  return self::$instances[$name];
              } else {
                  self::$instances[$name] = new self::$classes[$name];
                  return self::$instances[$name];
              }
          } else {
              throw new Exception('The following class name could not be found: ' . $name);
          }
      }

The ternary operator with null check can be simplified using a null coalescing operator:
 $name = $name ?? self::$lastRegistered;

And with PHP 7.4+ the null coalescing assignment operator is available:
  $name ??= self::$lastRegistered;

By returning early there is need for else - this can decrease indentation levels, which can result in ones that aren’t as long, as well as make the method shorter over-all:
public static function get($name = NULL) {
    $name = $name ?? self::$lastRegistered;
    if (!self::$classes[$name]) {
        throw new Exception('The following class name could not be found: ' . $name);
    }
    if (self::$instances[$name]) {
        return self::$instances[$name];
    }
    self::$instances[$name] = new self::$classes[$name];
    return self::$instances[$name];
}

In addition to requiring fewer lines of code, there is also no need for the else keyword.

How is this called ? Singleton ? Container ? Dependency injection ?

This appears to be a variation of the Singleton pattern. Typically a singleton class would have a private constructor and a static method like getInstance() that would return a static property after conditionally assigning that property to a newly instantiated object if it hadn’t been set.


for loop in one line (for python -c) I want to use python -c to remove wheel files from the dist/ directory in my project. (*)
Actually, I use the following command:
python -c "from pathlib import Path; [p.unlink() for p in Path('dist').glob('Pascal_Scraper-*.whl')]"

Is there a shorter/cleaner way to do that?

(*) the setuptools command python setup.py rotate --keep=1 --match=.whl doesn't work for me.

If you need to use python for being crossplattform and if you need to do this in one line then this is the way to go, yes.
For some reason
Alternatives to consider:

put the code in a file so you can make it more readable
create a Makefile and run this under a target (like clean:)
use os instead of pathlib to see if you like that better (pathlib is fine, though)
use map and a lambda instead of list comprehension, to see if you like that better (probably not though)

I would have expected that a one-line for loop would work, but it doesn't seem to in python -c when there is a command before it.
I think there is no much cleaner solution


Is it ok to use HttpContext inside View? I have a pretty common issue: I need to know the name of current user and also name of the current controller so I can highlight/disable some links.

Here's some solutions I found by myself:

Get all what's needed inside PartialView.

I have a PartialView inside my page layout, that takes user name like this:

@{ var userName = Context.ToContext().GetUser<Agent>().Name; }

And here's the way I take controller name:

@{ var controllerName = HttpContext.Current.Request.RequestContext.RouteData.Values["controller"].ToString(); }

I have some problem with this code, for it is suspected to ruin MVC pattern.  I have another solution, which I consider even more stupid.

I've created new controller called like ContextController:

public class ContextController : BaseController // BaseController is just a facade
{
    public string GetUserName() {
        return Context.GetUser<Agent>().Name;
    }
}

Here's the problem: I cannot get controller name with this way, because it will always be like "Context", which is useless for me. And I also get the name on view like this:

@{ var userName = Html.Action("GetUserName", "Context"); }

The last way I figured out is to pass needed strings through ViewData. But I have about 18 controllers and some of 'em have like 3 methods, returning ViewResult. It's not bad design, it's just reality. And I don't actually want to create and pass ViewDataDictionary for every single method, that returns ViewResult. Maybe I could extend my BaseController, so ViewDataDictionary for every page will always have userName and controllerName?

But here comes another problem: I have whole lot of AJAX in my project, and it seems to me not legit enough to pass unused data every time controller action is called.

If you know of a better solution, I'd be happy to hear it.

You should try creating ViewModel classes - they're simple classes which just have a bunch of properties, and you pass them to the view instead of using ViewData or ViewBag.  You can make them all inherit off of a base model which has a bunch of show/hide flags (like user1614543 suggested) or put the flags on individual models (the better choice) which get returned to the views that use those flags.  Either way, you can put in sane default values so you only need to set them when you're displaying different data.  You can even pass the name of the controller as a string, or use an enum to categorize them.

This will make your markup more readable, and ensure type-safety of your data, which will make working with it easier.

Here's an example, although you'll have to convert it from ASPX to Razor.


Parsing files with county codes I am running through a file and inserting elements one by one. The counties all contain specific county codes which are duplicated many times throughout the file. I am looking for a way to assign these codes to a specific county while ignoring duplicate county codes.

I have two versions I wrote below with runtimes:

getCodes()

def get_codes(data):
    code_info = {}
    for row in data:
        county = row["county"]
        code = int(float(row["code"]))
        if code > 100000:
            code = code/100
        if county not in code_info:
            code_info[county] = []
        code_info[county].append(code)
    for county in code_info:
        code_info[county] = list(set(code_info[county]))
    return code_info

get_codes2()

def get_codes2(data):
    code_info = {}
    for row in data:
        county = row["county"]
        code = int(float(row["code"]))
        if code > 100000:
            code = code/100

        if county in code_info:
            if not code in code_info[county]:
                code_info[county].append(code)
        else:
            code_info[county] = []
            code_info[county].append(code)
    return code_info

county_data = csv.DictReader(open("county_file.txt"))
start = time.time()
county_codes = get_codes(county_data)
end = time.time()
print "run time: " + str(end-start)

county_data = csv.DictReader(open("county_file.txt"))
start = time.time()
county_codes = get_codes2(county_data)
end = time.time()
print "run time: " + str(end-start)

Also, it's probably obvious from this, but county codes that are greater than 100000 can have trailing zeroes accidentally added, so I'm removing them by dividing by 100. As another note, the int(float()) conversion is intentional. Sometimes the county codes are values such as "27.7" and need to be converted to "27", other times they are just basic ints.

The runtimes on my system:

get_codes: 9 seconds
get_codes2: 14 seconds

How can I improve this further for better performance?

I think you're pretty close to optimal here. I made a few tweaks, to avoid some conditionals by using sets instead. I don't actually know if it'll run faster; you'll need to benchmark it, and it likely depends on the breakdown of how many dupes per county there are.

def get_codes3(data):
    from collections import defaultdict
    codeinfo = defaultdict(set)
    for row in data:
        county = row["county"]
        # remove float() if you can get away with it - are there
        # actually values like '1.5' in the data?
        code = int(float(row["code"]))
        if code > 100000:
            code = code/100

        codeinfo[county].add(code)
    # if you can get away with sets instead of lists in your return
    # value, you're good
    return code_info
    # otherwise:
    # return dict([(county, list(info)) for county, info in code_info.iteritems()])

You don't need float() unless the data is actually like "123.45" - it's not clear if that's the case
sets can work in most places that lists work, so you might not need to convert to a list
It might be worth it to write a script that does just the x = x/100 if x > 100000 part and writes that out to a new file


Usage of StringBuilder for returning copies of a string Given a string and a non-negative int n, we'll say that the front of the string is the first 3 chars, or whatever is there if the string is less than length 3. Return n copies of the front:

frontTimes("Chocolate", 2) → "ChoCho"
frontTimes("Chocolate", 3) → "ChoChoCho"
frontTimes("Abc", 3) → "AbcAbcAbc"
frontTimes("X", 3) → "XXX"

I've solved above problem using following code but I'm using a new StringBuilder to store intermediate string result. Does it really needed can I optimize and remove the for loop in the else?

public String frontTimes(String str, int n) {

        StringBuilder sb = new StringBuilder(str);
        StringBuilder newsb = new StringBuilder();

        if(sb.length() >= 3)
        {
            for(int i = 0; i < n; i++)
            {
                newsb.append(sb.substring(0,3));
            }

            return newsb.toString();
        }
        else
        {
            for(int i = 0; i < n; i++)
            {
                newsb.append(sb.toString());
            }

            return newsb.toString();

        }

    }

I'd just like to make a slight change to the code. When you are instantiating a StringBuilder, you have the opportunity to specify the capacity of it. Using this constructor, we can avoid having it resized more than necessary.

public String frontTimes(String str, int n) {
    StringBuilder stringBuilder = new StringBuilder(3 * n);
    if (str.length() > 3) {
        str = str.substring(0, 3);
    }
    for (int i = 0; i < n; i++) {
        stringBuilder.append(str);
    }
    return stringBuilder.toString();
}

Note how many times that 3 is declared? Would be better to extract that to a constant, or even better, a parameter. And with a few other changes to the above (as suggested by @rolfl in chat):

public String frontTimes(final String str, final int frontLength, final int times) {
    String copyString = str.length() > frontLength ? str.substring(0, frontLength) : str;
    StringBuilder stringBuilder = new StringBuilder(copyString.length() * times);
    for (int i = 0; i < times; i++) {
        stringBuilder.append(copyString);
    }
    return stringBuilder.toString();
}


Sieve of Eratosthenes in Scheme (R7RS) I've seen many implementations of Sieve of Eratosthenes in Scheme, but I thought I'd try to write one that is both space- and time-efficient:

Space-efficient: I use R7RS bytevectors as a bitset, which encodes odd numbers starting with 3 (so bit 0 = 3, bit 1 = 5, bit 2 = 7, etc.).
Time-efficient: Many implementations scan the table looking for the next viable prime. I instead use first-set-bit (analogous to Integer.numberOfTrailingZeros in Java) to avoid scanning each bit individually.

This code generates prime numbers up to at least n, and usually a handful more. It depends on SRFI 60 for the bitwise operations.

(define (primes-up-to-about n)
  (define table
    (make-bytevector (+ (arithmetic-shift n -4) 1) 255))
  (define len (bytevector-length table))
  (define bitlen (arithmetic-shift len 3))

  (define (clear! i)
    (define q (arithmetic-shift i -3))
    (bytevector-u8-set! table q
                        (copy-bit (bitwise-and i 7) (bytevector-u8-ref table q) #f)))
  (define (next-marked-from i)
    (let loop ((q (arithmetic-shift i -3))
               (m (arithmetic-shift -1 (bitwise-and i 7))))
      (and (< q len)
           (let ((v (bitwise-and (bytevector-u8-ref table q) m)))
             (cond ((zero? v) (loop (+ q 1) -1))
                   (else (+ (arithmetic-shift q 3) (first-set-bit v))))))))
  (define (index->value i)
    (+ i i 3))

  (let loop ((rv '(2)) (i 0))
    (define next (next-marked-from i))
    (if (not next)
        (reverse rv)
        (let ((nextval (index->value next)))
          (do ((j next (+ j nextval)))
              ((>= j bitlen))
            (clear! j))
          (loop (cons nextval rv) next)))))

I'm looking for ways to make the code more compact and/or elegant without sacrificing the space or time efficiency.

In particular, if you can make a SRFI 42 :bitset generator that actually does the equivalent of my next-marked-from, I can just turn the whole function into a list-ec comprehension. What's not to like? :-)

For people who want to test this program in Racket (since there aren't very many R7RS implementations currently around), add the following lines before the function definition:

(require srfi/60)
(define bytevector-length bytes-length)
(define bytevector-u8-ref bytes-ref)
(define bytevector-u8-set! bytes-set!)
(define make-bytevector make-bytes)

To be honest, if it weren't for the hint in the first line (define (primes-up-to-about n)), I'd have no idea that this was an implementation of the Sieve of Eratosthenes.

The main problem, I think, is that names like table, clear!, and next-marked-from are all too low-level, not much better than the bytevector primitives, and devoid of contextual meaning.

To start, I think you should be able to rearrange the definitions.

  (define bytelen (+ (arithmetic-shift n -4) 1))
  (define primes-table (make-bytevector bytelen 255))
  (define bitlen (arithmetic-shift len 3))

Instead of clear!, try a more meaningful name like mark-as-composite!.

To complete the abstraction of the bytevector as a sieve, you would also need a helper function (define (next-prime-above n) …) that combines your next-marked-from and index->value.

(reverse rv) seems suspiciously expensive.  I think you should be able to build the list of primes in ascending order using smarter recursion.


Selecting random class from weighted class probability distribution I have a solution that I've scratched out, but it's a tad messy:

First, let's say we have a random number from a uniform distribution we'll be using to select a random class from a class probability distribution:

r = 0.525325235325

And a list of classes, along with a corresponding list of values representing a class probability distribution for the four classes:

classes = ["w", "x", "y", "z"]

distribution = [0.1, 0.2, 0.3, 0.4]

Now, to get the cumulative probabilities:

b = [sum(distribution[0:x + 1]) for x in range(len(distribution))]
>>> b
[0.1, 0.30000000000000004, 0.6000000000000001, 1.0]

So far so good. Now, select a class by the weighted probability:

c = [y for y in b if y > r][0]
>>> c
0.6000000000000001

And to get the selected class:

selected = classes[b.index(c)]
>>> selected
'y'

Putting it all together:

r = 0.525325235325
classes = ["w", "x", "y", "z"]
distribution = [0.1, 0.2, 0.3, 0.4]

b = [sum(distribution[0:x + 1]) for x in range(len(distribution))]
selected = classes[b.index([y for y in b if y > r][0])]

This works well enough. The thing is, it will be called each time the class probability distribution is updated along with a complex code base – so efficiency matters.

Are there any ways I can make this code more efficient in terms of speed and/or memory usage (eliminating b without the expense of additional processing would be nice) in Python? Alternatively, is there an established algorithm for this problem that I've overlooked?

Using dictionaries is faster than lists. Your distributions can serve as keys to find the corresponding values instead of using indexed lists.


Functional Sieve of Eratosthenes in Clojure For some reason, this is the first time I've ever written an implementation of the Sieve of Eratosthenes. I basically just stared at the Wikipedia walkthrough of the algorithm until it made sense, then tried making the algorithm in idiomatic Clojure.
I'm not using an boolean array representing every number. Instead, I have a marked set, and a primes vector that are maintained separately.

My main concern is performance. I was hoping for better performance than I'm getting. It's only about 2x faster than the naïve implementation (included below the Sieve code for reference). I'd mainly like recommendations for speeding this up.

I decided to go overboard with the use of ->> here. It just fit so perfectly everywhere, but I think I may have taken it too far.

Is there a way to avoid the cond dispatch in naïve-prime??

Anything else notable.

(ns irrelevant.sieves.eratosthenes)

; ----- Sieve -----

(defn sieve-primes [n]
  (loop [p 2 ; First prime
         marked #{} ; Found composites
         primes []]
    (let [mults (->> n
                     (range p)
                     (map #(* p %))
                     (take-while #(< % n)))

          next-p (->> p
                      (inc)
                      (iterate inc)
                      (remove marked)
                      (first))

          new-primes (conj primes p)]

      (if (>= next-p n)
        new-primes
        (recur next-p (into marked mults) new-primes)))))

; ----- Naive -----

(defn naive-prime? [n]
  (cond
    (< n 2) false
    (= n 2) true
    :else (->> n
               (Math/sqrt)
               (inc)
               (range 2)
               (some #(zero? (rem n %)))
               (not))))

(defn naive-primes [n]
  (->> n
       (range 2)
       (filterv naive-prime?)))

Runtime Complexity:
I wrote up a testing function to automate timing with the help of Criterium for benchmarking:
(defn test-times [start-n end-n step-factor]
  (->> (iterate #(* step-factor %) start-n)
       (take-while #(< % end-n))
       (mapv (fn [n] [n (->> (c/quick-benchmark*
                               (fn [] (sieve-primes n))
                               nil)
                             (:mean)
                             (first))]))))

This tests how long it takes for various values of n, then packages the results in a vector of [n time-taken] pairs. Here are the results (times in seconds):
(test-times 1 1e7 2)
=>
[[1 2.3196602948749615E-7]
 [2 2.3105128053865377E-7]
 [4 1.42932799280724E-6]
 [8 5.63559279071997E-6]
 [16 1.2984918224299065E-5]
 [32 3.289705735278571E-5]
 [64 7.087895492662475E-5]
 [128 1.4285673446327686E-4]
 [256 2.923177758112095E-4]
 [512 6.273025793650794E-4]
 [1024 0.0012981272479166666]
 [2048 0.0025652252416666667]
 [4096 0.005141740791666667]
 [8192 0.01068772265]
 [16384 0.022486903166666666]
 [32768 0.05367695991666667]
 [65536 0.11212592816666668]
 [131072 0.23180363016666666]
 [262144 0.48056071016666674]
 [524288 1.1464995601666668]
 [1048576 2.8823068596666666]
 [2097152 6.4231157065]
 [4194304 15.808042165333335]
 [8388608 56.98961213533334]]

Plotting it out, it appears to be roughly O(n^3). The time taken seems to roughly double every time n is doubled (O(n)) until n=524288, then it explodes. A custom Wolfram regression calculator gave a best fit with a cubic curve.

Is there a way to avoid the cond dispatch in naïve-prime??

You could do naive-prime? this way:

(defn naive-prime? [n]
  (and
       (>= n 2)
       (->> n
            (inc)
            (Math/sqrt)
            (range 2)
            (not-any? #(zero? (rem n %))))))

This gets rid of the special cases 1 and 2. The trick is to increment before applying Math/sqrt, so that the edge case 2 works correctly.

I've also elided the not into the some to produce not-any?.

  I'd mainly like recommendations for speeding this up.

Try

(defn sieve-primes [n]
  (loop [p 2 ; First prime
         marked #{} ; Found composites
         primes []]
    (let [mults (range (* p p) (inc n) p)
          next-p (->> p
                      (inc)
                      (iterate inc)
                      (remove marked)
                      (first))

          new-primes (conj primes p)]

      (if (>= (* next-p next-p) n)
        (into new-primes (remove marked (range next-p (inc n)))

There are a couple of changes here.

Generate  mults as a range, starting at (* p p) - the first
number that needs to be tested with prime factor p.
Stop testing when the prime you would use, next-p, squared is
bigger than the limit n.

I don't like the idea of building a massive set of composites. Better turn the algorithm inside out and test each number as it occurs against only the necessary prime factors. But that's a quite different algorithm.


Std lib-like C++ function to find nearest elements in a container Initial problem

For a project I found myself in the need to search for elements in a container that are the closest neighbours to another precise element I have. In my case it was points in any dimension, but it can apply to various other things that we are not used to compute "distance" for.

So I decided to write a generic function to perform this search, so I can use it for whatever type I want, provided that I can compute the "distance" between two elements of this type. I tried to make it in the style of standard library's algorithm.

My solution

template<typename T, class Distance>
struct Comp
{
    using result_type = typename std::result_of<Distance(const T&, const T&)>::type;
    using type = std::less<result_type>;
};

template<class InputIt, typename T, class Distance,
class Compare = typename Comp<T, Distance>::type>

std::vector<T> find_n_nearest(
    InputIt start,
    InputIt end,
    const T& val,
    size_t n,
    Distance dist,
    Compare comp = Compare())
{
    std::vector<T> result{start, end};

    std::sort(std::begin(result), std::end(result),
        [&] (const T& t1, const T& t2)
        {
            return comp(dist(val, t1), dist(val, t2));
        });

    result.erase(std::begin(result) + n, std::end(result));

    return result;
}

What this (the function) basically does is :

create a copy of the range we want to look in
sort it, by comparing values returned from the distance function between val and the element of the range, so that we have the nearest neighbours of val at the begining of our vector
compare the distances with a custom operator if provided, std::less if not
only keep the n elements we want

Now about the boilerplate :

I want to default the Compare type to std::less, which needs a type parameter. So we need to find the return type of the Distance object provided, and my solution here is what I found to work with both functions and lambdas.

Examples

Simple use case : retrieve the nearest values to an int. Distance between two ints will be the absolute value of their substraction.

const auto distance_int =
    [] (int i, int j)
    {
        return std::abs(i-j);
    };

std::vector<int> v = {56, 10, 79841651, 45, 59, 68, -20, 0, 36, 23, -3256};
auto res = {0, 10, 23, -20, 36};
auto found = find_n_nearest(std::begin(v), std::end(v), 4, 5, distance_int);

if(std::equal(std::begin(res), std::end(res), std::begin(found)))
    std::cout << "Success !" << std::endl;

To illustrate the use of the comparator, let's say I now define my "neighbourhood" of ints as being as distant as possible. In a word, the opposite of the precedent example.

std::list<int> v = {56, 10, 79841651, 45, 59, 68, -20, 0, 36, 23, -3256};
auto res = {79841651, -3256, 68, 59, 56};
auto found = find_n_nearest(std::begin(v), std::end(v), 4, 5, distance_int, std::greater<int>());

if(std::equal(std::begin(res), std::end(res), std::begin(found)))
    std::cout << "Success !" << std::endl;

Review

When I used this function, it was convenient to me to receive the results as a std::vector<T> but this is not very good as a generic algorithm.

There are two problems with this : I think we should not alter the original range, so sorting it is not an option. Then we have to copy it elsewhere, and for that I used my cache vector (not that much time to think about it during the project, and it was not a critical piece of code). I thought of replacing this by providing an OutputIt to the function, indicating where to put the results (for example an user-provided vector or whatever container), but I don't think I could sort the range in my algorithm, because the Output Iterator concept is used only to ... output things.

If there are more efficient algorithms (instead of sorting from distance) to do it feel free to tell me, but that's not my main concern. I'd like to have an elegant solution, and pieces of advice on anything you think is not quite good in my code.

I think all you need is std::nth_element:

template<typename Q, typename I, typename Distance>
void find_n_nearest(const Q& q, I first, I nth, I last, Distance dist)
{
    using T = decltype(*first);
    auto compare = [&q, &dist] (T i, T j) { return dist(i, q) < dist(j, q); };
    std::nth_element(first, nth, last, compare);
    std::sort(first, last, compare);
}

int main()
{
    auto distance = [] (int i, int j) { return std::abs(i-j); };
    std::vector<int> v = {56, 10, 79841651, 45, 59, 68, -20, 0, 36, 23, -3256};
    auto res = {0, 10, 23, -20, 36};

    find_n_nearest(4, v.begin(), v.begin() + 5, v.end(), distance);
    assert(std::equal(v.begin(), v.begin() + 5, res.begin()));
}

If you just need the n nearest elements but not necessarily sorted, you can skip std::sort. Then complexity is linear in n.

I have skipped the initial copy part, because it's not always needed. Feel free to add it if you like.


Simulation to find max population of R-pentominos in Game of Life I'm brand new to Python and I feel my code is really like what coded in Java.

I try to practice python by small problem and that is to find the maximum population of R-pentomino. Wiki says:

  During this early research, Conway discovered that the R-pentomino
  failed to stabilize in a small number of generations. In fact, it
  takes 1103 generations to stabilize, by which time it has a population
  of 116 and has fired six escaping gliders (these were the first
  gliders ever discovered).

So I write this code:

from sets import Set

__author__ = 'Sayakiss'

dx = [0, 0, 1, -1, 1, -1, 1, -1]
dy = [1, -1, 0, 0, -1, 1, 1, -1]

def is_ng_alive(x, y, original_set):
    cnt = 0;
    for i in range(len(dx)):
        nx = x + dx[i]
        ny = y + dy[i]
        if (nx, ny) in original_set:
            cnt += 1
    if (x, y) in original_set:
        if cnt in [2, 3]:
            return True
    else:
        if cnt == 3:
            return True
    return False

def sim(original_set):
    new_set = Set()
    for (x, y) in original_set:
        for i in range(len(dx)):
            nx = x + dx[i]
            ny = y + dy[i]
            if is_ng_alive(nx, ny, original_set):
                new_set.add((nx, ny))
        if is_ng_alive(x, y, original_set):
            new_set.add((x, y))
    return new_set

def print_cell_set(cell_set, x_size=10, y_size=10):
    for x in range(-x_size, x_size):
        for y in range(-y_size, y_size):
            if (x, y) in cell_set:
                print '*',
            else:
                print '.',
        print ''

cell_set = Set([(0, 1), (0, 2), (1, 0), (1, 1), (2, 1)])
max_size = 0
max_gen = 0
for i in range(1500):
    cell_set = sim(cell_set)
    gen_size = len(cell_set)
    if gen_size > max_size:
        max_size = gen_size
        max_gen = i + 1
    print str(i + 1) + " generation population: " + str(gen_size)

print str(max_gen) + "-" + str(max_size)

Output of my code should be 821-319. It means the maximum population is 319 and occurs in 821-th generation. I'm quite sure it's correct because my friend's code gives the same answer.

Could anyone give me some suggestions about my code?

Global

Python has deprecated sets, instead use the __builtins__.set.
This allows us to add some nice sugar to your code, for creating the set.

cell_set = {(0, 1), (0, 2), (1, 0), (1, 1), (2, 1)}

Your names are good, but a few 'let you down', as I don't know what sim means, I assume simulate.
You may want to use str.format to format strings for print.
This is as it will be simpler to add strings and numbers.

print str(i + 1) + " generation population: " + str(gen_size)
print "{} generation population: {}".format(i + 1, gen_size)

You can use Python's max to find the maximum.

max([1, 2, 3]) # 3
max([(1, 3), (2, 2), (3, 1)]) # (3, 1)

As you mutate cell_set you will have to use a full-fledged generator, rather than a generator comprehension.
Where you yield (gen_size, i).

def generate_cells(cell_set, generations=1500):
    for i in range(1, generations + 1):
        cell_set = sim(cell_set)
        print "{} generation population: {}".format(i, len(cell_set))
        yield len(cell_set), i

max_size, max_gen = max(generate_cells(cell_set))

In my opinion that is much easier to understand.
Python has a dedicated style guide called PEP8, it states that constants, dx and dy, should be upper-snake case.
So DX and DY.

is_ng_alive

You should use zip, to loop through both dx and dy.

for ax, ay in zip(dx, dy):
    if (x + ax, y + ay) in original_set:
        cnt += 1

You can use sum and a generator comprehension to not have to add up cnt manually.

cnt = sum((x + ax, y + ay) in original_set for ax, ay in zip(dx, dy))

You can simplify your logic, as if cnt is 3 it will return true.
Then you can just return the boolean from the new truth statement.

if cnt == 3:
    return True

return (x, y) in original_set and cnt == 2

I mistakenly thought that zip(dx, dy) would be a line.
Instead you may want the algorithm to be 'more square', which would be:

for ax in dx:
    for ay in dy:
        (x + ax, y + ay)

sim

Using the changes from above you can change the for loop with ease.
However to make things easier to read you may want to make (x + ax, y + ay) for ax, ay in zip(dx, dy) a function.

def new_coords(x, y):
    return ((x + ax, y + ay) for ax, ay in zip(dx, dy))

for item in ((nx, ny) for (nx, ny) in new_coords(x, y)
             if is_ng_alive(nx, ny, original_set)):
    new_set.add(item)

print_cell_set

This is very bad on performance, instead you would want to build a string, and then limit prints.
You can do this with another list comprehension, a turnery and str.join.

for x in range(-x_size, x_size):
    print ''.join('*' if (x, y) in cell_set else '.' for y in range(-y_size, y_size))

Overall you have nice code, you just missed a few things to improve clarity.

And I got:

__author__ = 'Sayakiss'

DX = [0, 0, 1, -1, 1, -1, 1, -1]
DY = [1, -1, 0, 0, -1, 1, 1, -1]

def new_coords(x, y):
    return ((x + ax, y + ay) for ax, ay in zip(DX, DY))

def is_ng_alive(x, y, original_set):
    cnt = sum((nx, ny) in original_set for nx, ny in new_coords(x, y))
    if cnt == 3:
        return True

    return (x, y) in original_set and cnt == 2

def sim(original_set):
    new_set = set()
    for (x, y) in original_set:
        for item in ((nx, ny) for (nx, ny) in new_coords(x, y)
                 if is_ng_alive(nx, ny, original_set)):
            new_set.add(item)
        if is_ng_alive(x, y, original_set):
            new_set.add((x, y))
    return new_set

def print_cell_set(cell_set, x_size=10, y_size=10):
    for x in range(-x_size, x_size):
        print ''.join('*' if (x, y) in cell_set else '.' for y in range(-y_size, y_size))

def generate_cells(cell_set, generations=1500):
    for i in range(1, generations + 1):
        cell_set = sim(cell_set)
        print "{} generation population: {}".format(i, len(cell_set))
        yield len(cell_set), i

cell_set = {(0, 1), (0, 2), (1, 0), (1, 1), (2, 1)}
print "{1}-{0}".format(*max(generate_cells(cell_set)))


Should I return true or false if both of the IP address strings are empty? I am validating two IP addresses. I simply wrote a peice of code like

if(string.IsNullOrEmpty(ip1) && string.IsNullOrEmpty(ip2)
  return true;

But is this logically correct? What should we done with empty strings? If both are empty strings, is it okay to return true?

May be a complete code snipet can give better insight.

public static bool IsIPAddressMatching(string ipAddressA, string ipAddressB)
        {
            try
            {
                if(string.IsNullOrEmpty(ipAddressA) || string.IsNullOrEmpty(ipAddressB))
                    return true;
                var address1 = IPAddress.Parse(ipAddressA);
                var address2 = IPAddress.Parse(ipAddressB);

                if (address1.AddressFamily == address2.AddressFamily)
                    return (address1.Equals(address2));

               // some code here to check mapped IP addresses

            }
            catch (Exception)
            {
                return false;
            }
        }

From the IP validation point of view null or empty IP addresses are exceptional actions so you can not say that two null or empty strings are the same IP addresses becouse they aren't IP addresses and sometimes not even strings.

And you don't have to sorrund the logic with try-catch block becouse it will eat usefull information when things will get ugly. If you don't want to deal with parsing exception then use IPAddress.TryParse it will be faster becouse throwing an exception is always a heavy weight stuff (check it with profiler).

public static bool IsIPAddressMatching(string ipAddressA, string ipAddressB)
{
    if (ipAddressA == null)
    {
        throw new ArgumentNullException("ipAddressA");
    }
    if (ipAddressB == null)
    {
        throw new ArgumentNullException("ipAddressB");
    }

    if (string.IsNullOrEmpty(ipAddressA))
    {
        throw new ArgumentException("ipAddressA cannot be an empty string", "ipAddressA");
    }
    if (string.IsNullOrEmpty(ipAddressB))
    {
        throw new ArgumentException("ipAddressB cannot be an empty string", "ipAddressB");
    }

    var address1 = IPAddress.Parse(ipAddressA);
    var address2 = IPAddress.Parse(ipAddressB);

    if (address1.AddressFamily == address2.AddressFamily)
    {
        return (address1.Equals(address2));
    }

   // some code here to check mapped IP addresses

   return ...
}


Numbers to words converter Please review my code:

import java.lang.*;
import java.util.*;
import java.io.*;

class NumberToWords
{
  static public boolean HelperConvertNumberToText(int num, String[] result)
  {
      String [] strones = {
        "One", "Two", "Three", "Four", "Five", "Six", "Seven", "Eight",
        "Nine", "Ten", "Eleven", "Twelve", "Thirteen", "Fourteen",
        "Fifteen", "Sixteen", "Seventeen", "Eighteen", "Nineteen",
      };

      String [] strtens = {
          "Ten", "Twenty", "Thirty", "Fourty", "Fifty", "Sixty",
          "Seventy", "Eighty", "Ninety", "Hundred"
      };

      result[0] = "";
      int single, tens, hundreds;

      if(num > 1000)
          return false;

      hundreds = num / 100;
      num = num - hundreds * 100;
      if( num < 20)
      {
          tens = 0; // special case
          single = num;
      }
      else
      {
          tens = num / 10;
          num = num - tens * 10;
          single = num;
      }

      if(hundreds > 0)
      {
         result[0] += strones[hundreds-1];
         result[0] += " Hundred ";
      }
      if(tens > 0)
      {
         result[0] += strtens[tens - 1];
         result[0] += " ";
      }
      if(single > 0)
      {
         result[0] += strones[single - 1];
         result[0] += " ";
      }
      return true;
  }

  static public boolean ConvertNumberToText(int num, String[] result)
  {
      String tempString[] = new String[1];
      tempString[0] = "";
      int thousands;
      int temp;
      result[0] = "";
      if(num < 0 || num > 100000)
      {
          System.out.println(num + " \tNot Supported");
          return false;
      }

      if( num == 0)
      {
         System.out.println(num + " \tZero");
         return false;
      }

      if(num < 1000)
      {
          HelperConvertNumberToText(num, tempString);
          result[0] += tempString[0];
      }
      else
      {
          thousands = num / 1000;
          temp = num - thousands * 1000;
          HelperConvertNumberToText(thousands, tempString);
          result[0] += tempString[0];
          result[0] += "Thousand ";
          HelperConvertNumberToText(temp, tempString);
          result[0] += tempString[0];
      }
      return true;
  }

  public static void main(String[] args)
  {
      String [] result = new String[1];
      result[0] = "";
      int  i, num;
      int [] arrNum =
      {
        -1, 0, 5, 10, 15, 19, 20, 21, 25, 33, 49, 50, 72,
        99, 100, 101, 117, 199, 200, 214, 517, 589, 999,
        1000, 1010, 1018, 1200, 9890, 10119, 13535, 57019,
        99999, 100000, 100001
      };

      for (i = 0; i < arrNum.length; i++)
      {
          num = arrNum[i];
          if( ConvertNumberToText(num, result) == true)
             System.out.println(num  + "\t" + result[0]);
      }
  }
}

"Strone"?  What's a "strone"?  Oh… of course!

In seriousness, naming could be improved.  They are obviously arrays of strings; the Hungarian prefix is unnecessary.  Also, as constants, they should be declared static final and named with ALL_CAPS.

public class NumberToWords {
    private static final ONES = { "One", "Two", … };
    private static final TENS = { "Ten", "Twenty", … };
    …
}

Helper functions should be private.

It is customary to put the static keyword after the access modifier, as in public static void main(…).  It is unconventional to say static public boolean ConvertNumberToText(…).  Also, functions should be namedStartingWithLowerCase().

Using a single-element array as a parameter so that you can pass back the result as an out-parameter is very weird.  Using a StringBuilder would be a bit more obvious, but still very odd.  The right approach should be to return a String, and throw an exception if necessary — perhaps NumberFormatException.  Ideally, your code should be able to handle every possible int input, making errors impossible.


Central Database class I have this web site I inherited as part of my job and it has some old code.  I'd like to optimize it, but I don't really have the time to do it right, so I'm looking for some "low hanging fruit" to optimize. Basically I want minimal change with the greatest performance improvement.

One of the things I've come across is an old static method used to grab data from a remote database:

public static IDataReader GetRS(string sql, string connString)
{
    SqlConnection dbconn = new SqlConnection();
    if (connString.ToLower().Trim() == "shop")
        dbconn.ConnectionString = ConfigurationManager.ConnectionStrings["Shop.DbConnection"].ConnectionString;
    else
        dbconn.ConnectionString = connString;
    dbconn.Open();
    SqlCommand cmd = new SqlCommand(sql, dbconn);
    return cmd.ExecuteReader(CommandBehavior.CloseConnection);
}

I think this code is contributing to a high number of NumberOfReclaimedConnections we are seeing on the servers due to the calling code not cleaning up properly.  If I rewrite the above method like the one below, will it help make things more efficient without having to rewrite all the hundreds of instances of calling code?

public static IDataReader GetRS(string sql, string connString)
{
    IDataReader ans = null;
    using (DataTable tbl = new DataTable())
    {
        using (SqlConnection dbconn = new SqlConnection())
        {
            if (connString.ToLower().Trim() == "shop")
                dbconn.ConnectionString = ConfigurationManager.ConnectionStrings["Shop.DbConnection"].ConnectionString;
            else
                dbconn.ConnectionString = connString;
            using (SqlCommand cmd = new SqlCommand(sql, dbconn))
            {
                using (SqlDataAdapter adapter = new SqlDataAdapter(cmd))
                { adapter.Fill(tbl); }
            }
        }
        if (tbl.Rows.Count > 0)
        {
            ans = tbl.CreateDataReader();
        }
    }
    return ans;
}

I'm pretty sure this will do a better job at cleaning up the connection as it will be disposed before the method returns and even if there is an error it will be disposed. Does the DataTable being disposed interfere with the caller's ability to extract data from the DataTableReader?  If I do need to remove the using around the DataTable, will not cleaning up the DataTable cause similar issues to not cleaning up the SqlConnection as it is now?

I'm also open to writing this code in a completely different way as long as I'm fulfilling the following:

I don't have to rewrite code on all the calling references to this method, I just don't have time to do that right now. But it's on the list.
Due to the above requirement, the return signature has to be compatible with an IDataReader. I'm open to writing a custom class that inherits this interface if that is what helps.
The code targets and will compile in a .NET 2.0 project. We're behind the technology curve.

I would like to point some opportunities here. As you are refactoring your old code, I suggest:

try to avoid the use of literal string inside methods. Instead, use const variables or Resources (I do prefer Resources);
create your variables where you need them. Move your using to the nearest point of interest;
try to use string.Equals, it will be faster;
get your DbCommand directly from your connection, you have a method to do this;
try to use a Factory Pattern in this class, so in the future, if you need to change your database, you won't have to search for all your SqlConnection objects, you'll just need to change your Factory creator;
don't forget your XML comments

I made an example just to show those points. Sorry for the simplicity, but it's more difficult without knowing all your solution, but here is the idea:

using System;
using System.Configuration;
using System.Data;
using System.Data.Common;

namespace ClassLibrary1
{
    public class Class1
    {
        public const string shopStr = "shop";
        public const string shopDbConn = "Shop.DbConnection";
        public const string factoryDb = "System.Data.SqlClient";

        private static DbProviderFactory factory = DbProviderFactories.GetFactory(factoryDb);

        /// <summary>
        /// Get data from the database.
        /// </summary>
        /// <param name="sql">SQL command to retrieve data.</param>
        /// <param name="connString">Connection string for the database.</param>
        /// <returns><c>IDataReader</c> with the first result set, or <c>null</c> if no result was found.</returns>
        public static IDataReader GetRS(string sql, string connString)
        {
            IDataReader result = null;

            using (DbConnection conn = factory.CreateConnection())
            {
                if (string.Equals(connString, shopStr, StringComparison.OrdinalIgnoreCase))
                    conn.ConnectionString = ConfigurationManager.ConnectionStrings[shopDbConn].ConnectionString;
                else
                    conn.ConnectionString = connString;

                using (DbCommand cmd = conn.CreateCommand())
                {
                    cmd.CommandText = sql;

                    using (DbDataAdapter adapter = factory.CreateDataAdapter())
                    {
                        DataTable tbl = new DataTable(); // don't dispose it
                        adapter.Fill(tbl);
                        if (tbl.Rows.Count > 0) result = tbl.CreateDataReader();
                    }
                }
            }

            return result;
        }
    }
}


jQuery parallax plugin I'm working in my project with the following script:

;(function($, doc, win) {
"use strict";

 // cache the window object
 var jQuerywindow = $(window);

 $('section[data-type="background"]').each(function(){

   // declare the variable to affect the defined data-type
   var jQueryscroll = $(this);

    $(window).scroll(function() {

      // HTML5 proves useful for helping with creating JS functions!
      // also, negative value because we're scrolling upwards
      var yPos = -(jQuerywindow.scrollTop() / jQueryscroll.data('speed'));

      // background position
      var coords = '50% '+ yPos + 'px';

      // move the background
      jQueryscroll.css({ backgroundPosition: coords });
    }); // end window scroll
 });  // end section function

})(jQuery, document, window);

The script allows me to have several sections with parallax background effect.  So, I can have one or ten in a page. I'm a bit concern about performance. Can the script be improved?

First of all, your code looks quite good. You've encapsulated the plugin well, and cached all your jQuery instances, which are, in my experience, the most common pitfalls.

One thing I noticed is that you create a scroll event handler for every section[data-type="background"] element. Performance wise, this might not be optimal since the scroll event fires quite often. Instead of creating multiple scroll events, something like the following could be done:

var backgrounds = [];
$('section[data-type="background"]').each(function(){
  backgrounds.push($(this));
}

$(window).scroll(function() {
  for(var i=0; i < backgrounds.length; i++) {
    var $elem = backgrounds[i];
    var yPos = -(jQuerywindow.scrollTop() / $elem.data('speed'));
    var coords = '50% '+ yPos + 'px';
    $elem.css({ backgroundPosition: coords });
  }
});

John Resig wrote about scroll event performance quite some time ago, and it's still a good read (including the comments!). You might want to check that out too, and choose a technique of your liking.

And one more, very personal opinion: jQuerywindow is not a variable name I would choose. If you're determined to prepend every jQuery variable with jQuery, then write jQueryWindow, but I would suggest just using $window. I actually don't know where I got this from, but prepending every jQuery variable (including function parameters) with $ makes for very easily readable code (again, this is only a personal opinion).

Edit: Oh, and you probably should cache the jQueryscroll.data('speed') invocation, so that you don't have to read from the DOM every time a scroll event happens.

Edit2: And it is generally a good idea to do a little profiling when you're having performance problems, as guessing what might be the bottleneck is usually not very effective. Most modern browsers have some kind of JavaScript profiler available.


Probe for open application on a TCP port I need to examine an application port to see if it opens and I need to put the retry count as a parameter. Since I'm new to Node I wanted to get your feedback on it
for improvements.

checkAppPr: function (port) {
        var result = Promise.defer();
        var checkStatus = function (next, result, times) {
            portscanner.checkPortStatus(port, '127.0.0.1', function (error, status) {
                if (error) {
                    result.reject(error);
                } else {
                    if (status === 'open') {
                        console.log("Application status: open");
                        result.resolve();
                    } else {
                        times--;
                        if (times > 0) {
                            setTimeout(function () {
                                next(next, result, times);
                            }, 1000);
                        } else {
                            result.reject("start timeout");
                        }
                    }
                }
            })
        }
        checkStatus(checkStatus, result, 20);
        return result.promise;
    }

First, I'd suggest to improve on naming. checkAppPr doesn't really tell me anything about what this function does. Naming it to checkAppPort would make more sense.

Next, you've got some hard-coded literals in your code like 127.0.0.1, 20, 1000. Without some sensible names, they don't mean anything. Put them in constants along with the convention of using capital letters and underscores as names.

Your code is an example of one function trying to jam too much functionality. You can split this into two functions, one that checks for any port and resolves/rejects, and another function that repeatedly calls the first one. You've done it already, except the function is nested in the other.

Here's my take on your code. Longer, but splits out responsibilities.

// Returns a promise that resolves when the port is open
checkPortStatus: function(port, host){
  return new Promise((resolve, reject) => {
    portscanner.checkPortStatus(port, host, function(error, status) {
      if(error)
        reject(error);
      else if(status === 'open')
        resolve(status);
      else
        reject(new Error('Port is not open'));
    });
  });
},

// Your API function
checkAppPort: function(port, retriesLeft) {

  const TIME_BETWEEN_CHECKS = 1000;
  const HOST = '127.0.0.1';
  const RETRIES = 20;

  // Setting a default to retriesLeft
  retriesLeft = retriesLeft === void 0 ? RETRIES : retriesLeft;

  if(!port) throw new Error('Port is required');
  if(retriesLeft === 0) return Promise.reject('Timed Out');

  return new Promise((resolve, reject) => {

    // We call our checker. When it resolves, it calls this promise's resolve.
    // If it rejects, we do added work.
    this.checkPortStatus(port, host).then(resolve, error => {
      setTimeout(() => {

        // Call this function again, with one less retry. However, we hook our
        // resolve and reject to the promise of the new call effectively making
        // a chain when it keeps failing.
        this.checkAppPort(port, retriesLeft - 1).then(resolve, reject);

      }, TIME_BETWEEN_CHECKS);
    });
  });
}


Method to remove outliers from a set of values I'm removing outliers from a set of data using the method described here.

The method removes the outliers and returns the quartile values of the set after all outliers have been removed.

(Quartiles is a class I wrote with properties for Q1, Q2, Q3, and the Inter-quartile range, IQR. The .Quartiles() returns these values for a sequence of decimals.)

public Quartiles RemoveOutliers(ICollection<MyObject> objects)
{
    if (objects.Count == 0)
    {
        return new Quartiles();
    }

    // Calculate Mean value of the set.
    decimal meanValue = objects.Average(o => o.MyProperty);

    // Find the Object whose value is farthest from the Mean.
    MyObject objectFarthestFromMean = objects.OrderByDescending(o => Math.Abs(o.MyProperty - meanValue)).First();
    Quartiles quartiles = objects.Quartiles(o => o.MyProperty);

    // Remove Object if its value is more than 1.5*IQR from the Mean.
    decimal minValue = meanValue - (1.5m * quartiles.IQR);
    decimal maxValue = meanValue + (1.5m * quartiles.IQR);

    if ((objectFarthestFromMean.MyProperty < minValue) || (objectFarthestFromMean.MyProperty > maxValue))
    {
        objects.Remove(objectFarthestFromMean);

        return RemoveOutliers(objects);
    }

    // No outlier found, we're finished.
    return quartiles;
}

Looking for any ways to improve or fix any pitfalls I haven't encountered in testing.

You used Math.Abs(o.MyProperty - meanValue) to check both upper and lower cases.

But then you do specify minValue and maxValue separately. You could apply the same logic here:

decimal maxRange = (1.5m * quartiles.IQR);

if (Math.Abs(objectFarthestFromMean.MyProperty - meanValue) > maxRange)
{

Other than that, the calculation logic seems about as efficient as it can be (keeping readability in mind).


Evaluate whether string is in alphabetical order or reverse alphabetical order Purpose

This problem comes from this question on the dailyprogrammer subreddit.

Basically, the task is to write an implementation that identifies if the characters in a String are in

Alphabetical order (A to Z)
Reverse Alphabetical order (Z to A)
Neither Alphabetical nor Reverse Alphabetical order ("abba")
Both Alphabetical and Reverse Alphabetical order (only one distinct character in String)

Implementation

public class StringCharactersInAlphabeticalOrderEvaluatorImpl implements StringCharactersInAlphabeticalOrderEvaluator {
  private final List<Character> charactersFromAtoZ;

  public StringCharactersInAlphabeticalOrderEvaluatorImpl(final List<Character> charactersFromAtoZ) {
    this.charactersFromAtoZ = charactersFromAtoZ;
  }

  @Override public AlphabeticalOrderState evaluateAlphabeticalOrderOfCharacters(final String candidate) {
    final char[] stringChars = candidate.toUpperCase().toCharArray();
    int index = 0;
    boolean fromAtoZ = true;
    boolean fromZtoA = true;
    while (index < stringChars.length - 1) {
      if (charactersFromAtoZ.indexOf(stringChars[index]) > charactersFromAtoZ.indexOf(stringChars[index + 1])) {
        fromAtoZ = false;
      }

      if (charactersFromAtoZ.indexOf(stringChars[index]) < charactersFromAtoZ.indexOf(stringChars[index + 1])) {
        fromZtoA = false;
      }

      index++;
    }

    if (!fromAtoZ && !fromZtoA) {
      return AlphabeticalOrderState.IN_NEITHER_ALPHABETICAL_NOR_REVERSE_ALPHABETICAL_ORDER;
    }

    if (fromAtoZ && fromZtoA) {
      return AlphabeticalOrderState.IN_BOTH_ALPHABETICAL_AND_REVERSE_ALPHABETICAL_ORDER;
    }

    if (fromAtoZ) {
      return AlphabeticalOrderState.IN_ALPHABETICAL_ORDER;
    }

    if (fromZtoA) {
      return AlphabeticalOrderState.IN_REVERSE_ALPHABETICAL_ORDER;
    }

    throw new RuntimeException("unexpected character string");

  }
}

Feedback

I'd like feedback on my implementation, specifically

Am I missing any corner cases? Am I over-specifying my cases (for example, the BothAlphabeticalAndReverseAlphabetical case)?
Is this the most efficient way of implementing a solution? Is there a better implementation technique (particularly when it comes to my if statements, which I'm not in love with).

I think that your solution is overengineered.  I don't think that there is much reason to define a StringCharactersInAlphabeticalOrderEvaluator interface, containing one evaluateAlphabeticalOrderOfCharacters(String) method.  Do you plan to write an implementation other than StringCharactersInAlphabeticalOrderEvaluatorImpl?  Is there even any reason to have an object at all?

To be fair, different languages have different collation rules.  For example, in Spanish, the letter ñ follows n, and, until the rules were simplified in 1994, the digraph ch was treated as a letter between c and d, and ll was treated as a letter between l and m.  However, if we were going to get fancy and generalize, I would recommend doing it "properly", using a Collator.

This exercise, though, is marked as "easy".  It looks like "ASCIIbetical" order is good enough: we don't have to consider case folding, digraphs, diacritics, Unicode surrogate pairs, or locale-specific rules.  For that, we can just compare the code points using char subtraction or Character.compare(char, char).

You have implemented evaluateAlphabeticalOrderOfCharacters() as a monolithic function, which violates the Single Responsibility principle.  There isn't much advantage to having it do two kinds of ordering checks and classify the result into four categories.  You would be better off exposing two independent checks.  In fact, by splitting it up, you might save a few loop iterations, since each loop could exit early as soon as it detected an out-of-sequence character.

Suggested solution

The principal functions in this class are isAlphabeticalOrder() and isReverseAlphabeticalOrder().  They both delegate to a isMonotonic() helper function to avoid code duplication.  There is a classifyOrder() helper to produce the result strings to be printed in main().

I've used CharSequence rather than String, as a hint that the string is to be interpreted more as a sequence of characters.  I think that a List<Character> would be overkill.

import java.util.Scanner;

public class AlphabeticalOrderChecker {
    private static int ASCENDING = +1,
                       DESCENDING = -1;

    private static boolean isMonotonic(CharSequence cs, int trend) {
        for (int i = 1; i < cs.length(); i++) {
            if (trend * Character.compare(cs.charAt(i - 1), cs.charAt(i)) > 0) {
                return false;
            }
        }
        return true;
    }

    public static boolean isAlphabeticalOrder(CharSequence cs) {
        return isMonotonic(cs, ASCENDING);
    }

    public static boolean isReverseAlphabeticalOrder(CharSequence cs) {
        return isMonotonic(cs, DESCENDING);
    }

    private static String classifyOrder(CharSequence cs) {
        boolean asc = isAlphabeticalOrder(cs);
        boolean desc = isReverseAlphabeticalOrder(cs);
        return (asc && desc) ? "IN ORDER AND REVERSE ORDER" :
               (asc)         ? "IN ORDER" :
                      (desc) ? "REVERSE ORDER" :
                               "NOT IN ORDER";
    }

    public static void main(String[] args) {
        Scanner sc = new Scanner(System.in);
        while (sc.hasNextLine()) {
            String s = sc.nextLine();
            System.out.printf("%s %s\n", s, classifyOrder(s));
        }
    }
}


AppEngine get/post on webapp2.RequestHandler I was receiving 405 method not supported errors on get requests to my save controller, so I created a get method which just calls the existing post one as follows:

class Save(webapp2.RequestHandler):

    def get(self):
        self.post()

    def post(self):
        #...

This seems to be working, but is there a more Pythonic way to define both request handlers at once?

Yes, you can easily alias one method to another:

class Save(webapp2.RequestHandler):

    def post(self):
        #...

    get = post

But although I don't know anything about your application, you shouldn't alias GET to POST like this: the two methods have different semantics. GET is a safe method — it must only retrieve data and not update or store it — while POST is an unsafe method — it requests the server to store or update the data in the request. See "Safe methods" in Wikipedia.


Find The Parity Outlier I am trying to solve this coding exercise:

You are given an array (which will have a length of at least 3, but could be very large) containing integers. The array is either entirely comprised of odd integers or entirely comprised of even integers except for a single integer N. Write a method that takes the array as an argument and returns N.
For example:
[2, 4, 0, 100, 4, 11, 2602, 36]
Should return: 11
[160, 3, 1719, 19, 11, 13, -21]
Should return: 160

Here is my code:
import java.util.ArrayList;
public class FindOutLier {
    public static int search(ArrayList<Integer> lists, int num) {
        int count = 0;
        int index;
        Integer a[] = new Integer[lists.size()];
        a = lists.toArray(a);
        for (index = 0; index < a.length; index++) {
            if (num == a[index]) {
                count++;
            }
        }
        return count;
    }

    public static boolean checkEvenOrOdd(int[] integers) {
        int t = 0;
        double result = 0.0;
        ArrayList<Integer> l = new ArrayList<Integer>(integers.length);
        while (t < integers.length) {
            result = integers[t] % 2;
            if (result == 0.0) {
                l.add(1);
                t++;
            } else {
                l.add(0);
                t++;
            }
        }
        int counter = search(l, 1);
        if (counter == 1)
            return true;
        else
            return false;
    }

    public static int find(int[] integers) {
        int t = 0;
        double newresult = 0.0;
        boolean result = checkEvenOrOdd(integers);
        if (result == false) //
        {
            while (t < integers.length) {
                newresult = integers[t] % 2;
                if (newresult != 0.0) {
                    break;
                } else {
                    t++;
                }
            }
        } else {
            while (t < integers.length) {
                newresult = integers[t] % 2;
                if (newresult == 0.0) {
                    break;
                } else {
                    t++;
                }
            }
        }
        return integers[t--];
    }
}

It passed Junit Test in Eclipse. Please review it/ suggest any improvements.
Thanks.

Your approach is far too complicated.

In checkEvenOrOdd() you create a ArrayList<Integer> containing
zeros or ones.
In search() you create an Integer a[] from that list.
Then you iterate over the array and count the number of zeros or ones.

You can iterate over int[] integers directly to find the number of odd (or even) entries:

    int oddCount = 0;
    for (int n : integers) {
        if (n % 2 != 0) {
            oddCount += 1;
        }
    }

Note how a "enhanced for statement" can be used to iterate over
the array elements, instead of a for-loop or a while-statement for
the array indices, as in your code.

There is also no need to use a double result or floating point literals
when doing an integer remainder calculation.

    if (counter == 1)
        return true;
    else
        return false;

can be simplified to

    return (counter == 1)

But in this case the entire logic can be put into a single function,
e.g. like this:

public static int find(int[] integers) {

    int oddCount = 0;
    for (int n : integers) {
        if (n % 2 != 0) {
            oddCount += 1;
        }
    }

    if (oddCount == 1) {
        for (int n : integers) {
            if (n % 2 != 0) {
                return n;
            }
        }
    } else {
        for (int n : integers) {
            if (n % 2 == 0) {
                return n;
            }
        }
    }
    return 0;
}

Then one could try to avoid the code repetition for the even/odd
case.

But actually I would suggest a completely different, more efficient  approach:

Check the first two array elements if they are even or odd.
If they are both even, the outlier can be found
by search for the first odd element, starting at index 2.
If they are both odd, the outlier must be even, and can be found
similarly.
If the first two array elements have different parity, one of
them is the outlier. Which one, can be determined by checking the third
array element.


Using std::future to manipulate objects in C++11 I'm experimenting with std::future, but haven't found any materials that deal with the usecase I have in mind and was hoping someone could either confirm or deny the validity of the approach excerpted below:

class Thing {
private:

    std::string mValue;

public:
    Thing() : mValue( "" ) {}

    void set(const std::string& s)
    {
        mValue = s;
        reverse( mValue.begin(), mValue.end() );
    }

    const std::string& getValue() const
    {
        return mValue;
    }
};

int main(int argc, const char * argv[])
{
    Thing* tA = new Thing();
    Thing* tB = new Thing();
    Thing* tC = new Thing();

    auto tLambda = [](const string& iStr, Thing* oThing) -> Thing* {
        oThing->set( iStr );
        return oThing;
    };

    future<Thing*> tA1 = async( launch::async, tLambda, "This is a test...A", tA );
    future<Thing*> tB1 = async( launch::async, tLambda, "This is a test...B", tB );
    future<Thing*> tC1 = async( launch::async, tLambda, "This is a test...C", tC );

    future<Thing*> tA2 = async( launch::async, tLambda, tA1.get()->getValue(), tA );
    future<Thing*> tB2 = async( launch::async, tLambda, tB1.get()->getValue(), tB );
    future<Thing*> tC2 = async( launch::async, tLambda, tC1.get()->getValue(), tC );

    cout << tA2.get()->getValue() << endl;
    cout << tB2.get()->getValue() << endl;
    cout << tC2.get()->getValue() << endl;

    return 0;
}

In essence, I'd like to use std::future to update a pre-existing object's member variables rather than initializing and returning a new object (as is shown in most std::future examples I've seen). In the example above, the model seems a little suspect. I would love to hear any thoughts on the best way to achieve this sort of functionality. Ultimately, I would like to embed this in a thread-pool sort of context.

(Note: On another user's suggestion, this is reposted from Stack Overflow).

There are two things that concern me in the code you offer:

The Thing instances pointed to by tA, tB, tC are leaked. In this case it looks like it would be trivial to address this by stack allocating them, and passing their address to the async calls: Thing tA; async( ... , &tA);
The cross thread synchronization dependencies are not explicitly addressed. Instead it's all implicit by the way the code is written. Any mistake in the code to lead to data races. For example if in creating tB2 it called the wrong get() or pass the wrong Thing*, not only would it be the wrong value, but it might race.

I spent some time trying to figure out if proposed functionality such as Resumable Functions (pdf) would allow this to be nicer, and while I expect resumable functions would help in more general code, in your code I think a bigger problem is the factoring. Namely, since nothing uses the middle state, the code passed to the future should encompass both operations.

auto tLambdaBoth = [](const string& iStr, Thing* oThing) -> Thing* {
    auto pthing = tLambda(iStr, oThing);
    return tLambda(pthing->getValue(), oThing);
}

And writing this showed me a new angle on my second misgiving. The original code mixed uses of the original pointer and the pointer returned by tLambda. We know that they currently are the same pointer, but assuming that could restrict your options later. In tLambdaBoth I'd rather see return tLambda(pthing->getValue(), pthing); so why not the same in the original code?

So we can address both points together by avoiding new, and then never reusing the original object; instead it always uses the pointer that's returned from the future:

Thing tA, tB, tC;

// launch futures
std::vector<std::future<Thing*>> fut1;
fut1.emplace_back(async(launch::async, tLambda, "This is a test...A", &tA));
fut1.emplace_back(async(launch::async, tLambda, "This is a test...B", &tB));
fut1.emplace_back(async(launch::async, tLambda, "This is a test...C", &tC));

// launch second set; using values clearly no longer in flight
std::vector<std::future<Thing*>> fut2;
for (auto& fut : fut1)
{
    auto pthing = fut.get();
    fut2.emplace_back(async(launch::async, tLambda, pthing->getValue(), pthing));
}

// report results
for (auto& fut : fut2)
{
    cout << fut.get()->getValue() << endl;
}

While I definitely like this a lot better, I'm not sure I understand your use case well enough to say whether this addresses it. Perhaps you should digest what I'm saying here, and then clarify your use case if this did not address what you were trying to determine about the validity of your approach.


Template with boost::operators extremely verbose and repeating I got the following ugly code:

template < class _Coeff,
       unsigned _nVars,
       typename _Expo=int,
       template <class, class> class _Map = _Map >
class Polynomial
      : boost::ring_operators< Polynomial<_Coeff,_nVars,_Expo,_Map>
      , boost::addable       < Polynomial<_Coeff,_nVars,_Expo,_Map>, _Coeff
      , boost::addable       < Polynomial<_Coeff,_nVars,_Expo,_Map>,
                   std::pair< std::array<_Expo,_nVars>, _Coeff >
      , boost::subtractable  < Polynomial<_Coeff,_nVars,_Expo,_Map>, _Coeff
      , boost::subtractable  < Polynomial<_Coeff,_nVars,_Expo,_Map>,
                   std::pair< std::array<_Expo,_nVars>, _Coeff >
      , boost::multipliable  < Polynomial<_Coeff,_nVars,_Expo,_Map>, _Coeff
      , boost::multipliable  < Polynomial<_Coeff,_nVars,_Expo,_Map>,
                   std::pair< std::array<_Expo,_nVars>,_Coeff >
                > > > > > > > {

In particular, I hate to repeat Polynomial<_Coeff,_nVars,_Expo,_Map>. Is there a way to improve this syntax without using the preprocessor ? I can't manage to get any help from the using keyword here.

Just for the infos: I'm writing multivariate polynomials where _Coeff is the class of the coefficients and the terms X1^4x2^4 are represented as std::array<_Expo,_nVars>.

Easy, with default template arguments:

template < class _Coeff,
       unsigned _nVars,
       typename _Expo=int,
       template <class, class> class _Map = _Map
       typename Poly = Polynomial<_Coeff,_nVars,_Expo,_Map>,
       typename Pair = std::pair< std::array<_Expo,_nVars>, _Coeff > >
class Polynomial
      : boost::ring_operators< Poly
      , boost::addable       < Poly, _Coeff
      , boost::addable       < Poly, Pair
      , boost::subtractable  < Poly, _Coeff
      , boost::subtractable  < Poly, Pair
      , boost::multipliable  < Poly, _Coeff
      , boost::multipliable  < Poly, Pair
                > > > > > > > {


Max and Min in array using minimum comparisons Is this the most robust and fastest way for finding the min and max out of an array without using STL functions? How can I improve it?

#include <stdio.h>
#include <stdlib.h>
int main(int argc, char const *argv[]) {
system("clear");
int c=9,n[10],max,min,i;
while (c>=0) {
n[c]=random()%100;
printf("%d\n",n[c]);
c--;}c=10;
max=n[0];min=n[1];
for ( i =0; i<c; i++) {
  if(max<n[i])max=n[i];
  else if(min>n[i])min=n[i];
}
printf("\n%d\n%d\n",max,min);
return 0;
}

First of all, format your code. Indentation goes along way. If everything is left-justified, you can't make out any of the logic.

Secondly, this is a good idea:

max=n[0];min=n[1];

But we should really initialize both to n[0], and on separate lines:

max = n[0];
min = n[0];

The reason is this allows us to start at index 1:

for (i=1; i<c; ++i) {
    if (n[i] > max) {
        max = n[i];
    }
    else if (n[i] < min) {
        min = n[i];
    }
}

We could also consider putting this into a function:

void minmax(int* arr, int len, int* min, int* max) { ... }

minmax(n, c, &min, &max);

Any time you're doing something k times for some constant, or filling in an array - do it front to back. It will be much easier to understand. Don't fill n from index 9 to 0... fill it from 0 to 9. Also, prefer a for loop to a while loop and use i both times:

int c=10, n[10];
for (i=0; i<c; ++i) {
}

And lastly, c is a bad name for the max size of your array. Something like SIZE or LENGTH or anything to that effect would be better.


Casting calls with the same parent class I'm using Telegram API and I've different methods which returns different type of information like this:
public SendMessage getTextMessage(Long chatId, String text) {
        return new SendMessage()
                .enableMarkdown(false)
                .setChatId(chatId)
                .setText(text);
    }

public AnswerCallbackQuery getPopUpAnswer(String callbackId, String text) {
        return new AnswerCallbackQuery()
                .setCallbackQueryId(callbackId)
                .setText(text)
                .setShowAlert(true);
    }

public EditMessageText getEditedMessage(Long chatId, Integer messageId, String text){
        return new EditMessageText()
                .setChatId(chatId)
                .setMessageId(messageId)
                .setText(text);
    }

public SendPhoto getPhotoMessage(Long chatId, GooglePlayGame game) {
        return new SendPhoto().setChatId(chatId)
                .setPhoto(game.getPictureURL())
                .setCaption(game.toString());
    }

They have one parent PartialBotApiMethod.
In main method I handle and execute (send) them:
PartialBotApiMethod<?> responseToUser = updateReceiver.handleUpdate(update);
try {
     execute(responseToUser); //error
    }
catch (TelegramApiException e){
     e.printStackTrace();
    }

But I can't execute PartialBotApiMethod object, only BotApiMethod and their subs. SendPhoto is not in one hierarchy with others, that's why I can't put SendPhoto result to BotApiMethod type variable.
And I can't cast like this:
try {
     execute((BotApiMethod)responseToUser);
    }
catch (TelegramApiException e){
     e.printStackTrace();
    }

Because SendPhoto doesn't extends BotApiMethod and I'll give a ClassCastEx.
So I gonna use instanceof and cast "personally":
PartialBotApiMethod<?> responseToUser = updateReceiver.handleUpdate(update);
        if (responseToUser instanceof SendMessage) {
            try {
                execute((SendMessage)responseToUser);
            } catch (TelegramApiException e) {
                e.printStackTrace();
            }
        }
        else if (responseToUser instanceof SendPhoto) {
            try {
                execute((SendPhoto) responseToUser);
            } catch (TelegramApiException e) {
                e.printStackTrace();
            }
        }
        else if (responseToUser instanceof AnswerCallbackQuery) {
            try {
                execute((AnswerCallbackQuery)responseToUser);
            } catch (TelegramApiException e) {
                e.printStackTrace();
            }
        }
        else if (responseToUser instanceof EditMessageText) {
            try {
                execute((EditMessageText)responseToUser);
            } catch (TelegramApiException e) {
                e.printStackTrace();
            }
        }

Do I've better options or just stay with instanceof?
Can I replace instanceof if I cannot use a common interface (third pary API)?
UPDATE
updateReceiver.handleUpdate()
public PartialBotApiMethod handleUpdate(Update update) {
   if (updateHasTextMessage(update)){
       try {
           return processInputMessage(update.getMessage());
       } catch (NotSupportedChatCommandException e) {
           return replyMessageService.getTextMessage(update.getMessage().getChatId(), "Not available");
       }
   }
   else if (updateHasCallbackQuery(update)) {
       return callbackQueryHandler.handleCallbackQuery(update.getCallbackQuery());
   }
   else {
       return replyMessageService.getTextMessage(update.getMessage().getChatId(),
                    "Can't handle");
        }
}

I'm not sure if there is enough context for me to fully understand the restrictions you are wporking with but I will try anyway... 25 years ago an elder university student half-jokingly taught me that the solution to any problem is always to "add another layer of indirection." In this case, split handleUpdate so that each message type has their own handler object and place the type specific request handling into those classes. Furthermore, separate each type specific response handler into their own class (to maintain single responsibility principle) and inject that dependency to each update handler.
I.e. move type specific handling to the module where the type is still known. The function of the instanceof-block you want to get rid of will now effectively be moved to the if-block in the handleUpdate method and you don't need to repeat it again in the response handling code.


Pairwise adding tuples in a list I have two list of tuples:

 [[1,8],[2,7],[3,14]]
 [[1,5],[2,10],[3,14]]

and a desired sum 20. I need to find two tuples from two list whose second element either add it to the sum i.e 20 or the next lowest sum. In this case if we consider [3,14] and [1,5] the sum is 14+5=19 hence the output should be [3,1]

[1,3000],[2,5000],[3,7000],[4,10000]
[1,2000],[2,3000],[3,4000],[4,5000]]

the sum is 10000. Here we have [2,5000], [4,5000] and [3,7000], [2,3000] so the output should be [2,4] and [3,2]

[[1,2000],[2,4000],[3,6000]]
[[1,2000]]

the sum is 7000. Here since I don't have a combination that sum up to 7000 I consider all the possible combinations 4000(2000+2000), 6000(4000+2000) and 8000(6000+2000) and consider the next lowest number from the desired sum which is 600. For 6000 my output should be [2,4000] and [1,2000] which is [2,1].

import itertools

def optimalUtilization(maximumOperatingTravelDistance,
                       forwardShippingRouteList, returnShippingRouteList):

    result=[]
    t1=[]
    t2=[]
    for miles in forwardShippingRouteList:
        t1.append(miles[1])

    for miles in returnShippingRouteList:
        t2.append(miles[1])

    result.append(t1)
    result.append(t2)
    total_sum=set()

    for element in list(itertools.product(*result)):
        if sum(element)<=maximumOperatingTravelDistance:
            total_sum.add(sum(element))

    total_sum=sorted(total_sum,reverse=True)
    return optimalUtilizationhelper(total_sum[0],
                       forwardShippingRouteList, returnShippingRouteList)

def optimalUtilizationhelper(maximumOperatingTravelDistance,
                       forwardShippingRouteList, returnShippingRouteList):

    dist_dict={}
    for carid,miles in forwardShippingRouteList:
        dist_dict.update({miles:carid})

    result=[]

    for carid,miles in returnShippingRouteList:
        if (maximumOperatingTravelDistance-miles) in dist_dict:
            result.append(list((dist_dict[maximumOperatingTravelDistance-miles],carid)))

    return result

I get the desired result here but the complexity here is \$O(n^2 \log n)\$. What is a better way to do this?

Review

No need for all these intermediate temp variables

You loop over a and b to append to the result which is unnecessary.

Only afterwards you create the products but this can be done before.
The sort is not needed if you keep a running minimum
Use a if __name__ == '__main__'
Consider adding Docstring and Testcases, (or both in the form of doctests) to make the solution more understandable/testable
Read PEP8 the python style guide

Functions and variables should be snake_case for instance

Revised Code

def optimal_shipping(a, b, target):
    """
    Finds the optimal shipping by compairing pairwise lists to the target sum

    >>> optimal_shipping([[1,2000],[2,4000],[3,6000]], [[1,2000]], 6000)
    [[2, 1]]
    >>> optimal_shipping([[1,8],[2,7],[3,14]], [[1,5],[2,10],[3,14]], 20)
    [[2, 3]]
    >>> optimal_shipping([[1,3000],[2,5000],[3,7000],[4,10000]], [[1,2000],[2,3000],[3,4000],[4,5000]], 10000)
    [[2, 4], [3, 2]]
    """
    best = 0
    for i, x in a:
        for j, y in b:
            if abs(x + y - target) < abs(target - best):
                best = x + y
                res = [[i, j]]
            elif x + y == best:
                res += [[i, j]]
    return res

if __name__ == '__main__':
    import doctest
    doctest.testmod()

Note
If those lists prices are in ascending order, maybe something with a bisect search could work.


Bubble sort implementation in C++ I was wondering if this implementation of bubble sort could be improved. Are there any things that I have done wrong?

template<typename Element>
void bubble_sort(Element arr[], size_t size) {
    auto sorted_elements = 1u;
    bool sorted = false;
    while (!sorted) {
        sorted = true;
        for (auto index = 0u; index < size - sorted_elements; ++index) {
            if (arr[index] > arr[index + 1]) {
                auto temp = arr[index + 1];
                arr[index + 1] = arr[index];
                arr[index] = temp;
                sorted = false;
            }
        }
        ++sorted_elements;
    }
}

Implementation

size - sorted_elements might not return what you expect if size == 0 (it returns std::numeric_limits<unsigned int>::max()). This makes the intended check fail for size == 0. This can be fixed by changing the while loops condition to sorted_elements < size && !sorted.
You could use std::swap(arr[index], arr[index + 1]); to swap the elements. This way, the preferred assignment operation (copy or move) for Element will be used (if Element provides its own specialization). Currently, you always copy, which might not always be preferred (e.g. with large std::strings).


Filters for a JSON collection I am trying to create a class that parses a JSON and selects all labels, or just a particular label that you prefer to later do a calculation with the value pair extracted off that label. This is working well for .all() This is the first function, wich selects ALL available labels. But then I want to make it flexible and be able to call names = IncomeSources(result).spa() to see the revenue results for the spa only. The code is working fine, but I will die if I have to write a function for each label separately. Is there more efficient way to do it ?

class IncomeSources(object):

    def __init__(self, result):
        self.result = result

    def all(self):
        labels_list = []
        for index in self.result:
            for key, value in index.items():
                if 'accountLabelType' in key:
                    names = value
                    labels_list.append(names)
                    collection = set(labels_list)

        return collection

    def brickyard(self):
        name = 'Brickyard'
        labels_list = []
        for index in self.result:
            for key, value in index.items():
                if 'accountLabelType' in key and value.startswith('BYD'):
                    names = value
                    labels_list.append(names)
                    collection = set(labels_list)
        return collection

    def schoolhouse(self):
        name = 'Schoolhouse'
        labels_list = []
        for index in self.result:
            for key, value in index.items():
                if 'accountLabelType' in key and value.startswith('SCH'):
                    names = value
                    labels_list.append(names)
                    collection = set(labels_list)
        return collection

    def spa(self):
        name = 'Spa'
        labels_list = []
        for index in self.result:
            for key, value in index.items():
                if 'accountLabelType' in key and value.startswith('Spa'):
                    names = value
                    labels_list.append(names)
                    collection = set(labels_list)
        return collection

Duplicated code

In order to remove duplicated code, the key is to identify the parts that are identical/different from one place to another. In your case, you could add a filter parameter to your all function (which would then deserve to be named differently).

You could write something like :

def get_result(self, beginFilter=None):
    labels_list = []
    for index in self.result:
        for key, value in index.items():
            if 'accountLabelType' in key and (beginFilter is None or value.startswith(beginFilter)):
                names = value
                labels_list.append(names)
                collection = set(labels_list)
    return collection

def brickyard(self):
    return self.get_result('BYD')

def schoolhouse(self):
    return self.get_result('SCH')

def spa(self):
    return self.get_result('Spa')

As a side-note, I had not given enough thought before writing the code above but if I had to re-do it from the beginning, I'd choose '' as a default value because it wouldn't need a special handling (all strings start with '' as far as I can tell).

Simplifying code

All your name variables were useless. Your names variable is not really useful neither. Also, labels_list could be named labels.

Finally, your conversion from list of labels to set of labels could be done only once at the very end of the function.

def get_result(self, beginFilter=None):
    labels = []
    for index in self.result:
        for key, value in index.items():
            if 'accountLabelType' in key and (beginFilter is None or value.startswith(beginFilter)):
                labels.append(value)
    return set(labels)

Now, it looks clearer that labels could be a set directly instead of converting it afterward.

def get_result(self, beginFilter=None):
    labels = set()
    for index in self.result:
        for key, value in index.items():
            if 'accountLabelType' in key and (beginFilter is None or value.startswith(beginFilter)):
                labels.add(value)
    return labels

You could also write this as a single set comprehension. Set comprehension is a convenient way to define set (you also have list comprehension and dict comprehension and even something called generator expression but this is out of scope at the moment).

It usually goes like this

my_list = [func(x) for x in my_iterable]  # list comprehension
my_set = {func(x) for x in my_iterable}  # set comprehension
my_dict = { my_key(x): my_val(x) for x in my_iterable}  # dict comprehension

In all 3 cases, one could add an optional filtering part so you could write something like:

my_set = {func(x) for x in my_iterable if some_property(x)}

Finally, you can also add more nested iterations. In your case, we could write (but some people would consider that this is going too far):

def get_result(self, beginFilter=None):
    return { value
        for index in self.result
        for key, value in index.items()
        if 'accountLabelType' in key and (beginFilter is None or value.startswith(beginFilter))}

Back to where we came from

Now that I understand better the point of your code, we can try to clean up the get_result function. The filtering can actually be moved out of the function.

def all(self):
    return { value
        for index in self.result
        for key, value in index.items()
        if 'accountLabelType' in key}

def brickyard(self):
    return {v for v in self.all() if v.startswith('BYD')}

def schoolhouse(self):
    return {v for v in self.all() if v.startswith('SCH')}

def spa(self):
    return {v for v in self.all() if v.startswith('Spa')}

or with a helper function :

def all(self):
    return { value
        for index in self.result
        for key, value in index.items()
        if 'accountLabelType' in key}

def filter(self, begin):
    return {v for v in self.all() if v.startwith(begin)}

def brickyard(self):
    return self.filter('BYD')

def schoolhouse(self):
    return self.filter('SCH')

def spa(self):
    return self.filter('Spa')

More

In order not to call all multiple times, one could imagine saving the result.


Trying multiple regexes against a single string I have a huge list of regexes (>1,000 but <1,000,000) that I want to test against (many) single strings.

It is unlikely and unintended that more than one such expression would match a single string. I could just maintain a big list of each compiled, individual regex and iterate over that for every input string. However I have it in my head that I should be handing the problem over to the regex compiler to simplify the common substrings since it can (at least theoretically) produce a very neat single DFA.

import re
import uuid

class multiregex(object):
    def __init__(self,rules):
        merge = []
        self._messages = {}
        for regex,text in rules:
            name = "g"+str(uuid.uuid4()).replace('-','')
            merge += ["(?P<%s>%s)" % (name,regex)]
            self._messages[name] = text

        self._re=re.compile('|'.join(merge))

    def __call__(self,s):
        result = self._re.search(s)
        if result:
            groups = result.groupdict()
            return ((self._messages[x], groups[x]) for x in groups.keys() if groups[x]).next()

rules = [("foobar", "Hit a foobar"),
         ("f.*b.*r", "fbr"),
         ("foob.z", "Frobination"),
         ("baz", "Hit a baz"),
         ("b(ingo)?", "b with optional ingo")]

m=multiregex(rules)
tests=["foobar", "foobaz", "foobazr", "b", "bingo"]
for text,hit in (m(x) for x in tests):
    print "Message: '%s' (because of '%s')" % (text,hit)

The code above works, but am I have a few outstanding issues with it:

Is it needlessly over complicating the whole thing? Or is it pushing the problem off to code that's heavily researched and optimised.
Is there a neater way of finding just the named capture group that matched than what I've done with groupdict()?
Are there any more gotchas than the obvious one of two 'rules' each containing the same group name? e.g.:

rules = [("(?P<hello>foobar)", "Hit a foobar"),
         ("(?P<hello>foob.z)", "Frobination")]

(The issue of a single syntax error in a single 'rule' killing the whole thing is easy enough to workaround by validating the inputs at rule creation time)

I think it's a neat idea, because you're indeed using well-tested code, which reduces the chance of errors.
Looking at the re API, you do need to retrieve all possibles matches using groupdict().
Even that one is not a gotcha since you're naming your groups yourself. Right? I can't think of anything else.

Other comments:

You have a small bug in your last loop where text and hit need to be exchanged.
There's no easy to way to be sure that the DFA version will be faster than the normal version. Since you're not producing a factorized DFA but a sum of DFAs, naive code could be as slow as testing each regex one by one. Of course it's possible that it's much faster, but measure it if this is what you want to achieve!
You don't need uuids, a simple counter would be enough. Eg ?P<g1> instead of ?P<fd897214dd9d4dd28f591a412ef5d3ea>.


Optimize nested loops for counting the total number of selections The question is: To select 2 numbers out of n positive integer inputs (order matters), where the number that is placed anywhere before the second must be greater than the second by a given number m. Count the total number of selections.
An example answer would be, given the inputs 5 1 3 2 4 and m is 0, the total number of selections is 5 (51,53,52,54,32)
Another example answer would be, given the inputs 6 5 4 3 2 1 and m is 2, the total number of selections is 6 (63,62,61,52,51,41)
How do I improve my double for-loop method in terms of time?
using namespace std;

int main()
{
    int n, m, i, j, m = 0, sum = 0;
    cin >> n >> m;
    int *a=new int[n];
    for (i = 0; i < n; i++) {
        scanf("%d", &a[i]);
    }

    for(i=0;i<n-1;i++)
    {
        if (a[i] - m <= 0)  continue;
        int temp=a[i]-m;
        for(j=i+1;j<n;j++)
        if(a[j]<temp) sum++;
    }
    cout<<sum<<endl;
    return 0;
}
`

using namespace std;

Don't do this.  We have namespaces for a reason, you know.  And we seem to be missing the necessary Standard Library headers <iostream> and <cstdio>.

    int n, m, i, j, m = 0, sum = 0;

These variable names tell us nothing about what they are used for.  Please use more expressive names.

    cin >> n >> m;

When reading from a stream such as std::cin, it's vital to check that the conversion was successful before using the values.  That could be as simple as
if (!cin) {
    return EXIT_FAILURE;
}

    int *a=new int[n];

Prefer a standard container (e.g. std::vector).

    for (i = 0; i < n; i++) {

If we had a std::vector, we could have avoided hand-coding this loop:
auto a = std::vector{};
a.reserve(n);
std::copy_n(std::istream_iterator<int>(std::cin), n, a.begin());

        scanf("%d", &a[i]);

Why are we mixing C++ streams and C stdio?  It's better to use just one (preferably the streams).  Again, we're missing the test whether the conversion was successful.