How do I remove nested if else conditions? I'm playing around with the concept of functional programming.
I have flattened the nested if statements and made by code more verbose.
The refactored code looks overly complicated.
What could be a simpler approach?
Nested Ifs
public static int Main(string[] args)
{
    int computer = 1;
    string brand = "PC";

    if (computer < 2)
    {
        if (brand != "Apple")
        {
            // perform complex calculation 1
            Console.WriteLine("you don't own a Mac");
            return 0;
        }
        else
        {
            // perform complex calculation 2
            Console.WriteLine("you own less than 2 Macs");
            return 1;
        }
    }
    else
    {
        // perform complex calculation 3
        Console.WriteLine("you own two or pcs");
        return 2;
    }
}

Flattened Main
public static int Main(string[] args)
{
    int computer = 1;
    string brand = "PC";

    return CalculatedResult = DetermineComputer(computer, brand);
}

Extension Methods
public static class extensions
{
    public static int ComplexCalc1(this bool val)
    {
        Console.WriteLine("you don't own a Mac");
        return 0;
    }
    public static int ComplexCalc2(this bool val)
    {
        Console.WriteLine("you own less than 2 Macs");
        return 1;
    }
    public static int ComplexCalc3(this bool val)
    {
        Console.WriteLine("you own greater than two pcs");
        return 2;
    }
    public static int ComplexCalc4(this bool val)
    {
        Console.WriteLine("Condition not accounted for");
        return -1;
    }

    public static bool IsLessThanTwo(int computer) => computer < 2;

    public static bool IsApple(string brand) => brand == "Apple";

    public static (Func<bool, int> func, bool result) IsLessThanTwoAndNotApple(this int computer, string brand)
        => (ComplexCalc1, IsLessThanTwo(computer) && (IsApple(brand) == false));

    public static (Func<bool, int> func, bool result) IsLessThanTwoAndIsApple(this int computer, string brand)
        => (ComplexCalc2, IsLessThanTwo(computer) && IsApple(brand));

    public static (Func<bool, int> func, bool result) Unaccountedfor(this int computer, string brand)
        => (ComplexCalc4, true);

    public static (Func<bool, int> func, bool result) IsGreaterThanTwoAndIsNotApple(this int computer, string brand)
        => (ComplexCalc3, (IsLessThanTwo(computer) == false) && (IsApple(brand) == false));
}

Calculation
public static int DetermineComputer(int computer, string brand)
{
    var methodlist = new List<(Func<bool, int>, bool)>
    {
        computer.IsLessThanTwoAndNotApple(brand),
        computer.IsLessThanTwoAndIsApple(brand),
        computer.IsGreaterThanTwoAndIsNotApple(brand),
        computer.Unaccountedfor(brand)
    };

    var tupleResult = methodlist.Where(x => x.Item2 == true).Select(x =>
        new {
            functor = x.Item1,
            result = x.Item2
        }).First();

    return tupleResult.functor(tupleResult.result);
}

Unit Tests
[DataRow(3, "Apple", -1)]
[DataRow(2, "Apple", -1)]
[DataRow(1, "Apple", 1)]
[DataRow(0, "Apple", 1)]
[DataRow(3, "PC", 2)]
[DataRow(2, "PC", 2)]
[DataRow(1, "PC", 0)]
[DataRow(0, "PC", 0)]
[DataTestMethod]
public void NestedIfs_Demo(int first, string second, int expected)
{
    int computer = first;
    string brand = second;

    var CalculatedResult = DetermineComputer(computer, brand);

    Assert.AreEqual(expected, CalculatedResult);
}

