Dining Philosophers using C11 threads I wanted to try multithreading out in C, so I did Dining Philosophers using C11 threads with the approach of having one of the philosophers left-handed. Any suggestions?

#include <stdio.h>
#include <threads.h>
#include <stdlib.h>

#define NUM_THREADS 5

struct timespec time1;
mtx_t forks[NUM_THREADS];

typedef struct {
    char *name;
    int left;
    int right;
} Philosopher;

Philosopher *create(char *nam, int lef, int righ) {
    Philosopher *x = malloc(sizeof(Philosopher));
    x->name = nam;
    x->left = lef;
    x->right = righ;
    return x;
}

int eat(void *data) {
    time1.tv_sec = 1;
    Philosopher *foo = (Philosopher *) data;
    mtx_lock(&forks[foo->left]);
    mtx_lock(&forks[foo->right]);
    printf("%s is eating\n",  foo->name);
    thrd_sleep(&time1, NULL);
    printf("%s is done eating\n",  foo->name);
    mtx_unlock(&forks[foo->left]);
    mtx_unlock(&forks[foo->right]);
    return 0;
}

int main(void) {
    thrd_t threadId[NUM_THREADS];
    Philosopher *all[NUM_THREADS] = {create("Teral", 0 ,1),
                    create("Billy", 1, 2),
                    create("Daniel", 2,3),
                    create("Philip", 3, 4),
                    create("Bennet", 0, 4)};
    for (int i = 0; i < NUM_THREADS; i++){
        if (mtx_init(&forks[i], mtx_plain) != thrd_success){
            puts("FAILED IN MUTEX INIT!");
            return 0;
        }
    }
    for (int i=0; i < NUM_THREADS; ++i) {
        if (thrd_create(threadId+i, eat, all[i]) != thrd_success) {
            printf("%d-th thread create error\n", i);
            return 0;
        }
    }

    for (int i=0; i < NUM_THREADS; ++i)
        thrd_join(threadId[i], NULL);
    return 0;
}

when calling malloc(), always check the returned value to assure the operation was successful

use meaningful variable names.   x is not a meaningful name and just makes the code more difficult to understand.

it seems the threads.h header file is not available everywhere.

For portability, suggest changing to the POSIX threads

#include <pthreads.h>

and using the related functions like:

pthread_create()
pthread_exit()
pthread_join()
pthread_t
pthread_mutex_lock()
pthread_mutex_unlock()

when a function fails, most likely one or more of the Philosopher entities have already been created via malloc()  to avoid a memory leak, those memory allocation pointers need to be passed to free()

When returning out of main, due to an error, return EXIT_FAILURE (or similar value) rather than returning 0 which indicates success.

in main(), the calls to create() need to check the returned value to assure the operation was a success.

the eat() function just zooms through and exits.  This is not the (typical) action of the Dining Philosophers problem.  Suggest searching stackoverflow.com for implementations of the problem and using the overall logic, with some modification for the 'left handed' diner.

The posted code fails to free() the malloc'd memory at the end of the main() function


Find two unique integers in sorted array I have to write the following method:

I have sorted array of integers and in this array every integer repeats twice, except two elements.  I have to find these two elements.

Input: 1, 1, 2, 3, 3, 4, 4, 5
Output: 2, 5

This is my solution:

private static int[] findUniques(int[] arr) {
    int[] res = new int[2];
    int start = 0;    // start can be 0 or 1 only; it indicates the position in array res
    for (int i = 0; i < arr.length-2; ) {
        if(arr[i] == arr[i+1]){
            i+=2;
            if(i == arr.length-1){
                res[1] = arr[arr.length-1];
            }
        }
        else{
            if(arr[i+1] == arr[i+2]){
                res[start] = arr[i];
                start++;
                i+=3;
                if(i == arr.length-1){
                    res[1] = arr[arr.length-1];
                }
            }
            else{
                res[start] = arr[i];
                res[start+1] = arr[i+1];
                break;
            }
        }
    }
    return res;
}

Is there a clearer way to find these two values? I don't like that I have so many if-else statements, but I don't know how to write it better.

For each number in the list, either

there is no next (at end of src list) - add to result list
the next is different - add to the result list, increment result count (if all answers found, return)
the next is the same; skip next, (add 2 to src pointer)

public int[] findUnique(int[] src){
    int[] res =  new int[2];
    int resIdx = 0;

    for(int i = 0; i < src.length;){
        // at the end of the list, or next is different
        if(i == src.length-1 || src[i] != src[i+1]){
            res[resIdx++] = src[i++];
            if (resIdx== 2) break;
        } else{
            // not different; skip next item
            i+=2;
        }
    }
    return res;
}


Create CSV file of domain groups and their users I'm fairly new to scripting in Python. I'm looking for suggestions on improving any of the following in the below script: my coding style, performance, readability, commenting, or anything else.

"""
META
Author: James Nicholson
Date: 8/16/2013

DESCRIPTION
This script takes a list of groups from a text file and creates a new text file
in CSV format that contains group members in the below format:

group_name1,user_name1
group_name1,user_name2
etc...

VARIABLES
server = 'domain name'
glist = 'path to text file containing desired groups'
"""

# Import the win32 module needed for accessing groups and members
import win32net

# Set server name and group_list text file variables
server = # enter server name here
group_list = 'groups.txt'
grps = []

# Loop over lines in group_list and store group names in a list
with open(group_list, 'rb') as my_groups:
    for line in my_groups:
        grps.append(line.rstrip('\r\n'))

# Loop over groups and store members in desired format in new text file 'groups_members_output.txt'
with open('groups_members_output.txt', 'wb') as blank_file:
    for group in grps:
        usrs = win32net.NetGroupGetUsers(server,group,0)
        for user in usrs[0]:
            blank_file.write('{},{}\r\n'.format(group,user['name']))

Some notes:

Comments: A lot (if not all) of the comments are pretty much repeating what the code already says. I'd remove them.
server = # enter server name here. Create functions with configurable elements as arguments instead of global variables.
with open('groups_members_output.txt', 'wb') as blank_file. Try to give more meaningful names to variables, be declarative. blank_file says hardly anything useful about the variable.
grps/usrs: Just for saving one or two characters now variables are harder to read.
Put spaces after commas.
Use (functional) list-comprehensions expressions instead of (imperative) for-loop statements whenever possible.
Use always "\n" for line endings (read and write), Python takes care of portability in all platforms.

I'd write:

import win32net

def write_group_members(server, input_groups, output_groups):
    with open(input_groups) as input_groups_fd:
        groups = [line.rstrip() for line in input_groups_fd]

    with open(output_groups, "w") as output_groups_fd:
        for group in groups:
            users = win32net.NetGroupGetUsers(server, group, 0)[0]
            for user in users:
                contents = "{0},{1}\n".format(group, user["name"])
                output_groups_fd.write(contents)

write_group_members("someserver.org", "groups.txt", "groups_members_output.txt")


Analyzer for randomly generated names I recently wrote a name generator that uses a DTMC underneath (I asked about it here) and, since I'm not entirely confident I did it right, I wrote a script to check my code, or at least its output.

It works pretty well, but, being new to the language, I want to know how to make it more idiomatic. Performance boosts (in terms of speed and memory efficiency) would also be a plus, but since this is just a simple test script they're not as important.

# arguments: '-dDELIMITER'
if ARGV[0] == '-h'
    [
            'Should be used in the form:',
            '<invocation of name_gen.rb> | <ruby> name_gen_test.rb -d<delimiter>',
            'The delimiter MUST be specified in name_gen.rb and it MUST NOT be ``.'
    ].each { |line| puts line }
end

DELIMITER = ARGV[0] || abort('You must specify a delimiter as the sole command-line argument')

connections = Hash.new { |hash, key| hash[key] = Hash.new 0 }
start = Hash.new 0

until (cur_line = STDIN.gets).nil?
    cur_line.chomp!
    individual_syllables = cur_line.split DELIMITER
    individual_syllables.each_with_index { |from, index|
        start[from] += 1 if index == 0
        connections[from][individual_syllables[index + 1] || !!false] += 1
    }
end

# % of start per syllable
# % of connections to each syllable it connected to

puts 'Start:'
total_start_count = start.values.inject(:+).to_f
max_len = start.keys.inject (0) { |memo, cur|
    (cur.length > memo) ? cur.length : memo
}
start.each { |text, percent|
    puts "  #{text.ljust max_len} : #{(percent * 100 / total_start_count).round.to_i}%"
    # Get the percent -> Truncate -> convert to string -> justify
}
puts

END_MARKER = '[end]'
puts 'Connections:'
connections.each { |from, links|
    total_connection_count = links.values.inject(:+).to_f
    max_len = links.keys.inject(END_MARKER.length) { |memo, cur|
        ((cur ? cur : '').length > memo) ? cur.length : memo
    }
    puts "  #{from}:"
    links.each { |to, probability|
        next unless to
        puts "    #{to.ljust max_len} : #{(probability * 100 / total_connection_count).round.to_i}%"
    }
    puts "    #{END_MARKER} : #{(links[false] * 100 / total_connection_count).round.to_i}%"
}

This is the name generator; this script is meant to be used something like this (on Windows, at least):

ruby name_gen.rb dict.txt 10000 -d_ | ruby name_gen_test.rb _

if the dictionary of syllables is located at 'dict.txt'.

Here's an example dictionary file:

a|1|1|b,2;c,2
b|0|3|a,0;c,2
c|0|0|a,1;b,1

And an example output for the script:

Start:
  a : 100%

Connections:
  a:
    c     : 40%
    b     : 40%
    [end] : 20%
  c:
    a     : 50%
    b     : 50%
    [end] : 0%
  b:
    c     : 40%
    [end] : 60%

For anyone interested, the final code is available here.

The ruby style guide suggests to use 2 spaces per indentation level.
You can use heredocs for multi-line strings, keep in mind that they preserve white space, here is a nice trick that could be used:

help = <<-END.gsub(/^\s+\|/, '')
  |Should be used in the form:,
  |<invocation of name_gen.rb> | <ruby> name_gen_test.rb -d<delimiter>,
  |The delimiter MUST be specified in name_gen.rb and it MUST NOT be ``.
          END
help.each_line { |line| puts line }

for multiline blocks please use do...end instead of {...}:

individual_syllables.each_with_index do |from, index|
start[from] += 1 if index == 0
connections[from][individual_syllables[index + 1] || !!false] +=
end

I don't understand why you use !!false, since !!false == false. Also I don't think it is needed here.
Do not put a space between a method name and the opening parenthesis:

max_len = start.keys.inject (0)

Avoid nested ternary operators, it makes the code hard to understand.

((cur ? cur : '').length > memo) ? cur.length : memo
# becomes
cur ||= ''
[cur.length, memo].max


INI File Parser in Haskell I'm learning Haskell at the moment, and have just written my first useful module - a parser fo INI files. I used Parsec. I'd like to know what can be improved here - or maybe I did some things completely wrong and there is a better way. Thanks.

module IniFile (iniFileToMap) where

