Cache mechanism with duration I want to implement in my code a cache mechanism with a fixed duration. For example an iframe will be cached for one hour, or a script file will be cached for 24 hours.

This is how I implemented it, with a rounded timestamp (simplified code assuming that the url doesn't have any query or hash part):

var duration=86400000; // example for 1 day = 86400000 milliseconds
url=url+"?_ts="+ getTimeStamp(duration);

function getTimeStamp(roundTime) {
  var timeStamp=new Date().getTime();
  if (roundTime) {timeStamp=Math.floor(timeStamp/roundTime);}
  return timeStamp;
}

My questions:

Are there any traps to watch for, or is the above code going to work just fine in all situations (dynamic script tag, iframe, ajax)?
Is there a better way to do it, for example with a JavaScript library?

I suggest you read more about rules on simple rules for HTTP caching, and simple rules for AJAX caching. These should give you a heads up on gotchas, especially in IE as well as the right headers to send for HTTP caching.

A more proper way to do this is to send the right headers. This would mean some server-side action and using the Expires header.

Anyways, let's do cache busting instead. This is the method you are trying to do, which is to append a variable value into a query string. This assumes that your resources are "forever cached" and needs busting to bring in a new version.

Here's a few gotchas in your code:

It doesn't persist during page load.

Since you don't have any persistence layer in your code, each page load will generate a new time stamp for the resource, and will reload them from the server instead of the cache.
What happens when it expires?

I don't seem to see code that tells if the resource has already expired, unless you have other code that does that for you.

If without it, you should build a collection that records the urls and their times. Then you refer to that collection to get the url of the resource. If the resource has not expired, then return the previously generated url. If it has expired, then generate a new url with a new time stamp and hand it for use.

Here's code that should get you on the right track. This assumes that the resources never expire, so that you need the cache-buster mechanism going.

(function(ns){

  //Read from local storage, if any, or use an empty object
  var collection = /*read from local storage*/ || {};

  ns.generate(url,duration){

    //return a runnable function that generates either an existing url
    //or a newly generated url
    return function(){
      var currentDate = Date.now();

      //if URL isn't in collection or has already expired
      if(!collection.hasOwnProperty(url) || collection[url].expires < currentDate){
        //generate the url and store
        collection[url] = {
          url : url,
          expires : currentDate + duration;
          stampedUrl : url += '?_ts='+ (currentDate + duration)
        }

        /*save in local storage*/

      }

      //return the url
      return collection[url].stampedUrl;
    }
  }
}(this.StampedUrl = this.StampedUrl || {}));

Sample usage:

//Generate and store our URL. This should return a runnable function
//that returns an unexpired URL or a newly generated url
var myScript = StampedUrl.generate('http://example.com/someScript.js',3600);

//Use function to generate the URL.
$.get(myScript(),function(){...});


Dice game for two players (Nea) The rules:

The points rolled on each player’s dice are added to their score.
If the total is an even number, an additional 10 points are added to their score.
If the total is an odd number, 5 points are subtracted from their score.
If they roll a double, they get to roll one extra die and get the number of points rolled added to their score.
The score of a player cannot go below 0 at any point.
The person with the highest score at the end of the 5 rounds wins.
If  both  players  have  the  same  score  at  the  end  of  the  5  rounds,  they  each  roll  1  die  and  whoever gets the highest score wins (this repeats until someone wins)

import pickle
import random
import time

print("")
print("welcome to the dice game")
print("")

with open('users.pickle', 'rb') as f:
    users = pickle.load(f)

print(users)

def login():

    logged_in = False

    username = input('please enter username: ')

    while not logged_in:

        if username in users:
            password = input("enter password: ")
            if password == users[username]:
                print("access granted")
                logged_in = True

        else:
            print("access denied")
            exit()

    return username

print("  ")

rounds = 0

def roll():
    die1 = random.randint(1, 7)
    die2 = random.randint(1, 7)
    change = 10 if (die1 + die2) % 2 == 0 else -5
    points = die1 + die2 + change
    if die1 == die2:
        points += random.randint(1, 6)
    return points

def game():
    player1_points = 0
    player2_points = 0
    for i in range(1, 6):
        player1_points += roll()
        print(f'After this round user1 you now have: {player1_points} Points')
        # time.sleep(1)
        player2_points += roll()
        print(f'After this round user2 you now have: {player2_points} Points')

    player1_tiebreaker = 0
    player2_tiebreaker = 0
    if player1_points == player2_tiebreaker:
        while player1_tiebreaker == player2_tiebreaker:
            player1_tiebreaker = random.randint(1, 6)
            player2_tiebreaker = random.randint(1, 6)

    player2_win = (player2_points + player2_tiebreaker)
    player1_win = (player1_points + player1_tiebreaker) \

    return (player1_points, player1_win), (player2_points, player2_win)

def add_winner(winner):

    with open('Winner.txt', 'a') as i:
        i.write(','.join(map(str, winner)))
        i.write('\n')

def get_leaderboard():
    with open('Leaderboard.txt', 'r+') as g:
        return [line.replace('\n', '') for line in g.readlines()]

def update_leaderboard(leaderboard, winner):
    for idx, item in enumerate(leaderboard):
        if item.split(',')[1] == winner[1] and int(item.split(',')[0]) < int(winner[0]):
            leaderboard[idx] = '{}, {}'.format(winner[0], winner[1])
        else:
            pass
    leaderboard.sort(reverse=True)

def save_leaderboard(leaderboard):
    with open('Leaderboard.txt', 'r+') as h:
        for item in leaderboard:
            h.write(f'{item}\n')

def main():
    user1 = login()
    user2 = login()

    (player1, player1_win), (player2, player2_win) = game()

    if player1_win:
        winner = [player1, user1]
    else:
        winner = [player2, user2]

    print("Well done: ", winner[1], "you won with", winner[0], "Points")

    add_winner(winner)
    leaderboard = get_leaderboard()
    update_leaderboard(leaderboard, winner)
    save_leaderboard(leaderboard)

if __name__ == '__main__':
    main()

Starting off, its not secure the way you are prompting for passwords and storing them. Instead, use the getpass module, and store the hash of the password. I'm not even certain that this way is 100% correct (I'm not a security guru), but its what the Python documentation shows for secure password hashing, and I imagine its better than plaintext.

from getpass import getpass
from hashlib import pbkdf2_hmac as pwhash

def login(users):
    username = input('please enter username: ')
    if not username in users:
        print("access denied")
        exit()

    password_hash = None
    while password_hash != users[username]:
        password_hash = pwhash("sha256", getpass().encode("ascii"), b"t4h20p8g24j", 100000)

    print("access granted")
    return username

Your roll function had a typo in it-  the third (optional) roll could only generate numbers [1, 5] inclusive. I also reordered the statements to remove an intermediate variable that didn't contribute anything.

def roll():
    die1 = random.randint(1, 7)
    die2 = random.randint(1, 7)
    points = die1 + die2
    points += 10 if (die1 + die2) % 2 == 0 else -5
    if die1 == die2:
        points += random.randint(1, 7)
    return points

I would replace your get_leaderboard body with a shorter and cleaner read call.

def get_leaderboard():
    with open('Leaderboard.txt', 'r+') as g:
        return g.read().split("\n")

And similarly for save_leaderboard

def save_leaderboard(leaderboard):
    with open('Leaderboard.txt', 'r+') as h:
        h.write("\n".join(leaderboard))

You should also try to avoid excessive use of tuples when you need to access the elements individually. Your code will be littered with thingy[0] and thingy[1] and nobody but you (if that) will know what the significance of 0 and 1 are. I revised update_leaderboard with these changes, but I'm still not quite satisfied with it.

def update_leaderboard(leaderboard, winner):
    winner_score, winner_name = winner
    for idx, item in enumerate(leaderboard):
        item_score, item_name = item.split(', ')
        if item.name == winner_name and int(item_score) < int(winner_score):
            leaderboard[idx] = '{}, {}'.format(winner_score, winner_name)
    leaderboard.sort(reverse=True)
    #leaderboard.sort(reverse=True), key=lambda x:int(x.split(', ')[0]))

The data really shouldn't be cast back and forth from strings to lists/tuples so much. It would be best to just make a dictionary of name:score pairs, and write a tiny bit of formatting logic into your get_leaderboard and save_leaderboard functions. I wasn't going to change that much though.

I also want to point out common mistake that could normally create a bug in sorting (in your case it does not because of the hard-coded ,, but still)-  you can't rely on str.sort to sort a list of strings by their numerical value. I left a comment showing how you would do this correctly if your delimiter evaluated as greater than "0".

Furthermore- you shouldn't really rely on changing leaderboard outside the function scope. I feel like this is partially why you chose to go with the (unnecessary) enumeration() logic. Instead, return the modified leaderboard and allow the caller to take it from there.

Anyways, the last major change I would make would be to move all your globals (and friends) inside your main() function.

def main():
    print("")
    print("welcome to the dice game")
    print("")

    with open('users.pickle', 'rb') as f:
        users = pickle.load(f)

    print(users)
    print("  ")

    user1 = login(users)
    user2 = login(users)
    ...


On What Day Is That Date I got bored on my long weekend so I decided to have my coding exercise. And I successfully made a program that let you input the day and the month then it gives you the day of given date. But sadly it's only for 2014 calendar.

import java.io.*;

public class DaysToYears{
public static void main(String[] args) throws ArrayIndexOutOfBoundsException{
  BufferedReader dataIn = new BufferedReader(new InputStreamReader(System.in));
  try{
    String []yearName = {"January","February","March","April","May","June","July","August","September","October","November","December"};
    String []days = {"Sunday","Monday","Tuesday","Wednesday","Thursday","Friday","Saturday"};
    int []yearsCode = {6, 2, 2, 5, 0, 3, 5, 1, 4, 6, 2, 4};
    int firstCase = 0;
    int secondCase = 0;
    String inputMonth = new String();
    int inputDate = 0;
      System.out.print("Enter your month: ");
        inputMonth = dataIn.readLine();
      System.out.print("Enter your date:  ");
        inputDate = Integer.parseInt(dataIn.readLine());
          for(int nodeCount=0;nodeCount<yearName.length;nodeCount++){
              if(inputMonth.equalsIgnoreCase(yearName[nodeCount])){
                if(inputDate < 7){
                    firstCase = (yearsCode[nodeCount] + inputDate + 3) % 7;
                    System.out.println(yearName[nodeCount] + " " + inputDate + " is " + days[firstCase]);
                    }else{
                      secondCase = (yearsCode[nodeCount] + inputDate + 3) % 7;
                      System.out.println(yearName[nodeCount] + " " + inputDate + " is " + days[secondCase]);
                      }
                    }
                  }
    }catch(IOException e){
      System.out.println("Error getting input!");
    }
  }
}

This "new String()" is unnecessary:

String inputMonth = new String();

I would leave it out completely:

String inputMonth;


Finding all the subsets in an array of integers that sum to a given target Problem Statement:

  Given an Array if ints, Find out all the subsets in the Array that
  sum to a given target value.

  Example:

  If the input array is:

  {1, 3, 2, 5, 4, 9} with target as 9

  The resulting subsets are:

135
324
9
54

Below is my implementation in Java. Please free to review in terms of time, space complexity, style etc and a more efficient solution is welcome.

 import java.util.HashSet;

 /**
  * Created by anirudh on 12/5/15.
 */
 public class FindSubsetsThatSumToATarget {

    /**
     * The collection for storing the unique sets that sum to a target.
     */
    public static HashSet<String> allSubsets = new HashSet<>();

    /**
     * The method for finding the subsets that sum to a target.
     *
     * @param input  The input array to be processed for subset with particular sum
     * @param target The target sum we are looking for
     * @param ramp   The Temporary String to be beefed up during recursive iterations(By default value an empty String)
     * @param index  The index used to traverse the array during recursive calls
     */
    private void FindTargetSumSubsets(int[] input, int target, String ramp, int index) {

        if(index > (input.length - 1)) {
            if(getSum(ramp) == target) {
                allSubsets.add(ramp);
            }
            return;
        }

        //First recursive call going ahead selecting the int at the currenct index value
        FindTargetSumSubsets(input, target, ramp + input[index], index + 1);
        //Second recursive call going ahead WITHOUT selecting the int at the currenct index value
        FindTargetSumSubsets(input, target, ramp, index + 1);
    }

    /**
     * A helper Method for calculating the sum from a string of integers
     *
     * @param intString the string subset
     * @return the sum of the string subset
     */
    private static int getSum(String intString) {
        int sum = 0;
        for (int i = 0; i < intString.length(); i++) {
            sum += Integer.parseInt(intString.substring(i, i + 1));
        }
        return sum;
    }

    /**
     * Cracking it up here : )
     *
     * @param args command line arguments.
     */
    public static void main(String[] args) {
        int [] n =  {1, 3, 2, 5, 4, 9};
        new FindSubsetsThatSumToATarget().FindTargetSumSubsets(n, 9, "", 0);
        for (String str: allSubsets) {
            System.out.println(str);
        }
    }
}

I have some across a problem with the above implementation that in case of repeated values in the input array I am getting duplicate permutes of sets that sum to a given target.

E.g.: In input is:

int [] n =  {2, 3, 2};

The resulting sets generated are:

32
23

Which in the given context are actually the same.

Violations of general good practices

Using String.substring to extract single characters is odd. Use String.charAt instead next time
Parsing single character strings using Integer.parseInt is odd. Use String.charAt(index) - '0' instead
allSubsets should be declared as Set instead of HashSet (use interface types instead of implementations)
Method names should be camelCase
No need to write JavaDoc for private methods, simple comments are good enough, if needed at all

Strange way to collect integers

The way you collect integers in a String is strange in many ways:

The program will not work for numbers > 9
Appending digits to a string one by one, and then finally parsing them from the string one by one is strange.

You could use a Set instead. Sure, maybe it seems a bit strange and inefficient to clone the set in each recursive call, but I still think it would be better. It would also solve the problem you found, with the duplicate sequences in the results.
Probably there is an even better way, but I have no more time now.

Strange static and non-static variables

allSubsets is static, but input and target are not.
It would make more sense to either make all of these static, or none of them.

Not suggested implementation

I can't say I suggest this implementation, but it solves some issues with yours:

private void find(int[] input, int target, Set<Integer> ramp, int index) {
    if (index > input.length - 1) {
        if (getSum(ramp) == target) {
            allSubsets.add(toString(ramp));
        }
        return;
    }

    find(input, target, updatedSet(ramp, input[index]), index + 1);
    find(input, target, ramp, index + 1);
}

private Set<Integer> updatedSet(Set<Integer> ramp, int integer) {
    Set<Integer> updated = new HashSet<Integer>();
    updated.addAll(ramp);
    updated.add(integer);
    return updated;
}

private String toString(Set<Integer> ramp) {
    StringBuilder builder = new StringBuilder();
    for (Integer integer : ramp) {
        builder.append(integer);
    }
    return builder.toString();
}

private static int getSum(Set<Integer> integers) {
    int sum = 0;
    for (Integer integer : integers) {
        sum += integer;
    }
    return sum;
}


Solution to the 0/1 knapsack in Python So I made a version for the 0/1 knapsack problem myself (using matrix dynamic programming algorithm). Given a list of items with name, value, and weight, my function computes correctly the optimal value with total weight <= allowed weight.

I don't know if my code is clean and pythonic enough, I would greatly appreciate it if you give me some comments, thanks.

def knapsack(W, items):
    """
    Given a list of items with name, value and weight.
    Return the optimal value with total weight <= allowed weight and
    list of picked items.
    """
    n = len(items)
    k = [[0 for x in range(W+1)] for x in range(n+1)]

    for i in range(n+1):
        for w in range(W+1):
            if i == 0 or w == 0:
                k[i][w] = 0
            elif items[i-1][2] <= w:
                k[i][w] = max(items[i-1][1] + k[i-1][w-items[i-1][2]], k[i-1][w])
            else:
                k[i][w] = k[i-1][w]

    picked = []
    set_trace(k, n, W, items, picked)
    return k[n][W], picked

# find which item are picked
def set_trace(k, n, W, items, picked):
    for i in range(n, 0, -1):
        if k[i][W] != k[i-1][W]:
            picked.append(items[i-1])
            set_trace(k, i-1, W-items[i-1][2], items, picked)
            break

if __name__ == '__main__':
    items = [('A', 1, 1), ('B', 4, 3), ('C', 5, 4), ('D', 7, 5)]
    max_value,  picked = knapsack(7, items)
    print("Maximum value:", max_value)
    print("Name", "Value", "Weight")
    for item in reversed(picked):
        print(item[0], ' '*2, item[1], ' '*3, item[2])

Output:

Maximum value: 9
Name Value Weight
B    4     3
C    5     4

Kudos for using list comprehensions. However picked = []; set_trace(k, n, W, items, picked) and picked.append(items[i-1]) doesn't feel as good.

One could expect set_trace to build the list and return it. Even better, would be to picked = list(set_trace(k, n, W, items)), meaning set_trace would be a generator (with a pretty terrible name).

In order to do so, you first have to come to the conclusion that the recursion is really not necessary. All you do, in that recursive call, is continue the iteration at the next step with a different W. And that's it, you break. You could have just replace these two lines with W -= items[i-1][2]. Doing so, you could replace the append with a yield and you have your generator:

def set_trace(k, n, W, items):
    for i in range(n, 0, -1):
        if k[i][W] != k[i-1][W]:
            yield items[i-1]
            W -= items[i-1][2]

this lets you define picked = list(set_trace(k, n, W, items)).

However, n is len(items) so you might as well not pass it as a parameter and compute it at the beginning of the function.

You can also get much better readability by improving your naming. I understand that you named your variables based of the math theory that this problem relly on, but single letter variable names are a poor choice.

Same for using a simple tuple to store your items: it doesn't convey meaning. Use a namedtuple instead.

Last thing, since you are skipping the first row and first column of k, you can use slices and the enumerate builtin to avoid using so much brackets; you can also improve set_trace in such a way by using zip to create pairwise values of potential weights:

from collections import namedtuple

Item = namedtuple('Item', 'name value weight')

def knapsack(allowed_weight, items):
    """
    Given a list of items with name, value and weight.
    Return the optimal value with total weight <= allowed weight and
    list of picked items.
    """
    k = [
        [0 for x in range(allowed_weight + 1)]
        for x in range(len(items) + 1)
    ]

    for next_idx, (item, weights) in enumerate(zip(items, k), 1):
        for w, current_weight in enumerate(weights[1:], 1):
            if item.weight <= w:
                k[next_idx][w] = max(
                    item.value + weights[w - item.weight],
                    current_weight
                )
            else:
                k[next_idx][w] = current_weight

    return k[-1][-1], list(fetch_items(k, allowed_weight, items))

# find which item are picked
def fetch_items(k, allowed_weight, items):
    for item, weights_p, weights_n in zip(items[::-1], k[-2::-1], k[::-1]):
        if weights_n[allowed_weight] != weights_p[allowed_weight]:
            yield item
            allowed_weight -= item.weight

if __name__ == '__main__':
    items = [
        Item('A', 1, 1),
        Item('B', 4, 3),
        Item('C', 5, 4),
        Item('D', 7, 5),
    ]
    max_value, picked = knapsack(7, items)
    print("Maximum value:", max_value)
    print("Name", "Value", "Weight")
    for item in reversed(picked):
        print(item.name, ' '*2, item.value, ' '*3, item.weight)


Hash function for strings Implementation of a hash function in java, haven't got round to dealing with collisions yet.
I tried to use good code, refactored, descriptive variable names, nice syntax.

My code must be efficient and extensible.

Would be interested to know how you guys would improve.
Three classes:

TestHash - main method is here, hash is initialised and data
inserted and read
HashTable -object were the hash table is held
HashEntry - each entry in the array is a 'HashEntry' object

HashTable

import java.io.*;

public class HashTable {
    // number of indexes in hash table
    private final static int TABLE_SIZE = 100;
    // initialise array of HashEntry (table)
    HashEntry[] table;

    // constructor, create array, assign all values to null
    HashTable() {
        // create array of HashEntry, predefined size
        table = new HashEntry[TABLE_SIZE];
        // assign all values to null
        for (int i = 0; i < TABLE_SIZE; i++) {
            table[i] = null;
        }
    }

    // from key, find the correct index, load the HashEntry object and return the value
    public String getValue(String key) {
        int index = this.getKeyIndex(key);
        HashEntry chosen_entry = table[index];
        String chosen_entry_value = chosen_entry.getValue(key);
        return chosen_entry_value;
    }

    // creates a new HashEntry object, inserts that into the table array at the correct index
    // determined by getKeyIndex(key)
    public void insert(String key, String value) {
        int index = this.getKeyIndex(key);
        HashEntry keyValuePair;
        keyValuePair = new HashEntry(key, value);
        table[index] = keyValuePair;
        return;
    }

    // ** PRIVATE FUNCTIONS ** //
    // get index from key
    private Integer getKeyIndex(String key) {
        int hash = 7;
        for (int i = 0; i < key.length(); i++) {
            hash = (hash*31 + key.charAt(i))%100;
        }
        return hash;
    }
}

HashEntry

public class HashEntry {
    private String key;
    private String value;

    HashEntry(String key, String value) {
        this.key = key;
        this.value = value;
    }

    public String getKey() {
        return this.key;
    }

    public String getValue(String key) {
        // use string key to move down linked list in case of collision
        // haven't implemented this yet
        return this.value;
    }
}

TestHash

public class TestHash {
    public static void main(String[] args)
    {
        HashTable tableHash = new HashTable();
        tableHash.insert("david", "england");
        tableHash.insert("joe", "united states");
        tableHash.insert("maria", "china");
        System.out.println(tableHash.getValue("joe"));
    }
}

Very nifty-looking code right there. However, you should declare table as private final. Also, you don't need to initialize each array component of table to null, Java already does this for you. What comes to insert, you could write simply

table[index] = new HashEntry(key, value);