I'm assuming that computer and brand are meant to be function parameters, otherwise any kind of branching would be unnecessary.
Here's something that pretty much does what you want, assuming you just want messages that describe in clear english the quantity and brand of computers, and not necessarily have the exact messages used in your examples:
public static void Message(int quantity, string brand)
{
    Console.WriteLine("You own " + quantity + " " + brand + (quantity !== 1 ? 's' : '');
}

The actual brand and quantities are variables that are inserted into the general message. If you want the message to be longer and use more variables, then you would probably want to create substrings to later be combined, for the sake of readability:
public static void Message(int quantity, string brand)
{
    string brandPlural = brand + (quantity !== 1 ? 's' : '');

    Console.WriteLine("You own " + quantity + " " + brandPlural;
}
```


FizzBuzz functional implementation I was playing around with ES6 arrow functions, currying and partial application and I decided to try a functional implementation of the FizzBuzz problem:

function* range(start, stop) {
  for (var i = start; i < stop; i++)
    yield i
}

var map = fn => res => x => fn(x) ? res : null
var isDividedBy = dividend => divisor => divisor % dividend === 0
var mapDividedBy = x => map(isDividedBy(x))
var mappers = [
  mapDividedBy(3)('Fizz'),
  mapDividedBy(5)('Buzz')
]
var applyMappers = mappers => x => mappers.map(fn => fn(x)).join('') || x

var result = Array.from(range(1, 100)).map(applyMappers(mappers))

console.log(result)

My main focus was to be able to easily add any other case like "print Zazz for numbers lower than 10" by simply adding map(x => x < 10)('Zazz') to the mappers array.

What do you think?

map vs if

var map = fn => res => x => fn(x) ? res : null

I would rename this to something more like ifThen or when -- calling this function map is confusing, to me at least, given the normal usage of map. This is an if where the else is always null.

In a larger program you may find that providing the true/false results as functions instead of constants would be more powerful, but here that would just add noise.

const

Most of your vars could be consts. Doesn't make much difference here but since you're deliberately being functional const would help declare your intent.

without mapDividedBy

I don't think you need this specialized function. Just pass the predicate and true-result into your when/if:

const when = pred => trueRes => x => pred(x) ? trueRes : null
const isDividedBy = dividend => divisor => divisor % dividend === 0
const mappers = [
  when(isDividedBy(3))('Fizz'),
  when(isDividedBy(5))('Buzz'),
]

naming

I'm not sure mappers is the most descriptive name. I can't think of anything better at present, so that's not very helpful, but any (pure) function is just a "mapper" from inputs to output(s). Maybe tests? Hmm.

In any event, since this is a FizzBuzz I suggest providing a final function named as such, for convenience and readability:

const fizzbuzz = applyMappers(mappers)
const result = Array.from(range(1, 100)).map(fizzbuzz)

misc

I like your currying approach.

Clojure has a function juxt which does the same thing as mappers.map(fn => fn(x)) but looks a little cleaner, being a separate function:

const fizzbuzz = x => juxt(mappers)(x).join('') || x

But whether this is more readable or more obscure depends on whether you know Clojure, so I've left this point to the end.


Transforming tuple with (int month, int year) in string "mm/yyyy" I would like to improve the following code:

    result = database.execute(script, fetch="0") # (6, 2016) for example
    month = str(result[0])
    year = str(result[1])
    if len(month) == 1:
        month = "0" + month
    return "%s/%s" % (month, year)

I'm feeling that my code is dumb, and could be more intelligent.

Python version 2.6.

You can use the datetime module:

import datetime as dt
result = (2, 2017)
date = dt.date(year=result[1], month=result[0], day=1)
date_str = date.strftime("%m/%Y")


Array-like data structure for fixed sized types with non-machine-word sizes I am writing an array-like data structure for types other than 8 16 32 64 – the usual type sizes.
Ideally, my interface is the following for addressing the array.
void setindex(uint8_t *array, size_t width, size_t index, uint64_t value);
uint64_t getindex(uint8_t *array, size_t width, size_t index);

This is basically an array of unsigned integers of size width. A uint8_t value would contain 4 elements for width=2, at max.
This should hold no more metadata than that. So in theory, it should work with any blob of allocated memory. Bound-checks should be done by the caller.
I have the following code, packed as a very small header library:
#include <cstdio>
#include <iostream>
#include <bitset>
#include <cassert>

using namespace std;

uint64_t getindex(uint64_t *A, size_t width, size_t index)
{
    uint64_t mask, mask1, mask2, ret, shift;
    uint64_t size, d, m;

    size = sizeof A[0] * 8;
    mask = (1 << width) - 1;
    shift = index * width;

    // Any decent compiler does this in one instruction
    d = (index + 1) * width / size;
    m = (index + 1) * width % size;

    if (!d) {
        ret = (*A & (mask << (shift))) >> shift;
    } else {
        mask1 = (1 << m) - 1;
        mask2 = (1 << (width - m)) - 1;

        ret = (A[d] & mask1) << (width - m) | (A[d - 1] & (mask2 << (size - (width - m)))) >> (size - (width - m));
    }

    return ret;
}

uint64_t setindex(uint64_t *A, size_t width, size_t index, uint64_t value)
{
    uint64_t mask, mask1, mask2, shift;
    uint64_t size, d, m;

    assert(value < (1 << width));

    size = sizeof A[0] * 8;

    mask = (1 << width) - 1;

    shift = index * width;

    // Any decent compiler does this in one instruction
    d = (index + 1) * width / size;
    m = (index + 1) * width % size;

    if (!d) {
        A[0] = (A[0] & ~(mask << (shift))) | (value << shift);

    } else {
        mask1 = (1 << m) - 1;
        mask2 = (1 << (width - m)) - 1;

        A[d] = (A[d] & ~mask1) | (((mask1 << (width - m)) & value) >> (width - m));
        A[d - 1] = A[d - 1] & ~(mask2 << size - m) | ((mask2 & value) << (size - (width - m)));
    }

    return value;
}

I come from C, so the code may be very C-like, as I don't fully know most of the C++ features well.
Can this be simplified and made more robust? The above code may have problems with bit shifting and undefined behavior. I have the feeling that this problem is very well suited for fors and divmods algorithms, like those used to construct gcd. But in my implementation, I did not manage to do that. Are there existing libraries I can better use?

using namespace std; is a bad practice.

The code is plain C. You have two options:

Admit this fact, change your #includes to the C-style, and declare your functions as extern "C". This way they are callable from both C and C++ code.

Make a class, and overload operator[](std::size_t) and operator[](std::size_t) const. The width shall be a class member. Much more C++ish.

An A parameter to getindex should be const-qualified.

Declare variables as close to use as possible. E.g. instead of
      uint64_t mask1;
      ....
      if () {
          ....
      } else {
          mask1 = ....;
      }

do
      ....
      if () {
      } else {
          uint64_t mask2 = ....


Inventories of iPods for different countries We were given two countries, namely Brazil and Argentina. Both had one inventory each with 100 iPods in them. The cost of an iPod in Brazil was 100/unit and in Argentina it was 50/unit. But to get iPods form Argentina, you also had to pay 400/10units. Now they give us some number and we had to find out which purchase would be cheaper.

#include<iostream.h>
#include<string.h>
#include<stdlib.h>
#include<conio.h>
class country
{
  public:
    int name;
    int items,unit;
    float cost,price;

  public:
    country();
    void purchase(std::string,int);
    void setData(int,int,float,float, int);
};
country::country()
{
}
void country::setData(int name,int items,float cost,float price,int unit)
{
  name=name;
  items=items;
  cost=cost;
  price=price;
  unit=unit;
}
void country::purchase(std::string a, int b)
{
  float custCost=0;
  std::cout<<a;
  cout<<"item"<<name;
  if(b<=items)
  {
    items=items-b;
    custCost=cost*b;
    cout<<name<<":"<<items<<":"<< custCost;
  }

}
int main()
{
  country t[2];
  t[0].setData(0,100,100,0,0);
  t[1].setData(1,100,50,400,20);
  std::string a;
  int b,con;
  cout<<"enter the country";
  std::cin>>a;
  cout<<"enter the items needed";
  cin>>b;
  if(a=="brazil")
    con=0;
  else con=1;
  t[con].purchase(a,b);

  getch();
  return 0;
}

It works but I am not totally satisfied with my design as I feel it's too cumbersome. Specifically, if I change the price etc., code changes have to be made.

Before looking at your design, a few trivial points that you can correct,

#include<iostream>
#include<string>
#include<cstdlib>

Avoid c style header usage. Instead use the c++ header style, and namespaces.
Instead of conio.h/getch() is not really portable. Avoid it.
Your setData method should really be the constructor. Using setData to populate the structure is a c habit. Also be sure to use the member initializers for construction.

You really want to keep the members private

class country {
  private:
    int name;
    int items,unit;
    float cost,price;

    bool try_purchase(std::string,int);

  public:
    country(int,int,float,float, int);
    void do_purchace(std::string a);
};
country::country(int name,int items,float cost,float price,int unit)
          :name(name),items(items),cost(cost),price(price){}

// false indicates that the purchase failed.
bool country::try_purchase(std::string a, int b) {
  if(b > items) return false;
  items = items - b;
  return true;
}
void country::do_purchace(std::string a) {
  int b;
  std::cout<<"enter the items needed: ";
  std::cin>>b;
  if (try_purchase(a,b))
    std::cout<<name<<":"<<items<<":"<<cost * b<<std::endl;
}
int main()
{
  country t[] = {country(0,100,100,0,0), country(1,100,50,400,20)};
  std::string a;
  std::cout<<"enter the country: ";
  std::cin>>a;
  int con = (a=="brazil") ? 0 : 1;
  t[con].do_purchace(a);
}

Avoid incorporating io statements with your logic. Refactor purchase so that logic is separated to another method.

Regarding your assertion that code changes have to be made if you change price, I do not see why. You can read the information to construct a country from a file and thus avoid hard coding it in the main file. However, this does not change the design as a whole.


Best Regular Expression for a one or two values optionally divided by a certain character I'm looking for the most concise regex that matches one or two 4-digit years in any the following setups:

year
year-
-year
year-year

I can't think of anything slicker than this:

[\\-]?\d{4}|\d{4}\[\\-](\d{4})?

This is a somewhat difficult one because regular expressions inherently lack memory, so you can't tell on the back whether the front existed, so I don't think one can get better than the one you wrote for that particular set.  If you wanted to allow some sort of variant, you could potentially find a better one.


Non-coder options in Javascript I have a page with a few options that the owner of a site needs to be able to set. He's not a coder and I'm trying to make it as user-friendly as possible.

It works, but my implementation feels sloppy and I was wondering if there's a better way to do it.

This first part from my main javascript file will be minified and the site owner won't even need to look at it. The unminified source might be passed on to another developer in the future, though:

/* If this Squarespace page title isn't in the onlyShowOn list of pages,
 * stop now. */
var pageTitle = $('meta[property="og:title"]').attr('content');
if (pageTitle !== undefined && onlyShowOn.indexOf(pageTitle) == -1 ) {
    return true;
}

if (pageOptions.hasOwnProperty(pageTitle) && pageOptions[pageTitle] !== undefined) {
    var pageOption = pageOptions[pageTitle];
    if (pageOption.hasOwnProperty('skipHeadings') && pageOption.skipHeadings !== undefined) {
        skipHeadings = pageOption.skipHeadings;
    }

    if (pageOption.hasOwnProperty('headingSubstitution') && pageOption.headingSubstitution !== undefined) {
        headingSubstitution = pageOption.headingSubstitution;
    }
}

I don't like all the hasOwnProperty and checking for undefined, but I want to cover the cases where he doesn't set the options for all his pages or doesn't set all of the options.

Here's the options file that he will modify before injecting all of the code into the footer of his Squarespace site:

/* skipHeadings:
    This variable is used to exclude headings from the ToC. For example, a
    value of 2 will exclude the first 2 headings from the ToC.  */
var skipHeadings = 1;

/* headingSubstitution:
    This variable is used to provide substitutions which will show in the
    ToC. Any heading that matches exactly the quoted text before the colon
    will appear in the table of contents using the quoted text after the
    colon.
     - The values have to be quoted.
     - The heading and the substitution have to be separated by a colon.
     - The substitutions have to be separated by commas.  */
var headingSubstitution = {
    'A really long heading to test box size issues.': 'Long Heading ...',
    'Playing with Tocify': 'Playing ...' // You can add comments here, too.
};

/* onlyShowOn:
    This variable contains the list of pages on which the ToC will show.
     - The values have to be quoted, separated by commas, and match the name
       of your page in Squarespace exactly.
     - You can use either single- or double-quotes.
     - The entries can be on separate lines as long as they're separated by
       commas.  */
var onlyShowOn = [
    'Some Other Page',  // You can add comments after the page names like this.
    'Tocify Test Page', // The name of the page I'm using on my development server.
    'Tocify Page', // The name of the page I'm using on my Squarespace test site.
];

/* pageOptions:
    This variable contains per-page options for skipHeadings and
    headingSubstitution. The values above are used as defaults if a value is
    set here, it completely overrides the default value. As with the onlyShowOn
    option, the page name must match the name of your page in Squarespace
    exactly. The rules for the value definitions are the same as for the
    default options. */
var pageOptions = {
    'Some Other Page': {
        'skipHeadings': 2,
        'headingSubstitution': {
            'A really long heading to test box size issues.': 'Long Heading ...',
        }
    },
    'Tocify Test Page': {
    },
    'Tocify Page': {
    }
};

Firstly, good job in commenting. The thing I usually look for in comments is why this thing is needed, which you really did in detail. However, your comments might need a bit of trimming. Explaining is good, making them too long though makes them visually distracting.

The onlyShowOn and pageOptions structures are redundant. You can just use pageOptions instead to check for existence.

var pageTitle = $('meta[property="og:title"]').attr('content');

I believe this can be replaced by the following, removing the dependency to jQuery.

var pageTitle = document.querySelector('meta[property="og:title"]').getAttribute('content');

In the end, your code is about getting skipHeadings and headingSubstitution based on the value of the title and falling back to default values if some parameters don't meet. So let's start with that.

var defaults = {
  skipHeadings: 1,
  headingSubstitution: {
    'A really long heading to test box size issues.': 'Long Heading ...',
    'Playing with Tocify': 'Playing ...'
  }
};

var substitutes = {
  'Some Other Page': {
    skipHeadings: 2,
    headingSubstitution: {
      'A really long heading to test box size issues.': 'Long Heading ...',
    }
  },
  'Tocify Test Page': {},
  'Tocify Page': {}
};

var pageTitle = document.querySelector('meta[property="og:title"]').getAttribute('content');

var hasSubstitute = pageOptions.hasOwnProperty(pageTitle) && substitutes[pageTitle];

// We check if there's a substitute. If not, use the default. Also, if our
// substitute has missing properties, Object.assign to fills them in with
// defaults.
var substitute = hasSubstitute ? Object.assign({}, defaults, substitutes[pageTitle]): defaults;

// In the end, substitute will always have a skipHeadings and
// headingSubstitution property, allowing us to write without checks.
var skipHeadings = substitute.skipHeadings;
var headingSubstitution = substitute.headingSubstitution;


Generate two random primes and find their totient I am supposed to make a basic RSA encryption/decryption code, where I am only given e.

I wrote this code to randomly generate two primes and then find their totient.  But the totient is supposed to not be co-prime with e which is given as 65537.  So this code would technically return a good value eventually, but in reality it would take ages.  Please help me speed it up and/or make suggestions on how it can be improved in general.

def primeGen(lim):
    """Generates all primes up to a certain limit (lim)"""
    fnl=[]
    primes=[2]
    p=0
    for n in range(2,lim+1):
        fnl.append(n)
    for v in fnl:
        p=0
        for i in range(2,v):
            if (v%i==0):
                p=1
                break
            if p==0 and i==(v-1):
                primes.append(v)
    return primes

import random
def randomPrimeGen(lim):
    """Generates a single random prime number using primeGen(lim)"""
    M=(len(primeGen(lim))-1)
    randomPrime=(primeGen(lim)[random.randint(int(M/10),M)])
    return randomPrime

def modtot():
    e=65537
    totient=1
    GCD=gcd(e,totient)
    while GCD==1:
        p=randomPrimeGen(30000)
        q=randomPrimeGen(30000)
        n=p*q
        totient=((p-1)*(q-1))
        GCD=gcd(e,totient)
        print(GCD, totient)
    return n, totient

One problem, pointed out in comment by @chepner is that you are recomputing the primes twice. Save the primes, so you don't have to recompute.

Also, in your inner loop, you are unnecessarily dividing by every number up to v. Instead, you should skip all non-primes and only test divisibility by the primes, up to your largest known prime (because all other numbers up to there can be factorized into these primes). Only after that do you need to test divisibility for each integer max(primes) < i < v. Skipping all non-primes up to max(primes) saves a lot of looping/divisions/comparisons for large lim (especially given that the distance between consecutive primes tends to increase as lim increases). Something like this will give you a big speedup:

def primeGenFast(lim):
    fnl=[]
    primes=[2]

    fnl = list(range(2,lim+1))
    for v in fnl:
        p=1
        for n in primes:
            if v%n == 0:
                p = 0
                break

        if p==1:
            for i in range(primes[-1],v):
                if (v%i==0):
                    break
                if i==(v-1):
                    primes.append(v)

    return primes

This gives a speedup of nearly 10-20x on my machine, increasing as the size of lim increases:

In [3]: timeit(primeGenSlow(10000))
1 loop, best of 5: 465 ms per loop

In [6]: timeit(primeGenSlow(25000))
1 loop, best of 5: 3.98 s per loop

In [8]: timeit(primeGenSlow(100000))
1 loop, best of 5: 38.1 s per loop

----

In [2]: timeit(primeGenFast(10000))
10 loops, best of 5: 29.6 ms per loop

In [5]: timeit(primeGenFast(25000))
1 loop, best of 5: 307 ms per loop

In [7]: timeit(primeGenFast(100000))
1 loop, best of 5: 1.91 s per loop


Game winning optimal strategy Consider a row of n coins of values v1 . . . vn, where n is even.  a player selects either the first or last coin from the row, removes it from the row permanently, and receives the value of the coin. Determine the maximum possible amount of money we can definitely win if we move first.
Let us understand the problem with few examples:
[8, 15, 3, 7] : The user collects maximum value as 22(7 + 15)

Does choosing the best at each move give an optimal solution?
Is there any way to improve this code?
def optimal_strategy_game(v):
    n = len(v)
    t = [[0 for x in xrange(n)] for x in xrange(n)]
    for gap in xrange(n):
        for i in xrange(n):
            j = i + gap
            if j<n:
                x =  0 if i+1 > j-1 else t[i+1][j-1]
                y = t[i+2][j] if i+2 <j else 0
                z = t[i][j-2] if i <= j-2 else 0
                t[i][j] = max(v[i]+min(x,y), v[j]+min(x,z))
    print t
    print t[0][n-1]

v = [8,15,3,7]
n = len(v)
print optimal_strategy(0,n-1,v)
optimal_strategy_game(v)

1. Review

It's usually more convenient to return a result rather than printing it. Then you can use the result in other computations, for example test cases.
It's not clear what the meaning of the table t is. It would be helpful to have a comment. I think that if \$i ≤ j\$ then t[i][j] gets filled in with the maximum money obtainable by a player who starts with the coins \$v_i, \dots, v_j\$.
There is some trouble in handling edge cases — you need to avoid an invalid lookup in the table t so instead of writing t[i+1][j-1] you have to write:

0 if i+1 > j-1 else t[i+1][j-1]

to avoid trouble in the cases i+1 == n and j == 0. What this suggests is that t would be better implemented as a mapping from pair (i, j) to money, using collections.defaultdict:

from collections import defaultdict
t = defaultdict(int)    # Map from (i,j) to max money on v[i:j+1].

Then you could look up any pair and it would default to zero even if it was out of bounds, like this:

x = t[i + 1, j - 1]
y = t[i + 2, j]
z = t[i, j - 2]
t[i, j] = max(v[i] + min(x, y), v[j] + min(x, z))

In this code:

for gap in xrange(n):
    for i in xrange(n):
        j = i + gap
        if j<n:

you could avoid the test j<n by iterating only over valid values of i:

for gap in xrange(n):
    for i in xrange(n - gap):
        j = i + gap

These lines of code:

x = t[i + 1, j - 1]
y = t[i + 2, j]
z = t[i, j - 2]
t[i, j] = max(v[i] + min(x, y), v[j] + min(x, z))

are looking two steps ahead (the next player chooses the minimum of their options, and the current player chooses the maximum of theirs).

But it is simpler to look only one step ahead, like this:

s = sum(v[i:j+1])
t[i, j] = s - min(t[i + 1, j], t[i, j - 1])

How does this work? Well, s is the sum of all the coins from i to j, so if the next player can win t[i + 1, j] (say), then the current player can win whatever's left.

(This is a standard trick when implementing a minimax algorithm. Instead of having different code for the even (max) and odd (min) players, use the same code for both players and negate the score at each step so that max and min swap roles.)

However, this is inefficient because of the computation of s, which requires iterating over all the coins from i to j. But we can avoid this by precomputing a list of running totals, using itertools.accumulate:

from itertools import accumulate
totals = [0] + list(accumulate(v))

and then the computation of s becomes:

s = totals[j + 1] - totals[i] # sum(v[i:j+1])

2. Revised code

from collections import defaultdict
from itertools import accumulate

def optimal_strategy_game(coins):
    """Return the maximum possible amount of money that can be won by a
    player starting on the given row of coins.
    """
    n = len(coins)
    totals = [0] + list(accumulate(coins)) # Running totals over coins
    money = defaultdict(int) # Map from (i,j) to max money on coins[i:j+1].
    for gap in range(n):
        for i in range(n - gap):
            j = i + gap
            s = totals[j + 1] - totals[i] # s = sum(coins[i:j+1])
            money[i, j] = s - min(money[i + 1, j], money[i, j - 1])
    return money[0, n - 1]

3. Alternative approach

The technique here (of building a table of solutions to sub-problems) is known as dynamic programming. But code that uses dynamic programming can always be rewritten to use recursion to visit the sub-problems and memoization to avoid duplicate work. Sometimes this results in clearer code. In Python 3.2 or later, many functions are easy to memoize using the @functools.lru_cache decorator:

from functools import lru_cache
from itertools import accumulate

def max_money(coins):
    """Return the maximum possible amount of money that can be won by a
    player starting on the given row of coins.
    """
    totals = [0] + list(accumulate(coins)) # Running totals over coins

    @lru_cache(maxsize=None)
    def money(start, stop):
        "Return maximum money that can be won on coins[start:stop]."
        if start >= stop:
            return 0
        else:
            s = totals[stop] - totals[start] # s = sum(coins[start:stop])
            return s - min(money(start + 1, stop), money(start, stop - 1))

    return money(0, len(coins))

(Python 2.7 lacks lru_cache but there's a backport.)


Makefile for a small JNI project So I'm going to publish this small project on GitHub and I thought I'd better do a makefile for it. The source code consists of

1 Java Class
1 Native JNI module written in C
1 stdlib wrapper object written in C (for LD_PRELOAD)

The purpose of the Makefile is to compile the two shared objects from the C-Source.

Here it is

CC=gcc
CFLAGS=-nostartfiles -fpic -shared -ldl -D_GNU_SOURCE -Wall -Werror
OUT=bld
OUTFILES = $(OUT)/bind.so $(OUT)/bindsetter.so
JNI_AUTO_HEADER=$(OUT)/com_daftdroid_droidlib_bind_ClientBind.h

#Two locations needed to get all the JNI header files
JNI_INCLUDE=/usr/lib/jvm/java-8-oracle/include \
    /usr/lib/jvm/java-8-oracle/include/linux

#Add the includes to the compiler flags
CFLAGS+=$(foreach d, $(JNI_INCLUDE), -I$d)

all: $(OUTFILES)

$(OUTFILES): $(OUT)/%.so: %.c $(JNI_AUTO_HEADER)
    $(CC) $(CFLAGS) -c $< -o $@

$(JNI_AUTO_HEADER): ClientBind.java
    javac ClientBind.java -d $(OUT) -h $(OUT)

.PHONY: clean
clean:
    rm bld/* -rf

So some questions: CC=gcc is this a bit presumptuous? CFLAGS - well these all assume GCC too. What about inflicting -Werror on people - on my computer there are no warnings as it stands.

I hate this: JNI_INCLUDE=/usr.... that is seriously installation dependent. What's a better way of doing this?

I have this header file JNI_AUTO_HEADER that javac can generate from the .java source, so therefore I consider it a build artifact which the makefile is responsible for. Do you agree? Also I've made all of the .so files dependent on JNI_AUTO_HEADER but hey, there's only two of them so I guess it's not worth worrying about.

Also I'm not sure if there's a better way of doing this in my source file:

// This include generated in the build directory at compile time
#include "bld/com_daftdroid_droidlib_bind_ClientBind.h"

(without getting into a much more complex build process)

Any other comments welcome.

CFLAGS conventionally control the compilation. Linking options, that is

-nostartfiles -shared -ldl

are better be separated into LDFLAGS.
-Werror is always a good idea. You don't want your users to run potentially incorrect code, do you?
I recommend to make $(OUT) directory automatically.

$(OUTFILES): | $(OUT)
$(OUT):
    mkdir $(OUT)

See order-only dependencies for details.
JNI_AUTO_HEADER is a build artifact, and yes make is responsible for it. I see no reason for confusion.
As for #include "bld/com_daftdroid_droidlib_bind_ClientBind.h", consider adding

-include $(JNI_AUTO_HEADER)

to CFLAGS, or passing it via -D option (see this discussion for details)
I don't know the correct way to figure out Java include paths; maybe java.home is a good place to start? You'd need a small Java app to print it though.


Simple Gender Guesser I have made a simple gender guesser for an app and thought that this simple bit of code may be useful to other developers. It takes in a string and returns an 'f' for Female and an 'f' Male depending on the name provided. I am not a PHP expert so I was wondering if anyone can give me some feedback on this code.

The full code can be found at the repository here

<?php

Class Gender{
    public $gender;
    protected $maleNames    = [];
    protected $femaleNames  = [];
    public $isMale      = FALSE;
    public $isFemale    = FALSE;
    public function __construct($name, $default, $unknown, $custom){
        $this->maleNames    = $this->getMaleNames($custom);
        $this->femaleNames  = $this->getFemaleNames($custom);
        $this->gender       = $default;
        if(in_array($this->getName($name), $this->maleNames)){
            $this->isMale = TRUE;
        }
        if(in_array($this->getName($name), $this->femaleNames)){
            $this->isFemale = TRUE;
        }
        if($this->isMale && !$this->isFemale){
            $this->gender = 'm';
        }elseif(!$this->isMale && $this->isFemale){
            $this->gender = 'f';
        }elseif($this->isMale && $this->isFemale){
            $this->gender = $default;
        }else{
            $this->gender = $unknown;
        }
    }
    public static function guess($name, $default = 'm', $unknown = 'm', $custom = []){
        $g = new Self($name, $default, $unknown, $custom);
        return $g->gender;
    }
    public static function isMale($name, $default = 'm', $unknown = 'm', $custom = []){
        $g = new Self($name, $default, $unknown, $custom);
        return ($g->gender === 'm')? TRUE : FALSE;
    }
    public static function isFemale($name, $default = 'm', $unknown = 'm', $custom = []){
        $g = new Self($name, $default, $unknown, $custom);
        return ($g->gender === 'f')? TRUE : FALSE;
    }
    protected function getName($name){
        $name = explode(' ', $name);
        $name = $name[0];
        $name = strtolower($name);
        return $name;
    }
    protected function getMaleNames($custom){
        $names = json_decode(file_get_contents(__DIR__ . '/names/male.json'));

        if(is_array($custom['male'])){
            foreach($custom['male'] as $name){
                $names[] = strtolower($name);
            }
        }
        return $names;
    }
    protected function getFemaleNames($custom){
        $names = json_decode(file_get_contents(__DIR__ . '/names/female.json'));

        if(is_array($custom['female'])){
            foreach($custom['female'] as $name){
                $names[] = strtolower($name);
            }
        }
        return $names;
    }
}

echo Gender::guess('anthony');

?>

Performance aspects regarding file access

In your getMaleNames($custom) function you first load the /names/male.json contents, then look at $custom and use this one if not empty: so in the latter case you spent time reading file while it's useless. You'd instead look at $custom first, then at file if needed.
Obviously the same applies to getFemaleNames().

In the other hand, when you call one of your static functions you create a new Gender instance, so invoking both of the get...Names() functions, which ends with reading two files for each external call as soon as they don't provide non empty $custom argument.
It could be far more efficient to create a unique instance once, then use its contents.
As a first thought this might be achieved by looking at the Gender instance from the static functions, and create it only when it doesn't exist yet. But look also at the strategy part below.

Compacting code

There are several places where this can be done.

The best example is in the already cited get...Names() functions: they can be simply factorized like this (also includes the change suggested about files):

protected function getNames($gender, $custom) {
  if (is_array($custom[$gender])) {
    foreach ($custom[$gender] as $name) {
      $names[] = strtolower($name);
    }
  }
  $names =
    json_decode(file_get_contents(__DIR__ . '/names/' . $gender . '.json'));
  return $names;
}

Another example is that, even if you want to keep separate functions available for the external calls (though guess() only is probably enough), you can simplify the two is...() functions, like this one (same for isFemale()):

public static function isMale($name, $default = 'm', $unknown = 'm', $custom = []) {
  return self::guess($name, $default, $unknown, $custom) === 'm';
}

Finally the getName() function can be simplified like this:

protected function getName($name) {
  return strtolower(explode(' ', $name)[0]);
}

Strategy

Analyzing what your class is made for, it appears that creating an instance is not required. The only data which needs to be saved between two external calls is the content of the /names/...json files (and only if we retain the first change I proposed above!).

So you might:

keep properties $maleNames and $femaleNames only, and make them static.
also make the protected functions static.
move all the logics currently in __construct() to guess() (working with simple variables rather than properties).
and not have __construct() at all.

Additionally observe that, in the current __construct() logics:

the affectation to $this->gender is never used further
getName() can be called only once and is very simple, so the function can be abandoned in profit of direct comptuting
the if/else sequence can be replaced by directs returns

So it ends with this full code simplified version:

Class Gender{
  protected static $maleNames;
  protected static $femaleNames;
  public static function guess($name, $default = 'm', $unknown = 'm', $custom = []){
    if (!self::$maleNames) {
      self::$maleNames = self::getNames('male');
    }
    if (!self::$femaleNames) {
      self::$femaleNames = self::getNames('female');
    }
    $name = strtolower(explode(' ', $name)[0];
    $isMale = in_array($name, self::$maleNames);
    $isFemale = in_array($name, self::$femaleNames);
    if ($isMale && !isFemale) {
      return 'm';
    }
    if (!$isMale && isFemale) {
      return 'f';
    }
    if ($isMale && isFemale) {
      return $default;
    }
    return $unknown;
  }
  public static function isMale($name, $default = 'm', $unknown = 'm', $custom = []) {
    return self::guess($name, $default, $unknown, $custom) === 'm';
  }
  public static function isFemale($name, $default = 'm', $unknown = 'm', $custom = []) {
    return self::guess($name, $default, $unknown, $custom) === 'f';
  }
  protected static function getNames($gender, $custom) {
    if (is_array($custom[$gender])) {
      foreach ($custom[$gender] as $name) {
        $names[] = strtolower($name);
      }
    }
    $names =
      json_decode(file_get_contents(__DIR__ . '/names/' . $gender . '.json'));
    return $names;
  }
}


Three edittext boxes: suggestions to clean up code I am a newbie at writing Android apps and using Java in general. I have went through most of the Android hello view tutorials but still seem to be lacking some understanding of the basics.  Here is a snippet of code I wrote for an app.  My goal was to have three edittext boxes for the user to input information.  I want the user to put information in two of the three boxes and when they hit the calculate button, it will calculate the third edittext value based on a specific equation.  Is there anyway that I can avoid initializing these 3 edittext objects in each of my methods within this class?  Any other suggestions for cleaning up or improving this code?

public class PlantPopulation extends Activity {

    /** Called when the activity is first created. */
    @Override
    public void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.main);

        EditText seedSpacing = (EditText)
                findViewById(R.id.seedSpacing);
        EditText rowSpacing = (EditText)
                findViewById(R.id.rowSpacing);
        EditText population = (EditText)
                findViewById(R.id.population);

        seedSpacing.setText("");  //clear values
        rowSpacing.setText("");
        population.setText("");

    }

    //check how many boxes are empty
    public int checkifempty(String array[]) {

        int i = 0;
        for (String string : array) {
            if (string.equals("")) {
                i = i+1;
            }
        }
        return i;
    }

    //run the calculation
    public void calc(View v) {

        EditText seedSpacing = (EditText)
                findViewById(R.id.seedSpacing);
        EditText rowSpacing = (EditText)
                findViewById(R.id.rowSpacing);
        EditText population = (EditText)
                findViewById(R.id.population);

        String sS = seedSpacing.getText().toString();
        String rS = rowSpacing.getText().toString();
        String pop = population.getText().toString();

        String boxes[] = {sS,rS,pop};
        //determine which box is empty
        if (checkifempty(boxes) <2) {

            if (sS.equals("")) {
                double calc1=(43560*144)/new Double(rS)/
                        new Double(pop);
                calc1 = Math.floor(calc1 * 100 +.5)/100;
                seedSpacing.setText(Double.toString(calc1));
            } else if (rS.equals("")) {
                double calc2=(43560*144)/new Double(sS)/
                        new Double(pop);
                calc2 = Math.round(calc2);
                rowSpacing.setText(Double.toString(calc2));
            } else if (pop.equals(""))  {
                Double calc3=((43560*144)/new Double(rS)/
                        new Double(sS));
                Integer calc = calc3.intValue();
                //calc3 = Math.round(calc3);
                population.setText(calc.toString());
            } else {
                Toast.makeText(PlantPopulation.this,
                    "Leave one item blank.", Toast.LENGTH_SHORT).show();
            }
        } else {
            Toast.makeText(PlantPopulation.this,
                    "You must fill in two of the three boxes.", Toast.LENGTH_SHORT).show();
        }
    }
}

Replace "43560*144" by a constant.
Store new Double(sS), new Double(srS), new Double(pop) in variables
before if (checkifempty(boxes) <2). It'll improve speed and clarify
the code.
Correct indentation in the onCreate method


Shortening my questions code Is there any other way to shorten this code without sacrificing readability?  I am fairly new to C#, let alone programming, and I wanted to know if this is as short as this code can possibly become.  Feedback would be greatly welcomed.

Side note: was Questions.Questask used properly here, or could there have been a better way?

 class Program
 {
    static void Main(string[] args)
    {
        string name = "";
        while (name.ToLower() != "none")
        {
            Console.WriteLine("Are you Kydd,Leo,Jay,Sha, or Zigg");
            name = Console.ReadLine();

            if (name.ToLower() == "kydd")
                Question.Questask("Input question here", "Kquestion","Link");
            else if (name.ToLower() == "leo")
                Question.Questask("Input question here", "Lquestion", "Link");
            else if (name.ToLower() == "jay")
                Question.Questask("Input question here", "Jquestion", "Link");
            else if (name.ToLower() == "sha")
                Question.Questask("Input question here", "Squestion", "Link");
            if (name.ToLower() == "zigg")
                Question.Questask("Input question here", "Zquestion", "Link");
       }
    }

    class Question
    {
        public static void Questask(string question, string questAnswer, string url)
        {
            Console.WriteLine(question);
            questAnswer = Console.ReadLine();
            if (questAnswer.ToLower() == "yes")
            {
                Console.WriteLine("You are right!");
                Console.ReadKey();
            }
            else
            {
                System.Diagnostics.Process.Start(url);
            }
        }
    }
}

When I need a simple strategy pattern I'll use a Dictionary. This can drastically reduce code by cutting repetition.

Note the use of StringComparer.InvariantCultureIgnoreCase for comparer to do away with the String.ToLower calls.

private static readonly IDictionary<string, Tuple<string, string>> NamesDictionary =
    new Dictionary<string, Tuple<string, string>>(StringComparer.InvariantCultureIgnoreCase)
    {
        { "kydd", Tuple.Create("question 1", "link1") },
        { "zydd", Tuple.Create("question 2", "link2") },
        ...
    };

private static void DoSomething(string name)
{
    Tuple<string, string> questionAndLinkTuple;

    if (!NamesDictionary.TryGetValue(name, out questionAndLinkTuple))
        return;

    var question = questionAndLinkTuple.Item1;
    var link = questionAndLinkTuple.Item2;

    Question.AskQuestion("Input answer here", question, link);
}


“Guess My Number” with Racket I am using the book "Realm of Racket" which was written by the people behind the Racket and Dr. Racket projects. It is a great book based on games, similar to the famous "Land of Lisp".

The first game we are supposed to code is the classic "Guess My Number". Following the instructions on the book you end up with this code:

(define lower 1)

(define upper 100)

(define (computer-guess)
  (round (/ (+ lower upper) 2)))

(define (smaller)
  (set! upper (max lower (sub1 (computer-guess))))
  (computer-guess))

(define (bigger)
  (set! lower (min upper (add1 (computer-guess))))
  (computer-guess))

(define (start n m)
  (set! lower (min n m))
  (set! upper (max n m)))

As a matter of practice I decided to try something different. On the previous code, you need to keep using the interactive panel to make new guesses until you reach the number.

As you can see, the algorithm uses binary search.

I am trying the best I can to practice the concept of a recursive definition which generates an iterative procedure.

Hence, I tried to create a code which would keep making new guesses until reaching the determined number. I also tried to create a code that would count the number of tries.

I ended up with the following code which works!

Despite providing the expected output and working fine, I think I could do some refactoring.

I am repeating my self but I do not see how to avoid this:

#lang racket

(define (guess lower-boundary upper-boundary)
  (round (/ (+ lower-boundary upper-boundary) 2)))

(require racket/trace)

(define (game-iter number)
  (define (iter number count lower-boundary  upper-boundary accu)
    (cond ((= (guess lower-boundary upper-boundary) number) (values count (guess lower-boundary upper-boundary)))
          ((> (guess lower-boundary upper-boundary) number)
           (iter  number
                  (add1 count)
                  lower-boundary
                  (sub1 (guess lower-boundary upper-boundary))
                  (guess lower-boundary upper-boundary)))
          (else (iter number (add1 count) (add1 (guess lower-boundary upper-boundary)) upper-boundary (guess lower-boundary upper-boundary)))))
  (trace iter)
  (iter number 0 1 100 (guess 1 100)))

(game-iter 10)

Racket isn't my Lisp of choice, but here's some general advice.

You're calling the function guess multiple times with the same arguments. The code would be much more readable (and slightly more efficient) if you bind the result to a local variable:

(define (game-iter number)
  (define (iter number count lower-boundary upper-boundary accu)
    (let ((guess (guess lower-boundary upper-boundary)))
      (cond ((= guess number) (values count guess))
            ((> guess number)
             (iter  number
                    (add1 count)
                    lower-boundary
                    (sub1 guess)
                    guess))
            (else (iter number
                        (add1 count)
                        (add1 guess)
                        upper-boundary
                        guess)))))
  (trace iter)
  (iter number 0 1 100 (guess 1 100)))

I'm not sure if the racket custom is to abbreviate variable names to avoid shadowing the function. Using the same name for the variable and the function shouldn't cause any problems in this case, so I'll just use guess here. Feel free to change it.

Your iter function has number as a parameter, but it never changes. It's unnecessary to pass it down, since it's already available from the outer scope. You also have a parameter called accu, which is never used. Unless you intend to use it later, it should be removed.

(define (game-iter number)
  (define (iter count lower-boundary upper-boundary)
    (let ((guess (guess lower-boundary upper-boundary)))
      (cond ((= guess number) (values count guess))
            ((> guess number)
             (iter (add1 count)
                   lower-boundary
                   (sub1 guess)))
            (else (iter (add1 count)
                        (add1 guess)
                        upper-boundary)))))
  (trace iter)
  (iter 0 1 100))

You coud collapse the two calls to iter into one by using if inside the call.

(define (game-iter number)
  (define (iter count lower-boundary upper-boundary)
    (let ((guess (guess lower-boundary upper-boundary)))
      (if (= guess number)
          (values count guess)
          (iter (add1 count)
                (if (< guess number)
                    (add1 guess)
                    lower-boundary)
                (if (> guess number)
                    (sub1 guess)
                    upper-boundary)))))
  (trace iter)
  (iter 0 1 100))

Notice also that you start counting guesses from zero. So if the number is 50, which is the first guess, the program will consider that to be zero guesses. It would make more sense to start from 1.

As pointed out by Renzo, you should also make sure that the number is in the range from 1 to 100:

(define (game-iter number)
  (define (iter count lower-boundary upper-boundary)
    (let ((guess (guess lower-boundary upper-boundary)))
      (if (= guess number)
          (values count guess)
          (iter (add1 count)
                (if (< guess number)
                    (add1 guess)
                    lower-boundary)
                (if (> guess number)
                    (sub1 guess)
                    upper-boundary)))))
  (trace iter)
  (if (< 0 number 101)
      (iter 1 1 100)
      ;; Raise an error or return something suitable
      #f))


Checking to see if all values in an array are equal I have an array that might be either one-dimensional, two-dimension, or three-dimensional. I'm wanting to check to see if all values in a given direction are constant.

Here are 3 functions:

# Check to see if all values in the first dimension are equal
def constant1D(arr)
  arr.inject(true) { |same, val| same and val == arr[0] }
end

# Check to see if all values in the second dimension are equal
def constant2Dy(arr)
  arr.inject(true) { |same, x| same and constant1D(x) }
end

# Check to see if all values in the third dimension are equal
def constant3Dz(arr)
  arr.inject(true) { |same, x| same and constant2Dy(x) }
end

And here is the code that would be checking:

constantX = constant1D(values)
constantY = ((mapType == :twoD) and constant2Dy(values))
constantZ = ((mapType == :threeD) and constant3Dz(values))

Here are some sample runs:

threeDa = [[[1, 2, 3], [4, 5, 6], [7, 8, 9]], [[1, 2, 3], [4, 5, 6], [7, 8, 9]], [[1, 2, 3], [4, 5, 6], [7, 8, 9]]]
threeDb = [[[1, 2, 3], [1, 2, 3], [1, 2, 3]], [[4, 5, 6], [4, 5, 6], [4, 5, 6]], [[7, 8, 9], [7, 8, 9], [7, 8, 9]]]
threeDc = [[[1, 1, 1], [2, 2, 2], [3, 3, 3]], [[4, 4, 4], [5, 5, 5], [6, 6, 6]], [[7, 7, 7], [8, 8, 8], [9, 9, 9]]]
>> constant1D(threeDa)
=> true
>> constant1D(threeDb)
=> false
>> constant1D(threeDc)
=> false
>> constant2Dy(threeDa)
=> false
>> constant2Dy(threeDb)
=> true
>> constant2Dy(threeDc)
=> false
>> constant3Dz(threeDa)
=> false
>> constant3Dz(threeDb)
=> false
>> constant3Dz(threeDc)
=> true

Is this the most "Ruby" way, or is there a better way? E.g., referencing arr[0] in the above has a little bit of code smell to me, but perhaps I'm overthinking it.

[Rewritten]

You want to avoid inject, as uniq.size is O(n) and clearer. Monkey patching array makes the code clearer and is idiomatic Ruby. Methods that return a boolean should end in ?. Method names should be snake_case. Here you go:

class Array
  # Check to see if all values in the first dimension are equal
  def constant_1d?
    uniq.size == 1
  end

  # Check to see if all values in the second dimension are equal
  def constant_2dy?
    all?(&:constant_1d?)
  end

  # Check to see if all values in the third dimension are equal
  def constant_3dz?
    all?(&:constant_2dy?)
  end
end


Guessing game for a five-digit code In this program, there is a code that the user has to try to solve in 10 guesses.  If you have any numbers in the right spot, it will tell you how many you have correct and the value of the correct numbers so you can figure it out.
I know there has to be a much simpler way to do this, I just can't put one together.  I could use arrays (although i'm not the best with them yet) to find certain values, but i'm not positive on how to do that.  Any tips?
(I apologize in advance for any sloppy coding or poor choice of variable names.  Also this is in a main method, i just didn't copy the class name or the main method)

    String code = "53492";
    int guesses = 10;
    Scanner s12 = new Scanner(System.in);
    System.out.println("Try to guess the 5 number code. You have 10 guesses: ");
    String guess = s12.nextLine();
    int right = 0;
    int combo = 0; //correct values combined
    int correctInt = 0;

    for(int i = 0; i < 10; i++) //will allow you to guess a certain amount of times
    {
        combo = 0;
        right = 0;
        for(int x = 0; x < 5; x++) //starts searching at first spot(0)
        {
            int guessInt = Integer.parseInt(guess);
            int codeInt = Integer.parseInt(code);
            String getCorrectInt = guess.substring(x, x + 1);
            correctInt = Integer.parseInt(getCorrectInt);  //stores value of num at the value of
            String getCodeGuess = code.substring(x, x + 1);
            int codeGuess = Integer.parseInt(getCodeGuess);
                if(guessInt == codeInt)
                {
                    System.out.println("Correct! You guessed it!");
                    System.exit(0);
                }
                if(correctInt == codeGuess)  //if the num at the position matches the code at the right position
                {
                    combo += correctInt;    //totals up the correct amount of numbers in right spot
                    right++; //keeps track of how many are right
                }
        }
        guesses--;
        System.out.println("You have " + right + " number(s) correct that equal to " + combo + " and you have " + guesses + " guesses left.");
        System.out.println("Guess again: ");
        guess = s12.nextLine();
    }
    System.out.println("You ran out of guesses!  The code was " + code);

Streams clean this up a little bit, however I removed print statements here so that the logic can be seen a little clearer:

String code = "53492";
int guesses = 10;
Scanner scanner = new Scanner(System.in);

while (guesses-- > 0) {
    String guess = scanner.nextLine();
    if (guess.equals(code)) {
        return; //guess was correct, can also just compare correct to guess length
    }

    int correct = compare(guess, code, Stream::count); //count elems
    int combo = compare(guess, code, s -> s.reduce(0, Integer::sum)); //sum
    // print information, note this requires three iterations since we stream twice
    // and have an #equals check. There are still some optimizations you
    // can make to remove this
}

//elsewheres
public <R> R compare(String one, String two, Function<IntStream, R> back) {
    //apply our stream function to an intstream for a mutable return
    return back.apply(IntStream.range(0, code.length())
                      .filter(i -> guess.charAt(i) == code.charAt(i)) //correct chars
                      .map(i -> guess.charAt(i) - '0')); //map to values
}

It seemed like it would be a neater solution before I actually sat and wrote it, but I figured it'd be nice to at least see an alternative.


Built-in help in a Makefile My colleagues often ask me, "Dude could you remind me what I have to type to build this target?". So I eventually implemented a nice workaround in my Makefiles.

I wanted my Makefiles to have a help target that give some usage clues to the users. This is achieved with an embedded Perl script and some markers.

Wherever I want a target to be documented, I simply add this trailing comment:

##@<category> <description>

Here is an example:

SCRIPT_VERSION=v1.0
SCRIPT_AUTHOR=John Doe

all:                ##@Build Build all the project
     echo "Hello World!" > all

clean:              ##@Cleaning Remove all intermediate objects
     rm all

mrproper: clean     ##@Cleaning Remove all output and interemediate objects

HELP_FUN = \
    %help; while(<>){push@{$$help{$$2//'options'}},[$$1,$$3] \
     if/^(\w+)\s*:.*\#\#(?:@(\w+))?\s(.*)$$/}; \
    print"$$_:\n", map"  $$_->[0]".(" "x(20-length($$_->[0])))."$$_->[1]\n",\
    @{$$help{$$_}},"\n" for keys %help; \

help: ##@Miscellaneous Show this help
    @echo -e "Usage: make [target] ...\n"
    @perl -e '$(HELP_FUN)' $(MAKEFILE_LIST)
    @echo -e "Written by $(SCRIPT_AUTHOR), version $(SCRIPT_VERSION)"
    @echo -e "Please report any bug or error to the author."

Here what I get if I type make help:

$ make help
Usage: make [target] ...

Miscellaneous:
  help                Show this help

Build:
  all                 Build all the project

Cleaning:
  clean               Remove all intermediate objects
  mrproper            Remove all output and interemediate objects

Written by John Doe, version v1.0
Please report any bug or error to the author.

In the case the Makefile takes time to parse the dependencies we can bypass the file for the help target:

ifeq (,$(filter help,$(MAKECMDGOALS)))
# Core of the makefile
endif

What do you think of this technique. Is it a good approach? Can I improve it?

It looks pretty good to me, but I see a few minor things that might be improved.

Specify which perl to run

I'd recommend not leaving it to chance as to which instance of perl is run.  Instead, for both safety and consistency, it might be better to do this:

PERL=/usr/bin/perl
##...
@$(PERL) -e '$(HELP_FUN)' $(MAKEFILE_LIST)

Allow multiword category tags

With a slight change in your regex, multiword categories could be supported. If the second line is changed to this:

if/^(\w+)\s*:.*\#\#(?:@([^@]*)@)?\s(.*)$$/}; \

It will support multiword categories, delimited with @,

nothing:         ##@Odds & ends@ Do nothing, but gracefully

Sort the keys for printing

On my machine, I get a different ordering each time I run make help which is a little confusing.  To fix that, sort the keys, rendering the last line:

@{$$help{$$_}},"\n" for sort keys %help

Note that you shouldn't have a line continuation there, and you don't need the trailing semicolon (although you may prefer one for style reasons).

Omit declaration of hash

Because %help is unambiguously used, you don't need to predeclare it.  It will automatically be created as an empty hash, so the first line can become:

while(<>){push@{$$help{$$2//'options'}},[$$1,$$3] \


Poke-a-Dot (Provider) I need to create variable-length strings of dots/periods/full-stops to add to some text content, in a way that is similar to a formatted table-of-contents:

Chapter 1 .................................... 1
  Section 1.1 ................................ 1
  Subsection 1.1.2 .......................... 12

I know that things like String.format exist, but that is not suitable for formatting periods this way.

The code is complicated by the need for there to be many threads doing similar work at the same time.

I have written this utility class that caches a 'base' string in a Thread-local, and uses that to supply the required data.

Are there any alternatives, improvements, or other recommendations you have?

public final class DotPadding {

    private static final ThreadLocal<String> PAD_BASE = new ThreadLocal<String>() {
        @Override
        protected String initialValue() {
            return "................";
        }
    };

    public static final String getPadding(final int length) {
        if (length <= 0) {
            return "";
        }
        String base = PAD_BASE.get();
        if (base.length() >= length) {
            return base.substring(0, length);
        }
        while (base.length() < length) {
            base = base.concat(base);
        }
        PAD_BASE.set(base);
        return base.substring(0, length);
    }

}

As an example usage, this is one of the ways that the above code is used. There are other places too:

String description = getFormattedDescription(title);
String index = getFormattedIndex(page);
String line = description
            + " "
            + DotPadding.getPadding(width - (2 + description.length() + index.length()))
            + " "
            + index;
System.out.println(line);

Naive approach: use StringUtils.repeat (source) without the ThreadLocal:

public static String repeat(final char ch, final int repeat) {
    final char[] buf = new char[repeat];
    for (int i = repeat - 1; i >= 0; i--) {
        buf[i] = ch;
    }
    return new String(buf);
}

Anyway, I suppose you did some profiling and it turned out that this is a bottleneck. Otherwise I'd really not complicate it with the ThreadLocal nor the doubling/concatenation logic.
substring in Java 6 used the same character array as the original String instance but it was changed in Java 7. So, substring calls copies the character array anyway, therefore string doubling and calling substring does not seem beneficial to me. (Anyway, it should be measured.)

There is one exception (according to the source which Eclipse brings up for Java 8) when beginIndex is zero and endIndex is the length of the original string. In this case substring returns the same String instance without copying. This case seems unlikely here, I guess there's a little chance that PAD_BASE.lenght will be the same as the length parameter since the while loop doubles the size of the padding string on every iteration.

So, in an average case the naive approach would create less new String (and underlying char[]) objects here than the one with substring.
Another idea is storing an array of strings in the ThreadLocal and extending the array on demand if the required lenght is longer than paddings.lenght:

String paddings[] = new String[max];
for (int i = 0; i < max; i++) {
    paddings[i] = StringUtils.repeat('.', i);
}

2 could be a named constant.


Do I need Generics for these node and tree classes? I want to implement KD tree. I have defined class Node as follows:

public static class Node{
    int id;
    public double x;
    public double y;
    public Node leftChild;
    public  Node rightChild;
    Node(int id, double x, double y){
        this.id = id;
        this.x = x;
        this.y = y;
        leftChild = null;
        rightChild = null;
    }
}

Then I have defined class KDTree in which I want to implement method contains(Node t):

public static class KDTree<T extends Node>{
    Node root;
    KDTree(Node root){
        this.root = root;
    }
    boolean contains(Node t){
        if (t == null)
            return false;
        if (t.x == root.x && t.y == root.y)
            return true;
        else
            return ...; //something else
    }
}

Do I need to use generics in class KDTree? I'm not aware of when it is a good practice to use generics and when it is not? It would be nice if someone can recommend me an article on when we have to use generics and when we don't have to.

As far as I read the declaration public static class KDTree<T extends Node> means "class KDTree contains objects that extends type Node". Is there any way to declare that class KDTree contains only objects of type Node?

Class Node represents a point in the plane, given by its x and y coordinate. Class KDTree is a tree-structure that stores the points in such a way that it is easy to answer queries of the type: give me m nearest points to my point p(x1, y1). This is the method that I use to construct KDTree:

public static Node buildKDTree(List<Node> list, boolean depth, int begin, int end){
    int middle = median(list, depth, begin, end);
    Node root = list.get(middle);
    root.leftChild = buildKDTree(list, !depth, begin, middle-1);
    depth = !depth;
    root.rightChild = buildKDTree(list, !depth, middle+1, end);
    return root;
}

I think that you have confused yourself with your code, and the way that data structures are intended to be implemented.

What you have done that is unconventional, and causing the confusion, is that you have exposed the Node class as a public class, and you have a constructor that takes Nodes as input.

Node is normally an implementation detail of a data astructure (your KDTree) that is transparent to others. Consider Java's LinkedList, that has Node instances, but you never see those Node instances, just like ArrayList has an internal array, etc.

Your KDTree should contain the Node class as an inner (nested) private class, and the users of your KDTree should never be able to see/access those nodes. Your KDTree class should abstact that interaction away.

The constructor for your KDTree should be able to construct off the base values of the node, or be an empty constructor, and have an add method that takes the base values.

Consider something that looks like:

public static class KDTree {

    private static class Node{
        int id;
        public double x;
        public double y;
        public Node leftChild;
        public  Node rightChild;

        Node(int id, double x, double y){
            this.id = id;
            this.x = x;
            this.y = y;
            leftChild = null;
            rightChild = null;
        }
    }

    private Node root = null;

    public KDTree(){
        // empty constructor
    }

    public boolean add(int id, double x, double y) {
        Node toadd = new Node(id, x, y);

        .... add toadd to hierarchy....

    }

    public boolean contains(int x, int y) {
        .....
    }

}

Note, the Node is private, and not visible outside the KDTree class.

It is possible that you need some other form of data structure to represent the actual data in the Node, like a Point class, and, your Node could have a reference to a Point instance. Point strikes me as being a data structure you need anyway, and probably already have.

Note that your x and y values are doubles, and any time I see == comparisons with doubles, I caution people that it is not as reliable as you would expect:

if (t.x == root.x && t.y == root.y)

Be careful there, it's possible that t.x and root.x are very, very, very close to each other, but not quite the same, due to some rounding value in a calculation, or something at the 15th decimal place, or something.


Build a dictionary based on split string I've written a simple script that takes a string filled with brackets and strings and builds them to a dictionary.

Example Input:

string_1 = '(()())'
string_2 = '((a)(b))'
string_3 = '(a(a)b(b)c)'
string_4 = 'a(a(a)b(b)c)b'
string_4 = 'a[a{a}b{b}c]b'
string_5 = 'beginning(content[word]{another word})outside'

Output for string5 and the other examples:

[
('0-8', 'beginning'),
('9', '('),
('10-16', 'content'),
('17', '['),
('18-21', 'word'),
('22', ']'),
('23', '{'),
('24-35', 'another word'),
('36', '}'),
('37', ')'),
('38-44', 'outside')
]

def sort(dictionary):
    return sorted(dictionary.items(), key=lambda v: int(v[0].split("-")[0]))

def join(a, b):
    return ''.join([repr(a), '-', repr(b - 1)]) if a != b else repr(b)

class Positions:
    def __init__(self, name):
        self.name = name
        self.content = {}  # creates a new empty list for each bracket

    def push(self, pos, content):
        self.content[pos] = content

def convert_string(string):
    string_content = string
    string = Positions(string)
    pos = 0
    start_of_str_pos = 0
    internal_string = ''
    for char in string_content:
        if char in ['(', '{', '[', ')', '<', '}', ']', '>']:
            string.push(repr(pos), repr(char))
            if internal_string != '':
                string.push(join(start_of_str_pos, pos), internal_string)
                internal_string = ''
            start_of_str_pos = pos + 1
        else:
            internal_string += char
        pos += 1

    if internal_string != '':
        string.push(''.join([repr(start_of_str_pos),
                             '-', repr(pos - 1)]), internal_string)
    print sort(string.content)

Obvious but easy

There are some issues that are immediately obvious and easy to correct.

In this statement:

return ''.join([repr(a), '-', repr(b - 1)]) if a != b else repr(b)

The multiple repr are awkward,
and so is the join.
It would be less awkward, simpler and more readable by using string formatting:

return '{}-{}'.format(a, b - 1) if a != b else repr(a)

This can be written a lot simpler:

if char in ['(', '{', '[', ')', '<', '}', ']', '>']:

Like this:

if char in '({[)<}]>':

But actually, it's a bit disturbing that the opening and closing brackets are ordered inconsistently. For example, looking at this string, it's not immediately obvious that it includes all matching closing brackets for all opening brackets. This way would be immediately obvious and less error prone:

if char in '(){}[]<>':

This is both confusing and disturbing:

def convert_string(string):
    string_content = string
    string = Positions(string)

Why shuffle around the string parameter?
It's a bad practice to reassign the value of a parameter,
as it makes it confusing what it really stands for throughout the function.

Another problem here is calling an instance of Positions a "string".
Later in the code when I read "string",
I think it's an actually string, but in fact it's not,
it's an instance of "Positions".
Very misleading.

Since empty strings are falsy, instead of this:

if internal_string != '':
    # ...

You can write simply:

if internal_string:
    # ...

Obvious but harder

There are some issues that are sort of obviously wrong,
but not so easy to correct.

This sorting function smells:

def sort(dictionary):
    return sorted(dictionary.items(), key=lambda v: int(v[0].split("-")[0]))

Instead of splitting a string and parsing to int,
my instinct says there should be a way to use the original int values directly.

In general, the code formats these strings from int values,
and it seems silly to later un-format the strings so you can sort.
There should be a better way.

The Positions class stinks.
It's called "Positions",
but its fields are "name" and "content".
Sounds more like a key-value pair.

Then there's this code with the comment:

   self.content = {}  # creates a new empty list for each bracket

The comment talks about "list", but the code uses a dictionary.

And there's the push method:

def push(self, pos, content):
    self.content[pos] = content

push is a term commonly used with stacks, or sometimes with lists.
But here the method puts a value into a dictionary,
an operation normally called "put".

Alternative implementation

Based on the above suggestions,
and reorganizing to more meaningful elements,
you could do something like this:

from functools import total_ordering

@total_ordering
class Item:
    def __init__(self, start, end, content):
        self.start = start
        self.end = end
        self.content = content

    @property
    def position(self):
        a = self.start
        b = self.end
        return '{}-{}'.format(a, b - 1) if a != b else repr(a)

    def __lt__(self, other):
        return self.start < other.start

def convert_string(string):
    start = 0
    internal_string = ''
    items = []
    for (pos, char) in enumerate(string):
        if char in '(){}[]<>':
            items.append(Item(pos, pos, char))
            if internal_string:
                items.append(Item(start, pos, internal_string))
                internal_string = ''
            start = pos + 1
        else:
            internal_string += char

    if internal_string:
        items.append(Item(start, len(string), internal_string))

    return items

Although with the implementation of __lt__,
I made it possible to sort simply by sorted(items),
actually there is no need to sort,
as the items are naturally sorted.

Note: the purpose of @total_ordering is to implement the other rich comparison operators like __le__, as recommended by PEP8.


HTML5 sidebar syntax I am trying to learn how to write clean HTML5 code and I came across a sidebar.

I need to get something like this:

My code:

Naujienos

2015-09-20

2013-10-30

Am I going in the right direction? Any feedback is appreciated.

The aside element already creates a section, there is no need for an additional section element.

If you use h1 for the aside element’s heading, the headings in the article elements should be h2 instead of h3 (or also h1, but that’s not recommended).

But using article for these short snippets might not be a good idea in the first place (unless they will contain more than just a date and an image), because each article also creates a section.

For the date you should use the time element.


Optimizing Dijkstra on grid Python (Microsoft Interview) Given a square grid of size N, each cell of which contains integer cost which represents a cost to traverse through that cell, we need to find a path from top left cell to bottom right cell by which the total cost incurred is minimum.
From the cell (i,j) we can go (i,j-1), (i, j+1), (i-1, j), (i+1, j).
Note: It is assumed that negative cost cycles do not exist in the input matrix.
Example 1:

Input: grid = {{9,4,9,9},{6,7,6,4},
{8,3,3,7},{7,4,9,10}}
Output: 43
Explanation: The grid is-
9 4 9 9
6 7 6 4
8 3 3 7
7 4 9 10
The minimum cost is-
9 + 4 + 7 + 3 + 3 + 7 + 10 = 43.

My working code:
global INT_MAX

# value of n is passed by the driver along with the grid
INT_MAX = 3 ** 38

class Solution:

    # <- this returns the minVertex's co-orinates ->
    def minimumVertex(self,distMat,visited):
        minVertexi = 0
        minVertexj = 0
        max1 = INT_MAX
        for i in range(n):
            for j in range(n):
                if distMat[i][j] < max1 and visited[i][j] == False:
                    max1 =  distMat[i][j]
                    minVertexi = i
                    minVertexj = j
        return minVertexi, minVertexj

    def minimumCostPath(self, grid):

        # <- visited matrix is created with all False ->
        visited = []
        for i in range(n):
            eachRow = []
            for j in range(n):
                eachRow.append(False)
            visited.append(eachRow)
        # <- distMat is created with all values initialized with INT_MAX ->
        distMat = []
        for i in range(n):
            eachRow1 = []
            for j in range(n):
                eachRow1.append(INT_MAX)
            distMat.append(eachRow1)
        distMat[0][0] = grid[0][0]

        drow = [-1, 1, 0, 0]
        dcol = [0, 0, -1, 1]

        for _ in range(n):
            for currj in range(n):
                mini,minj = self.minimumVertex(distMat,visited)

                visited[mini][minj] = True
                for k in range(len(drow)):
                    nbri = mini + drow[k]
                    nbrj = minj + dcol[k]
                    # print("outside nbri = ",nbri,"outside nbrj = ",nbrj)
                    if nbri >=0 and nbrj >= 0 and nbri < n and nbrj < n:
                        if visited[nbri][nbrj] == False:
                            # print("inside nbri = ",nbri,"inside nbrj = ",nbrj)

                            if distMat[nbri][nbrj] >  distMat[mini][minj] + grid[nbri][nbrj]:
                                distMat[nbri][nbrj] = distMat[mini][minj] + grid[nbri][nbrj]
        return distMat[n-1][n-1]

Expected Time Compelxity: O(n2*log(n))
Expected Auxiliary Space: O(n2)
Problem with my code:
My logic is alright however it is exceeding the time limit, how do I optimize it?
NOTE: The grid is always a square grid and N (dimension) is passed to the function along with the grid. I took N = 4 here because I was debugging the code for a single test case.

Actual time complexity
As you correctly state, the expected time complexity of Dijkstra's on a square matrix is O(n^2 log n). However, your algorithm has a complexity of at least O(n^4) due to the way you locate the minimum vertex.
for _ in range(n):
    for currj in range(n):
        # <--- executed n^2 times here
        mini,minj = self.minimumVertex(distMat,visited)
        ...

def minimumVertex(self,distMat,visited):
    minVertexi = 0
    minVertexj = 0
    max1 = INT_MAX
    for i in range(n):
        for j in range(n):
            # <--- takes n^2 time
            ...

In fact, the algorithm you implemented isn't purely Dijkstra's, but a blend of Dijkstra's algorithm and Floyd's algorithm.
You should read through the links above, particularly the Pseudocode sections to understand how each of the algorithms works, and the parts that you're mixing up.
Dijkstra's algorithm
The key data structure which allows Dijkstra's algorithm to work in O(n^2 log n) time is a priority queue which allows you to insert elements and remove the top element in O(log n) time.
This data structure is commonly implemented using a binary heap, and has a standard implementation in Python as the heapq library.
You should adapt your code to use this data structure instead of the distance table as shown below.
It is important that the first field of the tuple is the distance, since this determines the sorting.
def minimumCostPath(self, grid):
    # definitions of n, visited, drow, dcol

    queue = [(grid[0][0], 0, 0)]
    while queue:
        dist, i, j = heapq.heappop(queue)

        if visited[i][j]:
            continue
        visited[i][j] = True

        if i == j == n - 1:
            return dist

        for k in range(len(drow)):
            ni = i + drow[k]
            nj = j + dcol[k]
            if 0 <= ni < n and 0 <= nj < n:
                heapq.heappush(queue, (dist + grid[ni][nj], ni, nj))

    assert False, "queue is exhausted but we haven't reached the destination yet"

Benchmarking surprise
After writing all of this, I decided to quantify the improvement by benchmarking.
This is the code I used to see how my implementation compares against the original.
if __name__ == "__main__":
    import time
    N = 100
    grid = [[j for _ in range(N)] for j in range(N)]
    solver = Solution()

    start = time.time()
    print(solver.minimumCostPath(grid))
    end = time.time()
    print(f"Solved grid with size {N} in {1000*(end - start):.3f}ms")

To my surprise, your version was actually faster, but more importantly it gave incorrect results.

My logic is alright

No, it's not.
The issue is the usage of n in the minimumVertex function. Because you define n = 4 as a global, this value is used instead of the actual size of the grid. This makes your algorithm appear fast, but actually be incorrect for n > 4.
Conclusion
As it turns out, while the algorithm you (I imagine) intended to implement is O(n^4), your code was actually O(n^2) all along. If this really resulted in TLE, Python is not going to be able to handle this question.
What I find much more likely is that your code failed due to an incorrect response - using the correct implementation of Dijkstra's above is likely to fix this.


Button incrementing from 0 to 9 and repeating I have to create a button which counts from 0 to 9, and after 9 come 0 again. One of my friends said that this code is not good enough. Can you explain to me why it's a problem to use -1?

int szám = 0;
private void counterButton_Click(object sender, EventArgs e)
{
    szám++;
    this.Text = szám.ToString();
    if (szám == 9)
    {
        szám = -1;
    }
}

Your code is ok. But there are always other ways to get the same result which may be more elegant.

private void counterButton_Click(object sender, EventArgs e)
{
    szám=(szám +1) % 10;
    this.Text = szám.ToString();
}

or you can do:

private void counterButton_Click(object sender, EventArgs e)
{
    szám=(9==szám)?0:szám+1;
    this.Text = szám.ToString();
}


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

template<typename Element>
size_t partition_quickSort(Element arr[], size_t start, size_t end) {
    auto pivot = arr[end];
    size_t index_partition = start;

    for (size_t i = start; i < end; ++i) {
        if (arr[i] <= pivot) {
            std::swap(arr[index_partition++], arr[i]);
        }
    }

    std::swap(arr[index_partition], arr[end]);
    return index_partition;
}

template<typename Element>
void quick_sort(Element arr[], size_t start, size_t end) {
    if (start < end) {
        size_t partition = partition_quickSort(arr, start, end);
        quick_sort(arr, start, partition - 1);
        quick_sort(arr, partition + 1, end);
    }
}
template<typename Element>
void quick_sort(Element arr[], size_t size) {
    quick_sort(arr, 0, size - 1);
}

There is just one thing wrong with your code.
Don't ever call std::swap using a qualified name. Doing so cuts out user-defined functions which might be better, or actually viable. Use using std::swap; swap(*pa, *pb); or maybe std::iter_swap(pa, pb);.
Next, there's on thing sub-optimal. You are passing the range, as well as start and end. Knowing where in the overall range the part you are currently processing is does not help, so you can eliminate an argument.
Work on genericity by following C++ conventions.
Useing half-open iterator-ranges allows wider application, you only need mutable forward-iterators.
Finally, Don't use <= for comparing the elements. All the standard algorithms only use <, and there's no compelling reason to break with that convention.

template <class ForwardIter>
void quicksort(const ForwardIter begin, const ForwardIter end) {
    if (begin == end) return;
    auto cur = begin;
    if (++cur == end) return;
    auto mid = begin;
    for (; cur != end; ++cur)
        if (*cur < *begin)
            std::iter_swap(cur, ++mid);
    std::iter_swap(begin, mid);
    quicksort(begin, mid);
    quicksort(++mid, end);
}


Verifying value before parsing I am writing some Java code and am coming across a try/catch block in which I decided to verify the value is not null before parsing it:

String parsedValue = DEFAULT_VALUE;
try {
  if (myValue != null) {
    parsedValue = parse(myValue);
  }
} catch (ParseException e) {
  log(e);
}

Regarding the code above, my question is, is it better to nest the if loop into the try/catch block or vice-versa?

In other words, is the previous code better or the following:

String parsedValue = DEFAULT_VALUE;
if (myValue != null) {
  try {
    parsedValue = parse(myValue);
  } catch (ParseException e) {
    log(e);
  }
}

EDIT: As @Brythan raised the issue, for simplicity, let's assume here that log is a method in a random logging library, and has the 2 signatures log(Exception) and log(String). Like that we avoid creating an Exception to log a message, it makes the code clearer.

As @h.j.k raised another issue, here myValue could be null, this is no exceptional case, but if myValue has been assigned a value (not null), it should be parseable, and only in that case I expect an Exception to be thrown by parse(String).

I would put the try/catch over as little code as possible.  This also makes it easier to add an else case to the if statement if necessary.  For example:

String parsedValue = DEFAULT_VALUE;
if (myValue != null) {
  try {
    parsedValue = parse(myValue);
  } catch (ParseException e) {
    log(e);
  }
} else {
  log(new Exception("Empty myValue!"));
}

This allows for more flexibility than the other version.

As a general rule, it's best to put as little code inside a try block as possible.  Only things that can throw an exception that should be caught by the catch and connecting code.  One reason is that other code than you originally intended may throw the exception and the solution in the catch block may not apply to the new case.


Strategy pattern for four arithmetic operations I am studying design patterns, I was wondering if my approach makes sense.
I am trying to implement the strategy pattern. I think I have captured the essence which is

Define a family of algorithms,
encapsulate each one,
and make them interchangeable,

as shown in the example code below.

Have I captured the intent, am I missing something from the example?

#include "stdafx.h"
#include <iostream>

#include <iostream>
#include <fstream>
#include <string>

enum OperationType
{
    Add = 1, Subtract, Multiply, Divide
};

class Operation
{
public:
    Operation(int Value1, int Value2) :
        m_Value1(Value1),
        m_Value2(Value2) {      }

    virtual int Calculate() = 0;

private:

    int m_Result;
    int m_Value1;
    int m_Value2;

};

class Addition : public Operation
{
public:
    Addition(int Value1, int Value2) : Operation(Value1,Value2),
        m_Value1(Value1), m_Value2(Value2) {}

    int Calculate()
    {
        m_Result = m_Value1 + m_Value2;
        return m_Result;
    }

private:

    int m_Result;
    int m_Value1;
    int m_Value2;

};

class Subtraction : public Operation
{
public:
    Subtraction(int Value1, int Value2) : Operation(Value1, Value2),
        m_Value1(Value1), m_Value2(Value2) {}

    int Calculate()
    {
        m_Result = m_Value1 - m_Value2;
        return m_Result;
    }

private:

    int m_Result;
    int m_Value1;
    int m_Value2;

};

class Multiplication : public Operation
{
public:
    Multiplication(int Value1, int Value2) : Operation(Value1, Value2),
        m_Value1(Value1), m_Value2(Value2) {}

    int Calculate()
    {
        m_Result = m_Value1 * m_Value2;
        return m_Result;
    }

private:

    int m_Result;
    int m_Value1;
    int m_Value2;

};

class Division : public Operation
{
public:
    Division(int Value1, int Value2) : Operation(Value1, Value2),
        m_Value1(Value1), m_Value2(Value2) {}

    int Calculate()
    {
        m_Result = m_Value1 / m_Value2;
        return m_Result;
    }

private:

    int m_Result;
    int m_Value1;
    int m_Value2;

};

class Test
{
    public:

        Test()
        {
            m_Operation = NULL;
        }

        void SetOperation(int Value1, int Value2, int Operation)
        {
            switch (Operation)
            {
                case Add:
                {
                    m_Operation = new Addition(Value1, Value2);
                }
                break;

                case Subtract:
                {
                    m_Operation = new Subtraction(Value1, Value2);
                }
                break;

                case Multiply:
                {
                    m_Operation = new Multiplication(Value1, Value2);
                }
                break;

                case Divide:
                {
                    m_Operation = new Division(Value1, Value2);
                }
                break;
            }
        }

        int PerformOperation()
        {
            return m_Operation->Calculate();
        }

private:
    Operation *m_Operation;

};

int main()
{
    Test test;

    test.SetOperation(1, 2, OperationType::Add);
    std::cout << test.PerformOperation() << std::endl;

    test.SetOperation(2, 3, OperationType::Subtract);
    std::cout << test.PerformOperation() << std::endl;

    test.SetOperation(3, 4, OperationType::Multiply);
    std::cout << test.PerformOperation() << std::endl;

    test.SetOperation(40, 10, OperationType::Divide);
    std::cout << test.PerformOperation() << std::endl;

    int x;
    std::cin >> x;
    return 0;
}

Overkill

This program is using dynamite to crack nuts. Choosing one operation of four is a very simple problem, easily handled with an enum and a switch. Indeed, your program already contains those, so the strategy classes are just unnecessary complexity.

I'm going to ignore that and review this program as just an exercise in using strategies. But if you were actually trying to solve a problem, this would be a horribly complex design.

Unfortunately, this makes it hard to judge how well you're using strategies, because you aren't using them for anything. It's also hard to advise how to simplify the program, because everything is unnecessary complexity.

How to use strategies

The Design Patterns movement is not a good guide to how to structure programs. Most of the techniques it teaches are rather specialized, and strategies are one of those. They're a useful tool to know, but they're only one tool of many, and you won't use them regularly.

When implementing strategies, you should prefer λ to custom classes. Fall back to classes only if λ isn't available or won't work — for example, if the strategy has more than one operation, or needs to be saved to a file.

Using λ is also a better way to learn to use strategies. If you're comfortable with first-class functions, and use them without thinking they're a Fancy Design Pattern, then strategies look like an obvious (but clumsy) variation, so it will be easy to use them correctly.

If you aren't already comfortable with λ and functional programming, that's far more important to learn than design patterns.

Repetition

Now for some actual code review.

The three private members of Operation (m_Result, m_Value1, m_Value2) are unused. And they're private, so the subclasses can't use them either, and must redefine them! If they're part of Operation's interface, they should be public or at least protected.

Unnecessary state

There's no reason for the strategies to store m_Value1, m_Value2 or m_Result. Calculate can simply take them as arguments and return the result. The interface could be like this:

class Operation {
    virtual int Calculate(int a, int b) = 0;
};

Similarly, Test::SetOperation modifies this for no good reason. Is this just to have an excuse to store an Operation somewhere? It could simply return an Operation, or it could perform the operation directly without storing it anywhere. Then there's no reason for Test to be a class at all. It could simply be a function.

C++

Don't make a habit of using std::endl: it flushes the stream, which is seldom what you want.

Test leaks its Operations. This is harmless in a toy program, but you should habitually use std::unique_ptr for owned data, so you don't have leaks when they matter.

Superficial details

I see from stdafx.h that you're writing in Microsoft style, so I won't complain about the m_ prefixes.

There are a lot of unnecessary blank lines. I don't know if these are also a deliberate stylistic feature, but they're usually thought to make the program harder to read, because you can't see as much of it at once.

I suppose the std::cin >> x at the end is a hack to make the program's window not disappear instantly? You shouldn't have to do this. If you're running the program from an IDE, it should have an option to fix this for you; if you run it from the command line, this problem should not arise.

A better strategy problem

If you'd like a better problem to practice using strategies, try implementing a game like fairy chess, where there are a wide variety of pieces with unique moves. Don't make a subclass for each one; instead, the constructor should take a strategy (represented as a function) to generate the piece's possible moves. The usage might look like this (with functions that return move generators):

Piece camel("camel", "L", Leaper(1, 3));
Piece rook("rook", "R", Rider(1, 0));

If that's too complicated or doesn't sound fun, try a simpler game with a variety of pieces or cards or actions, where each one takes a function (not a class) to calculate what it does.


Split device path string in C So here is my input

\\?\usb#vid_04d9&pid_1702#6&12e3c9ed&0&2#{a5dcbf10-6530-11d2-901f-00c04fb951ed}

I need to extract 04d9 and 1702 from this string. Instead of copying the values I just point my pointer to their respective locations.

And here is my horrible code

#DEFINE FIRST_STRING 1
#DEFINE SECOND_STRING 2
#DEFINE THIRD_STRING 3

VOID
StringDelimiterReplace(
    PCHAR string,
    INT delimiter,
    INT replace
)
/*
Description:
    Replaces the delimiter in the string with the replace value specified
*/
{
    size_t stringLen = strlen(string);
    for (size_t size = 0; size < stringLen; size++) {
        if (string[size] == delimiter) string[size] = replace;
    }
}

VOID
GenericStringSplit(
    PCHAR string,
    INT delimiter,
    PCHAR *dest,
    INT tokenNum,
    INT replace
)
/*
Description:
    A generic string splitting function which returns the string at token number
    specified,
    Also provision to replace the delimiter with string
*/
{
    int i = 1;
    size_t size;
    if( delimiter != replace) StringDelimiterReplace(string, delimiter, replace);
    if (tokenNum == 1) *dest = string;
    for (size = 0; ; size++) {
        if (string[size] != replace) continue;
        if (i == tokenNum - 1) break;
        if (i > tokenNum - 1) return;
        i++;
    }
    *dest = &string[++size];
}

//main
GenericStringSplit(string, '#', &dest, SECOND_STRING, '\0');
GenericStringSplit(dest, '&', &vendorId, FIRST_STRING, '\0');
GenericStringSplit(dest, '\0', &productId, SECOND_STRING, '\0');
GenericStringSplit(vendorId, '_', &vendorId, SECOND_STRING, '\0');
GenericStringSplit(productId, '_', &productId, SECOND_STRING, '\0');

Here are some things that may help you improve your code.

Use the correct pragma

There is no #DEFINE in standard C.  It should instead be #define.  Case is significant in C.  Similarly, INT and VOID are not keywords in standard C, but int and void are.

Fix the formatting

The code is diabolically difficult to read with its current formatting.  First, put the description above the function itself.  Second, write a meaningful comment; the current description of GenericStringSplit is so vague and confusing as to be useless.  Third, don't put multiple statements on a line.  For example, instead of this:

if (string[size] == delimiter) string[size] = replace;

write this:

if (string[size] == delimiter) {
    string[size] = replace;
}

It's wise to get into the habit of using braces even with a one-liner.  Doing so lessens the likelihood of introducing subtle bugs as you maintain the code.

Use the required #includes

The code uses strlen and size_t both of which indicate that the code needs #include <string.h>.  It was not difficult to infer, but it helps reviewers if the code is complete.

Use standard functions

Your StringDelimiterReplace is, as it's currently used, very similar to the standard strtok function.  It could also benefit from using the standard strchr function.

/*
 * replaces all instances of delimiter with replace in string.
 *
 * Note that the passed string is modified in place and so cannot
 * be const and cannot be NULL.  Also, the NUL char ('\0') cannot
 * be used as the delimiter. The use of an int that does not
 * fit within a char for either the delimiter or the replace
 * parameters is not supported.  Wide strings are also not supported.
 *
 * In short, don't actually use this function in production code.
 */
static void StringDelimiterReplace(char *string, int delimiter, int replace) {
    while (NULL != (string = strchr(string, delimiter))) {
            *string++ = replace;
    }
}

Note that this has the effect of only traversing the string once instead of twice in the original code (once for strlen and again in the loop).

Do one thing well

The GenericStringSplit does multiple things.  It modifies the passed string and it passes back a pointer.  I'd strongly suggest that the code would be easier to understand if each function only did one thing.

Allow the use of const strings

The current code modifies the passed string in place, but an enhancement would be to allow const strings to be passed in.

Rethink the interface

I'd suggest that an alternative approach would be to actually look for "vid_" or "pid_" and then extract the corresponding ID.  One approach:

unsigned hexval(const char *string, const char *target) {
    char *found = strstr(string, target);
    if (found == NULL) {
        return 0;
    }
    return 0xffff & strtoul(&found[strlen(target)], NULL, 16);
}

Usage:

unsigned vid=hexval(string, "vid_");
unsigned pid=hexval(string, "pid_");
printf("%4.4x:%4.4x\n", vid, pid);