import Text.ParserCombinators.Parsec
import qualified Data.Map as Map
import Char

parseIniFile :: Parser ([(String, [(String, String)])])
parseSection :: Parser (String, [(String, String)])
parseSectionHeader :: Parser String
parseNameValue :: Parser (String, String)
parseComment :: Parser ()
parseNormalValue :: Parser String
parseQuotedValue :: Parser String
parseValue :: Parser String
parseName :: Parser String
parseInsideQuotes :: Parser String
parseCmtOrSpace :: Parser ()
iniFileToMap :: FilePath -> IO (Map.Map String (Map.Map String String))

parseCmtOrSpace = do{
   spaces;
   skipMany parseComment;
}

parseInsideQuotes = many (noneOf "\"")

parseQuotedValue = between (char '"') (char '"') parseInsideQuotes

parseNormalValue = many1 (satisfy (\c -> isPrint c && not (isSpace c)))

parseValue = parseQuotedValue <|> parseNormalValue

parseName =  many1 (satisfy (\c -> isAlpha c || isDigit c || c == '_' || c == '.'));

parseComment = do{
   skipMany1 (char ';');
   skipMany (noneOf "\n");
   spaces;
}

parseNameValue = do{
   name <-  parseName;
   between (skipMany (oneOf " \t")) (skipMany (oneOf " \t")) (char '=');
   value <- parseValue;
   return (name, value);
}

parseSectionHeader = do{
   name <- between (char '[') (char ']') parseName;
   return name;
}

parseSection = do{
   name <- between parseCmtOrSpace parseCmtOrSpace parseSectionHeader;
   values <- endBy1 (parseNameValue) (parseCmtOrSpace);
   return (name, values);
}

parseIniFile  = do{
   result <- many1 (parseSection);
   return result;
}

list2Map list = Map.fromList (map (\e -> (fst e, Map.fromList (snd e))) list)

iniFileToMap path = do{
   result <- parseFromFile parseIniFile path;
      case (result) of
         Left err -> error (show err)
         Right xs -> return (list2Map(xs));
}

The first thing I noticed is that you have grouped all the type signatures together, away from the function bodies. This seems very strange to me. It's way more common (and as far as I'm concerned also way more readable) to have a value's type directly before its definition.

The second thing I noticed were all the braces and semicolons. Unless you have a specific reason to use them, I'd strongly recommend making use of Haskell's significant indentation instead. Much less visual noise. I'm especially confused by the semicolon at the end of parseName's definition as parseName doesn't use do-notation and you didn't use a semicolon on any other definition which didn't use do.

You sometimes surround variables with parentheses (e.g. in values <- endBy1 (parseNameValue) (parseCmtOrSpace), but not in the line name <- between parseCmtOrSpace ... directly above it). I can't see any system to when you do this and when you don't, so it just seems random and inconsistent. So I'd recommend to get rid of the needless parentheses.

Speaking of parentheses, you might also want to get rid of some more parentheses by sprinkling a couple of $ signs in your code, to give it a more "haskellish" feel.

parseSectionHeader = do{
   name <- between (char '[') (char ']') parseName;
   return name;
}

parseIniFile  = do{
   result <- many1 (parseSection);
   return result;
}

Whenever you have something of the form:

foo <- bar
return foo

You can simply just write bar. I.e.:

parseSectionHeader = between (char '[') (char ']') parseName

parseIniFile  = many1 parseSection

iniFileToMap path = do{
   result <- parseFromFile parseIniFile path;
      case (result) of
         Left err -> error (show err)
         Right xs -> return (list2Map(xs));
}

Here the indentation is off - case should not be indented further than result <-. It should be:

iniFileToMap path = do
    result <- parseFromFile parseIniFile path
    case result of
        Left err -> error $ show err
        Right xs -> return $ list2Map xs


Implementing multiprocessing to compare large number of entries in a python dictionary I am a beginner in python programming, and I am trying to implement multiprocessing to the code I use daily for my data treatment. Essentially, the input file has around 10 - 30 millions of reads where each read contains 4 lines of information. There are two elements in the first line of every four lines that I am looking at. First I search if the first element is in the dictionary. If not, then I add to the dictionary the first element is the key while the second element is the value. But if the first element is already in the dictionary, I proceed to compare whether the second element is within the value list corresponding to the key. Similarly, if the second element is not already present in the list of values, it will be added, otherwise it is considered a duplicate.
Now as you can see, the dictionary is going to grow as more lines are read. In the end, it will just be too slow for a single process to complete the task efficiently. For my data it sometimes can take a week to do finish. Now I have a simple understanding that if I can implement something such multiple processes can be utilized to simultaneously compare with my dictionary, it would speed things up significantly (I can reserve as many as 16 CPU threads dedicated to my task). But unfortunately, multiprocessing/multithreading seems to be a complicated thing. I would appreciate if someone can direct me a path on what should I be focusing on.
Below is the code that I am currently using:

def sequence_mismatch (input_seq, input_dict, input_linker_length, j):
    local_smm_list=[]
    for stored in input_dict[input_seq[0:j:]]:
        local_smm=0
        for x in range(0, 7):
            if input_seq[input_linker_length+j+x] != stored[x]:
                local_smm += 1
        local_smm_list.append(local_smm)
    if min(local_smm_list) == 0:
        return 1
    else:
        return 0

# Main
from datetime import datetime
import itertools
import sys
try:
    input_fastq = sys.argv[1]
    linker_length_str = sys.argv[2]
    output_file_name = sys.argv[3]
    randomer_length = sys.argv[4]
except:
    print("Usage: ./PCRDupRm.py your_fastq.fastq linker_length output_file_name randomer_length")
    sys.exit(1)
startTime = datetime.now()
linker_length=int(linker_length_str)
i = int(randomer_length)
output_file = open(output_file_name, 'w+')
scanned_dict = {}
with open (input_fastq, 'r') as seq_data:
    for line1, line2, line3, line4 in itertools.izip_longest(*[seq_data]*4):
        if line2[0:i:] not in scanned_dict:
            scanned_dict.update({line2[0:i:]: [line2[(linker_length + i):(linker_length + i + 7):]]})
            output_file.write(line1)
            output_file.write(line2)
            output_file.write(line3)
            output_file.write(line4)
        else:
            comparison = sequence_mismatch (line2, scanned_dict, linker_length, i)
            if comparison == 1:
                continue
            else:
                scanned_dict[line2[0:i:]].extend([line2[(linker_length + i):(linker_length + i + 7):]])
                output_file.write(line1)
                output_file.write(line2)
                output_file.write(line3)
                output_file.write(line4)

output_file.close()
print (datetime.now() - startTime)

I just figured out that I could have simply used a dictionary nested inside a dictionary instead of a list nested inside a dictionary. Now the script finishes within minutes for each sample.


Compose n functions where the innermost function expects multiple arguments I want to perform function composition like this:

f(g(...(z(x) (y));

I have already compn:

const id = x => x;

const comp = f => g => x => f(g(x));

const foldr = f => acc => xs =>
 xs.reduceRight((acc, x, i) => f(x) (acc, i), acc);

const compn = foldr(comp) (id);

I tried to derive the desired function from compn, but failed. All I've got is a hideous wrapper function that applies the composition accordingly:

// generic functions

const id = x => x;

const last = xs => xs[xs.length - 1];

const init = xs => xs.slice(0, -1);

const sqr = x => x * x;

const inc = x => x + 1;

const sub = y => x => x - y;

const uncurry = f => (y, x) => f(x) (y);

const comp = f => g => x => f(g(x));

const foldr = f => acc => xs =>
 xs.reduceRight((acc, x, i) => f(x) (acc, i), acc);

// derived functions

const compn = foldr(comp) (id);

// wrapper function

const compn2 = fs => x => compn([...init(fs), last(fs) (x)]);

// arbitrary computation

const f = uncurry(compn2([sqr, inc, sub]));

console.log(f(5, 2)); // yields 16

Is there a way to derive compn2 from compn or from other combinators?

First, there's something weird happening here

const f = uncurry(compn2([sqr, inc, sub]));
console.log(f(5, 2)); // yields 16

I would assume that this composition is doing this

sqr (inc (sub (5) (2)))
// sqr (inc (2 - 5))
// sqr (inc (-3))
// sqr (-2)
// => 4

But upon closer inspect, it's because your uncurry function is unconventionally flipping the arguments

const uncurry = f => (y, x) => f(x) (y);

Nevermind that tho, we don't have to agree on it. I'll show you a way to solve your problem without even depending on it.

I'm going to introduce a function papply which procedurally applies a curried function

const papply = f => ([x,...xs]) =>
  x === undefined ? f : papply (f(x)) (xs)

console.log(papply (x => y => y - x) ([2,5])) // 3
// (x => y => y - x) (2) (5)
// (y => y - 2) (5)
// 5 - 2
// => 3

I'm also going to introduce a dramatically simplified compn – there's nothing wrong with yours, but it will help reduce some of the complexity of the example I provide.

const compn = ([f,...fs]) => x =>
  f === undefined ? x : f(compn (fs) (x))

const inc = x => x + 1

const f = compn ([inc, inc, inc])

console.log(f(3))
// compn ([inc, inc, inc]) (3)
// inc (compn ([inc,inc]) (3))
// inc (inc (compn ([inc]) (3)))
// inc (inc (inc (compn ([]) (3))))
// inc (inc (inc (3)))
// inc (inc (4))
// inc (5)
// => 6

OK, let's get to it then !

const sqr = x => x * x

const inc = x => x + 1

const sub = y => x => x - y

const compn = ([f,...fs]) => x =>
  f === undefined ? x : f(compn (fs) (x))

const papply = f => ([x,...xs]) =>
  x === undefined ? f : papply (f(x)) (xs)

const f = compn([sqr, inc, papply(sub)])

console.log(f([2, 5]));
// sqr (inc (papply (sub) ([2,5]))
// sqr (inc (papply (sub (2)) ([5])))
// sqr (inc (papply (x => x - 2) ([5])))
// sqr (inc (papply (5 - 2) ([])))
// sqr (inc (papply (3) ([])))
// sqr (inc (3))
// sqr (4)
// => 16

Of course it requires that the function is applied a bit differently ...

// this
f([2,5])

// instead of
f(5,2)

I don't think that change will bother you much tho, especially considering every other function of yours (including compn) is manually curried. This stays true to calling all functions with one arg. If multiple values are required, a tuple is provided – or in this case, an Array, as Tuple is absent in JS.

It's an interesting question and I think you had an interesting approach. The complexity ultimately steered me to thinking about a different solution. Just my 2 cents, hope it helped some ^_^


Simple Credit card validation I'm a beginner to Haskell (intermediate in web-dev stuff and JavaScript) and I'm not really sure what I could do better for Homework 1 of CIS194 that I'm taking online.
The problem statement is as follows (shortened):

Validate a credit card number by the following steps:

Double the value of every second digit from the right
Take the sum of the digits of the new values
Check whether the sum modulo 10 is 0.

Write the functions toDigits, toDigitsRev and doubleEveryOther for the first task, sumDigits for the second, and validate for the third.

toDigits :: Integer -> [Integer]
toDigits x
    | x <= 0       = []
    | divBy10 < 10 = [divBy10, remainder]
    | otherwise    = toDigits divBy10 ++ [remainder]
    where remainder = x `mod` 10
        divBy10   = x `div` 10

toDigitsRev :: Integer -> [Integer]
toDigitsRev x = reverse (toDigits x)

doubleEveryOther :: [Integer] -> [Integer]
doubleEveryOther xs = reverse $ doubleEveryOther' (reverse xs)
    where   doubleEveryOther' []       = []
            doubleEveryOther' (x:[])   = [x]
            doubleEveryOther' (x:y:[]) = [x, y*2]
            doubleEveryOther' (x:y:xs) = [x, y*2] ++ doubleEveryOther' xs

sumDigits :: [Integer] -> Integer
sumDigits xs = sum $ map sum $ map toDigits xs

validate :: Integer -> Bool
validate x = ((sumDigits (doubleEveryOther (toDigits x))) `mod` 10) == 0

First of all, it's great that you've used type signatures. So let's have a look at the contents of your functions.

Use divMod instead of div and mod

In toDigits, you both div and mod x by 10. However, there's a function that combines them both: divMod. With that in mind, we can write

toDigits :: Integer -> [Integer]
toDigits x
    | x <= 0       = []
    | divBy10 < 10 = [divBy10, remainder]
    | otherwise    = toDigits divBy10 ++ [remainder]
    where (divBy10, remainder) = x `divMod` 10

Avoid left-recursion on ++

However, your function is currently non-optimal. If you use x ++ [y] repeatedly, you end up with \$\mathcal O(n^2) \$ instead of \$\mathcal O(n) \$, as ++ is \$\mathcal O(n) \$ (where \$n\$ denotes the length of the first list).

It's easier to write toDigitsRev:

toDigitsRev :: Integer -> [Integer]
toDigitsRev x = case x `divMod` 10 of
                  (0, 0) -> []
                  (0, m) -> [m]
                  (d, m) -> m : toDigitsRev d

The case is just a matter of preference, you can continue to use your guards. Note that we now use (:), which is alwas \$\mathcal O(1) \$. Our toDigits is now only

toDigits :: Integer -> [Integer]
toDigits = reverse . toDigitsRev

Use simpler patterns where possible

Your patterns in doubleEveryOther are a little bit contrived. If we look closely, we can see that we always return the input, except for the two-element case. Let's reorder the patterns first to see that:

doubleEveryOther :: [Integer] -> [Integer]
doubleEveryOther xs = reverse $ doubleEveryOther' (reverse xs)
    where   doubleEveryOther' (x:y:xs) = [x, y*2] ++ doubleEveryOther' xs
            doubleEveryOther' []       = []
            doubleEveryOther' (x:[])   = [x]
            doubleEveryOther' (x:y:[]) = [x, y*2]

Now the first pattern handles both (x:y:xs) and (x:y:[]), so we can get rid of the last pattern:

doubleEveryOther :: [Integer] -> [Integer]
doubleEveryOther xs = reverse $ doubleEveryOther' (reverse xs)
    where   doubleEveryOther' (x:y:xs) = [x, y*2] ++ doubleEveryOther' xs
            doubleEveryOther' []       = []
            doubleEveryOther' (x:[])   = [x]

In our empty and single-element pattern we return our input, so we can combine both variants into a single pattern:

doubleEveryOther :: [Integer] -> [Integer]
doubleEveryOther xs = reverse $ doubleEveryOther' (reverse xs)
    where   doubleEveryOther' (x:y:xs) = x : y*2 : doubleEveryOther' xs
            doubleEveryOther' xs       = xs

Also, it's common to use short names for workers, so let's call the worker go:

doubleEveryOther :: [Integer] -> [Integer]
doubleEveryOther = reverse . go . reverse
    where   go (x:y:xs) = x : y*2 : go xs
            go xs       = xs

The point-free definition of doubleEveryOther is a matter of preference.

Other remarks

sumDigits and validate are fine, although I'd write them as follows

sumDigits :: [Integer] -> Integer
sumDigits  = sum . map (sum . toDigits)

validate :: Integer -> Bool
validate x = checksum == 0
  where
    checksum = sumDigits (doubleEveryOther $ toDigits x) `mod` 10

That's only a matter of personal preference, though. The compiler will change map f $ map g to map (f . g) automatically, so your variant of sumDigits didn't traverse the list twice.


Rotate a given number of variables in-place A very simple but useful algorithm: rotate the values of a given number of variables in-place to the left so that the first variable gets the value of the second, the second gets the value of the third, etc... and the last variable gets the value of the first one.

#include <type_traits>
#include <utility>

template<typename Head, typename... Tail>
struct are_same:
    std::conjunction<std::is_same<Head, Tail>...>
{};

template<typename T, typename... Args>
auto rotate_left_inner(T& first, T& second, Args&... args)
    -> T&
{
    first = std::move(second);
    if constexpr (sizeof...(Args) == 0) {
        return second;
    } else {
        return rotate_left_inner(second, args...);
    }
}

template<typename Head, typename... Tail>
auto rotate_left(Head& first, Tail&... args)
    -> void
{
    static_assert(are_same<Head, Tail...>::value,
                  "elements passed to rotate_left shall have the same type");

    if constexpr (sizeof...(Tail) > 0) {
        auto tmp = std::move(first);
        auto& last = rotate_left_inner(first, args...);
        last = std::move(tmp);
    }
}

This function can be used as follows:

int a=0, b=1, c=2, d=3, d=4;
rotate_left(a, c, e);
rotate_left(b, d);
// Now (a, b, c, d, e) = (2, 3, 4, 1, 0)

The code above generated the same assembly than the hand-rolled version when I gave it five integers to rotate in the compiler explorer, with both GCC and Clang.

I wanted to make a companion rotate_right function, but repeatedly finding the last element of a template parameter pack is not as easy, so the easiest solution for a programmer is to pass the variables to rotate to rotate_left in reverse order.

Do you see any way to improve this simple algorithm? Anything that I might be missing?

Issues (for me)
I get a couple of errors when I try and compile.
This error I have a feeling is caused because you are using a more up to date compiler (I presume this is a C++17 feature).
if constexpr (sizeof...(Tail) > 0) {
   ^^^^^^^^^

This problem causes my second error (because I remove constexpr to make it compile):
rt.cpp:12:6: note: candidate function template not viable: requires at least 2 arguments, but 1 was provided
auto rotate_left_inner(T& first, T& second, Args&... args)

Assuming that constexpr forces that expression to be evaluated at compile time and thus removes that whole block of code. Then it should compile.
Review
You and I differ on how we like to express the return value. I believe we have already had the discussion. So I just want to point it out for other readers.
// You
auto rotate_left(Head& first, Tail&... args)
    -> void

// Me
void rotate_left(Head& first, Tail&... args)

But I don't see anything wrong with yours either, its a simple stylistic thing.
Personally I don't like recursive template meta programming. Two reasons:

This was because when templates were first introduced there was a recursive limit on most compilers. I think this is purely historical now as most compilers have really huge recursive limits now.

Recursive template functions needed a terminating version of the function. But this new constexpr if statement seems to be in response to that. So you no longer need a special case and it can be handled by the compiler.

So it looks like both my reasons for disliking recursive template functions have been addressed by the new version of the language.
But I still prefer the style of the unrolling the argument loop in a function. So I present my version of the same function to show readers how it is done without using recursion (template iterative style).
#include <type_traits>
#include <utility>
#include <tuple>

template<typename T, std::size_t... index>
void rotate_left_inner(T&& tuple, std::index_sequence<index...> const&)
{
    auto tmp = std::move(std::get<0>(tuple));
    auto mover = {0, (std::get<index>(tuple) = std::move(std::get<index + 1>(tuple)), 0)...};
    std::get<sizeof...(index)>(tuple) = std::move(tmp);
}

template<typename... List>
void rotate_left(List&... args)
{
    rotate_left_inner(std::tie(args...), std::make_index_sequence<sizeof...(args) - 1>());
}

#include <iostream>

int main()
{
    int t1 = 5;
    int t2 = 6;
    int t3 = 7;

    rotate_left(t1, t2, t3);
    std::cout << t1 << " " << t2 << " " << t3 << "\n";

    rotate_left(t1);
    std::cout << t1 << " " << t2 << " " << t3 << "\n";
}


Fast edit of Excel file I have this code to edit certain cells in my Excel file using Apache POI, but it is really slow. How can I improved the performance?

Ideally I would like to edit 20000 rows in less than one minute. At the moment it does ~100/min. Any suggestions would be great.

public static void main(String[] args) throws IOException, InvalidFormatException{
        InputStream inp = new FileInputStream("test.xls");
        FileOutputStream fileOut = new FileOutputStream("edited-test.xls");
        Workbook wb = WorkbookFactory.create(inp);
        Sheet sheet = wb.getSheetAt(0);
        for(int i=2;i <20002;i++){
            Row row = sheet.getRow(i);

            Cell cell4 = row.getCell(4);
            cell4.setCellValue(i);

            Cell cell6 = row.getCell(6);
            cell6.setCellValue("aa"+i);

            Cell cell8 = row.getCell(8);
            cell8.setCellValue("2");

            wb.write(fileOut);
            System.out.println(i);
        }
        fileOut.close();
        System.out.println("Done!");
    }

First thing you should do is only write the file out once, not 20,000 times ;-)

Move the wb.write(fileOut); to be outside the loop.....

Additionally, there may be some improvement by reversing the loop:

for(int i=2;i <20002;i++){

can become:

for(int i=20001;i >= 2;i--){

This may make some memory management in the API faster.


Starting a new transaction inside a single bean I thinking of a way to start a new transaction inside a single EJB bean. Typical use case would be to process every item from a list in a separate transaction.

One way to do it would be to use UserTransaction:

@Stateless
@TransactionManagement(TransactionManagementType.BEAN)
public class ManagerBean implements Manager {
    @Resource
    private UserTransaction tx;

    @Override
    public void processAll(List<Object> list) throws Exception {
        for (Object obj : list) {
            tx.begin();
            try {
                processOne(obj);
                tx.commit();
            } catch (Exception ex) {
                tx.rollback();
            }
        }
    }

    private void processOne(Object obj) {
        /* ... */
    }
}

But I find it error prone to directly use commit() and rollback(). I'd prefer to leave transaction handing to the container. So I've come up with this pattern:

ManagerBean.java

@Stateless
public class ManagerBean implements Manager, ManagerInternal {
    @Resource
    private SessionContext context;

    @Override
    public void processAll(List<Object> list) {
        ManagerInternal txThis = context.getBusinessObject(ManagerInternal.class);
        for (Object obj : list) {
            txThis.processOne(obj);
        }
    }

    @Override
    @TransactionAttribute(TransactionAttributeType.REQUIRES_NEW)
    public void processOne(Object obj) {
        /* ... */
    }
}

@Local
interface ManagerInternal {
    void processOne(Object obj);
}

Manager.java:

@Local
public interface Manager {
    void processAll(List<Object> list);
}

Though I'm unsure if it's ok to have package-private local interface. It works on JBoss 4.2, but I don't know about other app servers.
What do you think about this pattern?

Alternative using self-injection:

Instead of using SessionContext.getBusinessObject(interface) I suppose you can inject EJB into itself:

ManagerBean.java

@Stateless
public class ManagerBean implements Manager, ManagerInternal {
    @EJB
    private ManagerInternal txThis;

    @Override
    public void processAll(List<Object> list) {
        for (Object obj : list) {
            txThis.processOne(obj);
        }
    }

    @Override
    @TransactionAttribute(TransactionAttributeType.REQUIRES_NEW)
    public void processOne(Object obj) {
        /* ... */
    }
}

@Local
interface ManagerInternal {
    void processOne(Object obj);
}

Manager.java stays the same.

EJB interfaces with package access seem to be not allowed by Wildfly.

Variable Names:
Your variable names are very generic. The List<Object> you pass in is just called list. This might be fine at coding-time, but as soon as you have to analyze the code weeks or maybe even months later, you will probably wonder, what purpose that list serves.
I'd thus propose you change the variable name a bit. Something like listForPersisting or something should be fine ;) The same may apply to your UserTransaction and the SessionContext, but as they are defined enough by their typename, that shouldn't be a big problem.
Spacing / Comments
I realy like your spacing. Your code is speaking enough, that you can leave out comments and still you understand it on first reading.
That's very good, keep it up!
Approach
I don't think you can solve your specific problem in many other ways as elegantly as you did.
What I do not like about your code is that you needlessly create the interface ManagerInternal  as if you needed a function to override.
You name that interface "Internal" and go through the hassle of making it @Local. But then you do something totally incomprehensive. You expose the overriden method.
If you don't need the method anywhere else, the principle of information hiding applies.

In computer science, information hiding is the principle of segregation of the design decisions in a computer program that are most likely to change, thus protecting other parts of the program from extensive modification if the design decision is changed. The protection involves providing a stable interface which protects the remainder of the program from the implementation (the details that are most likely to change).
Written another way, information hiding is the ability to prevent certain aspects of a class or software component from being accessible to its clients, using either programming language features (like private variables) or an explicit exporting policy.


Checking if a sentence is considered valid Exercise: Checking if a sentence is considered valid under the following conditions:

Numbers less than 13 are only allowed in their written form, e.g "twelve", but "14" is considered valid.
Only 1 period marker is allowed and it must be the final character
First letter must be uppercase
number of quotation marks in the sentence must be even

if the sentence meets all above criteria, it is considered valid.

Here is my approach:

public class SentenceValidator implements ValidatesSentences {

  private static final int MINIMUM_ALLOWED_NUMBER = 13;

  public SentenceValidator() {
        //default (auto-generated without explicit declaration anyway)
  }

    /**
     * Takes a string sentence and returns if it meets the requirements
     * @param sentence -> The string sentence to analyze for validity
     * @return boolean -> if the string meets the validation requirements
     */
  @Override
  public final boolean isSentenceConsideredValid(String sentence) {
    String temporary = guardTheSentence(sentence);
    return temporary.length() > 1 && areConditionsValid(temporary);
  }

  private boolean areConditionsValid(String sentence) {
      return isTheFirstCharacterAnUppercaseLetter(sentence)
              && isTheCountOfQuotationMarksEven(sentence)
              && isTheOnlyPeriodMarkTheLastCharacterOfTheSentence(sentence)
              && isTheLowestNumberInTheSentenceGreaterThan13(sentence);
  }

  private String guardTheSentence(String sentence) {
    Objects.requireNonNull(sentence, "null is not an acceptable value");
    return sentence.trim();
  }

  private boolean isTheFirstCharacterAnUppercaseLetter(String sentence) {
    return Character.isLetter(sentence.charAt(0))
        && sentence.charAt(0) == sentence.toUpperCase().charAt(0);
  }

  private boolean isTheCountOfQuotationMarksEven(String sentence) {
    return (retrieveCountOfAGivenCharacter(sentence,'"') % 2) == 0;
  }

  private boolean isTheOnlyPeriodMarkTheLastCharacterOfTheSentence(String sentence) {
    return sentence.charAt(sentence.length() - 1) == '.' && retrieveCountOfAGivenCharacter(sentence,'.') == 1;
  }

  private int retrieveCountOfAGivenCharacter(String sentence, Character charToCount) {
    int counter = 0;
    for (char c : sentence.toCharArray()) {
      if (c == charToCount) {
        counter++;
      }
    }
    return counter;
  }

  /**
   * We don't need really need to care about words for the numbers, that is trivial All we want to
   * check is for all numeric value(s) within the string and ensure none are below 13
   * supports negative numbers using the optional (?:-)
   */
  private boolean isTheLowestNumberInTheSentenceGreaterThan13(String sentence) {
    String temp = sentence.replaceAll("[^-?0-9]+", " ");
    temp = temp.trim();

    if (temp.isEmpty()) {
        return true;
    }

    int lowestNumber = MINIMUM_ALLOWED_NUMBER;
    for (String number : temp.split(" ")) {
      lowestNumber = Math.min(lowestNumber, Integer.parseInt(number));
    }

    return lowestNumber >= MINIMUM_ALLOWED_NUMBER;
  }
}

public interface ValidatesSentences {

    boolean isSentenceConsideredValid(String sentence);
}

A jump out problem I see already is the NumberFormatException when number is greater than a max int, I should use a long/Big Integer there and handle a NumberFormatException.

What you have here is a series of boolean-result conditions that you want to apply to a given String.

The design can be expanded to allow List of conditions to be applied.  This way you can add or remove conditions with minumal changes.
You can leverage Java 8 Predicate interface that fits the conditions you want to write.  You can write at least some of the conditions as lambda expressions, saving you the need to write concrete clases

List<Predicate<String>> conditions = new ArrayList<>();
conditions.add((sentence) -> Character.isLetter(sentence.charAt(0))
        && sentence.charAt(0) == sentence.toUpperCase().charAt(0))

  private boolean areConditionsValid(String sentence) {
      return conditions.stream().allMatch(condition -> condition.test(sentence));
  }


Getting MAC addresses from systems I have written code to get all the MAC addresses in from systems having multiple MAC addresses and I think there's something which needs to be corrected albeit the code runs fine.

Note: I have just tested it on Android.

import re

def getMacs():
    macs = []
    file = os.popen("getmac").read()
    file = file.split("\n")

    for line in file:
        found = re.search(r'([0-9A-F]{2}[:-]){5}([0-9A-F]{2})', line, re.I)
        if found:
            mac = found.group().replace('-', ':')
            macs.append(mac)

    return macs

There are a few things that I think could be improved.

Include all required imports

The code uses os.popen() but doesn't import os.  It should.

Consider other platforms

On my Linux machine, there is no getmac command, but one could get the required information via ifconfig.  Consider either passing the command in to the function or making OS-specific versions.

Don't use deprecated functions

The os.popen() command is deprecated since version 2.6.  Use the subprocess module instead.


Dict lookup with type conversion and fallback Given the following working code:

def get(self, key, default=None, type=None):
    """Return the default value if the requested data doesn't exist.
    If `type` is provided and is a callable it should convert the value,
    return it or raise a :exc:`ValueError` if that is not possible.  In
    this case the function will return the default as if the value was not
    found:

    >>> d = TypeConversionDict(foo='42', bar='blub')
    >>> d.get('foo', type=int)
    42
    >>> d.get('bar', -1, type=int)
    -1

    :param key: The key to be looked up.
    :param default: The default value to be returned if the key can't
                    be looked up.  If not further specified `None` is
                    returned.
    :param type: A callable that is used to cast the value in the
                 :class:`MultiDict`.  If a :exc:`ValueError` is raised
                 by this callable the default value is returned.
    """
    try:
        rv = self[key]
        if type is not None:
            rv = type(rv)
    except (KeyError, ValueError):
        rv = default
    return rv

While it is clear that if we attempt to get a the value of a non-existent key, we will get a KeyError, it is not immediately discernible what will lead to a ValueError. It's only when we pass a string that does not contain an int do we see that this is where type() fails, and a ValueError is caught and handled.

How many actions to try before calling except? Should it not be written more explicitly and with more granularity thus:

try:
    rv = self[key]
except KeyError:
    rv = default
try:
    if type is not None:
        rv = type(rv)
except ValueError:
    rv = default
return rv

I prefer the explicit form of the code, because we’re minimising the amount of code that can throw. It makes it very clear exactly where we expect the exception to occur, and we don’t risk catching an unexpected exception that’s hiding in our try block.

As it happens, we can actually do away with the first try/except block. This operation – look up a key, if it’s missing, use a default – is common enough that it’s covered by the standard library; specifically, dict.get(). You can use the single line:

rv = self.get(key, default)

Separately, I’d rename the type keyword. It’s better to avoid using the same names as the built-in functions – it can be a source of subtle bugs, and just causes confusion.


Prime Number Generator Time Limit Exceeded PRIME1- Prime Generator
The code is working fine and it's generating a prime number from  M to N but the only problem is I'm getting TLE when I submit my answer. How do I optimise my code to pass the time limit?

  number-theory

  Peter wants to generate some prime numbers for his cryptosystem. Help him! Your task is to generate all prime numbers between two given
  numbers!

  Input

  The input begins with the number t of test cases in a single line (t<=10). In each of the next t lines there are two numbers m and n (1
  <= m <= n <= 1000000000, n-m<=100000) separated by a space.

  Output

  For every test case print all prime numbers p such that m <= p <= n, one number per line, test cases separated by an empty line.

  Example

  Input:
   2

  1 10

  3 5

  Output:

  2
  3
  5
  7

  3
  5

  Warning: large Input/Output data, be careful with certain languages (though most should be OK if the algorithm is well designed)

Time limit:  6s

Source limit:    50000B

Memory limit:    1536MB

#include <iostream>
#include <math.h>
#include <vector>

int main() {
int T;
uint64_t m, n;
std::scanf("%d", &T);
while (T--) {
    std::scanf("%llu %llu", &m, &n);

    std::vector<bool> flags(n+1);
    std::fill(flags.begin(),flags.end(),true);

    if (m == 1) {
        m++;
    }
    double halfMax = ceil(sqrt(n));
    for (uint64_t i = 2; i <= halfMax; ++i) {

        if (flags[i]) {
            for (uint64_t j = i * 2; j <= n; j += i) {
                flags[j] = false;
            }

        }
    }

    for (uint64_t k = m ; k <= n; ++k) {
        if (flags[k]) {
            printf("%lld", k);
            printf(" ");
        }

     }
     printf("\n");
   }
}

I see some things that may help you improve your code.  I'll start with the stylistic pieces and then move on to the basic algorithm and the speed-up you're seeking.

Fix your formatting

This code has peculiar indentation that makes it difficult to tell when a function begins and ends.  Fixing that would hlep.

Use <cmath> instead of <math.h>

The difference between the two forms is that the former defines things within the std:: namespace versus into the global namespace.  Language lawyers have lots of fun with this, but for daily use I'd recommend using <cmath>.  See this SO question for details.

Use all of the required #includes

The function std::scanf is used but its declaration is in #include <cstdlib> which is not actually in the list of includes.  Similarly std::fill needs #include <algorithm>.

Avoid non-portable type assumptions

The code currently contains this line

std::scanf("%llu %llu", &m, &n);

That might be just fine with your compiler on your machine, but on mine, the compiler complains:

  warning: format ‘%llu’ expects argument of type ‘long long unsigned int*’, but argument 2 has type ‘uint64_t* {aka long unsigned int*}’

The simplest way to avoid that problem is to use the C++ stream extractors instead:

std::cin >> m >> n;

There is a parallel problem (and fix) with std::printf.

Use the right constructor

The code currently contains these two lines:

std::vector<bool> flags(n + 1);
std::fill(flags.begin(), flags.end(), true);

Why not simply use the constructor version that does both of these?

std::vector<bool> flags(n + 1, true);

Use your knowledge of the mathematical domain

The only even prime number is 2, so you could optimize for both space and performance by only storing bits that correspond with odd numbers.  Further, it's clear that if we eliminate even numbers, the inner sieve loop can look like this instead:

for (uint64_t j = i * i; j <= n; j += 2 * i) {

Avoid doing the same work twice

In the case that there are multiple trials, one simple optimization would be to compute the sieve only once (based on the largest n) and then simply use it multiple times rather than re-calculating the sieve for every trial.

Do work at compile time instead of runtime

Another optimization would be to compute the sieve at compile time rather than runtime.  If you're using C++14 or better, one can compute the sieve at compile time.


Winning strategy for the dice pig game I am writing a computer opponent in java to play the dice game pig. I am trying to code it to be optimized based off of this paper.

    @Override
public boolean decideIfShouldRoll(int turnNumber, int rollNumber,
        int poolSize, int myScore, int opponentsScore) {
    if (myScore < 30 && opponentsScore < 30) {
        if (poolSize <= 20)
            return true;
        else
            return false;
    }

    // Checks for if the opponent has a greater score
    if (opponentsScore > myScore) {

        // opponent is near winning?
        if (100 - opponentsScore <= 20) {

            // checks if computer is near winning
            if (100 - myScore <= 20) {
                if (poolSize <= 20)
                    return true;
                else
                    return false;
            }

            // if computer not near winning, take a go for broke strategy
            if (100 - myScore > 20) {
                if (poolSize <= 30)
                    return true;
                else
                    return false;
            }

        }
        if (opponentsScore > 20) {
            if (poolSize <= 20)
                return true;
            else
                return false;
        }

        return true;
    }

    if (myScore > opponentsScore) {

        // if near winning and the opponent's score is within 10 try a small
        // wins strategy
        if ((100 - myScore) <= 20 && (myScore - opponentsScore) <= 10) {
            if (poolSize <= 15)
                return true;
            else
                return false;
        }
        // if 10 points from winning, only build a pool up to 10 points
        if ((100 - myScore) <= 10) {
            if (poolSize <= 10)
                return true;
            else
                return false;
        }
        // if the difference between the computer score and opponent score
        // is > 10, go up to the standard optimal pool size of 20
        if ((myScore - opponentsScore) > 10) {
            if (poolSize <= 20)
                return true;
            else
                return false;
        }

        // if near winning and the opponents score is within 10 points, take
        // a reduced standard pool size
        if ((100 - myScore) <= 20
                && Math.abs(myScore - opponentsScore) <= 10) {

            if (poolSize <= 15)
                return true;
            else
                return false;
        }

        return true;
    }
    return true;

}

Bugs

The outline of the function is

@Override
public boolean decideIfShouldRoll(int turnNumber, int rollNumber,
        int poolSize, int myScore, int opponentsScore) {
    if (myScore < 30 && opponentsScore < 30) {
        …
    }
    if (opponentsScore > myScore) {
        …
    }
    if (myScore > opponentsScore) {
        …
    }
    return true;
}

The return true at the end is problematic.  You're saying that, except in the opening moves, anytime the score is tied, you want to roll forever, essentially forfeiting your turn.

If we have 95 points and a pool of 5, we should stop rolling.  Your code tries to amass a pool of 15 points or 10 points.

Redundancy

The opponentScore > 20 test should always be true, since the opponent is in the lead, and the leader has at least 30 points.

@Override
public boolean decideIfShouldRoll(int turnNumber, int rollNumber,
        int poolSize, int myScore, int opponentsScore) {
    if (myScore < 30 && opponentsScore < 30) {
        …
    }
    if (opponentsScore > myScore) {
        …
        if (opponentsScore > 20) {
            …
        }
        return false;
    }
    if (myScore > opponentsScore) {
        …
    }
    return true;
}

Math.abs() is superfluous here, since we have already established that we are in the lead:

if (myScore > opponentsScore) {
    if ((100 - myScore) <= 20 && (myScore - opponentsScore) <= 10) {
        …
    }
    …
    if ((myScore - opponentsScore) > 10) {
        …
    }
    if ((100 - myScore) <= 20
            && Math.abs(myScore - opponentsScore) <= 10) {
        …
    }
    return true;

In fact, if you eliminate the Math.abs(), then you are redundantly restating the (100 - myScore) <= 20 && (myScore - opponentsScore) <= 10 case exactly.

Verbosity

Anytime you have if (condition) return true; else return false; you should just write return condition;.

I'm not a fan of needless arithmetic like if (100 - opponentsScore <= 20).  It would be shorter to write if (opponentScore >= 80), and just as clear.

Interface

The function name could be simplified to shouldRoll.

Assuming a fair die, there is no logical strategy in which the turnNumber or the rollNumber could be a factor in the decision.  It doesn't matter how you arrived at the current score and pool, because past performance is no indicator of future expectations.  Anything else would be a gambler's fallacy.  Therefore, I suggest eliminating turnNumber and rollNumber from the method signature.

I suggest renaming opponentsScore to theirScore, both to save a few characters and to mirror myScore.

Rewrite

There is too much code altogether!  You are really just trying to express a simple idea: in the face of an imminent loss, go for a pool of 30 points; if you have a comfortable lead, aim for just 15; otherwise, try to gather 20.

@Override
public boolean shouldRoll(int poolSize, int myScore, int theirScore) {
    if (myScore + poolSize >= 100) {
        return false;
    }
    int myLead = myScore - theirScore;
    int wantPool = (myScore < 80 && theirScore >= 80) ? 30              // Imminent loss, be aggressive.
                 : (myScore >= 80 && (0 < myLead && myLead <= 10)) ? 15 // Comfortable lead
                 : 20;                                                  // General case
    return poolSize < wantPool;
}


Small tree generator If one enters, say, "2, j" then it prints:

j
jj

If there is no second it should be *, so if it's just 2, it's * **.

#include <iostream>
#include <string>
#include <vector>

using std::string; using std::cin; using std::cout; using std::endl; using std::ostream; using std::vector;

class tree {
public:
    friend ostream &output(tree &oTree, ostream &os);
    bool makeTree(int, const string&);
private:
    vector<string> Tree;
};

bool tree::makeTree(int newBase, const string &newCont = "*") {
    if (!Tree.empty())
        Tree.clear();
    string tempString;
    for(int tempbase = 0; tempbase != newBase; ++tempbase) {
        tempString += newCont;
        Tree.push_back(tempString);
    }
  return 1;
}

ostream &output(tree &oTree, ostream &os) {
    for (auto c: oTree.Tree) {
        os << c << endl;
    }
    return os;
}

int main() {

}

Don't do this (its not readable).

using std::string; using std::cin; using std::cout; using std::endl; using std::ostream; using std::vector;

One declaration per line.

using std::string;
using std::cin;
using std::cout;
using std::endl;
using std::ostream;
using std::vector;

But actually prefer not to do it at all. If you must do it then confine it to the most narrow scope possible. The whole reason that the standard library is std:: is so that it does not cost much to prefix members by it.

std::cout << "Hello World" << std::endl; // Not that hard

It cost nothing to clear an empty tree.

if (!Tree.empty())  // Why do this test. Just call clear.
    Tree.clear();

In C++ the & and * are usually placed with the type (its type information). This is different from C.

bool tree::makeTree(int newBase, const string &newCont = "*") {
                                             ^^^  Looks strange.

// I would write it like this:
// Note: I also put the const the other side (but that's more a personal
// preference and there is no clear better style there).
bool tree::makeTree(int newBase, string const& newCont = "*") {

Don't use integers to represent bools. At the end of makeTree() I see.

return 1;

// Use a boolean value.
return true;

But there is no failure value so why return anything at all?

Prefer '\n' over std::endl. The difference between the two is that std::endl does an extra flush. This can make using it very inefficient. The whole point of using the buffer is to make printing efficient and the streams are flushed at optimal points automatically and forcing a flush will make the use of stream less optimal.


jQuery click handlers I'm working with Codeigniter and I have a jQuery file to add some functionalities. But I think I'm doing it in a wrong way.

$(function() {
    var list = '../indoamericana/intranet/employeesList/';
    var home = '../indoamericana/administrar/callHome/';
    var profile = '../indoamericana/user/';
    var addUser = '../indoamericana/user/addUSer';

    $(document).on('click', '#documentacion', function(e) {
        var head = $('iframe').contents().find('head');
        head.append($('<link/>', {
            rel: 'stylesheet',
            href: '/indoamericana/webroot/css/style.css',
            type: 'text/css' }));
        head.append($('<link/>', { rel: 'stylesheet',
            href: '/indoamericana/webroot/css/custom.css',
            type: 'text/css' }));
        e.preventDefault();
    });

    $(document).on('click', 'a[data-target=#ajax]', function(ev){
        var target = $(this).attr("href");
        $("#ajax").load(target, function() {
            $("#ajax").modal("show");
        });
        ev.preventDefault();
    });

    var loadPage = function(page){
        $(".page-content").load(page);
    };

    $(".sub-menu .menu-item, .module-item").click(function(event) {
        $(".page-content").load($(this).data('target'));
        $("title").text( $( this ).text() );
    });

    $( document ).on( 'click', '.page-sidebar-menu li', function(e) {
        $( this ).addClass('active');
        $( this ).siblings().removeClass('active');
    });

    var last_clicked = '';
    var order = 'asc';

    $(document).on('click', '#admin-support a i', function(e){
        var index = $( this ).attr('data-sort');
        if( last_clicked == index )
            order = ( order == 'asc' ) ? order = 'desc' : order = 'asc';
        last_clicked = index;
        var page  = '../indoamericana/soporte/allSupports/' + index + '/' + order;
        $(".page-content").load(page);
    });
});

I'm calling this file (call-functionalities.js) in a script type of my 'head':

<script src="webroot/scripts/call-functionalities.js" type="text/javascript"></script>

I don't want to use anonymous functions. I have seen some pages that use

jQuery(document).ready(function() {
  App.init();
  Index.init();
});

Apart from the fact I can't find any use of var loadPage It doesn't need to be an expression.
Instead try:

function loadPage(page) {
}

It is still accessible only within your code here, it is cleaner and easier to debug.
Keep your click events consistent

$(".sub-menu .menu-item, .module-item").click(function(event) {
    $(".page-content").load($(this).data('target'));
    $("title").text( $( this ).text() );
});

Is the only place you're using .click( I'd stick with the $(parent).on('click', '.selector', function(){ style.


Java helper function to round a number to the specified number of decimal places The function I need:

  Rounds a double to the specified number of decimal places

I figured this would be a part of java.lang.math, but inexplicably it doesn't have a round function that returns a double. Thus, my proposed solution is to add my own round function in a MathEx class.

/**
 * Holds generic math functions to extend the capabilities of the java.lang.Math class
 */
public class MathEx {

    /**
     * Rounds a double to the specified number of decimal places using the ubiquitous "half up" rounding method
     * @param numberToRound The number to be rounded
     * @param numberOfDecimals The number of decimals to include in the rounded number
     * @return The rounded number
     */
    public static double round(double numberToRound, int numberOfDecimals) {

        return new BigDecimal(numberToRound)
                    .setScale(numberOfDecimals, BigDecimal.ROUND_HALF_UP)
                    .doubleValue();
    }
}

I'd appreciate feedback on the overall strategy, the Javadoc comments, and the naming conventions.

This question makes assumptions which are not supported by the actual implementations of double in Java (or most other languages).

double is a floating-point number format, which has a number of odd behaviours that simply do not support 'rounding' in a consistently accurate way.

Consider the following code:

    double d = (v * 10 + 4) / 10_000_000.0;
    double rescale = round(d, 6) * 1_000_000_000.0;

That code takes an input value v, multiplies it by 10, adds 4, and then divides it by 10 million.

Consider an input value 65. Take that, multiply by 10, and add 4, to get 654. Now, divide 654 by 10 million, to get 0.0000654.

Now, round that value to 6 decimals, and get: 0.0000650, right?

Then multiply that by 1 billion to get 65000

Unfortunately, the actual result is:

64999.999999999990000000

The reason is that 0.000065 does not exist. Sure, many things that print numbers in Java will print a rounded value for you, but any math you do will have small errors inconsistent with your expectations.

The bottom line?

Rounding floating-point numbers to any fractional value will result in occasional errors.

The solution is to keep everything as a BigDecimal number. Do not use your method unless you know what you are sacrificing.

Here is an ideone, it loops through small values until it finds one which causes errors. The first error it finds is at 65 millionths.


Node.JS module to retrieve configuration from environment variables I don't like the idea of sprinkling process.env all over the place and wanted to abstract it out a bit. I'm new to Node and know there must be a better way to do a few things.

Not repeat the name 4 times (twice as a string and twice as a field)
Any cleaner way to create properties and centralize existence checks than how I've done it? Mind you, I am trying to keep this Node 4.3.2 compatible sans Babel transpilers to keep it simple and working in AWS Lambda's NodeJs environment.

// module

module.exports = {

}

var configError = function (keyName) {
  console.error("ExternalVars error! Could not load a value for: "+keyName);
  process.exit(1);
}

// TODO: extract repeated function?
Object.defineProperty(module.exports, 'SAMPLE_SECRET', {
    get: function() {
        if (!process.env.SAMPLE_SECRET)
            configError('SAMPLE_SECRET');
        else
            return process.env.SAMPLE_SECRET;
    }
});

Object.defineProperty(module.exports, 'SAMPLE_SECRET_MORE', {
    get: function() {
        if (!process.env.SAMPLE_SECRET_MORE)
            configError('SAMPLE_SECRET_MORE');
        else
            return process.env.SAMPLE_SECRET_MORE;
    }
});

// usage example

var externalvars = require('./config/externalvars.js');

var sampleSecret = externalvars.SAMPLE_SECRET;
var sampleSecretMore = externalvars.SAMPLE_SECRET_MORE;

Disclaimer: since I'm not familiar with export/import realm, my suggestion below might be too naive and not work. If so, rather than only downvoting, thanks in advance for a little comment pointing me to a resource which would enlight how and why I'm wrong.

That's said, with merely my pure JS skills, here is how I'd work:

var keys = [
  'SAMPLE_SECRET',
  'SAMPLE_SECRET_MORE'
];
keys.forEach(function(keyName) {
  Object.defineProperty(module.exports, keyName, {
      get: function() {
          if (!process.env[keyName]) {
              console.error(
                "ExternalVars error! Could not load a value for: " + keyName
              );
              process.exit(1);
          } else {
              return process.env[keyName];
          }
      }
  });
});


How to good code for multiple if conditions? We have to check Git and SVN folders.
    public static String getREPOType(String repoPath){
        boolean gitRepo = new File(repoPath, Constants.GIT_FOLDER).exists();
        String repoType = null;
        if(gitRepo){
            repoType = Constants.GIT;
        }else{
            boolean svnRepo = new File(repoPath, Constants.SVN_FOLDER).exists();
            if(svnRepo){
                repoType = Constants.SVN;
            }else{
                // New type, need to be handled
            }
        }
        return repoType;
    }

Please help if this can be refactored in Java

One trick that improves readability of if's called early return: idea is to return from method as soon as you know what will be returned.
public static String getRepoType(String repoPath) {
    boolean gitRepo = new File(repoPath, Constants.GIT_FOLDER).exists();
    if (gitRepo) {
        return Constants.GIT;
    }
    boolean svnRepo = new File(repoPath, Constants.SVN_FOLDER).exists();
    if (svnRepo) {
        return Constants.SVN;
    }

    // New type, need to be handled
}

Take a look at this good question about similar stuff.


Haskell merge sort inversions I'm working on a task from the one of Coursera programming courses. The purpose is to estimate the amount of inversions that are required to sort the array of numbers.

Also I'm trying to learn some Haskell basics, here is my solution:

fsthalf :: [a] -> [a]
fsthalf xs = take (length xs `div` 2) xs

sndhalf :: [a] -> [a]
sndhalf xs = drop (length xs `div` 2) xs

union :: ([Int], Int) -> ([Int], Int) -> ([Int], Int)
union (a, ac) (b, bc) = (a++b, ac+bc)

merge :: ([Int], Int) -> ([Int], Int) -> ([Int], Int)
merge ([], ac) (bs, bc) = (bs, ac + bc)
merge (as, ac) ([], bc) = (as, ac + bc)
merge (a:as, ac) (b:bs, bc)
  | a <= b = ([a], 0) `union` merge (as, ac) (b:bs, bc)
  | otherwise = ([b], length(a:as)) `union` merge (a:as, ac) (bs, bc)

mergeSort :: ([Int], Int) -> ([Int], Int)
mergeSort (s, c)
  | length s `elem` [0, 1]  = (s, c)
  | otherwise = merge (mergeSort (fsthalf s, c)) (mergeSort (sndhalf s, c))

main :: IO()
main = do
  _ <- readLn :: IO Int
  rawValues <- getLine
  let intValues = map read $ words rawValues :: [Int]
  print $ snd $ mergeSort (intValues, 0)

It seems to work properly, but exceeds time limits. It takes from 9 to 10 seconds to sort the longest possible list of integers (100000 values), while the limit is 6 seconds. Could anyone please point out some possible optimizations for this solution.

General review tips:

It'd be better to document your code. In particular functions like

union :: ([Int], Int) -> ([Int], Int) -> ([Int], Int)

need documentation - it's not clear which Int has what purpose at all without examining the source.
Some of your functions are polymorphic (sndhalf), others not. Why? I'd suggest to have all types consistent. Either just sort Ints, or make the types as generic as possible, for example

union :: (Ord a) => ([a], Int) -> ([a], Int) -> ([a], Int)

Notice how this generality also makes the types easier to understand.
I'd suggest to wrap list + its inversion count into a newtype such as:

newtype List a = List [a] Int

This makes the code better structured, and also allows you to use smart
constructors, if you decide
so.
Investigate Haskell's
sort
for inspiration.

Now specific to performance:
- For each pass you traverse the list several times,
which can impact the performance considerably. In particular: take and
length in fsthalf, the same in sndhalf. Also length in mergeSort. This
makes it 5 traversals before the merging pass. I'd try to reduce this number.

Matching on length is wasteful, if you need just to distinguish if it's 0 or 1, you traverse the whole list needlessly:

mergeSort (s, c) | length s `elem` [0, 1]  = (s, c)

Instead:

mergeSort ([], _) = ([], 0)
mergeSort (xs@[_], _) = (xs, 1)
...

Make sure you're compiling with -O.

I hope this helps!


Morse Translator in Python I am a beginner in Python, and I made a Morse translator that converts letters to Morse, and Morse to letters (where a decimal point is 'dot' and underscore is 'dash'). Is there any way I could make my code shorter and more efficient? For reference, here's the translation of Morse to letter characters.
dot = '.'
dash = '_'
letter_list = ['A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P', 'Q', 'R', 'S',
               'T', 'U', 'V', 'W', 'X', 'Y', 'Z']
morse_list = [dot + dash, dash + dot * 3, dash + dot + dash + dot, dash + dot * 2, dot, dot * 2 + dash + dot,
              dash * 2 + dot, dot * 4, dot * 2, dot + dash * 3, dash + dot + dash, dot + dash + dot * 2,
              dash * 2, dash + dot, dash * 3, dot + dash * 2 + dot, dash * 2 + dot + dash, dot + dash + dot,
              dot * 3, dash, dot * 2 + dash, dash * 3 + dot, dot + dash * 2, dash + dot * 2 + dash,
              dash + dot + dash * 2, dash * 2 + dot * 2]
# The for loop below prints the entire translation (Eg. 'A: ._ B: _...') so the user could reference it
for n in range(len(letter_list)):
    print(letter_list[n] + ': ' + morse_list[n], sep=' ', end=' ', flush=True)
while True:
    print("\nTYPE 'EXIT' if you want to EXIT.")
    user_input = input("INPUT LETTERS OR MORSE CHARACTERS (DECIMAL AND UNDERSCORE) TO CONVERT."
                       "\nSEPERATE LETTERS WITH SPACES: ")
    # Converts user input into an iterable list
    user_list = user_input.split()
    if user_input == 'EXIT':
        print("THANK YOU FOR USING THE MORSE TRANSLATOR")
        break
    for i in range(len(user_list)):
        if user_list[i] in letter_list:
            for n in range(len(morse_list)):
                if user_list[i] == letter_list[n]:
                    print(morse_list[n], sep=' ', end='  ', flush=True)
        elif user_list[i] in morse_list:
            for n in range(len(letter_list)):
                if user_list[i] == morse_list[n]:
                    print(letter_list[n], sep=' ', end='  ', flush=True)

Since it is really difficult to tell what letter maps to what, it will be much easier to validate your code (for yourself & future maintainers) if you write out the dictionary directly:

CHAR_TO_MORSE = {
    'A': '._',
    'B': '_...',
    'C': '_._.',
    'D': '_..',
    'E': '.',
    'F': '.._.',
    'G': '__.',
    'H': '....',
    'I': '..',
    'J': '.___',
    'K': '_._',
    'L': '._..',
    'M': '__',
    'N': '_.',
    'O': '___',
    'P': '.__.',
    'Q': '__._',
    'R': '._.',
    'S': '...',
    'T': '_',
    'U': '.._',
    'V': '___.',
    'W': '.__',
    'X': '_.._',
    'Y': '_.__',
    'Z': '__..',
}
MORSE_TO_CHAR = {v: k for k, v in CHAR_TO_MORSE.items()}
CHAR_OR_MORSE_TO_INVERSE = {**CHAR_TO_MORSE, **MORSE_TO_CHAR}

Avoid string concatenation (+) when possible (mostly for performance):

print(' '.join("{}: {}".format(k, v) for k, v in CHAR_TO_MORSE.items()))

Very minor but check exit condition before parsing:

if user_input == 'EXIT':
    break
user_list = user_input.split()

What happens if the input contains neither a single char nor valid morse? Your program will eat it, potentially at the confusion of the user. Maybe you can default to printing a question mark; something like:

print(' '.join(CHAR_OR_MORSE_TO_INVERSE.get(c, '?') for c in user_list))

Note that this uses a single dictionary lookup per item user_list, which is about as efficient as you could ask for.


Number to word converter (1 to 99 inclusive) Program

It takes a numerical input in range [1, 99] and outputs its word equivalent.

Concerns

Procedural approach
General code style

Code

#include <iostream>

void print1to20(int num)
    {
           switch(num)
           {
           case 1: std::cout << "one"; break;
           case 2: std::cout << "two"; break;
           case 3: std::cout << "three"; break;
           case 4: std::cout << "four"; break;
           case 5: std::cout << "five"; break;
           case 6: std::cout << "six"; break;
           case 7: std::cout << "seven"; break;
           case 8: std::cout << "eight"; break;
           case 9: std::cout << "nine"; break;
           case 10: std::cout << "ten"; break;
           case 11: std::cout << "eleven"; break;
           case 12: std::cout << "twelve"; break;
           case 13: std::cout << "thirteen"; break;
           case 14: std::cout << "fourteen"; break;
           case 15: std::cout << "fifteen"; break;
           case 16: std::cout << "sixteen"; break;
           case 17: std::cout << "seventeen"; break;
           case 18: std::cout << "eighteen"; break;
           case 19: std::cout << "nineteen"; break;
           case 20: std::cout << "twenty"; break;
           default: std::cout << "1 to 20 switch error";
           }
    }

void printTens(int num)
    {
           switch(num)
           {
           case 20: std::cout << "twenty"; break;
           case 30: std::cout << "thirty"; break;
           case 40: std::cout << "fourty"; break;
           case 50: std::cout << "fifty"; break;
           case 60: std::cout << "sixty"; break;
           case 70: std::cout << "seventy"; break;
           case 80: std::cout << "eighty"; break;
           case 90: std::cout << "ninety"; break;
           default: std::cout << "tens switch error";
           }
    }

int main()
{
    int num;
    do
    {
    std::cout << "Insert a number 1-99: ";
    std::cin >> num;
    } while(num < 1 || num > 99);

    int num_a, num_b; // num = (a)(b);
    num_a = num - num%10;
    num_b = num%10;

    std::cout << "Your number is ";

    if(num <= 20)
        print1to20(num);
    else
    {
        printTens(num_a);
        std::cout << ' ';
        print1to20(num_b);
    }

    return 0;
}

The code is broken. If a number is greater than 20 and ends with 0, print1to20(num_b) will not do what it should.
Don't mix the logic with the output. I'd change the return type of your functions to std::string. It'll make the code more testable and reusable.
Don't just print something to std::cout in a case of an error. Throw an exception instead.
Declare each variable as late as possible:

int num_a, num_b; // num = (a)(b);
num_a = num - num%10;
num_b = num%10;

should be

int num_a = ...;
int num_b = ...;

Use more descriptive names. I have no clue what num_a mean. What is a? Something like first_digit and second_digit would be better (well the first one is not exactly a digit, so you can come with even better names).
There's no need to return 0 from the main function explicitly (sure, you can if you want to).
Write proper automated tests for your code to reduce the number of bugs (your bug is very likely to get caught by tests). In fact, you can check all possible valid inputs. There're only 100 of them.
There's nothing wrong in having stand-alone functions. There's no entity here, so I don't see the point of modeling this problem in terms of objects. A function from an int to its string representation is just a function.


ProcessPoolExecutor KMZ file search I've written a script to look for coordinates in KMZ files. I've tried to speed execution up using the ProcessPoolExecutor. I'm quite new to Python, so any recommendations are more than welcome.

#!/usr/bin/env python

from zipfile import ZipFile
from lxml import html
import os
import concurrent.futures

def process_file(filename):
    results=[]
    try:
        saved_file=False;
        kmz = ZipFile(filename, 'r')
        for kml_name in kmz.namelist():
            if 'doc.kml' in kml_name:
                continue
            kml = kmz.open(kml_name, 'r').read()
            doc = html.fromstring(kml)
            for pm in doc.cssselect('Document Placemark'):
                tmp = pm.cssselect('track')
                if len(tmp):
                    # Track Placemark
                    tmp = tmp[0]  # always one element by definition
                    for desc in tmp.iterdescendants():
                        content = desc.text_content()
                        if desc.tag == 'coord':
                            lon = float(content.split()[0])
                            lat = float(content.split()[1])
                            search_lon = -47
                            search_lat = 47
                            if (abs(lat - search_lat) <= 1 and abs(lon - search_lon) <= 1):
                                if not saved_file:
                                    results.append('\nFile: ' + filename + '\n')
                                    saved_file=True
                                results.append(content + '\n')
    except:
        pass
    return results

def main():
    # Search all files
    kmz_files = []
    for root, subdirs, files in os.walk('raw/L1B_Catalogue'):
        for file in files:
            if '.kmz' in file:
                filename = os.path.join(root,file)
                kmz_files.append(filename)

    # Parallel execution
    f=open('kmz_search_output.txt', 'wt')
    count = 0
    with concurrent.futures.ProcessPoolExecutor(max_workers=8) as executor:
        for results in executor.map(process_file, kmz_files, chunksize=10):
            print("{} / {}".format(count,len(kmz_files)))
            count = count + 1
            for line in results:
                f.write(line);
            pass
    f.close()
    print('Done')

if __name__ == '__main__':
    main()

1. Review

There are no docstrings. What does process_file do? What does it return?
The name process_file is vague. It's better to use specific names, for example kmz_coordinates.
The saved_file logic should be unnecessary, because the caller already knows which filename they passed to process_file. So you could write something like:

results = executor.map(kmz_coordinates, kmz_files, chunksize=10)
for filename, coords in zip(kmz_files, results):
    print("{} / {}".format(count,len(kmz_files)))
    count = count + 1
    f.write(f"File {filename}:\n")
    for line in coords:
        f.write(line);

using zip to match up the filenames with the results, and remove the saved_file logic from kmz_coordinates.
Similarly, the count logic is unnecessary, because you could use enumerate to generate the counts:

results = executor.map(kmz_coordinates, kmz_files, chunksize=10)
for count, (filename, coords) in enumerate(zip(kmz_files, results)):
    print("{} / {}".format(count, len(kmz_files)))
    f.write(f"File {filename}:\n")
    f.writelines(coords)

The use of try: ... except: pass to suppress exceptions is a bad idea. There are two reasons for this. First, a bare except: catches all exceptions, including KeyboardInterrupt, which might make it difficult to stop the program by typing control-C. Second, the exceptions might be due to bugs in your program, but by suppressing them you make the bugs difficult to spot.

If you really do have a good reason for suppressing exceptions, then you should localize the try: ... except as tightly as you can around the line of code that might raise the exception, and you should pick the appropriate exception classs. For example, if the problem you are worrying about is that lxml.html.fromstring will raise lxml.etree.ParserError, write something like:

try:
    doc = html.fromstring(kml)
except lxml.etree.ParserError:
    continue # skip this file

When opening a file, it is a good idea to use with. For example, instead of:

f=open('kmz_search_output.txt', 'wt')
# ... code using f ...
f.close()

write:

with open('kmz_search_output.txt', 'w') as f:
    # ... code using f ...

This saves a line of code (no need to close the file explicitly), and ensures that the file gets closed in a timely manner even if an exception occurs inside the ... code using f ....
The content of a KML file XML, not HTML, so shouldn't you be using lxml.etree instead of lxml.html?
The code tries to parse every file in the KMZ as if it is a KML file:

for kml_name in kmz.namelist():
    if 'doc.kml' in kml_name:
        continue
    kml = kmz.open(kml_name, 'r').read()
    doc = html.fromstring(kml)

But a KMZ can include other kinds of data, for example images and audio, so I would expect some check on the filename, for example to see if has the .kml extension:

for kml_name in kmz.namelist():
    if not kml_name.endswith('.kml') or 'doc.kml' in kml_name:
        continue

If you did this then maybe you could omit the try: ... except: completely.
The name tmp is vague. Better to be as specific as possible, for example track.
This section of code loops over the first track in each placemark:

for pm in doc.cssselect('Document Placemark'):
    tmp = pm.cssselect('track')
    if len(tmp):
        # Track Placemark
        tmp = tmp[0]  # always one element by definition

But if there is only one track in each placemark, then it looks to me as though this can be as a single loop:

for track in doc.cssselect('Document Placemark track'):

Similarly, the code then loops through the descendents of the track looking for coord elements. So you could combine that into the same loop:

for coord in doc.cssselect('Document Placemark track coord'):

Instead of splitting the content of the coord element twice:

lon = float(content.split()[0])
lat = float(content.split()[1])

Split it just once and use map and tuple assignment:

lon, lat = map(float, content.split())

The search coordinates are always the same, so they should be assigned outside the loop. It would make sense for them to be keyword arguments to the kml_track_coordinates, then you could use the function to search for coordinates of your choice.
Similarly, the tolerance of 1 degree is arbitrary and should also be a keyword argument to the function.

2. Revised code

This is untested, so probably contains some mistakes.

def kmz_coordinates(filename, search_lon=-47, search_lat=47, tolerance=1):
    """Return list of track coordinates found in a KMZ file that are
    within tolerance degrees of (search_lon, search_lat).

    """
    coords = []
    with ZipFile(filename, 'r') as kmz:
        for kml_name in kmz.namelist():
            if not kml_name.endswith('.kml') or 'doc.kml' in kml_name:
                continue
            with kmz.open(kml_name, 'r') as kml:
                doc = html.fromstring(kml.read())
            for coord in doc.cssselect('Document Placemark track coord'):
                content = coord.text_content()
                lon, lat = map(float, content.split())
                if (abs(lat - search_lat) <= tolerance
                    and abs(lon - search_lon)) <= tolerance):
                    coords.append(content + '\n')
    return coords

def kmz_search_directory(directory, output_filename='kmz_search_output.txt'):
    """Search directory for KMZ files, find matching coordinates within
    the files, and write the results to output_filename.

    """
    kmz_files = []
    for root, _, files in os.walk(directory):
        for filename in files:
            if filename.endswith('.kmz'):
                kmz_files.append(os.path.join(root, filename))

    with open(output_filename, 'w') as f:
        with concurrent.futures.ProcessPoolExecutor(max_workers=8) as executor:
            results = executor.map(kmz_coordinates, kmz_files, chunksize=10)
            for count, (filename, coords) in enumerate(zip(kmz_files, results)):
                print("{} / {}".format(count, len(kmz_files)))
                f.writelines(coords)

    print('Done')


Boolean constant for assert in if..else block I have the following condition statement in Groovy:

if ( condition_1 ) {
    //some actions
} else if ( condition_2 ) {
    //some another actions
} else {
    assert false, 'Assertion description'
}

Code Narc gives me a warning about using boolean false constant in the assertion. I'm not sure if it is real issue, so please share your thoughts about it.

I thought about using exceptions in this code instead of assert false but it looks too heavy (exception handling is too heavy I think) for my particular case.

Throwing an assert error is probably heavier than throwing an Exception (as Groovy will parse the assert inputs to give you a pretty output string)

And unless you are going to be running this hundreds of times per second, I wouldn't worry about it either way...

A way of using assert that gets round the warning (and provides a nice error message) might be to do:

assert condition_1 || condition_2, 'Expected at least condition_1 or condition_2 to be true'


Clear data-model layer in Django MVC I have a Django model User

class Notification(TimeStampedModel):
    user = models.ForeignKey(settings.AUTH_USER_MODEL, verbose_name=_('user'),
                             related_name='notifications')
    viewed = models.BooleanField(verbose_name=_('has viewed'), default=False)

    def make_viewed(self):
            if self.viewed:
                return
            self.viewed = True
            User.objects.filter(id=self.user.id).update(not_viewed_notifications=F('not_viewed_notifications') - Value(1))
            self.save()

So I think that order methods in model is bad idea and actually
 it's anti-pattern, so next one solution was like this:

class Notification(TimeStampedModel):
    user = models.ForeignKey(settings.AUTH_USER_MODEL, verbose_name=_('user'),
                             related_name='notifications')
    _viewed = models.BooleanField(verbose_name=_('has viewed'), default=False)

    @property
    def viewed(self):
        return self._viewed

    @viewed.setter
    def viewed(self, value):
        if self._viewed:
            return

        self._viewed = True
        self.user.not_viewed_notifications = F('not_viewed_notifications') - Value(1)
        self.user.save(commit=False)
        self.save()

It looks better, but we have a couple of problems:

1) Model have methods yet

2) Property care side-effect, it doesn't obviously

3) That action require access to instance, so we can't move it to custom field.

What are you thinking about?

I would go with a third approach so you don't have to manually update your User model:

class Notification(TimeStampedModel):
    user = models.ForeignKey(settings.AUTH_USER_MODEL, verbose_name=_('user'),
                             related_name='notifications')
    viewed = models.BooleanField(verbose_name=_('has viewed'), default=False)

from django.contrib.auth.models import User as DjangoUser

class User(DjangoUser):
     @property
     def not_viewed_notifications(self):
         return self.notifications.filter(viewed=False).count()

This is twofold:

You don't need special logic in Notification and handle it in your view very simply (you can also have your users "unsee" some notifications);
You can never be out-of-sync between the number of not seen notifications for a user and the state of the notifications for said user.

This approach also have the advantage of not requiring anything from the selected User model (settings.AUTH_USER_MODEL). If using the proposed one, you will have access to the "not viewed" count from the property, if using the base User model from Django (or any other, for that matter), nothing will break and you'll still have access to the reverse relationship. Whereas, in your current implementation, you'll get into troubles when trying to access a non-existent not_viewed_notifications.


Project Euler code evaluation - digit fifth powers This is for Project Euler #30:

  Surprisingly there are only three numbers that can be written as the
  sum of fourth powers of their digits:

  \$1634 = 1^4 + 6^4 + 3^4 + 4^4\$
    \$8208 = 8^4 + 2^4 + 0^4 + 8^4\$
    \$9474 = 9^4 + 4^4 + 7^4 + 4^4\$

  As \$1 = 1^4\$ is not a sum it is not included.  The sum of these numbers
  is \$1634 + 8208 +  9474 = 19316\$.

  Find the sum of all the numbers that can be written as the sum of
  fifth powers of their digits.

list = []
2.upto(1_000_000) do |num|
 list << num if num.to_s.split("").map(&:to_i).map { |num| num ** 5 }.inject(:+) == num
end

puts list.inject(:+)

This works, and I am here for three reasons:

I use map twice, and this smells but I don't know an easier/better way.
How does the & work? Notice I do map(&:to_i) and inject(:+).  I have seen examples of this but don't know why they are different and what is going on.
Any suggestions on code improvements?

Here is a little simpler way to write your code:

list = (2..1_000_000).select do |num|
  num.to_s.chars.map {|digit| digit.to_i ** 5}.inject(:+) == num
end

Using map twice is not big issue here, you operate on small collection and memory is not an issue (for really big collections you would probably want to use lazy, it's slower but somehow smarter).
The ampersand converts a symbol to a proc.

Inject without ampersand works pretty much the same, see documentation for details:

  If you specify a symbol instead, then each element in the collection will be passed to the named method of memo

Given memo = 0 and element 12 ruby does: 0 passed-symbol 12.
I'm not sure improvements are worth it, you can't really make it shorter or easier to read and performance or design is not necessary an issue here.


Running Code Just Once I want a code run just once (say in Dispose). WriteOnceBlock<T> from TPL Data Flow could be used here; but again if we need to check if it is done (in a 'not data flow' friendly manner), we have to call Receive and timeout and things.

I wrote this and it works, but I am not sure about when comparison takes place (==); is it possible for that to be rearranged so something goes wrong and out of time/order?

public class Once
{
    const Int32 JobDone = 11011;
    const Int32 NotDone = 119;

    Int32 _done;

    public Once() { _done = NotDone; }

    public bool IsDone() { return Interlocked.CompareExchange(ref _done, JobDone, NotDone) == JobDone; }
}

And usage:

readonly Once _stopped = new Once();
public void OnStop()
{
    if (_stopped.IsDone()) return;

    //...
}

There are a few items to make this code better.

Naming. Once is an OK name for the class, but the method name IsDone is a problem. This is an 'atomic' operation that sets values, as well as gets values. A method called something like "Trigger", and changing the class name to a common term like OneShot, will give you the semantics like:

private readonly OneShot terminator = new OneShot();

if (terminator.Trigger())
{
    ... do something if we are the first trigger
}

or, using your semantics, the negated value:

if (!terminator.Trigger()) return;

Your fields should all be private.

private const Int32 JobDone = 11011;
private const Int32 NotDone = 119;

private Int32 _done;

otherwise other code can possibly reset or mess up your trigger.
Why use the bizarre numbers for the constants? What's wrong with 1, and 0. The special numbers make me think there's something especially magical with them.

Apart from that, the Interlocked.CompareExchange is the right tool for the job. It creates an atomic compare-and-set operation that makes it thread safe.


Implementation of an OkHttp singleton for Android I'm new to Android programming. I am going to make lots of HTTP requests: 500, 1k, 2k, 10k.

I have tried to create a singleton class for OkHttp. I wrote this:

public class OkSingleton extends OkHttpClient {

private  static  OkSingleton minstance;
private final OkHttpClient myClient;

private OkSingleton (){
    myClient= new OkHttpClient();
}

public static synchronized  OkSingleton getMinstance(){

    if (minstance==null){
        minstance=new OkSingleton();
    }

    return minstance;
}}

Can you show me my mistakes?

You don't need to create new instance of OkHttpClient. Remove it.
You can use Initialization-on-demand holder pattern to create singleton in Java. No need to use synchronized block
Change coding style in your code. Remove m before instance variable. See naming convention.

Here is the code:

public class OkSingleton extends OkHttpClient {
    private static class LazyHolder {
        private static final OkSingleton instance = new OkSingleton();
    }

    public static OkSingleton getInstance() {
        return LazyHolder.instance;
    }
}


Concatenating a SoundCloud Playlist Update JSON I am currently working on an implementation of SoundCloud for a 3rd party iOS app, and as there is no SDK anymore I am doing all my calls via Alamofire.

This is what SC expects you to send in your body params:

{ "playlist":
    {"tracks":[
        {"id":"__"},
        {"id":"__"},
        {"id":"__"}]
    }
}

I hand over the current playlist where the song should be added in the playlist object to extract all the ids from tracks that are part of it atm - as SC expects you so send all the old track ids + your new one in a .PUT call

This is what the ID dance looks at the moment:

var allTracks = [Int]() //SC track id comes in as an Int

allTracks.append(trackId) //trackId is the new track

for track in playlist.tracks {
    var I = track.id
    allTracks.append(I)
}

var myparams = Dictionary<String,AnyObject>() //my Alamofire body-params

var myTracks = [Dictionary<String,AnyObject>]()

for id in allTracks {
    var D = ["id":id]
    myTracks.append(D)
}

myparams["playlist"] = ["tracks":myTracks]

As I am still fairly new to Swift/iOS Development I was looking for a more elegant way than to just throw stuff around 3x and end up basically appending it the way it came in.

You can make this easier with mapping.

So, let's clarify what we have.

playlist.tracks - An array of "tracks" which have an id property (which is what we need).
trackID - an integer representing our new track
myTracks - an array of dictionaries in the form of key "id" and value being the track ID.
myParams - a dictionary with the key of "tracks" and the value being the aforementioned myTracks array.

So we need to turn playlist.tracks into an array of ["id":id] dictionaries with the new track ID also in there.

First, let's ignore the new track, and map the existing tracks into an array of dictionaries:

var trackIDdicts: [[String:Int]] = playlist.tracks.map { ["id":$0.id] }

And now insert our new track ID at the front:

trackIDdicts.insert(["id":trackId], atIndex:0)

And now for myParams?

myParams["playlist"] = ["tracks":trackIDdicts]

Altogether, three simple lines of code:

var trackIDdicts: [[String:Int]] = playlist.tracks.map { ["id":$0.id] }
trackIDdicts.insert(["id":trackId], atIndex:0)
myParams["playlist"] = ["tracks":trackIDdicts]

I'd highly recommend converting your JSON/Dictionary keys into named constant variables however.

struct JSONKeys {
    static let TrackID = "id"
    static let Tracks = "tracks"
    static let Playlist = "playlist"
}

So now we have:

var trackIDdicts: [[String:Int]] = playlist.tracks.map { [JSONKeys.TrackID:$0.id] }
trackIDdicts.insert([JSONKeys.TrackID:trackId], atIndex:0)
myParams[JSONKeys.Playlist] = [JSONKeys.Tracks:trackIDdicts]