Estimating the number of tanks based on a sample of serial numbers I created code that was supposed to determine how well my system of guessing is.

The scenario is that there are n amount of tanks. Each tank is uniquely numbered 1 to n. Seven of these tanks are captured so we only know their serial number (This is done by getting a random number from 1 to n for each of the seven tanks).

I am supposed to predict n based on those serial numbers. My method (the one I have to code) was to first find the max number in the list and the number three standard deviations above the mean of the list. Then, I get my own random numbers from 1 to n where n ranges from the max number and the three standard deviations number.

During this time, I calculate the standard deviation for each of the seven serial numbers generated and after all of the standard deviations are calculated, I find which one is closest to the standard deviation of the original list. Whichever one is the closest to the standard deviation would constitute as my guess for n. I repeat this process as many times as necessary (the more times the more accurate results) to generate a list of guesses. My final guess then would be the mean of that list.

import random
import statistics

chosenNum = 429 #n is given only to check how well our process works
numRuns = 100 #number of guesses - my teacher wants at least 100
numCorrect = 0
numGuessN = []
guesses = []
percentErrors = []
for x in range(numRuns):
  randNum = [random.randint(1,chosenNum + 1),random.randint(1,chosenNum + 1),random.randint(1,chosenNum + 1),random.randint(1,chosenNum + 1),random.randint(1,chosenNum + 1),random.randint(1,chosenNum + 1),random.randint(1,chosenNum + 1)] #generates the seven serial numbers.
  NumSTD = statistics.stdev(randNum) #standard deviation
  maxNumSTD = statistics.mean(randNum)+(3*(statistics.stdev(randNum))) #three standard deviations above the mean
  maxNum = max(randNum) #max number in list
  #print (NumSTD)
  #print (maxNumSTD)
  #print (maxNum)
  #print (randNum)
  for x in range(200): #the greater the range, the more accurate the results
    STDarray = []
    if (maxNum - maxNumSTD < 0):
      for y in range(maxNum,int(maxNumSTD)): #n is max number to max number from Standard Deviation
        randNumZwei =  [random.randint(1,y),random.randint(1,y),random.randint(1,y),random.randint(1,y),random.randint(1,y),random.randint(1,y),random.randint(1,y)] #my simulated serial numbers
        testNumSTD = statistics.stdev(randNumZwei)
        STDarray.append(testNumSTD)
    else:
      for y in range(int(maxNumSTD),maxNum):
        randNumZwei =  [random.randint(1,y),random.randint(1,y),random.randint(1,y),random.randint(1,y),random.randint(1,y),random.randint(1,y),random.randint(1,y)]
        testNumSTD = statistics.stdev(randNumZwei)
        STDarray.append(testNumSTD)
    for z in range((len(STDarray) - 1)):
      if((min(STDarray, key=lambda x:abs(x-NumSTD))) == STDarray[z]): #find closest number to original standard deviation
        numGuessed = z + maxNum
    numGuessN.append(numGuessed) #make a list of all the guessed numbers
  guess = int(statistics.mean(numGuessN)) #the final guess is simply mean of all the other guesses generated
  #print(guess)
  guesses.append(guess) #number of final guesses should be the same as number of trials
print ("Your guesses are: " + str(guesses))
for x in range(len(guesses) - 1):
  percentError = (abs(guesses[x] - (chosenNum))/float(chosenNum)) * 100
  percentErrors.append(percentError)
  if(guesses[x] == chosenNum):
    numCorrect = numCorrect + 1
  else:
    closestNumber = min(guesses, key=lambda x:abs(x-chosenNum))
averagePercentError = statistics.mean(percentErrors)
print ("The average Percent Error is: " + str(averagePercentError) + "%")
if (numCorrect > 0):
  print ("You got the correct number " + str(numCorrect/float(len(guesses))))
else:
  print ("Your closest number was: " + str(closestNumber))

This code works but takes way too long to give me my result. The whole point of this is to calculate accuracy but not take way too long. How can I make this code more efficient to run faster?

I'm not going to comment on speed, only on coding style. For speed you should probably look at the python numerics library numpy (often abbreviated to np) especially Quickstart, Random numbers and Statistics sections.

The biggest problem with the code is that one huge block is doing everything. There aren't any functions; the code is generating random numbers, attempting to reverse-engineer them, and then testing performance. It's very hard to distinguish what part of the code is responsible for what; I had trouble piecing it together until I read over several times. It would be very hard to re-use portions of this code, even with copy and paste.

These should all be separate functions. This won't get you any speed boost but will make analysis much easier. That way you can update each function on its own without having to comb through your code for where the logic is located. Each function should have one job, this is called the single responsibility principle.

In addition, there should be a main() type function that acts as the driver for all the other functions. And the only line that ought to be at top-level of the file is the main guard:

    if __name__ == "__main__":
        main()

In particular, main() should pass the parameters in to the functions that actually do the work. Don't rely on global variables; again it makes the code impossible to use elsewhere. Variables should be scoped to where they are actually used.

Use _ for dummy variables in loops

for x in range(numRuns):

Since the loop index is not being used, convention is to use _ for the variable name.
Don't repeat yourself, make sure code is modular.

randNum = [random.randint(1,chosenNum + 1),random.randint(1,chosenNum + 1),random.randint(1,chosenNum + 1),random.randint(1,chosenNum + 1),random.randint(1,chosenNum + 1),random.randint(1,chosenNum + 1),random.randint(1,chosenNum + 1)] #generates the seven serial numbers.