omitting the return statement. Point is to write only necessary statements. In getValue, you might want to check whether table[index] is null, since if that is the case, your implementation will throw NullPointerException.


Exponentiation in logarithmic time Input: Real number y and Integer number x
Output: x^y
Constraint: Must be done in O(log(y)) time.

Notice that O(log(y)) is O(length-of(y)).

public static double expon(double x, int y){
    double result = 1.0;

    double part = x;
    for (int i = 0; i < 32; i++){
        if (((y >> i) & 1) == 1){
            result *= part;
        }
        part *= part;
    }

    return result;
}

Your code is lacking comments that explain the why, ideally every non-trivial math programme should include a brief description of the algoritmh used.


Simple BMI calculator in Java A similar question must have been asked a thousand times, but for the life of me I can't figure this out. I am new to Java and I want to create a programme that asks a user for his or her height and weight, and calculate the corresponding BMI. I want to throw exceptions for the wrong input (has to be a double, non-negative and not zero) and I want to catch any mathematical errors that may occur. The follow code does that, but I have exact duplication for handling the input of height and weight. How can I simplify this into a more concise format without losing any functionality?

import java.util.InputMismatchException;
import java.util.Scanner;

public class BMIcalculator {
    public static void main(String[] args){
        double BMI = calculateBMI();
        System.out.print("\nYour BMI is " + BMI + ".");
    }

    public static double calculateBMI() {
        // Scanner allows us to request a users input
        Scanner scan = new Scanner(System.in);
        // Initialize doubles as zero values
        double w=0, h=0;

        // Ask for user input (weight)
        System.out.println("Please enter your weight");
        while(true) {
            try {
                w = scan.nextDouble();
                if (w <= 0) throw new ArithmeticException();
                break;
            } catch (InputMismatchException ime) {
                System.err.println("Ahw, error: " + ime.getMessage());
                continue;
            } catch (ArithmeticException ae) {
                System.err.println("Weight can't be less than or equal to zero!");
                continue;
            }
        }
        // Ask for user input (height)
        System.out.println("Please enter your height");
        while(true) {
            try {
                h = scan.nextDouble();
                if (h <= 0) throw new ArithmeticException();
                break;
            } catch (InputMismatchException ime) {
                System.err.println("Ahw, error: " + ime.getMessage());
                continue;
            } catch (ArithmeticException ae) {
                System.err.println("Height can't be less than or equal to zero!");
                continue;
            }
        }
        // try-catch not necessary; we only get here when our input is valid

        // we have to close the input connection to the user
        scan.close();
        return w / (h * h);
    }
}

Avoiding code repetition

As pointed out in some of the other answers, getting a double value from the keyboard can be abstracted into its own method to avoid code repetition:

private static double getDouble(Scanner scanner, String metric) {
    System.out.println("Enter " + metric);
    double result;
    while (!scanner.hasNextDouble() || (result = scanner.nextDouble()) <= 0) {
        System.out.println(metric + " must be a positive number.");
        scanner.next();
    }
    return result;
}

try-with-resources

If you are on Java 7 and above, you should use try-with-resources for efficient handling of the underlying I/O resource:

public static void main(String[] args) {
    try (Scanner scanner = new Scanner(System.in)) {
        double weight = getDouble(scanner, "Weight (kg)");
        double height = getDouble(scanner, "Height (m)");
        System.out.println("Your BMI is: " + /* calculation */);
    }
}

Units

It's important to note that BMI is expressed in units of \$\frac{kg}{m^2}\$, so you should definitely display the units of measurement too. Otherwise, you'll need extra methods to perform the conversion to kilograms and meters. :)

Comments

Simply put, comments should explain the why, not how. Most of your comments now are just descriptive, i.e. on the how, which is already apparent in the code. Comments are sometimes seen as redundant as they are prone to being outdated. Therefore, comment sparingly, and only when you really need it to explain something that is non-obvious. Even so, try to make the code 'obvious' first. :)


Is the following repository pattern properly applied? Is this repository code written according to best practices? The Last Section I included it in the repository as well.

class HR_Repository<T> : IRepository<T> where T : class
{
    private readonly  LoginDataContext dataContext;
    public HR_Repository(LoginDataContext dataContext)
    {
    this.dataContext = dataContext;
    }

    public void Commit()
    {
        dataContext.SubmitChanges();
    }

    public IList<T> FindAll()
    {
        var table = this.LookupTableFor(typeof(T));
        return table.Cast<T>().ToList();
    }

    public IQueryable<T> Find()
    {
        var table = this.LookupTableFor(typeof(T));
        return table.Cast<T>();
    }

    public void Add(T item)
    {
        var table = this.LookupTableFor(typeof(T));
        table.InsertOnSubmit(item);
    }

    public void Delete(T item)
    {
        var table = this.LookupTableFor(typeof(T));
        table.DeleteOnSubmit(item);
    }

    private ITable LookupTableFor(Type entityType)
    {
        return dataContext.GetTable(entityType);
    }
}

public static class UserQueries
{
    public static Employee ByUserName(this IQueryable<Employee> employees, string username)
    {
        return employees.Where(u => u.User_Name == username).FirstOrDefault();
    }
}

I don't like the underscore in the class name, HR_Repository. It's a generic name for a generic class, which is good, but seeing that it implements IRepository<T> I would expect something like this:
public class EmployeeRepository : IRepository<Employee>

I like that you're initializing your private readonly LoginDataContext in the constructor, from a constructor argument.
private readonly  LoginDataContext dataContext;
public HR_Repository(LoginDataContext dataContext)
{
    this.dataContext = dataContext;
}

I like it because this implementation of IRepository<T> has a dependency on the LoginDataContext - by passing it into the constructor, you're doing constructor injection and that's awesome. Ideally the dependency would be an abstraction (abstract class or interface), which makes it easier to inject as a mock or fake implementation when you're writing unit tests. Even if you never write these tests, the fact that your code would be testable is a very good thing.
I don't like that your static helper provides extension methods to IQueryable<Employee>, it's not clear how/where this code is being used, the benefits aren't immediately apparent.

As for the pattern itself, it looks like a hybrid Unit-of-Work / Repository implementation, because of the Commit method. But your implementation exposes ITable and IQueryable, which leaks out the ORM into the calling code.
If you consider the client as, say, a controller class (assuming MVC), the controller would have a dependency on the unit-of-work, which has a dependency on the data context and lets you work with one or more repositories, which all share the data context of the unit-of-work:

(I skipped UML classes, hope this doesn't look too weird)

Disclaimer
I'm biased. I'm not a fan of unit-of-work/repository pattern, at least not with Entity Framework. I think Entity Framework's DbContext is a unit-of-work, and the IDbSet<TEntity> properties it exposes are repositories - I think wrapping this in another level of abstraction has very little benefits and adds a considerable amount of complexity to the code.
Instead, I simply wrap DbContext with an interface:
public interface IUnitOfWork
{
    void SaveChanges();
    IDbSet<TEntity> Set<TEntity>();
}

public class MyContext : DbContext, IUnitOfWork
{
    IDbSet<MyEntity> MyEntities { get; set; }
    // ...
}

Then I usually have a "service" class that takes an IUnitOfWork dependency; I write closely related queries inside that class, but the "service" only returns materialized results, it doesn't expose IQueryable<T>, even less so ITable. A controller will take a factory that can instantiate such a "service", wrapping its instance in a using block to ensure proper disposal.


Factoring the powers of 2 in Python The powers of 2 are trivial to factor because all powers of 2 have one 1-bit and the rest 0-bits. Just iterate minus one 0-bit as follows: 10000,1000,100,10,1. And then convert the binary strings into base-10 and you got the factors for powers of 2.

Example

import operator
from operator import mul
from functools import reduce
import time

print('Find the factors of a power of 2')
print('This input number must be a power of 2')
X = 32

binary = "{0:b}".format(int(X))
l = len(binary)

# Checking if input is power of 2
# All binary strings that are a power of 2 must
# have only ONE 1 bit

if binary.count(str(1)) == 1:
  print('yes, this is a power of two')
else:
  print('no, this is not a power of 2')
  quit()

start = (round(time.time() * 1000000))
count = 0
result = [];
for j in range(0, l):
        count = count + 1
        # create list of factors called result
        result.append(int(binary[0:l-count], 2))
        if len(result) == l-1:
          # I don't want to forget X itself is a factor
          result.append(X)
          end = (round(time.time() * 1000000))
          break

print(result)
print('Factoring took', end-start, 'microseconds')

Output

Find the factors of a power of 2
This input number must be a power of 2
yes, this is a power of two
[16, 8, 4, 2, 1, 32]
Factoring took 0 microseconds

Question

What better names could I use to define my variables, and what could make the program run faster?

Is there a more efficient implementation to decide if a number is a power of 2?

There should be some function definitions. is_power_of_2, powers_of_2_below and a main.
It's good practice to use an if __name__ == '__main__' guard.
Please stop putting lots of spaces at the end of lines.
Many of the imports are not used.
Always indent with 4 spaces.

import time

def is_power_of_2(X):
    binary = "{0:b}".format(int(X))
    return binary.count(str(1)) == 1

def powers_of_2_below(X):
    binary = "{0:b}".format(int(X))
    l = len(binary)
    count = 0
    result = [];
    for j in range(0, l):
        count = count + 1
        # create list of factors called result
        result.append(int(binary[0:l-count], 2))
        if len(result) == l-1:
            # I don't want to forget X itself is a factor
            result.append(X)
            break
    return result

def main(X):
    print('Find the factors of a power of 2')
    print('This input number must be a power of 2')
    if is_power_of_2(X):
        print('yes, this is a power of two')
    else:
        print('no, this is not a power of 2')
        quit()
    start = (round(time.time() * 1000000))
    result = factors_of_power_of_2(X)
    end = (round(time.time() * 1000000))
    print(result)
    print('Factoring took', end - start, 'microseconds')

if __name__ == '__main__':
    main(X)

You can now use return rather than quit to exit main.
The code in is_power_of_2 is fine. There are a couple other ways you can implement it but they're all pretty underwhelming.
By replacing count with (j + 1) we can remove the need for that variable.
By using j rather than (j + 1) we can remove the need for the if in the for.
To get l in powers_of_2_below we can use X.bit_length().
We can instead use 1 << j to build the value from j alone. This however will invert the output.

import time

def is_power_of_2(x):
    binary = "{0:b}".format(int(x))
    return binary.count(str(1)) == 1

def powers_of_2_below(x):
    result = []
    for j in range(0, x.bit_length()):
        result.append(1 << j)
    return result

def main(x):
    print('Find the factors of a power of 2')
    print('This input number must be a power of 2')
    if is_power_of_2(x):
        print('yes, this is a power of two')
    else:
        print('no, this is not a power of 2')
        return
    start = (round(time.time() * 1000000))
    result = factors_of_power_of_2(x)
    end = (round(time.time() * 1000000))
    print(result)
    print('Factoring took', end - start, 'microseconds')

if __name__ == '__main__':
    main(32)

We can use a list comprehension instead of a for loop in powers_of_2_below.

def powers_of_2_below(x):
    return [1 << j for j in range(x.bit_length())]

Alternate solutions to is_power_of_2:

We can use powers_of_2_below with & and any.

def is_power_of_2(x):
    x = abs(x)
    if x == 0:
        return False
    return not any(x & i for i in powers_of_2_below(x - 1))

We can replace the any with just x & x - 1. This is because \$1000 - 1 = 0111\$. Logically this is doing exactly the same as 13.

def is_power_of_2(x):
    x = abs(x)
    if x == 0:
        return False
    return not x & x - 1

We can instead use x.bit_length() to construct the power of 2 it should be and check they are the same.

def is_power_of_2(x):
    if x == 0:
        return False
    return abs(x) == 1 << x.bit_length() - 1

Overall is_power_of_2 just doesn't look nice.


Race car with falling objects that make the car go back to its initial position game I saw a game made in Python and PyGame in a tutorial, so I thought I'd make a game like it in Allegro5 and C++. I think I made my style a little better than my style before since I stopped putting everything in the main function, and I also stopped using 'magic numbers'. I'd like to know of your opinions, and what else I can improve to make my code better.

#include <stdio.h>
#include <allegro5/allegro.h>
#include <allegro5/allegro_font.h>
#include <allegro5/allegro_primitives.h>
#include <allegro5/allegro_image.h>
#include <allegro5/allegro_color.h>
#include <allegro5/allegro_ttf.h>
#include <iostream>
ALLEGRO_DISPLAY *display;
ALLEGRO_EVENT ev;
ALLEGRO_EVENT_QUEUE *eventqueue;
ALLEGRO_TIMER *timer;
ALLEGRO_BITMAP *car;
ALLEGRO_BITMAP *object;
float FPS = 60.0;
int width = 800;
int height = 600;
int objectX = 810;
int objectY = 610;
int objectYStep = 5;
int playerPosX = width / 2;
int playerPosY = 510;
int playerStep = 10;
bool isOpen = true;
void moveObject(){
objectY = objectY + objectYStep;
}
void movePlayerRight(){
playerPosX = playerPosX + playerStep;
    }
void movePlayerLeft(){
playerPosX = playerPosX - playerStep;
}
bool objectOutOfBounds(){
return objectX > width || objectY > height;
}
bool intersects(ALLEGRO_BITMAP *objecte, ALLEGRO_BITMAP *cars, int objecteX,    int objecteY, int carsX, int carsY){
int objecteW = al_get_bitmap_width(objecte);
int objecteH = al_get_bitmap_height(objecte);
int careW = al_get_bitmap_width(cars);
int careH = al_get_bitmap_height(cars);
return objecteX<careW + carsX && objecteX + objecteW>carsX && objecteY<careH + carsY && objecteX + objecteH>carsY;
}

int main(){

al_init();
if (!al_init()){
    std::cout << "Failed to initialize allegro5." << std::endl;
}
al_init_image_addon();
if (!al_init_image_addon()){
    std::cout << "Failed to initialize image." << std::endl;
}
display = al_create_display(width, height);
if (!display){
    std::cout << "Failed to create display." << std::endl;
}
al_install_keyboard();
if (!al_install_keyboard()){
    std::cout << "Failed to install keyboard." << std::endl;
}

eventqueue = al_create_event_queue();
timer = al_create_timer(1.0 / FPS);
car = al_load_bitmap("racecar.png");
object = al_load_bitmap("brickonn.jpg");

al_register_event_source(eventqueue, al_get_display_event_source(display));
al_register_event_source(eventqueue, al_get_keyboard_event_source());
al_register_event_source(eventqueue, al_get_timer_event_source(timer));

while (isOpen){
    al_clear_to_color(al_map_rgb(255, 255, 0));
    ALLEGRO_EVENT event;
    al_wait_for_event(eventqueue, &event);
    al_start_timer(timer);
    if (objectOutOfBounds()){
        objectX = rand() % width;
        objectY = 0;
        objectYStep = objectYStep + 1;
    }
    if (event.type == ALLEGRO_EVENT_TIMER){
        moveObject();
    }
    if (event.keyboard.keycode == ALLEGRO_KEY_LEFT){
        movePlayerLeft();
    }
    else if (event.keyboard.keycode == ALLEGRO_KEY_RIGHT){
        movePlayerRight();
    }
    if (intersects(object, car, objectX, objectY, playerPosX, playerPosY)){
        std::cout << "CRASH!" << std::endl;
        playerPosX = width / 2;
    }
    al_draw_bitmap(car, playerPosX, playerPosY, 0);
    al_draw_bitmap(object, objectX, objectY, 0);
    al_flip_display();
}

al_flip_display();
al_rest(10);
}

indentation

A little bit of whitespace goes a long way in giving your code a visual structure. This makes the code more readable:

#include <stdio.h>
#include <allegro5/allegro.h>
#include <allegro5/allegro_font.h>
#include <allegro5/allegro_primitives.h>
#include <allegro5/allegro_image.h>
#include <allegro5/allegro_color.h>
#include <allegro5/allegro_ttf.h>
#include <iostream>

ALLEGRO_DISPLAY *display;
ALLEGRO_EVENT ev;
ALLEGRO_EVENT_QUEUE *eventqueue;
ALLEGRO_TIMER *timer;
ALLEGRO_BITMAP *car;
ALLEGRO_BITMAP *object;

float FPS = 60.0;
int width = 800;
int height = 600;
int objectX = 810;
int objectY = 610;
int objectYStep = 5;
int playerPosX = width / 2;
int playerPosY = 510;
int playerStep = 10;
bool isOpen = true;

void moveObject(){
    objectY = objectY + objectYStep;
}

void movePlayerRight(){
    playerPosX = playerPosX + playerStep;
}

void movePlayerLeft(){
    playerPosX = playerPosX - playerStep;
}

bool objectOutOfBounds(){
    return objectX > width || objectY > height;
}

bool intersects(ALLEGRO_BITMAP *objecte, ALLEGRO_BITMAP *cars, int objecteX, int objecteY, int carsX, int carsY){
    int objecteW = al_get_bitmap_width(objecte);
    int objecteH = al_get_bitmap_height(objecte);
    int careW = al_get_bitmap_width(cars);
    int careH = al_get_bitmap_height(cars);
    return objecteX<careW + carsX && objecteX + objecteW>carsX && objecteY<careH + carsY && objecteX + objecteH>carsY;
}

int main(){
    al_init();
    if (!al_init()){
        std::cout << "Failed to initialize allegro5." << std::endl;
    }
    al_init_image_addon();
    if (!al_init_image_addon()){
        std::cout << "Failed to initialize image." << std::endl;
    }
    display = al_create_display(width, height);
    if (!display){
        std::cout << "Failed to create display." << std::endl;
    }
    al_install_keyboard();
    if (!al_install_keyboard()){
        std::cout << "Failed to install keyboard." << std::endl;
    }

    eventqueue = al_create_event_queue();
    timer = al_create_timer(1.0 / FPS);
    car = al_load_bitmap("racecar.png");
    object = al_load_bitmap("brickonn.jpg");

    al_register_event_source(eventqueue, al_get_display_event_source(display));
    al_register_event_source(eventqueue, al_get_keyboard_event_source());
    al_register_event_source(eventqueue, al_get_timer_event_source(timer));

    while (isOpen){
        al_clear_to_color(al_map_rgb(255, 255, 0));
        ALLEGRO_EVENT event;
        al_wait_for_event(eventqueue, &event);
        al_start_timer(timer);
        if (objectOutOfBounds()){
            objectX = rand() % width;
            objectY = 0;
            objectYStep = objectYStep + 1;
        }
        if (event.type == ALLEGRO_EVENT_TIMER){
            moveObject();
        }
        if (event.keyboard.keycode == ALLEGRO_KEY_LEFT){
            movePlayerLeft();
        }
        else if (event.keyboard.keycode == ALLEGRO_KEY_RIGHT){
            movePlayerRight();
        }
        if (intersects(object, car, objectX, objectY, playerPosX, playerPosY)){
            std::cout << "CRASH!" << std::endl;
            playerPosX = width / 2;
        }
        al_draw_bitmap(car, playerPosX, playerPosY, 0);
        al_draw_bitmap(object, objectX, objectY, 0);
        al_flip_display();
    }

    al_flip_display();
    al_rest(10);
}

Pick a coding style and consistently use it. Prefer a coding style that's common or "standard" in your language. It greatly helps when getting help from people. If your code is easy to digest, it's more likely somebody will do so.

comments

There are no comments. That's a problem. Add some comments to your code to explain what's going on.

more functions

  I stopped putting everything in the main function

That's a good idea. You can go even further. Look at the first part of your main() function. If you had to put a comment over it, it'd probably look like that:

// initialisation
al_init();
if (!al_init()){
    std::cout << "Failed to initialize allegro5." << std::endl;
}
al_init_image_addon();
if (!al_init_image_addon()){
    std::cout << "Failed to initialize image." << std::endl;
}
display = al_create_display(width, height);
if (!display){
    std::cout << "Failed to create display." << std::endl;
}
al_install_keyboard();
if (!al_install_keyboard()){
    std::cout << "Failed to install keyboard." << std::endl;
}

eventqueue = al_create_event_queue();
timer = al_create_timer(1.0 / FPS);
car = al_load_bitmap("racecar.png");
object = al_load_bitmap("brickonn.jpg");

al_register_event_source(eventqueue, al_get_display_event_source(display));
al_register_event_source(eventqueue, al_get_keyboard_event_source());
al_register_event_source(eventqueue, al_get_timer_event_source(timer));

Instead, put all this code in a function and only invoke initialise() from main(). This takes away the gory details when reading the code at first glance. Ok, there's an initialisation going on. Am I interested in that? Yes -> dive into the function, No -> go ahead with main


Import function vs turning it into custom hook I have this ProductSearchContainer.js file that it's getting too long to handle and maintain.

What it's responsible for:

Doing network requests to get a list of products
Filter that list with filters values it gets from URL query string and update state with the new filteredList
It also holds functions to simulate the length of products on each filter before you click it. For example: a color filter with Black(5) indicating that you'll get 5 products if you click on Black.
Along other functions

The file was getting too big (600+ lines) and I decide to move some pieces of logic to other files.

For each filter category (brand, price, rating, etc) I have two functions:

1 to apply the active filters (get a list and the activeFilters array and it returns a filtered list for those filters.
1 to simulate the next filter length (like I explained above on the colors example)

Note: You'll see below that they rely on state variable activePriceFilters but they don't call any React Hook in their execution.

OPTION #1

My 1st thought was to turn into a custom hook. So I did. And it works (see snippet below).

function usePriceFilter(activePriceFilters) {

  function applyPriceFilter(list, activePriceFilters) {
    console.log('I will get a list and return it filtered by activePriceFilters');
  }

  function simulateNextPriceFilter(list, someTestPriceFilter) {
    console.log('I will get a list and return the length of the filtered list by price');
  }

  return [applyPriceFilter,simulateNextPriceFilter];

}

And I consume by doing:

const [applyPriceFilter,simulateNextPriceFilter] = usePriceFilter(activePriceFilters)

I mean, my custom hook is basically a higher order function but I think it still qualifies as a custom hook:

From React DOCS:

  A custom Hook is a JavaScript function whose name starts with ”use” and that may call other Hooks.

OPTION #2

But I guess I could also turn into a regular .js file, export both functions and just do the regular import on them. Like:

import {applyPriceFilter,simulateNextPriceFilter} from './priceFilterHelpers

QUESTION:

Is there a difference in functionaly or performance between the 2 approaches? Should I favor 1 instead of the other?

I think that the custom hook's is more readable, but is there anything else I'm gaining by doing this?

function App() {

  const [activePriceFilters,setActivePriceFilters] = React.useState(['10to20','50+']);

  const [applyPriceFilter, simulateNextPriceFilter] = usePriceFilter(activePriceFilters);

  return(
    <React.Fragment>
      <button onClick={applyPriceFilter}>Apply Price Filters</button>
      <button onClick={simulateNextPriceFilter}>Simulate Price Filter</button>
    </React.Fragment>
  );

}

function usePriceFilter(activePriceFilters) {

  function applyPriceFilter(list, activePriceFilters) {
    console.log('I will get a list and return it filtered by activePriceFilters');
  }

  function simulateNextPriceFilter(list, someTestPriceFilter) {
    console.log('I will get a list and return the length of the filtered list by price');
  }

  return [applyPriceFilter,simulateNextPriceFilter];

}

ReactDOM.render(<App/>, document.getElementById('root'));
<script src="https://cdnjs.cloudflare.com/ajax/libs/react/16.8.3/umd/react.production.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react-dom/16.8.3/umd/react-dom.production.min.js"></script>

<div id="root"/>

Well, in your case, your "hook" isn't really managing any state. I would say that you can safely hold it in a simple file and import it. But if your functions were to do something more than just logging, I would make a custom hook, since it would most likely work better with the lifecycle of components, triggering re-renders when needed.


Checking if a LinkedList is a palindrome I would like to know if my code is \$O(n)\$ space complexity.
The code checks if a doubly linked list is Palindrome.

public boolean isPalindromeList(LinkedList<Integer> list) {
        boolean isPalindrome = true;
        Stack<Integer> s = new Stack<Integer>();
        Iterator<Integer> iterator = list.iterator();
        // step1: iterate over the list and each put the element in the Stack
        while (iterator.hasNext()) {
            s.push(iterator.next());
        }
        // step2: pop elements from Stack and compare it while iterating through
        // the list
        Iterator<Integer> iterator2 = list.iterator();
        while (iterator2.hasNext()) {
            if (s.pop() != iterator2.next()) {
                return false;
            }
        }
        // for example List = {1,2,5,3}. Stack.pop()=3 do compare 3 == 1 if no
        // return false else continue
        // repeat pop() and compare with next element in the List
        return isPalindrome;
    }

According to my understanding, it's \$O(1)\$ space complexity, because it's just changing the node that iterator points to, not creating a new variable or reference. But, I'm not sure if pointing to another object results in creating a new variable in Java Stack memory referencing the current node object in the heap space.

As Ratchet Freak points out, your code uses an \$n\$ size stack to manage the reversal of the list, so it's \$O(n)\$ space complexity. The question is whether there's a better way too, though.

First up, your comparison here is very dubious:

if (s.pop() != iterator2.next()) {

That's doing a native object comparison, and you are comparing Integer, and not int. This is bound to fail, and I am surprised it works for you (I presume Java is reusing Objects for your int value boxing). You should be calling this instead:

if (!s.pop().equals(iterator2.next())) {

Then, your isPalindrome variable is useless. It's never set to false. Instead you return immediately if it's not a palindrome (which is a good thing).

Further, consider that your input is a LinkedList which is a double-linked implementation (as you pointed out) which means a reversed iteration is "easy".

public boolean isPalindromeList(LinkedList<Integer> list) {
    ListIterator<Integer> forward = list.listIterator(0);
    ListIterator<Integer> reverse = list.listIterator(list.size());
    while (forward.hasNext()) {
        if (!forward.next().equals(reverse.previous())) {
            return false;
        }
    }
    return true;
}

You can see this running in ideone: https://ideone.com/LmfQrB


Django Custom Decorator for user group check I wrote a custom decorator to check whether user is in particular group of not. Please provide the feedback on optimizing code. I currently moved from Java to Django-Python so I wasn't able to optimize the logic.

Check whether student is in labsheet_collector, assignment_collector or fund_collector.

def labsheet_collector(view_func):
    def _wrapped_view_func(request, *args, **kwargs):
        print request.user
        if request.user.is_anonymous:
            return redirect('/admin/') #?next=%s' % request.path)
        if not (request.user.groups.filter(name='labsheet_collector').exists()) or request.user.is_superuser:
            raise Http404
        return view_func(request, *args, **kwargs)

    return _wrapped_view_func

def assignment_collector(view_func):
    def _wrapped_view_func(request, *args, **kwargs):
        if request.user.is_anonymous:
            return redirect('/admin/')
        if not (request.user.groups.filter(name='assignment_collector').exists()) \
                or request.user.is_superuser:
            raise Http404
        return view_func(request, *args, **kwargs)

    return _wrapped_view_func

def fund_collector(view_func):
    def _wrapped_view_func(request, *args, **kwargs):
        if request.user.is_anonymous:
            return redirect('/admin/')
        if not (request.user.groups.filter(name='fund_collector').exists()) \
                or request.user.is_superuser:
            raise Http404
        return view_func(request, *args, **kwargs)

    return _wrapped_view_func

As an alternative to the answer given by jcollado, Django has some built in decorators to help.

From the docs:

from django.contrib.auth.decorators import user_passes_test

def email_check(user):
    return '@example.com' in user.email

@user_passes_test(email_check)
def my_view(request):

https://docs.djangoproject.com/en/1.6/topics/auth/default/#limiting-access-to-logged-in-users-that-pass-a-test

This could be modified to something like:

from django.contrib.auth.decorators import user_passes_test

def group_check(user):
    return user.groups.filter(name__in=['assignment_collector',
                                        'fund_collector'
                                        'labsheet_collector'])

@user_passes_test(group_check)
def my_view(request):
    ...

If they are not in these groups it will redirect by default to the login page.


Calculating chromosome coverage of a set of regions Some background: I work mostly in Python (specifically numpy), doing Bioinformatics type work. I am interested in moving towards Haskell. Initially, I hope it will be possible to replace some of the simple exploratory scripting I've been doing with Haskell. The following is not something that would actually be particularly useful, but I thought it might be a good starting point for my first program in Haskell!

The following code takes two files:

Describes regions of a chromosome that have been
categorized as similar. Line 1 shows the start basepair of the
region, line 2 the end basepair, and so on. A shortened example: http://pastebin.com/RMd13Kv3
Shows an interaction matrix between "bins" of the chromosome. A row has chrmName startBp endBp, followed by the row of the interaction matrix. This is not necessarily in order. Example: http://pastebin.com/Q5CaVHAu

The code is attempting to work out the fraction of the whole chromosome spanned by the regions.

import System.IO
import System.Environment(getArgs)
import Data.Either
import Data.List

joinLinePairs :: [a] -> ([a], [a])
joinLinePairs x = partitionEithers $ zipWith ($) (cycle [Left, Right]) x

zipLinesPairs :: ([a], [a]) -> [(a, a)]
zipLinesPairs (a, b) = zip a b

pairDifference :: (String, String) -> Int
pairDifference (a, b) = (read b) - (read a)

domainArea :: [String] -> Int
domainArea inpLines = sum (map pairDifference $ zipLinesPairs $ joinLinePairs inpLines)

processRawMat :: [String] -> [[Int]]
processRawMat x = map (processMatRow . words) x

processMatRow :: [String] -> [Int]
processMatRow (x:y:z:_) = [read y, read z]

maxMinBin :: [[Int]] -> (Int, Int)
maxMinBin lst = (minimum vals, maximum vals)
            where vals = concat lst

coverageFrac :: (Integral a, Fractional b) => (a, a) -> a -> b
coverageFrac (mn, mx) domArea = (fromIntegral domArea) / ((fromIntegral  mx) - (fromIntegral  mn))

main :: IO ()
main = do

args <- getArgs
    rawDomains <- readFile (args !! 0)
    let coveredArea = domainArea $ lines rawDomains
    print $ coveredArea
    rawMatrix <- readFile (args !! 1)
    let rawMatLines = lines rawMatrix
    let listolists = processRawMat rawMatLines
    print $ coverageFrac (maxMinBin listolists) coveredArea

One thing I noticed is that this code is very slow when running on real data (in reality the second file can run to tens of thousands of lines, and hundreds of mb). As most of the things I would be doing involve dealing with very large text files (these would be considered small, by comparison), this worries me a bit!

I would appreciate any feedback anyone could offer me. Thanks!

Try using the functions from then ByteString package instead of System.IO and String as they are much more efficient. Also, consider using lazy IO (using heGetContents from the link I sent) to read the huge input file as it will not load it all at once in memory while you parse it making it more efficient.


Collection-like classes in Java - taking advantage of varargs I quite often need to implement collection-like classes and - to make using them more comfortable - would like to take advantage of a constructor with variable arguments.

The following implementation does not work, though.

class FruitsStore
{
    private List<String> availableFruits;

    public FruitsStore(String... fruits)
    {
        this.availableFruits = Arrays.asList(fruits);
    }

    public void add(String fruit)
    {
        this.availableFruits.add(fruit);
    }

    ...
}

The following code snippet

FruitsStore store = new FruitsStore("banana", "melon");
store.add("orange");

throws the following exception

Exception in thread "main" java.lang.UnsupportedOperationException
    at java.util.AbstractList.add(Unknown Source)
    at java.util.AbstractList.add(Unknown Source)
    at cz.dusanrychnovsky.FruitsStore.add(FruitsStore.java:26)
    at cz.dusanrychnovsky.FruitsStore.main(FruitsStore.java:12)

To overcome this issue, I use a constructor like this:

public FruitsStore(String... fruits)
{
    this.availableFruits = new LinkedList<String>(Arrays.asList(fruits));
}

This compiles and works fine.

Is this a good coding practice? Does it have any drawbacks? What implementation would you suggest?

You have found the natural solution to this situation. Arrays.asList will give you a List with specific features - new xxList(Arrays.asList(...)) will give you a list with attributes you require.


Fill an array with last non-zero element to the end in PHP I need to fill an array with the last non-zero value.

Points of note I guess are that the array will either have all 0 values or end with 0 values.

Please review my code below.

<?php

$array = [1, 1, 5, 4, 0, 0];
$array2 = [0, 0, 0, 0, 0, 0];

function padArray($array) {
    $length = count($array) - 1;
    for ($i = $length; $i >= 0; $i--) {
        if($array[$i] != 0) {
            for ($j = $i; $j < count($array); $j++) {
                $array[$j] = $array[$i];
            }
            break;
        }
    }
    return $array;
}

var_dump(padArray($array));
var_dump(padArray($array2));

//should output [1,1,5,4,4,4]
//should output [0,0,0,0,0,0]

Do you really want this function to use loose comparison $array[$i] != 0?  What should intended behavior be for other potentially falsey values at the end of the array (false, "", [], etc.)?  If you truly only want to replace only trailing zeroes, then you must use exact comparison here !==.
Your function name seems unclear with regards to what the function does.  Perhaps arrayPadOverTrailingZeroes, arrayPadOverTrailingEmptyValues (depending on desired behavior).
PHP provides built-in array functions that may clean up this code a little bit (and perhaps perform slightly better since they are compiled).

For example:

function arrayPadOverTrailingZeroes($array) {
    $length = count($array);
    while (($value = end($array)) === 0) {
        array_pop($array);
    }
    // This conditional to support case where you have all zeroes,
    // in which case, the while loop above would have discarded all array members
    // and $value would be set to false
    if (count($array) === 0) {
        $value = 0;
    }
    return array_pad($array, $length, $value);
}

Answered this before I saw your all-zeroes use case.  Added some additional logic above to handle this case.


Python Search file for string and read into list This function of my program is searching a file that is formatted as first_name last_name rate hours:

John Smith 25 80
Mary Johnson 15.5 60.0
...

With one space between each item.

The user enters the first name and last name of the employee and while the line is not equal to null it searchs for the employee. If its found then found=True and it gets the rate and hours from that line and breaks out of the loop. Then if found=True, it will print the name of the employee and their gross pay (rate * hours), else it says employee is not found. The code is working in my program but I will be using something very similar later on for adding/modifying/deleting new employee functions.

# 'print_emp', display only a single employee's data chosen by the user displayed as
# firstname, lastname, grosspay (on one line of output)
def print_emp():
    menu_name = ' '*int(OFFSET/2) + "EMPLOYEE LOOKUP"
    dotted = (OFFSET+len(menu_name))*'-'
    found = False

    try:
        file= open('employees.txt')
        line = file.readline()
        name = input('Give me an employee to search for ')
        # run thru while loop & search for employee
        while line != ' ':
            line = line.split(' ') #split into a list
            if name == line[0]+' '+line[1]:
                found = True
                rate = float(line[2])
                hours = float(line[3])
                break
            line = file.readline() # read subsequent records
        # close file
        file.close()
        if found:
            print(name, '$%4.2f' % (rate*hours))
        else:
            print('Employee is not listed')

    except FileNotFoundError:
        print("Error: File not found.")

You could use the with..as construct to not worry anymore about closing the file.

Also iterating over the file and reading in the data can be simplified.

Also, you should not shadow the reserved name file.

menu_name and dotted seem not to be used atm.

Implementing these changes I get the following code:

def print_emp():
    # menu_name = ' '*int(OFFSET/2) + "EMPLOYEE LOOKUP"
    # dotted = (OFFSET+len(menu_name))*'-'
    search_name = input('Give me an employee to search for: ')
    try:
        with open('employees.txt') as employees:
            for line in employees:
                if not line:
                    continue
                first_name, last_name, rate, hours = line.split()
                name = "{} {}".format(first_name, last_name)
                rate, hours = map(float, (rate, hours))
                if name == search_name:
                    print("{} ${:4.2f}".format(name, rate*hours))
                    break
            else:  # Will be triggered if the loop was not interrupted with break
                print('Employee is not listed')
    except FileNotFoundError:
        print("Error: File not found.")

Now, you could actually separate the concerns here. Write a function that reads all employees, another that finds a name in all employees and a third that prints it. This can be efficiently implemented using classes. I would probably also not fail and continue if the file is missing:

class Employee:
    def __init__(self, name, rate, hours):
        self.name = name
        self.rate = rate
        self.hours = hours

    def __repr__(self):
        return "name: {}, ${:4.2f}/hour, {} hours".format(self.name, self.rate, self.hours)

    def __str__(self):
        return "{} ${:4.2f}".format(self.name, self.rate*self.hours)

def read_employees(file_name):
    employees = {}
    with open(file_name) as employees:
        for line in employees:
            *name, rate, hours = line.split()
            name = "{} {}".format(name)
            rate, hours = map(float, [rate, hours])
            if name in employees:
                raise ValueError("Employee with name {} already exists!".format(name))
            employees[name] = Employee(name, rate, hours)
    return employees

if __name__ == "__main__":
    employees = read_employees('employees.txt')
    name = input('Give me an employee to search for: ')
    print(employees[name])

This code will raise a KeyError if the name does not exist, To prevent this you can use this:

if __name__ == "__main__":
    employees = read_employees('employees.txt')
    name = input('Give me an employee to search for: ')
    try:
        print(employees[name])
    except KeyError:
        print('Employee is not listed')

Or even this to keep on asking until a valid name is given:

if __name__ == "__main__":
    employees = read_employees('employees.txt')
    name = input('Give me an employee to search for: ')
    while name not in employees:
        print('Employee is not listed, try again')
        name = input('Give me an employee to search for: ')
    print(employees[name])

This more modular design allows you now to easily implement a write_employees(employees, file_name) function to write the dict back to a file, delete employees (del employees[to_delete]), modify them (employees[name].rate = 10), etc.


How to refactor that content_tag adding method? How to get rid of that duplicaiton in if conditional?

def set_caption(result)
  if result.respond_to?(:app_name)
    content_tag(:div, result.type_name, class: 'type-name') +
      content_tag(:div, result.app_name, class: 'app-name')
  else
    content_tag(:div, result.type_name, class: 'type-name')
  end
end

Just use variables for repeated expressions:

def set_caption(result)
  type_div = content_tag(:div, result.type_name, class: 'type-name')
  if result.respond_to?(:app_name)
    type_div + content_tag(:div, result.app_name, class: 'app-name')
  else
    type_div
  end
end

Note that you can move the conditional further in: type_div + (condition ? app_div : ""). Use this style whenever this adds readibility (here I'd say it doesn't).


Mongoose promise & error handling I search for rails-like way of writing NodeJS code and it's more then hard to find the right way. The code quickly become bloated and unreadable. I would like my code to be as clean as possible.

Example (common ExpressJS middleware API controller):

var User = require('../models/user'); // mongoose model

exports.updateUser = function(req, res, next) {
  User.passwordToSecretWithBcrypt(req.body.password, function(err, secret) {
    if (err) return next(err);

    User.findById(req.user.id).exec(function(err, user) {
      if (err) return next(err);
      if (!user) return next(new Error('no user found'));

      if (req.body.name) user.name = req.body.name;
      if (req.body.email) user.email = req.body.email;
      if (secret) user.secret = secret;
      user.save(function(err, user) {
        if (err) return next(err);
        if (!user) return next(new Error('user not saved'));

        res.json(user);
      });
    });
  });
};

The main question is the error handling. A simple record update in the example above has 5 error handling statements. That's crazy in my opinion and you never know exactly what kind of error you'll get so it's very hard to construct the JSON error response that is shown to a user.

Is there a better way for handling errors?

I also tried to use promises but I couldn't find a best-practice example that would show me the whole example as a chain of the find, save, create and delete Mongoose methods.

How can I convert the example above to use promises?

Would promises fix the error handling?  If so, how (question1)?

Interesting question,

I will not comment on making this more rails-like, I have no clue there. But I can observe that you in essence do 3 things: so you could perhaps group your error handling into 3 locations.

Furthermore, I want to point out that your code handles some error situations silently, that's never a good idea.

Other than that the code is very straightforward and easy enough to follow.

For my counter proposal I would start with a function that takes a request and checks for mandatory fields:

function validateMandatoryRequestFields( req , fields ){
  for( var i = 0 ; i < fields.length ; i++){
    if( !req.body[fields[i]] ){
      return fields[i] + ' was not provided';
    }
  }
  return '';
}

So that your function can do something like this in the beginning:

exports.updateUser = function(req, res, next) {
  //Do we have all we need ?
  var mandatoryFields = [ 'password', 'name' , 'email' ],
    errorMessage = validateMandatoryRequestFields( req , mandatoryFields );
  //Do we have what we need ?
  if(errorMessage){
    return next(errorMessage);
  }

You should always make sure that the request has what you need upfront.

Then, for callbacks I tend to mangle the err handling and bogus data handling into 1 if statement with 1 return statement:

  //Encrypt password
  User.passwordToSecretWithBcrypt(req.body.password, function(err, secret) {
    if (err || !secret ) {
      return next(err || new Error('Password encryption failed') );
    }

If you keep applying this to all of the code, you end up with something like this:

function validateMandatoryRequestFields( req , fields ){
  for( var i = 0 ; i < fields.length ; i++){
    if( !req.body[fields[i]] ){
      return fields[i] + ' was not provided';
    }
  }
  return '';
}

exports.updateUser = function(req, res, next) {
  //Do we have all we need ?
  var mandatoryFields = [ 'password', 'name' , 'email' ],
    errorMessage = validateMandatoryRequestFields( req , mandatoryFields );
  //Do we have what we need ?
  if(errorMessage){
    return next(errorMessage);
  }
  //Encrypt password
  User.passwordToSecretWithBcrypt(req.body.password, function(err, secret) {
    if (err || !secret ) {
      return next(err || new Error('Password encryption failed') );
    }
    //Find user
    User.findById(req.user.id).exec(function(err, user) {
      if (err || !user){
        return next(err || new Error('no user found'));
      }
      //Update user
      user.save(function(err, user) {
        if (err || !user) {
          return next(err || new Error('user not saved') );
        }
        res.json(user);
      });
    });
  });
};


Copy-Paste-Find-Replace (weekly update) Heres a code that update weekly datas from production reports
Known flaws :

Im copy pasting the old line and replacing the week number in it to fit it to the one we want to update. The problem is that its not taking in account if cell emplacement of the data is changing.
I also tought it was simpler than re-writing all formulas in every-cell by coding every bits of formula (file emplacement, file name and cell emplacement changes every ranges)
Should i call my action if true in another sub to make this clearer?
(ex: if true, call(copy-paste-find-replace)
I have 1 code per sheet (3 sheets) cause of the ranges are hard coded and changes depending on the sheets, with your answer to (1) i could make it a single sub with variables depending on the sheet
I have a week and a half remaining to make this as clean as possible, i dont want to refactor it all the way .. :(

Heres one of the 3 code :
Sub AjoutSemaineajouterperfo()
    ' AjoutSemaineajouterperfo Macro
    ' Le code permet d'ajouter une nouvelle Semaineajouteraine

    Dim k As Long
    k = 3

    Do While (Cells(k, 3).Value <> "" And k <= 53) ' Boucle qui trouve la première ligne Semaineajouteraine vide
        k = k + 1
    Loop

    k = k - 2                                    ' La boucle while ajoute une Semaine de trop, on veut aussi revenir sur la dernière semaine rentrer (d'ou le -2)

    Dim Semaineajouter As Long
    Dim Destination As Long
    Dim Semaineavant As Long
    Semaineajouter = Cells(59, 3).Value          ' Valeur de la semaine à ajouter(case)
    Destination = Semaineajouter + 1             ' Ligne ou la prochaine semaine va se coller
    Semaineavant = Semaineajouter - 1            ' Ligne de la dernière semaine importer

    Dim semaineactuelle As Long
    semaineactuelle = WorksheetFunction.WeekNum(Now, vbMonday)

    If Semaineajouter > k And Semaineajouter <= 52 And Semaineajouter <> semaineactuelle Then
        ' Si le numéro de Semaineajouteraine entrée est plus grand que la dernière semaine ajouté  et différent de la semaine actuelle
        ' Aussi plus petit que 52 (préserver la mise en forme

        Application.EnableEvents = False
        Application.DisplayAlerts = False
        Application.ScreenUpdating = False

        ActiveSheet.Unprotect

        Dim semaineajoutertex As String
        Dim semaineavanttex As String
        semaineajoutertex = "sem " & CStr(Semaineajouter) ' Transfert des numéros de semaine en texte (pour search and replace)
        semaineavanttex = "sem " & CStr(Semaineavant)

        ' Range 1
        Dim RangeOrigine1 As String
        RangeOrigine1 = "C" & Semaineajouter & ":" & "AX" & Semaineajouter ' Range d'origine (semaine avant)
        Dim RangeDestination1 As String
        RangeDestination1 = "C" & Destination & ":" & "AX" & Destination ' Range à importer (semaine ajout)

        Range(RangeOrigine1).Copy Range(RangeDestination1)

        Range(RangeDestination1).Replace What:=semaineavanttex, Replacement:=semaineajoutertex, LookAt:=xlPart, _
                                         SearchOrder:=xlByRows, MatchCase:=False, SearchFormat:=False, _
                                         ReplaceFormat:=False

        ActiveSheet.Protect

        Application.ScreenUpdating = True
        Application.DisplayAlerts = True
        Application.EnableEvents = True

    Else
        MsgBox "Entrez un numéro de Semaineajouteraine valide", vbCritical, "Ne peut exécuter"
    End If

End Sub

In this case, pressing GO would call the macro and add week 33 by copy-pasting-find-replace the line from week 32, replacing "sem 32" by "sem 33" in every
newly paste entries.

I don't see the need to find a specific row number.  Simply work your way down until you reach the current week.
The key to reducing repeat code is to extract it to it's own method.
Refactored Code
Sub UpdateWeeklyReports()
    Application.ScreenUpdating = False
    Const WorksheetName As String = "Sheet1"
    Const FirstWeekNumberRow As Long = 3
    Dim LastWeek As Range
    Dim What As String, Replacement As String
    With Worksheets(WorksheetName)
        Dim Row As Long
        For Row = FirstWeekNumberRow + 1 To  Format(Now, "WW") - 1
            If Not .Cells(Row, 3).HasFormula Then
                ' What = "sem " & .Cells(Row - 1, 2).Value
                ' Replacement = "sem " & .Cells(Row, 2).Value
                AddNewRow SourceRange:=.Rows(Row - 1).Range("C1:J1"), What:=What, Replacement:=Replacement
                AddNewRow SourceRange:=.Rows(Row - 1).Range("M1:T1"), What:=What, Replacement:=Replacement
                AddNewRow SourceRange:=.Rows(Row - 1).Range("W1:AD1"), What:=What, Replacement:=Replacement
            End If
        Next
    End With
End Sub

Private Sub AddNewRow(ByVal SourceRange As Range, ByVal What As String, ByVal Replacement As String)
    With SourceRange
        .Offset(1).Formula = .Formula
        .Offset(1).Replace What:=What, Replacement:=Replacement, LookAt:=xlPart, SearchOrder:=xlByRows, MatchCase:=False, SearchFormat:=False, ReplaceFormat:=False
    End With
End Sub


Binary Search recursive & iterative solution I have implemented binary search solution by using recursion and iterative approach.
I there any way to improve it?
Tests
package test;

import main.algorithms.BinarySearchDemo;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;

public class BinarySearchTest {

    BinarySearchDemo binarySearchDemo;
    int[] array;

    @Before
    public void setUp(){
        binarySearchDemo = new BinarySearchDemo();
        array = new int[]{2, 3, 5, 6, 9, 11, 12, 15, 17, 21};
    }

    @Test
    public void testBinarySearchJavaAPI(){
        Assert.assertEquals(2, binarySearchDemo.binarySearchJavaAPI(5,array));
    }

    @Test
    public void testBinarySearchImpl(){
        Assert.assertEquals(2, binarySearchDemo.binarySearchImpl(5,array));
    }

}

Implementation
package main;

public class BinarySearch {

    private static boolean binarySearchRecursive(int[] array, int i) {

        return binarySearchRecursive(array,0,array.length-1,i);
    }

    private static boolean binarySearchRecursive(int[] array, int left, int right, int item) {

        if(left > right){
            return false;
        }
        int pivot =  (right - left) / 2 + left;

        if(item == array[pivot]){
            return true;
        }
        else if ( item < array[pivot] ){
            return binarySearchRecursive(array, left,pivot-1,item);
        }else {
            return binarySearchRecursive(array, pivot+1, right, item);
        }
    }

    private static boolean binarySearchIterative(int[] array, int item) {

        int left = 0;
        int right = array.length - 1 ;

        while (left < right) {
            int mid = (left + right)/2 + left;
            if (array[mid] == item) {
                return true;
            } else if(item < array[mid]) {
                right = mid-1;
            } else {
                left = mid + 1;
            }
        }
        return false;
    }

}

You haven't indicated the purpose for writing these implementations.  The natural alternative would be to use Arrays.binarySearch unless this is purely for learning purposes.
Testing
Your test suite is far from exhaustive, testing that both methods can find the item at position 2 is very minimal (at the moment both implementations could simply always return 2).  Consider adding additional tests for edge cases (end of array being searched, beginning of array being searched for example).  Consider what behaviour you're expecting if the item isn't in the array being searched and adding test cases for those scenarios.
array isn't a great name for the source data in your tests.  sortedArray or dataToSearch would be a bit better however consider declaring the array in each test method.  In a larger test file, scrolling up and down to see the data in the array, versus the test that's being performed adds extra overhead, which can be undesirable.
Implementations
All of your methods are declared as private.  Some of them are meant to be called from outside the class, some of them (such as the recursive call) probably aren't.  This should be reflected in the access declarations.
Naming is an important aspect of development.  The better your names are and the more consistent you are with them, the easier your code is to follow.  You're consistent with your use of left and right, but mid is called pivot in the alternate implementation.  The item to find is either i or item.  IDE's will often give parameter name prompts even when there is no Java Doc.  If you name your parameters well, it's clear that the parameters are (int[] arrayToSearch, int itemToFind)
Doh...
I should really have noticed this earlier... but both of your methods return boolean, however you're asserting they both return 2, which isn't a boolean...  Think about what you want the method to return...implement that...and write the tests accordingly...


Request for memory optimizations for solution to Google FooBar challenge, "ion_flux_relabeling," I just finished one of the Google FooBar Challenges.My Python is super rusty. I did the challenge in Haskell first and then attempted to covert the code. I am guessing that led to some bad Python Practices.

  Oh no! Commander Lambda's latest experiment to improve the efficiency
  of her LAMBCHOP doomsday device has backfired spectacularly. She had
  been improving the structure of the ion flux converter tree, but
  something went terribly wrong and the flux chains exploded. Some of
  the ion flux converters survived the explosion intact, but others had
  their position labels blasted off. She's having her henchmen rebuild
  the ion flux converter tree by hand, but you think you can do it much
  more quickly - quickly enough, perhaps, to earn a promotion!

  Flux chains require perfect binary trees, so Lambda's design arranged
  the ion flux converters to form one. To label them, she performed a
  post-order traversal of the tree of converters and labeled each
  converter with the order of that converter in the traversal, starting
  at 1. For example, a tree of 7 converters would look like the
  following:

      7
     /   \
   3      6
  /  \   / \
 1   2  4   5

  Write a function answer(h, q) - where h is the height of the perfect
  tree of converters and q is a list of positive integers representing
  different flux converters - which returns a list of integers p where
  each element in p is the label of the converter that sits on top of
  the respective converter in q, or -1 if there is no such converter.
  For example, answer(3, [1, 4, 7]) would return the converters above
  the converters at indexes 1, 4, and 7 in a perfect binary tree of
  height 3, which is [3, 6, -1].

  The domain of the integer h is 1 <= h <= 30, where h = 1 represents a
  perfect binary tree containing only the root, h = 2 represents a
  perfect binary tree with the root and two leaf nodes, h = 3 represents
  a perfect binary tree with the root, two internal nodes and four leaf
  nodes (like the example above), and so forth. The lists q and p
  contain at least one but no more than 10000 distinct integers, all of
  which will be between 1 and 2^h-1, inclusive.

How could I make my solution more memory efficient?

def splitInHalf(aList):
    full = len(aList)
    half = full/2
    return aList[0:half], aList[half:full]

class Tree(object):
    def __init__(self):
        self.left = None
        self.right = None
        self.data = None
    def __str__(self):
        return ("Data = " + str(self.data) + "Left = " + str(self.left) + "Right = " + str(self.right)  )

def makeTree(h):
    limit = (2**h)-1
    fluxConverters = list(range(limit,0,-1))
    result = generate_tree(list(fluxConverters))
    return result

def generate_tree(listInt):
    if not listInt:
        return None
    elif (len(listInt) == 1):
        rTreeOne = Tree()
        rTreeOne.data = listInt[0]
        return rTreeOne
    else:
        rTreeList = Tree()
        rTreeList.data   = listInt.pop(0)
        half1, half2 = splitInHalf(list(listInt))
        rTreeList.left   = generate_tree(half1)
        rTreeList.right  = generate_tree(half2)
        return rTreeList

def searchTree(tree,search):
    if tree == None:
        return -1
    else:
        if (nextHaz(tree.left,search) | nextHaz(tree.right,search)):
            return tree.data
        else:
            left = searchTree(tree.left,search)
            right = searchTree(tree.right,search)
            return mergeResults(left,right)

def mergeResults(x,y):
    if(x!=-1):
        return x
    else:
        return y

def nextHaz(tree, search):
    if(tree != None):
        return tree.data == search
    else:
        return False

def answer(h,q):
    tree = makeTree(h)
    partialSearch = lambda x : searchTree(tree,x)
    result = map(partialSearch,q)
    return result

answer(3,[7, 3, 5, 1])
testOneResult = [-1, 7, 6, 3]

answer(5,[19, 14, 28])
testTwoResult = [21, 15, 29]

The problem description says:

  The domain of the integer \$h\$ is \$1 \le h \le 30\$

In the worst case, where \$h=30\$, the perfect tree has \$2^{31}-1 = 2147483647\$ nodes. And to store each node requires at least 32 bytes for the Tree object:

>>> Tree().__sizeof__()
32

and a further 28 bytes for its data:

>>> (123456).__sizeof__()
28

making at least 128 GB in all.

So this should clue you into the possibility that you might be able to compute the answer without building the tree. After all, the function you're being asked to write takes some numbers and returns some other numbers:

>>> answer(7, [10, 25, 109])
[14, 29, 125]

so there might be some mathematical approach to computing the outputs based only on the numerical values of the inputs. (I won't spoil this for you by giving any more hints than that.)

This doesn't mean your work so far is wasted — as you try to figure out the mathematical approach, you can use the code you've written so far to check that the new code is producing the right answers.

(You didn't show us your original Haskell program, but I suspect it might have worked because Haskell's data structures are lazy — Haskell doesn't actually build them until just before they are needed. If you can figure out exactly which parts of the tree were being built in the Haskell version — and which were not — then maybe that will help you with your Python implementation.)


File presence check for an automated workflow I am working on a script for an automated workflow. It expects a CSV and a disk image to be present in the directory that is supplied as args.input.

I want to check for and handle every possible scenario: no CSV, no disk image, too many CSVs, too many disk images, and any possible combination thereto.

I've written the below, which works, and seems to be human readable, but it just seems incredibly excessive and verbose – is there any way I can make this more compact, yet still retain its legibility?

# CONFORMANCE CHECKS
def check_conformance():
    csv = glob.glob(args.input+'*.csv')
    disk_image = glob.glob(args.input+'*.E01')

    if len(csv) == 1:
        does_csv_exist = os.path.isfile(csv[0])
    elif len(csv) < 1:
        does_csv_exist = False
    elif len(csv) > 1:
        does_csv_exist = "too many CSVs!"
    if len(disk_image) == 1:
        does_E01_exist = os.path.isfile(disk_image[0])
    elif len(disk_image) < 1:
        does_E01_exist = False
    elif len(disk_image) > 1:
        does_E01_exist = "too many Disk Images!"
    if len(disk_image) > 1 and len(csv) > 1:
        does_csv_exist = "too many CSVs!"
        does_E01_exist = "too many disk images!"
        return (False, does_csv_exist, does_E01_exist,)
    if does_E01_exist is True and does_csv_exist is True:
        return True
    elif does_E01_exist is True and does_csv_exist is False:
        return (False, "CSV is missing")
    elif does_E01_exist is False and does_csv_exist is True:
        return (False, "E01 disk image is missing")
    elif does_E01_exist is False and does_csv_exist is False:
        return (False, "E01 disk image AND csv are missing")
    elif does_csv_exist is not True and does_csv_exist is not False:
        return (False, does_csv_exist)
    elif does_E01_exist is not True and does_E01_exist is not False:
        return (False, does_E01_exist)

I would recommend decomposing this function into smaller functions. For example, you would end up with something like this:

def check_conformance():
   csv_conformance  = check_csv_conformance()
   disk_conformance = check_disk_conformance()

Then you can return based on these checks as follows:

return csv_conformance && disk_conformance

Moreover, as a general rule, it is a bad habit to have different amount of return values based on branching. You should have a specific input-output length (again, in general. there are cases where you want a variable length array to be return)


GCD of several integers Exercise: Write a method that returns the gcd of an unspecified number of integers.

What I did:

Created a for loop that stores integers in an array. The array length is increased every time the else statement is reached. The loop will break upon receiving '0'.
Created a method to find the gcd. This method sorts the array using Arrays.sort. The first element of the array will be 0. The lowest integer (index 1) is assigned to the variable min.
To compute the gcd, I created a for loop that assigns min to i. I decrement this variable until the modulo operation for count % numbers.length - 1 equals 0. This indicates that a gcd has been found for all integers and will break out of the main for loop. The variable count will increment when an integer in the array is divisible by the current divisor.

I would really appreciate any feedback you can give me.

public class Exercise_07_14 {
    public static void main(String[] args) {
        Scanner input = new Scanner(System.in);

        System.out.println("Enter numbers: ");
        int[] numbers = new int[1];

        for (int i = 0; i < numbers.length; i++) {
            int number = input.nextInt();

            if (number == 0)
                break;
            else
                numbers[i] = number;

            numbers = incrementArraySize(numbers);
        }

        int gcd = getGCD(numbers);

        String output = "";

        for (int i = 1; i < numbers.length; i++) {
            if (i == numbers.length - 1)
                output += numbers[i];
            else
                output += numbers[i] + ", ";
        }

        System.out.println("The GCD of [" + output + "] is: " + gcd);
    }

    public static int getGCD(int... numbers) {
        Arrays.sort(numbers);

        int min = numbers[1];
        int gcd = 1;

        for (int i = min; i > 1; i--) {
            int count = 0;

            for (int k = 1; k < numbers.length; k++) {
                if (numbers[k] % i != 0)
                    break;
                else
                    count++;
            }

            if (count == (numbers.length - 1)) {
                gcd = i;
                break;
            }
        }

        return gcd;
    }

    public static int[] incrementArraySize(int[] array) {
        int[] tempArray = array;
        array = new int[tempArray.length + 1];

        System.arraycopy(tempArray, 0, array, 0, tempArray.length);

        return array;
    }
}

There are some issues, such as not using obvious built-in solutions, an inefficient algorithm and what looks like a logic error possibly caused by having odd array-growing logic.

I'll start with the error. In int min = numbers[1]; you assume that there are at least two numbers. That may not be true, if the user immediately enters 0 - which raises the question of what the GCD of the empty set is (0 or 1, depending on who you believe), but whatever it should be, it's bad form to crash with an out-of-bounds exception.

The odd array-growing logic is probably part of that problem, it leaves a spurious 0 in the array which you thereafter actively ignore - it's simpler to not have it there to begin with. Anyway, you wanted an array that can change size - that's usually called ArrayList which, conveniently, already exists. You can also avoid saving up the input entirely, by computing the GCD while reading the input.

The GCD algorithm is strange and inefficient. It's not the familiar GCD algorithm, which is already a strike against it by itself since that makes it hard to tell what it's doing, and there is no redeeming quality. This is an algorithm with a very bad worst case, as much as O(min*N) (if the GCD turns out to be 1). I strongly recommend using the modulo-based version of the Euclidean algorithm, which is efficient and recognizable, or maybe Binary GCD if you want something different. In the unlikely case that you rejected it because it takes only two inputs, of course gcd(a, b, c) = gcd(gcd(a, b), c) and so on, so you can simply loop over your input and call GCD on the gcd-so-far and the number from the input.

Or, as mentioned, you could compute the GCD while reading the input, something like this:

int gcdSoFar = 0;

while (true) {
    int number = input.nextInt();
    if (number == 0)
        break;

    gcdSoFar = gcd(number, gcdSoFar);
}

System.out.println("The GCD of the input is: " + gcdSoFar);

This loses the print-back of the input, if you want to preserve that you could use an ArrayList. By the way, in Java 8 you can use String.join(", ", list) to make a comma-separated string from your list, you don't have to do that manually.


Back to Basics: Tic Tac Toe - follow-up Follow-up from: Back to Basics - Tic Tac Toe

Again, any optimizations and critique is welcome! I'm particularly interested if it involves numpy, scipy | bitwise operators.

import numpy as np
from random import randint

def print_board(board):
    print("""
  +---+---+---+
c | {}   {}   {} |
  +   +   +   +
b | {}   {}   {} |
  +   +   +   +
a | {}   {}   {} |
  +---+---+---+
    1   2   3
""".format(*list(x if x != '0' else ' ' for x in board)))

def check_for_win(board):

    def check_diagonal(dia):
        return '0' not in dia and len(set(dia)) == 1

    a = np.array(board).reshape(3, 3)

    # check lr diagonal
    if check_diagonal(a.diagonal()):
        return True

    # check rl diagonal
    if check_diagonal(np.fliplr(a).diagonal()):
        return True

    # check rows & cols
    for matr in (a, np.transpose(a)):
        for row in matr:
            if '0' not in row and len(set(row)) == 1:
                return True

    return False

if __name__ == "__main__":
    board = ['0'] * 9
    codes = ('c1', 'c2', 'c3', 'b1', 'b2', 'b3', 'a1', 'a2', 'a3')
    turn, user = 1, 'X' if bool(randint(0, 1)) else 'O'
    print(f'Welcome! First to play is {user}!')
    print_board(board)

    while True:
        print(f'Player {user}: ')
        code = input().strip().lower()

        if code in codes:
            idx = codes.index(code)

            if board[idx] == '0':
                board[idx] = user
                print_board(board)

                if turn >= 5:
                    if check_for_win(board):
                        print(f'{user} won in {turn} turns! Congratulations!')
                        break
                    elif '0' not in board:
                        print("Aw, it's a draw!")
                        break

                user = 'X' if user == 'O' else 'O'
                turn = turn + 1
            else:
                print('Woops! You need to pick an empty space.')
        else:
            print("Hmm, that's not a valid input.")

Ways to improve:

simplifying list construction:

*list(x if x != '0' else ' ' for x in board)) --> *[x if x != '0' else ' ' for x in board]
check_diagonal function. The function performs a logical check if the passed row is fully selected/marked with the same mark/code and returns boolean result respectively.Therefore, it's better to give it a more meaningful and unified name, say row_crossed (reflecting boolean purpose):

def row_crossed(row):
    return '0' not in row and len(set(row)) == 1

Besides, renaming it will serve beneficially for the next improvements (see below)
2 consecutive conditions:

if check_diagonal(a.diagonal()):
    return True

# check rl diagonal
if check_diagonal(np.fliplr(a).diagonal()):
    return True

return the same boolean result. That's a sign for Consolidate conditional expression technique.Thus, the flow becomes:

# check lr diagonal
if row_crossed(a.diagonal()) or row_crossed(np.fliplr(a).diagonal()):
    return True

the for loop (in check_for_win function) which iterates through pair of  matrices (initial and transposed one) duplicates the same logical check

 ...
 if '0' not in row and len(set(row)) == 1:

as row_crossed (formerly check_diagonal) function does. Thus, replacing duplicated condition with function call.Eventually, the restructured check_for_win function would look as below:

def check_for_win(board):
    def row_crossed(row):
        return '0' not in row and len(set(row)) == 1

    a = np.array(board).reshape(3, 3)

    # check lr diagonal
    if row_crossed(a.diagonal()) or row_crossed(np.fliplr(a).diagonal()):
        return True

    # check rows & cols
    for matr in (a, a.transpose()):
        for row in matr:
            if row_crossed(row):
                return True

    return False

Actually, the above Consolidation of conditionals can be even more encompassing with applying np.concatenate and builtin any functions, though may seem less readable but quite viable for those who likes concise/compact code:

def check_for_win(board):
    def row_crossed(row):
        return '0' not in row and len(set(row)) == 1

    a = np.array(board).reshape(3, 3)

    if row_crossed(a.diagonal()) \
            or row_crossed(np.fliplr(a).diagonal()) \
            or any(row_crossed(row) for row in np.concatenate([a, a.transpose()])):
        return True

    return False

Minor "refinements":

for matr in (a, a.transpose()) looks a bit more "connected" than for matr in (a, np.transpose(a)):
optionally, to reduce typing misses of codes = ('c1', 'c2', 'c3', 'b1', 'b2', 'b3', 'a1', 'a2', 'a3') - a good alternative is:

codes = tuple(map(''.join, itertools.product('cba', '123')))