This line is too long, has repeated code, and is fixed at 7. What if the problem requirements change, and you need to solve the problem for capturing 13 tanks? Much better to have the number of captured tanks be a parameter, then use a list comprehension to call the randomness repeatedly. (I think since you used randint you don't need to add the + 1. randint is already inclusive. randrange is half-inclusive.) I suggest

tankSerialNumbers = [random.randint(1,chosenNum) for _ in range(numCapturedTanks)]

Choose meaningful variable names, and don't repeat yourself

maxNumSTD = statistics.mean(randNum)+(3*(statistics.stdev(randNum))) #three standard deviations above the mean

You just calculated stdev(randNum) on the line above, why calculate it again? And maxNumSTD is a poor variable name as it does not describe the variable at all. It's not a max of anything, and it's certainly not a standard deviation. It's a guess as to the highest tank number, so call it that!

highestTankSerial = statistics.mean(randNum) + 3*NumSTD

Don't repeat yourself. Seeing a trend here? In this section

if (maxNum - maxNumSTD < 0):
  for y in range(maxNum,int(maxNumSTD)): #n is max number to max number from Standard Deviation
    randNumZwei =  [random.randint(1,y),random.randint(1,y),random.randint(1,y),random.randint(1,y),random.randint(1,y),random.randint(1,y),random.randint(1,y)]
    testNumSTD = statistics.stdev(randNumZwei)
    STDarray.append(testNumSTD)
else:
  for y in range(int(maxNumSTD),maxNum):
    randNumZwei =  [random.randint(1,y),random.randint(1,y),random.randint(1,y),random.randint(1,y),random.randint(1,y),random.randint(1,y),random.randint(1,y)]
    testNumSTD = statistics.stdev(randNumZwei)
    STDarray.append(testNumSTD)

the code is almost exactly the same in the if and else blocks. Compare the two elements and swap them if necessary, then you don't need to duplicate your code.
Use Python's list features. In your code,

for z in range((len(STDarray) - 1)):
    if((min(STDarray, key=lambda x:abs(x-NumSTD))) == STDarray[z]): #find closest number to original standard deviation
        numGuessed = z + maxNum

you do use both the array index and the value. That's what enumerate is for. Also why are you skipping the last element of the list? And why are you recalculating the min each time through the loop? And why aren't you jumping out of the loop right away?

minStd = min(STDarray, key=lambda x:abs(x-NumSTD))
for (z,this_std) in enumerate(STDarray):
    if(minStd == this_std): #find closest number to original standard deviation
        numGuessed = z + maxNum
        break

But wait a minute. The whole goal of this is just to get the index of the minimal element. That's easy to do in one step with enumerate using tools you already know about.

(z, _) = min(enumerate(STDarray), key=lambda (_,x):abs(x-NumSTD))
numGuessed = z + maxNum


Optimize parsing more and more I have the below C# code for parsing. Do you think this is most optimized or  should I use a generic method in these functions themselves?

public static bool GetDBBool(object value)
{
    var result = false;

    if (value != null && string.IsNullOrEmpty(value.ToString()) == false)
        bool.TryParse(value.ToString(), out result);

    return result;
}

public static int GetDBInt(object value)
{
    var result = -999;

    if (value != null && string.IsNullOrEmpty(value.ToString()) == false)
        int.TryParse(value.ToString(), out result);

    return result;
}

public static double GetDBDouble(object value)
{
    var result = -999.00;

    if (value != null && string.IsNullOrEmpty(value.ToString()) == false)
        double.TryParse(value.ToString(), out result);

    return result;
}

public static DateTime GetDBDate(object value)
{
    var result = DateTime.Now;

    if (value != null && string.IsNullOrEmpty(value.ToString()) == false)
        DateTime.TryParse(value.ToString(), out result);

    return result;
}

public static DateTime? GetDBNullableDate(object value)
{
    DateTime date;
    if (value != null && string.IsNullOrEmpty(value.ToString()) == false)
        return DateTime.TryParse(value.ToString(), out date) ? date : (DateTime?)null;
    else
        return null;
}

== false is not very readable. Use ! instead.

If the "object value" is already of desired type (e.g. DateTime), it would be slower to convert it to string and then back again.

I would write instead

if (value is DateTime) return (DateTime)value;

I would not use var result = -999; as a return value.

If the conversion fails, you can either return Int32.MinValue (better than -999) or rather null.

Maybe also you would like to check for DBNull.Value?
So first I would check for null and DBNull.Value, then "value is DateTime" check and the last thing would be DateTime.TryParse.


lexical scope v let in scheme functions I've been chewing through a bit of The Little Schemer and the Peano examples got me thinking about operation size and time.

I got some help on making Peano Multiplication linear -- however (time ...) didn't show much difference between a lexically scoped version and the let-loop version.

I do however find a massive difference between the lexically scoped and let-loop versions when minting a Peano Exponent function.

The obvious question is why it makes such a difference (or perhaps it does not and my attempt at a lexically scoped Peano Exponent has some fatal error)?

-- i had thought i understood what the let-loop was doing in terms of storing the value in the parent enclosure (making the two roughly equivalent); in which way is this wrong?

;; lexically scoped version of Peano Exp
(define (exxp-lex b ex)
  (define (exxp-aux ex prod)
    (cond
      ((zero? ex) 1)
      ((= ex 1) (mX-let b prod))
      (else
        (exxp-aux (- ex 1) (mX-lex b prod)))))
  (exxp-aux ex 1))

;; let version of Peano Exp
(define (exxp-let b ex)
  (let loop ((ex ex)
              (prod 1))
    (cond
      ((zero? ex) 1)
      ((= ex 1) (mX-let prod b))
      (else
        (loop (- ex 1) (mX-let prod b))))
             ))

If it's relevant, I am using petite-chez scheme.

For reference, here are the Peano Multiplication functions.

;; a lexically scoped version Peano Multiplication
(define (mX-lex n m)
  (define (mX-aux m product)
    (if (zero? m)
      product
      (mX-aux (- m 1) (+ product n))))
  (mX-aux m 0))

;; using let for Peano Mult
(define (mX-let n m)
  (let loop ((m m)
             (product 0))
    (if (zero? m)
      product
      (loop (- m 1) (+ product n)))))

In exxp-aux, you call (mX-let b prod) and (mX-lex b prod).  You probably meant the two calls to be identical.

In exxp-let, you call (mX-let prod b).

mX-lex and mX-let are both implemented such that they would be faster if you call them with the larger multiplicand first and the smaller one second, because the second parameter controls the number of additions.  Since prod is likely to be larger than b, your exxp-aux will be slower.


Median of a Vector assignment I've been reading Bjarne's Principles in Programming and was given some code to demonstrate the usage of vectors and how we can return specific values for certain functionality such as mean and median. However, in the chapter I noticed that the code he provided was purposely made so that it doesn't always calculate the median correctly which was correct, he had purposely made it so we had to go back and alter the code as follows:

#include "../../std_lib_facilities.h"

int main()
{
  vector<double> temps;
  for (double temp; cin >> temp;)
      temps.push_back(temp);

  sort(temps);
  cout << "Median temperature: " << temps[temps.size() / 2] << '\n';
}

However, when asked in a question format it states:

  If we define the median of a sequence as “a number so that exactly as many elements come before it in the sequence as come after it,” fix the program in 4.6.3 so that it always prints out a median. Hint: A median need not be an element of a sequence.

I assumed he meant that we want equal values on both sides of our median as the previous algorithm only divides the vector size by 2 and returns, if odd the middle index + 1, therefore I'm assuming that he wanted the exact median value of the range rather than a specific element within the array? Regardless, the code I had created follows:

//Program to compute the median of a temperature vector
#include "../../std_lib_facilities.h"

int main()
{
    //Define variables and Read in values
    double median_value;
    vector<double> temps;
    for (double temp; cin >> temp;)
        temps.push_back(temp);

    //Compute Median Temperatures
    sort(temps);

    if (temps.size() % 2 == 0)
        median_value = (temps[temps.size() / 2] + temps[temps.size() / 2 - 1]) / 2;
    else
        median_value = temps[temps.size() / 2];

    cout << "Median Temperature of input values: " << median_value << '\n';
    for (int i = 0; i < temps.size(); ++i) cout << temps[i] << ", ";
}

I'm assuming this is what he meant by “a number so that exactly as many elements come before it in the sequence as come after it". I've probably missed the point of this exercise hence the post for some verification that I completed the exercise correctly.

#include "../../std_lib_facilities.h"

While PPP is structured around using this header, you will eventually be including only what you really need to make your code self-sufficiently compilable.  So instead of including a bunch of libraries you don't need, just include what you use.

#include <iostream>
#include <vector>

The header Stroustrup provides also imports all visible symbols in namespace std into the global namespace.  The global namespace is then polluted with a bunch of names that could lead to collisions.  The compiler could resolve function overloads in unexpected ways.  Essentially, using namespace std; is considered bad practice.  If you must use using directives, keep the use in the narrowest scope possible.  Otherwise, qualify names with their namespace as the namespace will help the reader understand what is being called and from where.

    double median_value;

Avoid declaring variables before you need them.  This technique is an artifact of older programming languages that didn't allow initialization of variables and constants after a statement.  Errors commonly associated with this technique include use-before-set, incorrectly initialized variables, and possibly non-contextual reuse due to programmer laziness.

The simplest solution is to declare the variable as close to its initialization point.

    double median_value;
    if (temps.size() % 2 == 0)
        median_value = ...;

Even better, directly initialize the variable with a value.

    const auto median_value = median(temps);

This reduces the chance of errors happening as well as reducing the cognitive load for readers.  Directly initializing variables also allows you to take advance of const/constexpr.

    sort(temps);

Do you need to sort the entire container? When we're looking to find the median, we're only interested in picking out either the middle value or the middle two values.  Rather than sort, we would rather select.  C++ implementations typically provide introselect for its selection algorithm (std::nth_element).

As you learn more about <algorithm> and iterators, keep this in mind.

        median_value = (/* ... */ + temps[temps.size() / 2 - 1]) / 2;

It's really important to keep in mind the possible states a variable can represent.  In the case of temps, that std::vector<double> could represent any number of doubles as well as nothing.  When temps is empty, temps.size() / 2 - 1 evaluates to 0 / 2 - 1. It's important to note that temps.size() returns size_type, an alias for an unsigned value.  Thus, subtracting one from unsigned zero results in the maximum value held by std::vector<double>::size_type. It's essentially a really large value almost guaranteed to be out of bounds when used in your context.  Out of bound index access is a common case of undefined behavior.

If the user doesn't provide you with values, you can't calculate a median.  temps having at least one value is a precondition for calculating the median.  Check your preconditions and notify the user if they mess up.  How you notify the user is up to you, but keep this in mind as you go over chapters on error and exception handling.

    for (int i = 0; i < temps.size(); ++i) cout << temps[i] << ", ";

As stated before, temps.size() returns an unsigned value.  The comparison i < temps.size() is a signed-unsigned mismatch.  If temps contains more values than i can represent, you'll have a bug.

If you are simply accessing the elements and don't need to apply transformations on the index, use the range-based for loop.

    for (auto temp : temps) std::cout << temp << ", ";


Project Euler 76: Counting summations Project Euler Problem 76 asks:

  How many different ways can one hundred be written as a sum of at least two positive integers?

My code is correct but just works slowly.

import time
start = time.perf_counter()

def con(H):
    J = []
    Q = []
    W = []
    for i in H:
        for j in range(len(i)-1):
            Cop = i[::]
            y = i[j]+i[j+1]
            del Cop[j:j+2]
            Cop.insert(j,y)
            J.append(Cop)
    for i in J:
        y =(sorted("".join(str(j) for j in i)))
        Q.append("".join(y))
    T = list(set(Q))
    for i in T:
        y = [int(j) for j in i]
        W.append(y)
    return W

num = 5
K = [[1 for i in range(num)]]
count = 0
for i in range(len(K[0])-2):
    K = con(K)
    print(K)
    count = count + len(K)
print(count+1)
end = time.perf_counter()
print(end-start,"second")

Here what this code does. For example, let us say num = 4. Then it creates an array of

[[1,1,1,1]]. Then it sums the consecutive numbers so our result becomes (from line 9-14)

[[2,1,1],[1,2,1],[1,1,2]] Since they are all same my code (from line 15-18) makes them just one function and returns ["112"] and then from 18-21 turns ["112"] to [[1,1,2]] and then I do this whole process again for the [[1,1,2]] and then I count the number of these arrays (from 25-29). The problem is its fast up to num = 20-30 but then it really slows down. Is there a way to increase the speed of my code or should I try a different algorithm?

The optimal solution to this is significantly simpler than what you have. We'll get to that in a second. First, let's examine your code:

Cop should not begin with a capital letter. Variables should be snake_case. Furthermore, cop is a bad name. At the least it should be copy (don't shorten names, it only makes things less clear--unless the shortening is a broadly understood acronym like HTML). But copy of what? From that name, I can't tell. I have to go hunting for the initialization of it (and all of the mutations to it) to figure out what it actually does. What are J, Q, and W? Again, they should be lowercase. But also, letters make awful variable names (except for maybe i and j for numeric loop variables).
n
Your code either has or is precariously close to an off by one issue with all of the +1ing that you're doing. Why is it print(count + 1) and not print(count)?
count = count + len(K) should be count += len(K)
[[1 for i in range(num)]] can just be [[1] * num]
PEP8 your code. It is messy and has little whitespace, which makes it hard to follow.
con is a poorly named function. What does that even mean? Clearly it doesn't count anything because you have to use it in a loop to get the answer to the problem.
Why are you maintaining Q as a list to only use it in T = list(set(Q))? Q should just be a set from the start, that's more efficient.
There's no need to do the list in T = list(set(Q)). You can iterate over T = set(Q) (without the need to create a list).
You need to comment this code. The fact that you need to spend a paragraph explaining how your code works is a pretty good sign of this. It would take a long time for me to walk through this an understand how each piece works. Comment things that aren't clear. Comment why not how. Add docstrings to functions. I wouldn't even know how to just use your code to get the answer to the problem. I assume setting num, but it's unclear to me if that gives the answer for 5 or 4, given that you do range(num) which is exclusive not inclusive (it's [0, 1, 2, 3, 4])

That's a not exhaustive list of issues with your code as it exists. But, we need to start from scratch, because what you have is way too complicated for the task at hand. If we take a step back and rethink the problem, we can come up with a much more elegant solution.

Let's start by looking at the problem:

  It is possible to write five as a sum in exactly six different ways:

4 + 1
3 + 2
3 + 1 + 1
2 + 2 + 1
2 + 1 + 1 + 1
1 + 1 + 1 + 1 + 1

  How many different ways can one hundred be written as a sum of at least two positive integers?

Here's a question: can we write out all of the ways to write four as a sum? Sure, it looks like:

3 + 1
2 + 2
2 + 1 + 1
1 + 1 + 1 + 1

Hmm, that looks suspiciously similar to the list above. Let's rewrite the sums for five. I'll put the parts that are unique to five in parenthesis:

(4 + 1)
(3 + 2)
3 + 1 (+ 1)
2 + 2 (+ 1)
2 + 1 + 1 (+ 1)
1 + 1 + 1 + 1 (+ 1)

We should notice a pattern here. Namely, the sums for five are the sums for four with + 1 added and an additional 2 new ways. The 2 new ways are just combinations of the first sum of four + 1. 3 + 1 + 1 if you merge the 1 + 1 is 3 + 2 and 3 + 1 + 1 if your merge the 3 + 1 is 4 + 1.

Similarly, we can write the sums for four as the sums of three with some extras added on:

(3 + 1)
(2 + 2)
2 + 1 (+ 1)
1 + 1 + 1 (+ 1)

This pattern continues all the way down to one where there is only one sum: 1 itself.

Hopefully, this pattern is hinting to you that there is a recurrence relation we could take advantage of. Namely, if we know the number of sums for 5 is the number of sums for 4 plus some extras. This goes all the way down to 1. This should hint to you that a recursive solution may be appropriate. However, there's a catch. While we can implement it recursively, it turns out that the dependency chain isn't linear. Each answer depends on every answer before it. Because of this, even though you could compute the answer recursively, it would be inefficient, because it would require recomputing the answer for every number below. But we're really close.

As an aside, there are other (perhaps more natural) phrasings of this problem, which may make this +1 structure more obvious. Consider you have n pennies. How many unique groups of all n pennies can you make (each group is like one of the terms in the addition). For example, if n = 5 then a group of 4 pennies and a group of 1 pennies (one of the sums for 5) is like 4 + 1.

The efficient solution recalls that we can take a dynamic programming approach where we memoize answers to smaller problems. We can do so by accumulating a list of the previous answers num_ways_to_sum and build this up from 1 up to the desired number:

def count_number_of_ways_to_sum(target_sum):
    """
    Compute the number of unique ways (order does not matter) of summing to target_sum
    using numbers from 1 to target_sum, exclusive.
    """
    if target_sum <= 1:
        raise ValueError('there are no numbers greater than 0 less than the target sum')

    # ith index contains the answer to the (i + 1)th subproblem
    num_ways_to_sum = [0] * (target_sum + 1)

    # There is only 1 way to sum to 1
    num_ways_to_sum[0] = 1

    # Consider sums involving numbers in [1, target_sum)
    for i in range(1, target_sum):
        # Every subproblem j >= i depends on i
        for j in range(i, target_sum + 1):
            # The number of ways to sum j includes all the ways to sum i
            # with all the ways to sum (j - i) appended (like the parenthesis)
            num_ways_to_sum[j] += num_ways_to_sum[j - i]

    return num_ways_to_sum[target_sum]

Note a few things:

We document and properly name the function
We are careful with domain. We raise ValueError when we are given an impossible target_sum. (1 has no solution, because there are no counting numbers less than 1 but greater than 0)
We name variables appropriately. It is clear what num_ways_to_sum is. We may even consider renaming i and j, although the comments perhaps make them clear enough (and given that they are only used over 3 lines, this information is close together and intelligible as a single unit.
We comment!
We use python standard library features appropriately (like lists, range, etc.)

Now, you should definitely test this with the unittest package.


AngularJS image size validation directive I've written a directive to validate the width and height of an image in AngularJS. I'm not a JS genius but I get along with it. So I would appreciate some feedback on this directives code if this can be done better in the context of Angular and the JS language itself.

angular.module('app').directive('imageFileValidator', ['fileReader', function (fileReader) {
return {
    restrict: 'A',
    require: '^ngModel',
    link: function ($scope, el, attr, ctrl) {
        // Binds the change callback for the element
        el.bind('change', function (e) {
            var file = ((e.srcElement || e.target).files[0]);

            fileReader.getImageSize(file, $scope).then(function(result) {
                var imageSize = result;

                if (attr.validateHeight != undefined) {
                    var heightRule = attr.validateHeight.split(' ');
                    var validHeight = validateImageSize(imageSize.height, heightRule[0], heightRule[1]);
                    ctrl.$setValidity('imageSizeY', validHeight);
                }

                if (attr.validateWidth != undefined) {
                    var widthRule = attr.validateWidth.split(' ');
                    var validWidth = validateImageSize(imageSize.width, widthRule[0], widthRule[1]);
                    ctrl.$setValidity('imageSizeX', validWidth);
                }
            });

            var validateImageSize = function(value1, operator, value2) {
                if (operator == '==') {
                    return (value1 == value2);
                }
                if (operator == '>=') {
                    return (value1 >= value2);
                }
                if (operator == '>') {
                    return (value1 > value2);
                }
                if (operator == '<=') {
                    return (value1 <= value2);
                }
                if (operator == '<') {
                    return (value1 < value2);
                }
                throw 'Invalid operator ' + operator + ' !';
            };
        });
    }
}
}]);

if (attr.validateHeight != undefined) {
    ...
}

if (attr.validateWidth != undefined) {
    ...
}

The parts != undefined are unnecessary and just add noise to your code; JavaScript automatically checks to see if a conditional is true, or that it is not a false value. And undefined is a false value.

throw 'Invalid operator ' + operator + ' !';

Don't just throw a string as an error: create your own error object. I recommend calling it InvalidOperatorError as the reason why the error would be thrown is if an invalid operator was passed into your function.

Here is what the class would look like:

/*
* An error thrown if an invalid operator
* is passed into a function.
*
* @param(string) -- The error message
*/
function InvalidOperatorError(message) {
    this.name = "InvalidOperatorError";
    this.message = message;
}

Notice how I added documentation to this error? You should add this to your function validateImageSize so you can describe what each argument means and what the return means. Also, so you can describe the purpose of the function.

Now, you can use an instance of this class when you are throwing an error. Here is what that will look like:

throw new InvalidOperatorError('Invalid operator ' + operator + ' !');

See? Now you have a descriptive error.


Finding the last successful route by ID using Python I have a script that iterates through a series of URLs that have the same pattern to parse them via BeautifulSoup. The URL structure is an aspx page that ends its route with a sequential id, very similar to this one here.

In my case the challenge is I'm iterating through several routes and I don't know the endpoint, so I don't know when to stop looping through the series.

My attempted remedy could be found in the below code sample. Here you see my soup function that makes a request to the url and returns the soup when the request returns a good status (200).

The next function is a binary search that loops through the range I give with a while loop which stops running when I get a last successful URL request.

import requests
from bs4 import BeautifulSoup

####
#
# Souped Up just makes the request and passes you the soup to parse when there is one available.  Just pass the URL.
#
####
def soupedUp(url):
    theRequest = requests.get(url, allow_redirects=False)
    if theRequest.status_code == 200:
        soup = BeautifulSoup(theRequest.text, "lxml")
    else:
        soup = None
    return soup

def binarySearch(theRange):
    first = 0
    last = len(theRange)-1
    while first <= last:
        middle = (first + last) // 2
        if soupedUp(url+str(middle)) is None:
            last = middle - 1
        else:
            first = middle + 1
    return middle

url = 'http://cornpalace.com/gallery.aspx?PID='

print(binarySearch(range(1,10000000)))

My functions are working, but I feel like there may be a faster, simpler or cleaner approach to find the last route in this URL.

Does anyone have a simpler way to handle this sort of looping through URLs while scraping the same pattern URL?

I'd be happy to see another approach at this, or any python modules that already offer this type of URL probing.

If I understand the problem correctly, the goal is still to parse all the available pages - you can initiate an endless loop and break it once the status code is not 200. And, we may reuse the same web-scraping session to improve on performance:

URL_TEMPLATE = 'http://cornpalace.com/gallery.aspx?PID={page}'

with requests.Session() as session:
    page = 1
    while True:
        response = session.get(URL_TEMPLATE.format(page=page), allow_redirects=False)
        if response.status_code != 200:
            break

         print("Processing page #{page}".format(page=page))

         soup = BeautifulSoup(response.text, "lxml")
         # parse the page

         page += 1


Formatter for converting links in a legacy markup to markdown This program converts legacy markup in the form seen in the tests to the resulting markdown seen in the test's respective comment.
The full program is as uploaded as a snippet, or you may see below
let tests = [
  "OneMatch$$$TwoMatch"                  // [One Match](OneMatch)[Two Match](TwoMatch)
, "OneMatch$$$TwoMatch$$$ThreeMatch"     // [One Match](OneMatch)[Two Match](TwoMatch)[Three Match](ThreeMatch)
, "E$$$scapeMatch"                       // Escape Match
, "SimpleMatch"                          // [Simple Match](SimpleMatch)
, "Prefixed$$$MatchyMatch"               // Prefixed [Matchy Match](MatchyMatch)
, "MatchMatchWith$$$Suffix"              // [Match Match With](MatchMatchWith) Suffix
, "MatchesWithPlural$$$s"                // [Matches With Plural](MatchesWithPlural)s
, "This$$$Gets$$$Escaped"                // This Gets Escaped
]

The $$$ delimits valid matches. Furthermore, a match is only valid if it is composed of two or more StartCased words.
Possible matches are checked against a regex and either formatted, or just returned:
function parseWord(word) {
  let linkRE = /\b[A-Z][a-z]+([A-Z][a-z]+)+\b/g
  let validLinks = word.match(linkRE)
  let stripped = word.replace(/\${3}/, '')

  if (validLinks) {
      return formatLinks(validLinks, stripped)
  }

  return _.startCase(stripped)
}

If we are to format the match we find the difference between the matched link and the entire found word. From there we decide whether to add the difference as a prefix or suffix to the link:
function formatLinks(matches, stripped) {
  let formatted_links = []
  for (match of matches) {
      let diff = difference(match, stripped)

      if (isSimpleLink(diff)) {
          formatted_links.push(formatAsSimpleLink(diff))
      } else if (hasPrefix(diff)) {
          formatted_links.push(formatAsPrefixedLink(diff))
      } else if (hasSuffix(diff)) {
          formatted_links.push(formatAsSuffixedLink(diff))
      }
  }

  // There might be multiple links within a word
  // in which case we join them and strip any duplicated parts
  // (which are otherwise interpreted as suffixes)
  if (formatted_links.length > 1) {
      return combineLinks(formatted_links)
  }

  // Default case
  return formatted_links[0]
}

The function, difference, is fast-diff from npm. Which is defined as such:
let result = difference('Good dog', 'Bad dog');
// [[-1, "Goo"], [1, "Ba"], [0, "d dog"]]

Here are the remaining helper functions (or you may view the snippet for the rest):
function combineLinks(links) {
  links.join('').replace(/(?<=\)).*?(?=\[)/g, '')
}

function formatAsSimpleLink(diff) {
  let link = diff[0][1]
  let text = _.startCase(link)
  return `[${text}](${link})`
}

function formatAsPrefixedLink(diff) {
  let link = diff[1][1]
  let text = _.startCase(link)
  let prefix = _.startCase(diff[0][1])
  return `${prefix} [${text}](${link})`
}

function formatAsSuffixedLink(diff) {
  let link = diff[0][1]
  let text = _.startCase(link)
  let suffix = diff[1][1]

  // don't capitalize plurals
  if (suffixIsPlural(suffix) == false) {
      suffix = _.startCase(diff[1][1])
      return `[${text}](${link}) ${suffix}`
  }

  return `[${text}](${link})${suffix}`
}

function isSimpleLink(diff) {
  return diff.length == 1
}

function hasPrefix(diff) {
  return diff[0][0] == 1 && diff[1][0] == 0
}

function hasSuffix(diff) {
  return diff[0][0] == 0 && diff[1][0] == 1
}

function suffixIsPlural(suffix) {
  return suffix.slice(-1) == 's'
}

My main concerns are:

The looping in formatLinks, which is really only useful if the word has a suffix which is also a link
The magic numbers used to check the diff, e.g., diff[0][1]
If there is a better way to tackle the problem.

The magic numbers used to check the diff, e.g., diff[0][0] == 1 && diff[1][0] == 0

One small improvement is that fast_diff exports enum values that can be used instead, eg:
differencesArr[0][0] === diff.INSERT
differencesArr[1][0] === diff.EQUAL

The looping in formatLinks, which is really only useful if the word has a suffix which is also a link

The whole looping mechanism looks quite suspicious to me:

Only the first $$$ is replaced by the .replace(/\${3}/, '') - if the word contains multiple $$$s, those beyond the first will be left in place

Inside formatLinks, you perform tests on and examine only the first element of the diff array, which is strange if there happen to be multiple matches

Then you have to clean the string afterwards - eg OneMatch$$$TwoMatch$$$ThreeMatch turns into [One Match](OneMatch) Two Match Three MatchOne Match [Two Match](TwoMatch)One Match Two Match [Three Match](ThreeMatch), which needs to be transformed again. Seems inelegant and error-prone.

If there is a better way to tackle the problem.

It looks like there are two fundamental categories a substring can fall into:

One which is escaped, and no []()s should be produced, like the whole span of E$$$scapeMatch
One which is not escaped, like the whole span of OneMatch$$$TwoMatch

To put each substring into one of these categories, I'd split by $$$ which split true, unescaped matches. For example:
OneMatch$$$TwoMatch -> ['OneMatch', 'TwoMatch']
E$$$scapeMatch -> ['E$$$scapeMatch']

Then you can iterate over each possible match, transforming the normal matches to their proper format, and leaving the escaped matches alone (save for removing their $$$s). Join the array of results, and there'll be 2 things left to do:

Add spacing at the edges of match brackets, if they're next to a prefix or suffix (which can be done with a simple regex)
Insert spaces between each word outside of brackets (which can be done with a simple regex)

const parseInput = input => input
    // Split into an array containing separate unescaped matches, escaped sequences, or prefixes/suffixes:
    .split(/\${3}(?=[A-Z]|s?$)/)
    // Add []() brackets around unescaped matches and remove all $s:
    .map(transformSubstr)
    .join('')
    // Add spacing around brackets:
    .replace(/(?<=\w)\[/g, ' [')
    // Except right before an "s" suffix:
    .replace(/\)(?!s)(?=\w)/g, ') ')
    // Add spaces between each word outside of brackets:
    .replace(
        /(?!s$)\w+(?=\[|$)/g,
        words => words.replace(/\B[A-Z]/g, ' $&')
    );

const transformSubstr = (substr) => {
    const normalWordsMatch = substr.match(/\b[A-Z][a-z]+(?:[A-Z][a-z]+)+\b/g);
    // If this section is an escaped match, a prefix, or a suffix, there won't be a match
    if (!normalWordsMatch) return substr.replace(/\$/g, '');
    const words = normalWordsMatch[0].match(/[A-Z][a-z]+/g);
    return `[${words.join(' ')}](${words.join('')})`;
};

const tests = [
  "OneMatch{a1fc471a-9483-4af3-8960-7104aa8d3b97}$TwoMatch{183d6425-9d46-4c32-8ce1-55b02e15b3c1}$scapeMatch"                       // Escape Match
, "SimpleMatch"                          // [Simple Match](SimpleMatch)
, "Prefixed{12abb1ad-bc6b-49b9-af33-314c963350ac}$Suffix"              // [Match Match With](MatchMatchWith) Suffix
, "MatchesWithPlural{d63f36d7-04f1-446b-80d3-6c09885cb6f3}$Gets$$$Escaped"                // This Gets Escaped
]

const parseInput = input => input
    // Split into an array containing separate unescaped matches, escaped sequences, or prefixes/suffixes:
    .split(/\${3}(?=[A-Z]|s?$)/)
    // Add []() brackets around unescaped matches:
    .map(transformSubstr)
    .join('')
    // Add spacing around brackets:
    .replace(/(?<=\w)\[/g, ' [')
    // Except right before an "s" suffix:
    .replace(/\)(?!s)(?=\w)/g, ') ')
    // Add spaces between each word outside of brackets:
    .replace(
        /(?!s$)\w+(?=\[|$)/g,
        words => words.replace(/\B[A-Z]/g, ' $&')
    );

const transformSubstr = (substr) => {
    const normalWordsMatch = substr.match(/\b[A-Z][a-z]+(?:[A-Z][a-z]+)+\b/g);
    // If this section is an escaped match, a prefix, or a suffix, there won't be a match
    if (!normalWordsMatch) return substr.replace(/\$/g, '');
    const words = normalWordsMatch[0].match(/[A-Z][a-z]+/g);
    return `[${words.join(' ')}](${words.join('')})`;
};
for (const input of tests) {
  console.log(parseInput(input));
}

I don't see any need for either of the external libraries anymore.

A couple nitpicks about your original code, if you were to keep using it:
Always use const when you can, which should be possible 95% of the time - only use let to warn future readers of the code that you may be reassigning the variable in the future.
Avoid sloppy comparison with == and != - better to use === and !==, whose rules are much easier to understand.
Always declare variables before using them - with for (match of matches), you're implicitly creating a global match variable. (Use for (const match of matches) instead)
parseWord name? There can be multiple words inside the variable passed to parseWord, eg SimpleMatch (though there's no space). Maybe call it something like parseInput.
Single-letter match? The current pattern being used in all the code here so far requires words to contain at least two characters. Is that really desirable? Consider, for example, what you'd want to happen given markup of ACuteCat$$$ADog. If that's permitted, you'd just need to change the [a-z]+ to [a-z]*.


Maximum distance between space stations HackerRank I am just working on Flatland Space Stations problem, and it is not seeming to be a tough question, but my algorithm lags in some aspects, since it solves majority of the test cases but fails for some. Which is causing me not to move forward.

The cause of the failure is timeout only, no wrong output.

My Algo:

To loop through the number of cities
Loop through the cities which has the space stations
Store the minimum distance in the array or dict for that particular city
Find out the maximum distance stores in the array of difference between n cities

I have written two codes based upon the following Algo:

1. With the use of Dictionary: Time out for three cases

def flatlandSpaceStations(n, c):
    # n is the number of cities
    # c is the array of space station exists in the city(s)
    cityDiff = {}
    maxDiff = 0
    if n == len(c):
        return 0
    else:
        for i in range(n):
            lst = []
            for j in c:
                lst.append(abs(i-j))
                cityDiff[i] = min(lst)

    for key, value in cityDiff.items():
        if value > maxDiff: maxDiff = value

    return maxDiff

2. Using lists in Python: Failed two cases in Timeout only

def flatlandSpaceStations(n, c):
    # n is the number of cities
    # c is the array of space station exists in the city(s)
    maxDiff = []
    if n == len(c):
        return 0
    else:
        for i in range(n):
            minDis = []
            for j in c:
                minDis.append(abs(i-j))
            maxDiff.append(min(minDis))
    return max(maxDiff)

I want to find some optimum solution, which I can understand. The above programs have a very bad time complexity I know, but this is only solution I can think of right now. Any help would be appreciated.

As per vnp's suggestion, I have made another attempt, and made the solution optimized, but it is not working out for the Two test cases due to timeout issue.

Solution \$O(n^2)\$:

def flatlandSpaceStations(n, c):
    # n is the number of cities
    # c is the array of space station exists in the city(s)
    maxDiff = []
    if n == len(c):
        return 0
    else:
        for i in range(n):
            minVal = float('inf')
            for j in c:
                if abs(i-j) < minVal: minVal = abs(i-j)
            maxDiff.append(minVal)
    return max(maxDiff)

I am attaching one of the two time out failure test case here for more insight.

TEST CASE

builting min

minVal = float('inf')
for j in c:
    if abs(i-j) < minVal: minVal = abs(i-j)

can be rewritten using min: min(abs(i-j) for j in c).

Apart from that, placing the code on the same line as the check looks very unreadable to me.

There is also no need to keep the values in a list,

for i in range(n):
    lst = []
    for j in c:
        lst.append(abs(i-j))
        cityDiff[i] = min(lst)

can be

max(min(abs(i - j) for j in c) for i in range(n))

variable names

i and j are useful variable names as counters. Here you use them as citie and space station. The same goes for c, which are the space stations.Then call em like that:

max(
    min(abs(space_station - city) for space_station in space_stations)
    for city in range(n)
)

space stations

A first improvement I would suggest if the city you want to check is a space station or not. In your naive algorithm you do  m x n checks. This will reduce it by m x m. If you use a set for the space stations, this inclusion check will be quick.

space_station_set = set(space_stations)
max(
    min(abs(space_station - city) for space_station in space_stations)
    for city in range(n) if city not in space_station_set
)

sorting space stations

If you sort the space stations, you can use this fact.

space_stations = [1, 4, 7]
space_stations_sorted = (
    [float("-inf")] + sorted(space_stations) + [float("inf")]
)
space_station_pairs = (
    (space_stations_sorted[i], space_stations_sorted[i + 1])
    for i in range(len(space_stations) + 1)
)

This will generate pairs of adjacent space stations. I added the infs to take care of the edges

list(space_station_pairs)

[(-inf, 1), (1, 4), (4, 7), (7, inf)]

Then you loop through the cities like this:

one, two = next(space_station_pairs)
distance_max = float("-inf")
for city in range(n):
    if city == two:
        one, two = next(space_station_pairs)
        continue
    distance = min((city-one), (two-city))
    distance_max = max(distance, distance_max)

This way you only loop once through the cities and twice through the space stations, so you reduced the complexity to O(n).

looping only over the space stations:

The furthest city will always be either in the middle between 2 space stations or at the edges of the map. You can use this fact to

space_stations_sorted = sorted(space_stations)
distance_max = max(
    (two - one) // 2 for one, two in pairwise(space_stations_sorted)
)
distance_max_total = max(
    distance_max, space_stations_sorted[0], n - space_stations_sorted[-1] - 1
)

this loops only once over the space stations, and leaves the cities untouched.


Compute interval for a node in a binary tree Given a binary tree organised as in the diagram below (a static tree represented by an array of 15 nodes), I am writing code to compute the interval represented by a given node at index i, given the interval of the entire tree (node 0)?  My motivation to do this is to get rid of the need to store the interval in a node instance.

Here's my stab at computing the value.  It appears to be correct but I'm wondering if there's a much smarter way of calculating it:

int Log2i(int x)
{
    auto r = 0;

    while (x >>= 1)
    {
        ++r;
    }

    return r;
}

void Interval(int i, int min, int max, int & a, int & b)
{
    auto d = Log2i(i);      // Depth of node i.
    auto k = 1 << d;        // At this level, there are k intervals.
    auto s = max - min;     // Total span over which we're partitioned.
    auto p = s / k;         // Therefore, size of each interval.
    auto f = k - 1;         // Index of first node for this partition.

    a = min + (p * (i - f));
    b = a + p;
}

int main(void)
{
    auto a = int(), b = int();

    Interval(6, 1000, 2000, a, b);

    return 0;
}

Not sure if it is much smarter, but

p = s / k;

could be replaced by

p = s >> d;

, which removes a division, and might produce faster code.

Instead of using the Log2 function, you may get better performance with such a function for computing k directly:

uint32_t msb32(uint32_t x)
{
    x |= (x >> 1);
    x |= (x >> 2);
    x |= (x >> 4);
    x |= (x >> 8);
    x |= (x >> 16);

    return x & ~(x >> 1);
}

source: http://aggregate.org/MAGIC/#Most%20Significant%201%20Bit

See also: https://stackoverflow.com/questions/671815/what-is-the-fastest-most-efficient-way-to-find-the-highest-set-bit-msb-in-an-i


Converting a csv to vcards in Python Context: I wrote a simple as an answer to a problem in Unix.SE and it was suggested to me to post it here for review.

A comma separated file is generated by an application. On the first column it contains a last name, on the second a first name and on the tenth column a telephone number. For each row of this csv an vcard entry should be generated.

This was my solution:

#!/usr/bin/python

import csv
import sys

def convert(file):
    reader = csv.reader(open(file, 'rb'))

    for row in reader:
        print 'BEGIN:VCARD'
        print 'VERSION:2.1'
        print 'N:' + row[0] + ';' + row[1]
        print 'FN:' + row[1] + ' ' + row[0]
        print 'TEL;HOME;VOICE:' + row[9]
        print 'END:VCARD'

def main(args):
    if len(args) != 2:
        print "Usage:"
        print args[0] + " filename"
        return

    convert(args[1])

if __name__ == '__main__':
    main(sys.argv)

Outputting the generated vcard on stdout was fine for me, I just redirected it into an appropriate file.

Read about the with statement and ALWAYS use it with file operations

with open( somefilename, 'rb' ) as source:
    reader = csv.reader( source )
    for row in reader:
       etc.

That guarantees that the file is closed and all OS resources released.


Testing Fibonacci conjecture The sequence of Fibonacci numbers is defined as F1 = 1, Fn = Fn−2 +
  Fn−1. It has been conjectured that for any Fibonacci number F, F2 + 41
  is composite.

  ... [T]ask is to either prove the conjecture to be true or find a
  counter-example that demonstrates it is false.

(source)

Here is my working code for this problem - although I don't know the conjecture is either true or false in general.

I tested it for first 107 natural numbers, also for first 1000 Fibonacci numbers - and it seems conjecture is true. but I takes a lot of time (80-90 seconds) to spit out a result.

As you see in last three functions, I have 3 method to test conjecture.

First one (fibonacci_conjecture) generates a sequence of Fib2+41s and returns result of primality test.
Second one is similar to first, but it first checks if numbers in a given range are Fibonacci numbers or not.
Third one seems more optimal. As we know from definition, for a given Fibonacci number f, 5f2+4 or 5f2-4 is perfect square. If we add 41 to both of them we'll have 5f2+45 or 5f2+37. The first one is always divisible by 5, so there's no need to check these values.

How can this code be written in an optimal way, for faster results? Or, which techniques should I follow (either within the code or within the math)?

import math

def is_perfect_square(x):
    return int(math.sqrt(x)) == math.sqrt(x)

def is_fibo(x):
    c = 5*x*x
    return is_perfect_square(c+4) or is_perfect_square(c-4)

def is_prime(x):
    c = int(math.sqrt(x))+2
    if (x%2==0 and x!=2) or is_perfect_square(x): return False
    else:
        for i in range(2, c):
            if x%i==0: return False
    return True

def minus_four(fib):
    if is_fibo(fib) and fib>0: return is_perfect_square(5*fib*fib-4)

def fibo(x):
    if x<2: return 1
    else: return fibo(x-1)+fibo(x-2)

def fibonacci_conjecture(end):
    return (True not in map(is_prime, [fibo(i)*fibo(i)+41 for i in range(end+1)]))

def second_way(end):
    return (True not in map(is_prime, [i**2+41 for i in range(end+1) if is_fibo(i)]))

def third_way(end):
    return (True not in map(is_prime, [fibo(i)*fibo(i)+41 for i in range(end+1) if minus_four(i)]))

There is also a brute-force like implementation. that checks if 5x2+37 is prime, then checks if that number is a Fibonacci number.

from fibonacci_conjecture import * # functions above
import time
from sys import argv

script, start, end = argv

_list = []

start_time = time.time()
for i in range(int(start), int(end)):
    if is_prime(5*i*i+37) and is_fibo(5*i*i+37): _list.append(i)
print _list
print time.time() - start_time

Results calling the last one from Terminal:

$ python timings.py 0 1000
[]
0.0163550376892
$ python timings.py 1000 2000
[]
0.0392298698425
$ python timings.py 2000 12000
[]
1.66012907028
$ python timings.py 12000 42000
[]
35.4742548466
$ python timings.py 42000 43000
[]
2.01177597046
$ python timings.py 43000 48000
[]
11.1937119961

As always, the first option is to use PyPy:

>>> time python2 p.py 0 30000
[]
14.4747481346
python2 p.py 0 30000  14.50s user 0.01s system 100% cpu 14.500 total

>>> time pypy p.py 0 30000
[]
0.990810871124
pypy p.py 0 30000  1.06s user 0.03s system 99% cpu 1.098 total

You shouldn't use star imports. Instead directly import the things you want:

from fibonacci_conjecture import is_fibo, is_prime

You should wrap the main body in a function called main.

The iteration would be better as a generator:

def get_falsifiers(start, end):
    for i in range(int(start), int(end)):
        if is_prime(5*i*i+37) and is_fibo(5*i*i+37):
            yield i

This would be run as:

_list = list(get_falsifiers(start, end))

Note that _list is a poor naming choice since it refers to the type and not the value. Something like falsifiers would be better.

These:

def fibonacci_conjecture(end):
    return (True not in map(is_prime, [fibo(i)*fibo(i)+41 for i in range(end+1)]))

def second_way(end):
    return (True not in map(is_prime, [i**2+41 for i in range(end+1) if is_fibo(i)]))

def third_way(end):
    return (True not in map(is_prime, [fibo(i)*fibo(i)+41 for i in range(end+1) if minus_four(i)]))

would be better as

def fibonacci_conjecture(end):
    return not any(map(is_prime, [fibo(i)*fibo(i)+41 for i in range(end+1)]))

def second_way(end):
    return not any(map(is_prime, [i**2+41 for i in range(end+1) if is_fibo(i)]))

def third_way(end):
    return not any(map(is_prime, [fibo(i)*fibo(i)+41 for i in range(end+1) if minus_four(i)]))

Further one should flatten the list comprehensions and map:

def fibonacci_conjecture(end):
    return not any([is_prime(fibo(i)*fibo(i)+41) for i in range(end+1)])

def second_way(end):
    return not any([is_prime(i**2+41) for i in range(end+1) if is_fibo(i)])

def third_way(end):
    return not any([is_prime(fibo(i)*fibo(i)+41) for i in range(end+1) if minus_four(i)])

You also then can just use a generator comprehension:

def fibonacci_conjecture(end):
    return not any(is_prime(fibo(i)*fibo(i)+41) for i in range(end+1))

def second_way(end):
    return not any(is_prime(i**2+41) for i in range(end+1) if is_fibo(i))

def third_way(end):
    return not any(is_prime(fibo(i)*fibo(i)+41) for i in range(end+1) if minus_four(i))

Your fibo(i)*fibo(i) requires two calls to fibo - this can be one with fibo(i) ** 2. Consider also using PEP 8 spacing. You can also remove the if in place and an and:

def fibonacci_conjecture(end):
    return not any(is_prime(fibo(i) ** 2 + 41) for i in range(end+1))

def second_way(end):
    return not any(is_fibo(i) and is_prime(i ** 2 + 41) for i in range(end+1))

def third_way(end):
    return not any(minus_four(i) and is_prime(fibo(i) ** 2 + 41) for i in range(end+1))

Some of these names (eg. minus_four) are particularly cryptic.

Your fibo actually takes \$\mathcal{O}(e^n)\$ time -  what you really want as a basis is a generator to let you do

return all(not is_prime(f ** 2 + 41) for f in fibonacci_numbers(count))

This generator is typically written something like

def fibonacci_numbers(count):
    prev = 1
    curr = 0

    for _ in range(count):
        yield curr
        prev, curr = curr, prev + curr

Now each number takes \$\mathcal{O}(1)\$ (approximately), which is much faster.

Doing this actually quickly leads to an

OverflowError: long int too large to convert to float

since math.sqrt(x) needs to convert

400624556078135004801005131049868376277487458971454665625744044750241868509910118684223898375755770833486910716679881483356186192835924621611785526885928263384281394311273430963137957057246385921466246089044680129333479765771922731594997403327473526588890161488739900887481571279985972340702546216307221018066

to a float.

One other problem is also precision issues. For large numbers your is_perfect_square fails:

is_perfect_square(100000000**2 + 1)

since

float(100000000**2 + 1) == 1e+16

which is rounded. So the results are actually meaningless.


Reflecting changes in the DataGridView back on the List and then updating database I have a DataGridView on the UI (a win form) and objects in the List <Earnings> are shown in the Grid.

View

class Form
{
    public BindingSource EarningDetails
    {
        set {dgEmployeeEarnings.DataSource= value;}
    }
}

Presenter Class

class WagePresenter
{
    BindingSource BS = new BindingSource();

    private void ShowEarnings()
    {
        bS.DataSource = _WageManager.PrepareEarnings ();
        _WageView.EarningDetails = BS;
    }
}

WageManager Class

class WageManager
{
    public List<Earning> PrepareEarnings()
            {
                return GetEarningObjedtList(CalculateEarnings(WagePeriodStartDate, WagePeriodEndDate));
            }

            private DataTable CalculateEarnings(DateTime startDate, DateTime endDate)
            {
                return _DataService.GetEarnings(startDate, endDate);
            }

            private List<Earning > GetEarningObjedtList(DataTable dt)
            {

                var convertedList = (from rw in dt.AsEnumerable()
                                     select new Earning()
                                     {
                                          EmployeeID=Convert.ToInt32(rw[0]),
                                          WorkDays= Convert.ToInt32 (rw[2]),
                                          DayOffs = Convert.ToInt32(rw[3]),
                                          LeaveDays  = Convert.ToInt32(rw[4]),
                                          ExtraShifts = Convert.ToInt32(rw[5]),
                                          ExtraShiftAmount = Convert.ToDecimal (rw[7]),
                                          BasicSalaryAmount = Convert.ToDecimal(rw[8]),
                                          BudjetoryAllowance = Convert.ToDecimal(rw[9]),
                                          NoPayDays = Convert.ToInt32(rw[10]),
                                          LessNoPayAmount = Convert.ToDecimal(rw[11]),
                                          AmountForEPF = Convert.ToDecimal(rw[12]),
                                          OverTimeAmount = Convert.ToDecimal(rw[13]),
                                          BroughtForwardAmount = rw[14] == DBNull.Value ? 0 : Convert.ToDecimal(rw[14]),
                                     }).ToList();

                return convertedList;
            }
}

Since this is MVP pattern, the DataSet returned from the service is converted to a list of Models (Earning) and then it's passed to the view by the presenter as a BindingSource.

All objects in the List are properly shown on the grid. Now What I'm trying to do is when user do the changes in the grid those changes should be reflected in the list (in object properties) and then it should be updated in the database.

Is it possible to do this with Lists/ ILists?
Any other notable flows in this approach?

I believe you will want to use a BindingList, which can notify when entries are updated.

You will need to handle the events raised in order to reflect the changes back to the original source.

See these sources for more:

Documentation : http://msdn.microsoft.com/en-us/library/ms132679%28v=vs.110%29.aspx
Previous question with some more info on List vs BindingList: https://stackoverflow.com/questions/2243950/listt-vs-bindinglistt-advantages-disadvantages
Basic example on Codeproject: http://www.codeproject.com/Articles/23222/BindingList-Example


Scraping articles on a web site I'm trying to create a scraping API in Express. The API scrapes the different articles featured on the home page.

Here are the issues I'm trying to resolve:

My code's turning into a ton of jquery select statements. How can I resolve this / How can I simplify my code or extract functions from this giant mass?
Are my variable names clear enough?
Are there any Node conventions I'm not following? I've seen a lot of let and I'm using var. If I'm doing anything wrong let me know.

Here is the block of code I'm running:

var express = require('express');
var fs      = require('fs');
var request = require('request');
var cheerio = require('cheerio');
var app     = express();

app.get('/scrape', function(req, res){
  url = 'http://anawesomewebsite.com';

  request(url, function(error, response, html){
    if(!error){
      var $ = cheerio.load(html);

      var title, url, image;
      var articles = [];
      var article = { title : "", url : "", image_src : ""};

      //Get articles featured at top of page
      var i = 0;
      $(".articles li .img img").each(function(){
          var image_src = $(this).attr('src');
          article.image_src = image_src;
          articles[i] = article;
          i = i + 1;
      });

      i = 0;
      $(".articles li a[class='title']").each(function(){
         var title = $(this).text();
         var url = $(this).attr('href');
         article.title = title;
         article.url = "http://vegnews.com" + url;
         articles[i] = article;
         i = i + 1;
      });

      //Get articles featured at bottom of page
      var number_of_articles_beforehand = articles.length;
      i = number_of_articles_beforehand;

      $(".article .image img").each(function(){
        var image_src = $(this).attr('src');
        article.image_src = image_src;
        articles[i] = article;
        i = i + 1;
      });

      i = number_of_articles_beforehand;
      $(".article .image a").each(function(){
         var url = $(this).attr('href');
         article.url = url
         articles[i] = article;
         i = i + 1;
      })

      i = number_of_articles_beforehand;
      $(".article .title").each(function(){
        var title = $(this).text();
        article.title = title;
        articles[i] = article;
        i = i + 1;
      })

    }
    res.send(articles);
  })
})

app.listen('8081')
console.log('Magic happens on port 8081');
exports = module.exports = app;

As for the jQuery/function usage, there is definitely room for improvement but it would be helpful to see an example HTML element that contains the article img, title, and url to understand the page structure a bit.
I think your variable names are clear, but could be simplified. Also, it is common practice to use Camel Case with the first letter lowercase for Node/Javascript instead of underscores between words.

For example, replace number_of_articles_beforehand with numArticlesBefore

Here is an article I found helpful regarding var, let, const

https://medium.com/javascript-scene/javascript-es6-var-let-or-const-ba58b8dcde75
Javascript allows variable hoisting (click here for more info) with the keyword var whereas variables/identifiers declared as let or const do not hoist.
Personally, I have stopped using var altogether to help improve code readability as mentioned in the article above. Also, it is common practice to use const for the Node package require statements instead of var.
Remove the var title, url, image; line because you are only using those variables within their respective functions.
Lastly, I would encourage you to replace i = i + 1; with i++;. It does the same thing and is easier to read. (FYI, i--; would decrement i by 1).


Find the longest sub string of a word after concatenation of given words array An Array of N words is given. Each word consists of small letters ('a'-'z'). Our goal is to concatenate the words in such a way as to obtain a single word with longest possible substring composed of one particular letter. Find the length of such substring.

Example 1: words = ["aaba", "aaaa", "bbab"] -> maxSubstring=6 of letter a, from the concatenated word = words[1] + words[0] + words[2] = "aaaaaababbab"

Example 2: words = ["xxbxx", "xbx", "x"] -> maxSubstring=4 of letter x, from the concatenated word = words[0] + words[2] + words[1] = "xxbxxxxbx"

My current implementation uses brute force:
find all combinations of words, and calculate the max sub-string.
Complexity is \$O(2^n)\$.

I'm looking for better ideas to reduce the time complexity.

class candiateCode{
    static Integer max = Integer.MIN_VALUE;

    public static void main(String[] args) {

        Scanner sc = new Scanner(System.in);
        int N = sc.nextInt();
        sc.nextLine();
        String[] arr = new String[N];

        for (int i = 0; i < N; i++) {
            arr[i] = sc.nextLine();

        }

        findCombi(arr, N, "", new HashMap<Integer, Boolean>());

        System.out.println(max);

    }

    private static void findCombi(String[] arr, int n, String prefix, Map<Integer, Boolean> map) {
        if (n == 0) {
            calculateMaxChar(prefix);
        }
        for (int i = 0; i < arr.length; i++) {
            if (!map.containsKey(i)) {
                Map<Integer, Boolean> map2 = new HashMap<>(map);
                map2.put(i, true);
                findCombi(arr, n - 1, prefix.concat(arr[i]), map2);
            }

        }
    }

    static void calculateMaxChar(String str) {
        Integer max1 = 1;
        Integer curr = 1;
        char ch = str.charAt(0);
        for (int i = 1; i < str.length(); i++) {
            if (str.charAt(i) == ch) {
                curr++;
            } else {
                ch = str.charAt(i);
                curr = 1;
            }
            if (max1 < curr)
                max1 = curr;

        }
        if (max1 > max)
            max = max1;

    }
}

Think of encapsulation and responsibilities

The main method parses the input and uses this piece of code to compute and print the answer:

findCombi(arr, N, "", new HashMap<Integer, Boolean>());

System.out.println(max);

This is ugly in many ways:

Where does the value of max come from? It's a global variable, its value gets set as a side effect of the call to findCombi. Try to organize code in a way to avoid side effects.
What are all those parameters passed to findCombi? They are low-level implementation details of findCombi. They should not be exposed outside that function. The main method shouldn't have to know how the solution is computed. It has the inputs (the array of words), it should pass just that to a function, and get an int back as the answer.
The parameter N is redundant, thanks to the .length field of arrays in Java.

This is what the snippet should have looked like:

int solution = findLongestSubstringCombination(arr);

There are no unnecessary parameters, and no side effects needed.


Adjusting dates in an array based on a start date I have a data structure like this which holds start/end dates:

----|-----------|-----------
Item| start     | end
----|-----------|-----------
1   |2017-05-12 | 2017-05-12
2   |2017-05-12 | 2017-05-13
3   |2017-05-13 | 2017-05-13
4   |2017-05-13 | 2017-05-14
5   |2017-05-14 | 2017-05-15
6   |2017-05-15 | 2017-05-16
7   |2017-05-16 | 2017-05-16

I want to move the start date of the first item to 2017-01-01 and every item should follow that start date and must stay within its ranges. The result should look like the following:

----|-----------|-----------
Item| start     | end
----|-----------|-----------
1   |2017-01-01 | 2017-01-01
2   |2017-01-01 | 2017-01-02
3   |2017-01-02 | 2017-01-02
4   |2017-01-02 | 2017-01-03
5   |2017-01-03 | 2017-01-04
6   |2017-01-04 | 2017-01-05
7   |2017-01-05 | 2017-01-05

This is how I implemented the method:

// ActiveItems() method (although it's bad naming) retrieves all the currently active hotels/transfers etc from a booking.
var items = booking.ActiveItems(); // each item has check-in and/or check-out dates, internals doesn't matter for this problem

var tempCurrentDate = newStartDate;
var previousCheckoutDate = newStartDate;

// go thru each item
for (var i = 0; i < items.Count; i++)
{
    var currentItem = items[i];
    // again, bad naming but internals doesn't really matter. method returns a check-in date (DateTime)
    var itemCheckinDate = currentItem.CheckInDate();
    var itemCheckoutDate = currentItem.CheckOutDate();

    // no of days between check-out and check-in
    // this difference later added to the new check-in date to get the new check-out
    var checkinToCheckoutDifference = (itemCheckoutDate - itemCheckinDate).Days;
    if (i == 0)
    {
        // check-in equals to start date if the item is the first one in the list
        itemCheckinDate = tempCurrentDate;
    }
    else
    {
        var prevDayToCurrentCheckinDifference = (previousCheckoutDate - currentItem.CheckInDate()).Days;
        itemCheckinDate = tempCurrentDate.AddDays(prevDayToCurrentCheckinDifference);
    }

    //SetCheckInDate() internals really doesn't matter here.
    currentItem.SetCheckInDate(itemCheckinDate);

    previousCheckoutDate = itemCheckoutDate;

    tempCurrentDate = itemCheckinDate.AddDays(checkinToCheckoutDifference);

    currentItem.SetCheckOutDate(tempCurrentDate);
}

Even though this code works as expected, is there a better and efficient way of doing this?

var items = booking.ActiveItems(); // each item has check-in and/or check-out dates

var offset = items[0].GetCheckInDate().Date - DateTime.Parse("2017-05-12");

foreach(var item in items)
{
   item.SetCheckInDate(item.GetCheckInDate() - offset);
   item.SetCheckOutDate(item.GetCheckOutDate() - offset);
}

That, however would not account for e.g. holidays, weekends, availability

And the only reason to do iterative adjustmens (as in original code) would be if SetDate would automatically make adjustments to holidays/weekends inside.

In this case original code lacks comments around that area explaining that SetDate might change the date by itself and absolute day offset would change.


Given N and M find all stepping numbers in range N to M The stepping number:

  A number is called as a stepping number if the adjacent digits have a
  difference of 1. e.g 123 is stepping number, but 358 is not a stepping
  number.

  Example:

  N = 10, M = 20

  All stepping numbers are 10, 12

  Return the numbers in sorted order.

The code I wrote does accomplish what is asked, but the complexity is \$O(n^2)\$ I think, and I'm sure there is a better way of doing this.

class Solution {
public List<int> stepnum(int A, int B) {
    List<int> result = new List<int>();
    for (int i=A;i<=B;i++)
    {
        string num = i.ToString();
        int amount = num.Count();
        int less = 0;
        bool step=true, ascending=true;

        if(amount>=2)  //0 - 1
        {
            for(int j=0;j<amount;j++)
            {
                ascending=true;
                if(j<amount-1)
                {
                    if((int)Char.GetNumericValue(num[j])>(int)Char.GetNumericValue(num[j+1]))
                    {
                        ascending=false;
                    }
                    if(ascending)
                        less=(int)Char.GetNumericValue(num[j+1])-(int)Char.GetNumericValue(num[j]);
                    else
                        less=(int)Char.GetNumericValue(num[j])-(int)Char.GetNumericValue(num[j+1]);
                    if(less!=1)
                    {
                        step=false;
                        break;
                    }
                }
            }
            if(step)
            {
                result.Add(Convert.ToInt32(num));
            }
        }
        else
            result.Add(Convert.ToInt32(num));
    }
    return result;
}
}

Code review

Avoid unnecessary indentation whenever possible. An early exit block should always be on top. What I mean by that is this:

for(...)
{
    // this saves your 1-level of indentation
    if (amount < 2)
    {
        // update result
        continue;
    }

    // rest of code here
}

Use intermediary variables if the code gets too long and loses its meaning

int current = (int)Char.GetNumericValue(num[j]), next = (int)Char.GetNumericValue(num[j+1]);
if(current > next)
{
    ascending=false;
}
if(ascending)
    less = next - current;
else
    less = current - next;

BTW, this can be simplfied to remove ascending variable:

if(current > next)
{
    less = current - next;
}
else
{
    less = next - current;
}

Or, even better: Math.Abs(current - next)
Put spaces around operators and control structures like for, if, while.
You can also compare the char directly, as "the value of a char object is a 16-bit numeric (ordinal) value": '2' - '1' == 1
Don't work on numbers as a string and convert it back. It is highly inefficient.
Divide your code. The block inside the first for-loop can be extracted into a bool IsStepNumber(int).

The Task

  The stepping number:

  A number is called as a stepping number if the adjacent digits have a difference of 1. e.g 123 is stepping number, but 358 is not a stepping number.

Basically, we need to (1)extract the digits, and (2)check if the adjacent digits have a difference of 1.

If you understand the positional notation, the extraction is pretty straightforward :

public static IEnumerable<int> GetDigitsReverse(int value)
{
    while(value > 0)
    {
        yield return value % 10;
        value /= 10;
    }
}

Due to the nature of this algorithm, the digits are in reserve order (right to left). Although it is really simple to re-reverse it (by chaining .Reverse()), it doesn't matter in which direction we are check the adjacent digits.
To compare adjacent items in a sequence, we can pair up a sequence with itself offset-ed by 1:

public static bool IsSteppingNumber(int value)
{
    var digits = GetDigitsReverse(value).ToList();
    return digits
        .Zip(digits.Skip(1), Tuple.Create)
        .All(x => Math.Abs(x.Item1 - x.Item2) == 1);
}

To enumerate all step numbers in a defined range, we can use the following method:

public static IEnumerable<int> ListSteppingNumbersBetween(int a, int b)
{
    return Enumerable.Range(a, b - a + 1)
        .Where(IsSteppingNumber);
}


Function that converts file eol's I've been unable to find a PHP function that will convert eol in files.  This is what I have so far. It works, no errors.

Your educated opinions, thoughts, improvements, potential bugs and suggestions are welcome.

/*
    $file_name - full/path/to/file.name

    $eol         - wanted end of line: "\n", "\r", "\r\n"
        from "\n" to "\r" || "\r\n"
        from "\r\n" to "\r" || "\n"
        from "\r" to "\n"

    $len    - buffer size

    return true on success or false on fail
*/
function file_eol($file, $eol, $len=8192) {

    $ok = FALSE;
    $extension = '.my_temp';

    $hread = FALSE;
    if (is_string($file))   $hread = fopen($file, 'rb');
    if ($hread === FALSE)   e("Bad file in file_eol(): $file");
    if (!is_resource($hread) || get_resource_type($hread) !== 'stream') e("Bad hread in file_eol(): $hread");

    $hsave = fopen($file.$extension, 'wb');
    if ($hsave === FALSE)       e("Bad hsave file in file_eol(): $file.my_temp");

    switch ($eol) {
    case "\n":      $order = array("\r\n", "\r");   // from "\r\n" || "\r" -> "\n"
    break;
    case "\r":      $order = array("\r\n", "\n");   // from "\r\n" || "\n" -> "\r"
    break;
    case "\r\n":    $order = array("\n", "\r\r\n"); // from "\n" -> "\r\n"
    break;

    default:
        e("Bad eol in file_eol(): $eol");
    }

    while (($str = fread($hread, $len)) && $str !== FALSE) {    // error or eof

        $new = str_replace($order, $eol, $str);
        // if (is_string($new)) $ok = fwrite($hsave, $new);     // this ???
        if ($new !== '')    $ok = fwrite($hsave, $new);
        if ($ok === FALSE) break;   //  or  e("Error writing file: $file$extension");
    }
    $ok = $ok === FALSE ? FALSE : TRUE; // ok could be valid 0bytes
    $ok = $ok && ((bool)$str || feof($hread));          // check if fread and feof failed
    $ok = $ok && fclose($hread) && fclose($hsave);  // ALWAYS check for fclose!

    if ($ok)    $ok = rename($file.$extension, $file);  // overwrite $file, and delete $file.$extension
    // else leave both files for later inspection

return $ok;
}

P.S. copy/paste should work if you create error handler function e() that will take string inputs or just change e() to trigger_error or whatever you want.

Edit:

Must include feof()

$ok = $ok && ((bool)$str || feof($hread));          // check if fread and feof failed

First of all, your function name sounds weird. The name of the function should brefly describe what it does, so, a name like convertEol would be more appropriate.

Second, I see much redundancy on your code, like this part:

if ($hread === FALSE)   e("Bad file in file_eol(): $file");
if (!is_resource($hread) || get_resource_type($hread) !== 'stream') e("Bad hread in file_eol(): $hread");

As you can see on the PHP manual of fopen function, it will return false if it is not able to open de file, otherwhise, it will ALWAYS return a resource, no matter what (except that you are doing something really crazy, like opening more files than a process is allowed to do).

So, your second verification is redundant.

Also, returning booleans in functions that do not check for some condition it too old-fashioned, too 80's. The same way, triggering uncatchable errors is almost a crime.

You should really learn how to work with Exceptions.

Here is what I would do, not necessarily the only one correct implementation:

function convertEol($fileName, $eolChar) {
    /* For windows, unix and mac.
     * IMPORTANT: for this implementation, \r\n MUST be the first element on the array
     */
    static $replaceableChars = array("\r\n", "\n", "\r");

    $originalFile, $tmpFile;

    try {
        $originalFile = new SplFileObject($fileName, 'r'); // open a file for reading
    } catch (RuntimeException $e) {
        throw new Exception("Unable to open file {$fileName} for reading", null, $e);
    }

    try {
        $tmpFile = new SplFileObject(uniqid() . '.tmp', 'w+'); // open or create a file for writing
    } catch (RuntimeException $e) {
        throw new Exception("Unable to create temp file", null, $e);
    }

    // iterates over the original file, line by line
    foreach ($originalFile as $line) {
        // writes the replaced string
        $tmpFile->fwrite(str_replace($replaceableChars, $eol, $line));
    }

    $tmpFile->fflush(); // makes sure file was written

    rename($tmpFile->getPathname(), $originalFile->getPathname());
}

Usage:

// Normal
convertEol('path/to/file', "\n");

// Unexistent file
convertEol('path/to/unexistent/file', "\n"); // will throw an exception

// Catching error
try {
    convertEol('path/to/unexistent/file', "\n");
} catch (Exception $e) {
    echo 'Oops, something went wrong:' . $e->getTraceAsString();
}
// ...


Rotate matrix 90 degrees clockwise Please suggest improvements and a possible better way of doing it in place.

public static void main(String[] args) {
    int[][] mat = { { 1, 2 }, { 3, 4 } };
    rotate(mat);
}

public static void rotate(int[][] matrix) {
    if (matrix == null) {
        return;
    }
    int n = matrix.length - 1;
    int temp = 0;
    int[][] mat2 = new int[n + 1][n + 1];
    mat2 = matrix;
    for (int row = 0; row <= n; row++) {
        for (int col = 0; col <= n; col++) {
            // mat2[n-col][row] = matrix[row][col];
            temp = matrix[col][n - row];
            matrix[col][n - row] = matrix[row][col];
            matrix[row][col] = temp;
        }
    }
    for (int i = 0; i <= n; i++) {
        for (int j = 0; j <= n; j++) {
            matrix[i][j] = mat2[i][j];
        }
    }
}

Yes there is a better way to do it. It makes the computation really simple and elegant.

If you take the transpose of the matrix and then rotate the matrix row-wise along the mid row, you can get the same result as rotating the matrix by 90 degrees counter clock-wise.

For example:

[1 2 3]
[4 5 6]
[7 8 9]

Step 1: take its transpose:

[1 4 7]
[2 5 8]
[3 6 9]

Step 2: rotate the matrix across mid row:

[3 6 9]
[2 5 8]
[1 4 7]

This is exactly what you get when you rotate the matrix by 90 degrees left.

Here is the code:

public static void rotateMatrix(int[][] matrix){
    if(matrix == null)
        return;
    if(matrix.length != matrix[0].length)//INVALID INPUT
        return;
    getTranspose(matrix);
    rorateAlongMidRow(matrix);
}

private static void getTranspose(int[][] matrix) {
    for(int i = 0; i < matrix.length; i++){
        for(int j = i+1; j < matrix.length ; j++){
            int temp = matrix[i][j];
            matrix[i][j] = matrix[j][i];
            matrix[j][i] = temp;
        }
    }
}

private static void rorateAlongMidRow(int[][] matrix) {
    int len = matrix.length ;
    for(int i = 0; i < len/2; i++){
        for(int j = 0;j < len; j++){
            int temp = matrix[i][j];
            matrix[i][j] = matrix[len-1 -i][j];
            matrix[len -1 -i][j] = temp;
        }
    }
}

Edit: As suggested in the comment, for making it rotate clock-wise, just change the function getTranspose() to rotateAlongDiagonal() in rotateMatrix() function.

private static void rotateAlongDiagonal(int[][] matrix) {
    int len = matrix.length;
    for(int i = 0; i < len; i++){
        for(int j = 0; j < len - 1 - i ; j++){
            int temp = matrix[i][j];
            matrix[i][j] = matrix[len -1 - j][len-1-i];
            matrix[len -1 - j][len-1-i] = temp;
        }
    }
}


Dice-rolling program I am sort of new to programming.  I would like to understand the code areas that are garbage and could be better written keeping the exact (if possible) behavior the code has now.

from random import randint
import time
from collections import OrderedDict

cast_again = 'r'
statistics = OrderedDict()
casts = 0
menu_options = """
> Options:
>    R - to cast again;
>    S - view stats;
>    E - exit the programme.
> """

def rollDice(dice):
    input('> Press Enter to cast dice!')
    time.sleep(1)
    cast_result = []
    for a_cast in range(0,dice):
        cast_result.append(randint(1,6))
    return cast_result

def atLeastALetterInput(text_to_display):
    cast_again = 'x'
    try:
        cast_again = input(text_to_display)[0].lower()
        return cast_again
    except IndexError:
        print('> Please make sure input is valid before pressing enter...')

while 1:
    try:
        num_dice = int(input('> Enter the number of dice you would like to cast! (max 10):'))
        if num_dice not in range(0,11):
            print('> Please enter a value between 1 and 10! ')
            continue
        break
    except ValueError:
        print('> Non-numerical characters detected. Try again!')

while 1:
    if cast_again not in ('r', 's', 'e'):
        print('> Ups! Available options R(new cast), S(statistics) and E(exit).')
        cast_again = atLeastALetterInput(menu_options)
        continue
    if cast_again == 'r':
        next_cast = casts + 1
        print('> Here comes cast number {}'.format(next_cast))
        casts +=1
        cast_results_to_stats = rollDice(num_dice)
        statistics[casts] = cast_results_to_stats
        print('> Here is your result: {}'.format(cast_results_to_stats))
        cast_again = atLeastALetterInput(menu_options)
    elif cast_again == 's':
        for key in statistics:
            print('> *** Cast {} resulted in {}'.format(key, statistics[key]))
        cast_again = atLeastALetterInput(menu_options)
    elif cast_again == 'e':
        print('> The game finished after {} casts.'.format(casts))
        break

print('> Good bye! ')
input()

About rollDice

The name of the function does not follow PEP 8, the Python style guide.

The name of the parameter is not that great.

You don't need 0 as a first parameter of range as it is the default behavior.

The value a_cast is not used. Usually, the name for throw-away values in Python is _.

The loop could be written in a list compregension.

After taking into account these details, your code looks like :

def roll_dice(nb_dice):
    input('> Press Enter to cast dice!')
    time.sleep(1)
    return [randint(1,6) for _ in range(nb_dice)]

About atLeastALetterInput

Here again, the name does not seem correct.

The variable is not required.

After rewriting this, code looks like :

def get_letter_from_user(text_to_display):
    try:
        return input(text_to_display)[0].lower()
    except IndexError:
        print('> Please make sure input is valid before pressing enter...')

About your first while loop

It should probably be put in a function with a signature like def get_nb_from_user(min_val, max_val):.

You could remove the need for a continue by re-organising the conditions :

    num_dice = int(input('> Enter the number of dice you would like to cast! (max 10):'))
    if num_dice in range(0,11):
        break
    else:
        print('> Please enter a value between 1 and 10! ')

Checking if an integer is in an interval by checking if it is in the result from range is not efficient at all : if you were to check if a number is between 1 and 100000000, it would take a very long time even though this should be pretty simple.

Your code becomes :

def get_nb_from_user(min_val, max_val):
    while True:
        try:
            nb = int(input('> Enter the number of dice you would like to cast! (max %s):' % max_val))
            if min_val <= nb <= max_val:
                return nb
            else:
                print('> Please enter a value between %s and %s! ' % (min_val, max_val))
        except ValueError:
            print('> Non-numerical characters detected. Try again!')

num_dice = get_nb_from_user(1, 10)

About your second while loop

To keep things simple, you should try to call get_letter_from_user from a single place.

This is quite easy to do :

while True:
    cast_again = get_letter_from_user(menu_options)
    if cast_again == 'r':
        next_cast = casts + 1
        print('> Here comes cast number {}'.format(next_cast))
        casts +=1
        cast_results_to_stats = roll_dice(num_dice)
        statistics[casts] = cast_results_to_stats
        print('> Here is your result: {}'.format(cast_results_to_stats))
    elif cast_again == 's':
        for key in statistics:
            print('> *** Cast {} resulted in {}'.format(key, statistics[key]))
    elif cast_again == 'e':
        print('> The game finished after {} casts.'.format(casts))
        print('> Good bye! ')
        input()
        break
    else:
        print('> Ups! Available options R(new cast), S(statistics) and E(exit).')

Then again, as explained in a different answer, there is no need for an ordered dictionnary as a list would do the trick.

Then your code begins (variable definition included) :

num_dice = get_nb_from_user(1, 10)
statistics = []

while True:
    cast_again = get_letter_from_user(menu_options)
    if cast_again == 'r':
        print('> Here comes cast number {}'.format(len(statistics)+1))
        cast_results_to_stats = roll_dice(num_dice)
        statistics.append(cast_results_to_stats)
        print('> Here is your result: {}'.format(cast_results_to_stats))
    elif cast_again == 's':
        for i, cast in enumerate(statistics):
            print('> *** Cast {} resulted in {}'.format(i, cast))
    elif cast_again == 'e':
        print('> The game finished after {} casts.'.format(len(statistics)))
        print('> Good bye! ')
        input()
        break
    else:
        print('> Ups! Available options R(new cast), S(statistics) and E(exit).')

Now, in Python, it is usually a good thing to move the part of your code actually doing stuff behind a main guard. Thus, your code begins :

def main():
    """Main function"""
    num_dice = get_nb_from_user(1, 10)
    statistics = []

    while True:
        cast_again = get_letter_from_user(menu_options)
        if cast_again == 'r':
            print('> Here comes cast number {}'.format(len(statistics)+1))
            cast_results_to_stats = roll_dice(num_dice)
            statistics.append(cast_results_to_stats)
            print('> Here is your result: {}'.format(cast_results_to_stats))
        elif cast_again == 's':
            for i, cast in enumerate(statistics):
                print('> *** Cast {} resulted in {}'.format(i, cast))
        elif cast_again == 'e':
            print('> The game finished after {} casts.'.format(len(statistics)))
            print('> Good bye! ')
            input()
            break
        else:
            print('> Ups! Available options R(new cast), S(statistics) and E(exit).')

if __name__ == "__main__":
    main()

Then, on top of this, you should try to add docstrings to your functions.


Count the number of unique elements in a sorted array Write a method named numUnique that accepts a sorted array of integers as a parameter and that returns the number of unique values in the array. The array is guaranteed to be in sorted order, which means that duplicates will be grouped together.

I'm mainly concerned with following proper conventions and maximizing readability; both of which I feel I know little about. In the last question I posted, I learned a bit about structuring if-statements. How can I think about these statements so that it's easier for me to write them, as well as easier for others to read them? Is there a sort of PEMDAS of programming logic that would help me understand writing conditional code with precedence? Any feedback is appreciated!

public static int numUnique(int[] array){
    if(array.length > 0){
        int countUnique = 1;
        for(int i = 0; i < array.length - 1; i++){
            if(array[i] != array[i+1]){
                countUnique++;
            }
        }
        return countUnique;
    } else {
        return 0;
    }
}

Just a few minor suggestions:

If you invert the condition on the length, then you can reduce the level of nesting, which is slightly easier to read
An array will never have a negative length. It's pure paranoia to check for array.length <= 0, I suggest to replace with array.length == 0
The spacing is not perfect. Use an IDE to reformat the code
This maybe a matter of taste, but I would rename countUnique to simply count, and the method from numUnique to countUnique

Putting it together:

public static int countUnique(int[] array) {
    if (array.length == 0) {
        return 0;
    }
    int count = 1;
    for (int i = 0; i < array.length - 1; i++) {
        if (array[i] != array[i + 1]) {
            count++;
        }
    }
    return count;
}


Linq version of the simulated annealing algorithm I decided to try and implement (a version of) the simulated annealing algorithm using just LINQ, just to see if I could.

I'd love it if anybody could see any ways to improve it, or give advice on any cool tricks for doing this kind of thing.

var result = (from res in Enumerable.Range(0, 1)
                  let R = new Random()
                  let initial = Enumerable.Range(0, 10).Select(i => R.Next(-10, 10))
                  let iterations = Enumerable.Range(0, 100)

                  let Schedule = (Func<int, float>)
                                 (X => 4 + (float)Math.Sin(X))

                  from iteration in iterations
                      let temperature = Schedule(iteration)
                      let state = Enumerable.Range(0, 10).Select(i => R.Next(-10, 10))
                      let DeltaE = state.Sum() - initial.Sum()

                      where DeltaE > 0 ||
                            Math.Pow(Math.E, DeltaE / temperature) > R.NextDouble()
                      select state.ToList()
              ).OrderBy(S => S.Sum()).First();

You have a lot of small errors here.

What is the purpose of from res in Enumerable.Range(0, 1)? It looks like you did it to force some local variables into 'let' queries, which makes no sense to me.
Instead of using OrderBy(X).First() you should use MinBy(X) (write it yourself if it doesn't exist). There's a significant performance difference.
The value of initial changes every time it is enumerated. Confusing. (You can cache the results with ToArray or ToList to prevent re-enumerating from running more Random.Next calls)
Ensure new Random() is only performed once (do it outside the query). The default seed is the current time, which is highly correlated between calls. You're likely to end up with two instances of Random with the exact same seed.
Random.Next(-10, 10) returns values in [-10, +9]. You probably wanted [-10, +10].


Goldbach’s Conjecture in Python Input Format

Input starts with an integer n (1 ≤ n ≤ 100) indicating the number of cases. The following n lines each contain a test case of a single even number x (4 ≤ x ≤ 32000).

Output Format

For each test case x, give the number of unique ways that x can be represented as a sum of two primes. Then list the sums (one sum per line) in increasing order of the first addend. The first addend must always be less than or equal to the second to avoid duplicates.

Sample Input

2
26
100

Sample Output

26 has 3 representation(s)
3+23
7+19
13+13

The above problem was originally posted in 2013 ICPC North America Qualifier, I encountered it in HackerRank's Round-I Holiday Cup contest.

My Python 3.x Code

import math
import time
def sieve(n):
    "Return all primes <= n using sieve of erato."
    np1 = n + 1
    s = list(range(np1))
    s[1] = 0
    sqrtn = int(round(n**0.5))
    for i in range(2, sqrtn + 1):
        if s[i]:
            s[i*i: np1: i] = [0] * len(range(i*i, np1, i))
    return filter(None, s)
def primeSum(n):
    p=2
    limit=math.floor(n/2)
    for p in primes:
        q=n-p
        if(p>q):
            break
        if(q in primes):
            out.extend([p,q])
    print (n,"has",round(len(out)/2),"representation(s)")
    for i,j in zip(out[0::2],out[1::2]):
        print (i,"+",j,sep='')

    print ("")

primes = sorted(set(sieve(32000)))
for __ in range(int(input())):
    out = []
    primeSum(int(input()))

This code almost took over 5 seconds for larger values of n but ran less than second in most cases. Is it because of time taken to output all those representations for larger values (because 32000 has over 300 representations.

Can this be optimized? and also review my general coding (I'm new to python).

Pass more arguments to functions

primes is global, but passing it as an argument to primeSum is trivial:

def primeSum(n, primes):

and then:

primeSum(int(input()), primes)

This makes the reasoning about and testing the function easier.

Do not modify random global variables

out.extend([p,q]) where out is a global variable. Don't. It makes the code impossible to use in different situations and therefore wastes your programming effort.

Do not print from a computation function

primeSum should find a prime sum. Printing the couples nicely is the job for another function. Again a computer friendly return format improves re-usability.

Remove dead code

You write:

limit=math.floor(n/2)

But limit is never been used, so delete that line.

Also p=2 can be eliminated as it does nothing.

Meaningful names

Avoid single-letter names, be descriptive. Good names can really make the difference between un-readable and readable code. And remember to use snake_case in Python.

Final version

Putting all my advice together gives this nice prime_sum function:

def prime_sums(number, primes):
    """
    Checks how a number may be written as a sum of two primes.

    >>> list(prime_sums(100, sorted(set(sieve(100)))))
    [(3, 97), (11, 89), (17, 83), (29, 71), (41, 59), (47, 53)]
    """
    for prime in primes:
        difference = number - prime
        if(prime > difference):
            break
        if(difference in primes):
            yield (prime, difference)