Sorting numbers using Java Collections.sort() Input is a file containing numbers in this format:

1.2 3 5.1 2 7 4 6
5 2 9.22 1 0.0 4 3

I am doing the sorting in the simplest way, using Collections.sort method. The running code is as follows:

import java.io.FileNotFoundException;
import java.io.FileReader;
import java.util.Collections;
import java.util.LinkedList;
import java.util.List;
import java.util.Scanner;

public class SortedNumbers {

    public static Double integerFromString(String str) {
        return Double.valueOf(str.substring(0, str.length()));
    }

    public static void main(String[] args) throws FileNotFoundException {
         String fileName = "";

        //scanner to read the file
        Scanner fileInput = null;

        List<String> temps = new LinkedList<String>();

        //first check if there is an input parameter
        if(args.length > 0) {
            fileName = args[0];
        }

        else {
            System.out.println("No file specified");
            System.exit(1);
        }

        //connect to the file
        FileReader file = new FileReader (fileName);
        fileInput = new Scanner(file);

        //start reading from the file, store all the lines in the list
        while(fileInput.hasNextLine()) {
            String num = fileInput.nextLine();
            temps.add(num);
        }

        //close the scanner
        fileInput.close();

        //convert the list into array of strings
        String[] str = temps.toArray(new String[0]);

        /**
         * for each string in str array, split it using space as the delimiter and store the splitted strings in an array,
         * for each string value in the array, convert it into Double and store it in a collection
         * Sort the collection, convert it into String
         */
        for(int i = 0; i < str.length; i++){
            List<Double> num = new LinkedList<Double>();
            String[] arr = str[i].split("\\s");
            for(int j = 0; j < arr.length; j++){
                Double currentNumber = integerFromString(arr[j]);
                num.add(currentNumber);
            }
            Collections.sort(num);
            String sortedNumbers = num.toString();
            int indexOfOpenBracket = sortedNumbers.indexOf("[");
            int indexOfLastBracket = sortedNumbers.lastIndexOf("]");
            System.out.println(sortedNumbers.substring(indexOfOpenBracket+1, indexOfLastBracket));
        }
    }
}

I am concerned about the performance, splitting the string and storing it in array, parsing each string from array into double and then adding in the list.  It seems like there could be an efficient way.

Are there any redundant steps in the code which I can avoid?

There are a few improvements I can see, but not really related to performance (though it may run a bit faster anyway).

If the files are small (less than 10Megabytes or so), then the relative performance of different approaches are slight (obviously, you can bork your code, but the IO part will remain about the same). So, it comes down to readability.

For readability, I would recommend using the 'cool' toys in Java7. Consider the following:

List<String> lines = Files.readAllLines(Paths.get(args[0]));
for (String line : lines) {
    String[] parts = line.split("\\s+");
    double[] values = new double[parts.length];
    for (int i = 0; i < parts.length; i++) {
        values[i] = Double.parseDouble(values[i]);
    }
    Arrays.sort(values);
    StringBuilder sb = new StringBuilder();
    for (int i = 0; i < values.length; i++) {
        if (i > 0) {
            sb.append(", ");
        }
        sb.append(values[i]);
    }
    System.out.println(sb.toString());
}

Note the key elements here:

use Files and Paths classes
use StringBuilder for the string concatenation
use String.split, not a scanner....
use primitive data structures (arrays of double, not Double)
that means using Arrays.sort, not Collections.sort

Other things to consider.....

a method called integerFromString ... that returns a double is ... confusing.
using List.toString() and the stripping off the [ and ] is backward.


Console output of multiplication table I have a method, which outputs multiplication table sized by given maximum number. Also, it visually separates first row and column from the rest table. How my code could be improved or optimized?

public class Example {
    public static void main(String args[]) {
        printMultiplicationTable(15);
    }

    private static void printMultiplicationTable(int max) {
        for (var row = 1; row <= max; row += 1) {
            if (row == 2) {
                for (var column = 2; column <= max; column += 1) {
                    System.out.print("----");
                }
                System.out.println();
                continue;
            }

            for (var column = 1; column <= max; column += 1) {
                int result = row * column;

                if (column == 1) {
                    if (row < 10) {
                        System.out.print(result + " |\t");
                    } else {
                        System.out.print(result + "|\t");
                    }

                } else {
                    System.out.print(result + "\t");
                }
            }

            System.out.println();
        }
    }
}


String Encoder with lower-case output Question: Take an input string and output the "encoded" string by using the following rules:

 [1] vowels are replaced with number: a->1, e->2, etc
 [2] consonants are replaced with previous letter b->a, c->b, etc
 [3] y goes to space
 [4] space goes to y
 [5] numbers are reversed
 [6] other characters remain unchanged(punctuation, etc.)
 [7] all output should be lower case
 E.g.: Hello World! => g2kk4yv4qkc!

My Solution:

  /// pre processed conversions for letters
  private static Dictionary<char, char> Convert;

  public static string encode(string stringToEncode) {

     // approach: pre process a mapping (dictionary) for letter conversions
     // use a Dict for fastest look ups.  The first run, will take a little
     // extra time, subsequent usage will perform even better
     if (Convert == null || Convert.Count == 0) BuildConversionMappings();

     // our return val (efficient Appends)
     StringBuilder sb = new StringBuilder();

     // used for reversing the numbers
     Stack<char> nums = new Stack<char>();

     // iterate the input string
     for(int i = 0; i < stringToEncode.Length; i++) {

        char c = stringToEncode[i];

        // we have 3 cases:
        // 1) is alpha ==> convert using mapping
        // 2) is number ==> peek ahead to complete the number
        // 3) is special char / punctunation ==> ignore

        if(Convert.ContainsKey(c)) {
           sb.Append(Convert[c]);
           continue;
        }

        if(Char.IsDigit(c)) {
           nums.Push(c);

           // we've reached the end of the input string OR
           // we've reached the end of the number
           if (i == stringToEncode.Length - 1
              || !Char.IsDigit(stringToEncode[i + 1])) {
              while (nums.Count > 0) {
                 sb.Append(nums.Pop());
              }
           }

           continue;
        }

        // not letter, not digit
        sb.Append(c);
     }
     return sb.ToString();
  }

  // create our mappings for letters
  private static void BuildConversionMappings() {

     Convert = new Dictionary<char, char>();

     // only loop once for both
     for(char c = 'B'; c<='Z'; c++) {
        // add capitals version
        char val = (char)(c - 1);
        val = Char.ToLower(val);
        Convert.Add(c, val);
        // add lower case version
        Convert.Add(Char.ToLower(c),val);
     }

     // special cases
     Convert['y'] = ' ';
     Convert['Y'] = ' ';
     Convert.Add(' ', 'y');

     // vowels
     char[] vowels = new char[] { 'a', 'e', 'i', 'o', 'u' };
     for(int i = 0;i < vowels.Length;i++) {
        var letter = vowels[i];
        var value = (i+1).ToString()[0];
        Convert[letter] = value;
        Convert[Char.ToUpper(letter)] = value;
     }
  }

I was asked this question for an interview and the only feedback I received was building a dictionary was a very poor choice and they were no longer interested in continuing the application process.  I was surprised and disappointed, but I want to improve for the next time.  What should I do better?

There are couple of things that might have been done better...

Initilize the dictionary form a static constructor or call the method to initialize the field. The method should not access it internally but return a dictionary as a result. The field itself should be readonly.
foreach could be used to iterate the stringToEncode.
Names of internal variables could be better:

Convert sounds like a method name. The dictionary should be named Conversions.
nums should be digits
sb should be called encoded

TryGetValue could have been used instead of ContainsKey
The dictionary is case-insensitive only because you put all letters there. Instead you should use the StringComparer.OrdinalIgnoreCase and make the dictionary <string, char> but this would be inconvenient. You could create your own IEqualityComparer<char>. This will cut in half its size .
I wish more var (optional)
I wish more LINQ

Things that are already good:

I find using a dictionary was a good choice.
The use of a Stack to reverse the order is very clever.
Using StringBuilder for efficiency is definitely a good choice either.

When we apply all suggestions the code could look like this:

Creating the IReadOnlyDictionary which returns a result and uses a couple of additional helper variables. No for loops.

// pre processed conversions for letters
private static readonly IReadOnlyDictionary<char, char> Conversions = BuildConversionDictionary();

private static IReadOnlyDictionary<char, char> BuildConversionDictionary()
{
    var conversions = new Dictionary<char, char>(CaseInsensitiveCharComparer);

    var alphabet = Enumerable.Range('a', 'z' - 'a' + 1).Select(x => (char)x);
    var vowels = new char[] { 'a', 'e', 'i', 'o', 'u' };
    var consonants = alphabet.Except(vowels);

    // consonants are replaced with previous letter b->a, c->b, etc
    foreach (var c in consonants)
    {
        conversions.Add(c, (char)(c - 1));
    }

    // y goes to space
    // space goes to y
    conversions['y'] = ' ';
    conversions[' '] = 'y';

    // vowels are replaced with number: a->1, e->2, etc
    foreach (var (c, i) in vowels.Select((c, i) => (c, i + 1)))
    {
        conversions.Add(c, (char)('0' + i));
    }

    return conversions;
}

The alternative equality comparer (here I'm using a helper-factory from my libraries):

private static IEqualityComparer<char> CaseInsensitiveCharComparer =
    EqualityComparerFactory<char>.Create
    (
        (x, y) => StringComparer.OrdinalIgnoreCase.Equals(x.ToString(), y.ToString()),
        obj => StringComparer.OrdinalIgnoreCase.GetHashCode(obj.ToString())
    );

The refactored encode (which should be Encode). What has changed here is that I first check for digits. If it's not one and we have some then dump the stack to the builder and clear it, othewise do the rest. Below the loop we have to dump it once again in case there are some digits left at the end.

public static string Encode(string stringToEncode)
{
    if (string.IsNullOrEmpty(stringToEncode)) throw new ArgumentException(nameof(stringToEncode));

    // our return val (efficient Appends)
    var encoded = new StringBuilder();

    // used for reversing the numbers
    var digits = new Stack<char>();

    // iterate the input string
    foreach (var c in stringToEncode)
    {
        if (char.IsDigit(c))
        {
            digits.Push(c);
            continue;
        }

        if (digits.Any())
        {
            encoded.Append(digits.ToArray());
            digits.Clear();
        }

        if (Conversions.TryGetValue(c, out var converted))
        {
            encoded.Append(converted);
            continue;
        }

        // something else, undefined
        encoded.Append(c);
    }

    // "dump" what's left
    encoded.Append(digits.ToArray());

    return encoded.ToString();
}

I think you cannot do anything else having only 60 minutes.


Replacing MySQL's AUTO_INCREMENT with Postgres' SERIAL I am making changes to a Python script that converts MySQL scripts to PostgreSQL, and I want to replace strings such as id INTEGER NOT NULL AUTO_INCREMENT with id SERIAL NOT NULL.

This is the code I got to work:

import re

line = 'id INTEGER(11) NOT NULL AUTO_INCREMENT,'

numeric_types = ['(BIG|MEDIUM|SMALL|TINY)*INT(EGER)*(\(.*?\))*',
'DEC(IMAL)*(\(.*?\))*', 'NUMERIC(\(.*\))*', 'FIXED(\(.*\))*',
'FLOAT(\(.*\))*', 'DOUBLE( PRECISION)*(\(.*?\))*', 'REAL(\(.*\))*',
'BIT', 'BOOL(EAN)*']

for i in range(len(numeric_types)):
    type = numeric_types[i]
    if (re.search(type, line)):
        line = re.sub(type, "SERIAL", line).replace(" AUTO_INCREMENT", "")
        print line
        break

Notes:

line will be a column from a CREATE TABLE statement inputted by the user
I could probably join all regular expressions into one using ORs, but I do not know if that would be a good practice or not

The MySQL syntax for a CREATE TABLE command is, in part:

column_definition:
    data_type [NOT NULL | NULL] [DEFAULT default_value]
      [AUTO_INCREMENT] [UNIQUE [KEY] | [PRIMARY] KEY]
      [COMMENT 'string']
      [COLUMN_FORMAT {FIXED|DYNAMIC|DEFAULT}]
      [STORAGE {DISK|MEMORY|DEFAULT}]
      [reference_definition]
  | data_type [GENERATED ALWAYS] AS (expression)
      [VIRTUAL | STORED] [UNIQUE [KEY]] [COMMENT comment]
      [NOT NULL | NULL] [[PRIMARY] KEY]

data_type:
    BIT[(length)]
  | TINYINT[(length)] [UNSIGNED] [ZEROFILL]
  | SMALLINT[(length)] [UNSIGNED] [ZEROFILL]
  | MEDIUMINT[(length)] [UNSIGNED] [ZEROFILL]
  | INT[(length)] [UNSIGNED] [ZEROFILL]
  | INTEGER[(length)] [UNSIGNED] [ZEROFILL]
  | BIGINT[(length)] [UNSIGNED] [ZEROFILL]
  | REAL[(length,decimals)] [UNSIGNED] [ZEROFILL]
  | DOUBLE[(length,decimals)] [UNSIGNED] [ZEROFILL]
  | FLOAT[(length,decimals)] [UNSIGNED] [ZEROFILL]
  | DECIMAL[(length[,decimals])] [UNSIGNED] [ZEROFILL]
  | NUMERIC[(length[,decimals])] [UNSIGNED] [ZEROFILL]
  | DATE
  | TIME[(fsp)]
  | TIMESTAMP[(fsp)]
  | DATETIME[(fsp)]
  | YEAR
  | CHAR[(length)] [BINARY]
      [CHARACTER SET charset_name] [COLLATE collation_name]
  | VARCHAR(length) [BINARY]
      [CHARACTER SET charset_name] [COLLATE collation_name]
  | BINARY[(length)]
  | VARBINARY(length)
  | TINYBLOB
  | BLOB
  | MEDIUMBLOB
  | LONGBLOB
  | TINYTEXT [BINARY]
      [CHARACTER SET charset_name] [COLLATE collation_name]
  | TEXT [BINARY]
      [CHARACTER SET charset_name] [COLLATE collation_name]
  | MEDIUMTEXT [BINARY]
      [CHARACTER SET charset_name] [COLLATE collation_name]
  | LONGTEXT [BINARY]
      [CHARACTER SET charset_name] [COLLATE collation_name]
  | ENUM(value1,value2,value3,...)
      [CHARACTER SET charset_name] [COLLATE collation_name]
  | SET(value1,value2,value3,...)
      [CHARACTER SET charset_name] [COLLATE collation_name]
  | JSON
  | spatial_type

To handle the general case is a non-trivial task!  I highly recommend writing your parsing regexes such that they are recognizably based on the documentation.  (One question I have is why you look for BOOL(EAN)* when MySQL has no boolean type.)

According to the PostgreSQL documentation:

  The data types smallserial, serial and bigserial are not true types, but merely a notational convenience for creating unique identifier columns (similar to the AUTO_INCREMENT property supported by some other databases). In the current implementation, specifying:

CREATE TABLE tablename (
    colname SERIAL
);

  is equivalent to specifying:

CREATE SEQUENCE tablename_colname_seq;
CREATE TABLE tablename (
    colname integer NOT NULL DEFAULT nextval('tablename_colname_seq')
);
ALTER SEQUENCE tablename_colname_seq OWNED BY tablename.colname;

Note that the equivalent MySQL syntax for that would be

CREATE TABLE tablename (
    colname integer NOT NULL AUTO_INCREMENT
);

If I were trying to machine-translate a database schema specification, I would prefer to independently translate the size-and-range aspect of the numeric type and the auto-incrementing nature of the column.  Otherwise, trying to handle the column name, the data type, any NOT NULL constraints, primary and foreign keys, etc. all at once will result in a mess.

Specifically, I would say that MySQL's AUTO_INCREMENT keyword translates directly to PostgreSQL's DEFAULT nextval(…), with a CREATE SEQUENCE … preamble and an ALTER SEQUENCE … OWNED BY … postamble.

Such a translation might not result in pretty and idiomatic PostgreSQL code, but the translation code would be more robust and maintainable.


Euclid's algorithm GCD finder and fraction simplifier This program is a practise exercise in using instance methods, and also in recursion which I struggle to get my head around. I want to know how my technique for the recursion could be improved, and if there's anything else which could be made more efficient.

The program creates a Rational object type (a fraction) which has numerator and denominator variables. It runs a method to find the greatest common divisor using Euclid's algorithm, and then prints the simplified form of the fraction.

Any feedback is helpful, but things I'm interested in:

Is there a more efficient way for me to take the two integers of input (which may both be positive or negative and in any size order) and turn them into two positive integers with the largest first? The if/else section I've used feels excessively big for the simplicity of the task but I couldn't think of another way.
Is there a way that I could combine the getGcd and reduce methods into a single method? I couldn't work out how to make a method where a subsection of it is recursive but the rest isn't.
Is there anything that could be improved about my recursive Euclid's algorithm bit? It's pretty simple but I wonder if I missed any tricks to make it extra efficient.

Code:

public class Rational {

public static void main(String[] args) {

    Rational test = new Rational(-462, 1071);
    System.out.println(test.reduce());

}

private int nume;
private int deno;

public Rational(int nume, int deno) {
    this.nume = nume;
    this.deno = deno;
}

public String toString() {
    return this.nume + "/" + this.deno;
}

public Rational reduce() {
    int gcd = this.getGcd();
    return new Rational(this.nume / gcd, this.deno / gcd);
}

public int getGcd() {

    // Set up variables
    int a;
    int b;
    int gcd;
    if (Math.abs(this.nume) > Math.abs(this.deno)) {
        a = Math.abs(this.nume);
        b = Math.abs(this.deno);
    } else {
        a = Math.abs(this.deno);
        b = Math.abs(this.nume);
    }

    // Euclid's algorithm
    if (a%b == 0) {
        gcd =  b;
        return gcd;
    } else {
        gcd = new Rational(b, a%b).getGcd();
        return gcd;
    }

}

}

First thing: let's make the getGcd static:

public static int gcd(int a, int b){
    if(a % b == 0)
        return b;
    else
        return gcd(b, a % b);
}

This makes things faster if you want to stick with recursion since random instances are not made on every recursive step (otherwise, the while loop tends to win).

In order to handle the signs, I would implement the following (to make later code more readable):

public static boolean isNeg(int a){
    return a < 0;
}

and add a condition to your constructor (this would make this.nume the only possible negative value, beautifying printing and making later code easier):

public Rational(int n, int d){
    if(isNeg(n) == isNeg(d)){
         this.nume = Math.abs(n);
    }else{
         this.nume = -1*Math.abs(n);
    }
    this.deno = Math.abs(d);
}

Then, reduce would be the following:

public Rational reduce(){
    int tNume = Math.abs(this.nume);
    //Math.max and Math.min get maximal and minimal values. This is great to
    //reduce the amount of code.
    int gcd = gcd(Math.max(tNume, this.deno), Math.min(tNume, this.deno));
    return new Rational(this.nume / gcd, this.deno / gcd);
}

I would actually probably always reduce for efficient comparison and ease of use (especially for math).

Also: consider throwing an error if this.deno is ever 0 in the constructor.


Python YouTube Video Downloader I'm looking for some feedback on my first 'real' Python program. Started with Python a few days ago and wanted to get right into it and make something hands on.
Currently the program lets you download audio and video for a YouTube video using Pytube. It allows you specify the location to save the download, whether to download Audio or Video and what resolution to download videos at.

#global variables
yt = None
res = None
stream = None
file_path = None
av_swtich = None

#see if the user wants to download audio or video

def av_select():
    global av_switch
    print('Would you like to download Video or Audio:')
    while True:
        av_switch = input('Please enter \'1\' for Video and \'2\' for Audio\n')
        if av_switch not in ('1', '2'):
            print('Pleae make a valid selection')
            continue
        else:
            break

#Download Audio Stream

def audio_download():
    global yt
    global file_path
    print('The Video has the following audio streams')
    print(*yt.streams.filter(only_audio=True), sep='\n')
    while True:
        itag = input('Please Enter the Itag of the stream you want to download:\n')
        try:
            yt.streams.get_by_itag(str(itag)).download(output_path=str(file_path), filename_prefix='Audio-')
            print('The audio stream has been downloaded')
            return True
        except:
            print('You have entered an invalid Itag')
            continue

#Define Download location
def download_location():
    global file_path
    while True:
        file_path = input('Please enter the Folder to save the Video in:\n')
        x = os.path.exists(path = str(file_path))
        if x == True:
            break
        else:
            print('The specified Folder does not exit')
            continue

#Select the video that the user wants to use
def video_select():
    global yt
    while True:
        video = input('Please enter the video URL:\n')
        try:
            yt = YouTube(video)
            print('The title of the video you selected is: ' + yt.title)
            return False
        except:
            print('The URL is not Valid\n')
            continue

#Select resoultion and display possible stings
def resoultion_select():
    global res
    global res_list
    while True:
        res = input('Please enter a Resoultion:\n')
        try:
            res_list = yt.streams.filter(resolution= str(res))
            return False
        except:
            print('You have entered an invalid resoultion')
            continue

#Select the stream to download based on the itag
def download_stream():
    global file_path
    global yt
    global res_list
    print('The Video has the following streams')
    print(*res_list, sep='\n')
    while True:
        itag = input('Please Enter the Itag of the stream you want to download:\n')
        try:
            yt.streams.get_by_itag(str(itag)).download(str(file_path))
            print('The video has been downloaded')
            return True
        except:
            print('You have entered an invalid Itag')
            continue

def Main():
    global av_switch
    print('Welcome to the Youtube Video Downloader\n')
    download_location()
    video_select()
    av_select()
    if av_switch == '1':
        resoultion_select()
        download_stream()
    else:
        audio_download()

Main()

Global variables

one major suggestion - don't (over)use global variables, it's generally not a good practice and it will make more difficult to test the code. For example if you'd like to add a functionality for downloading multiple items at the same time you'll have a hard time with it.

So instead of this:
def av_select():
    global av_switch # this function is setting value for this variable

def Main():
    global av_switch # this function is reading this variable
    av_select()

do this:
def av_select():
    while True:
        av_switch = input('Please enter \'1\' for Video and \'2\' for Audio\n')
        if av_switch not in ('1', '2'):
            print('Please make a valid selection')
            continue
        return av_switch # add return statement with the value

def Main():
    av_switch = av_select() # use the value returned from function
    if av_switch == '1':

same applies to other global variables.
Python coding guidelines

follow PEP 8 guidelines, in this case the name of the function Main is violating it - function names should be lowercase.

recommended way how to do code comments is to use docstrings. Instead of this:

#Define Download location
def download_location():

do this:
def download_location():
   """define download location"""

this way the comment can be accessed as download_location.__doc__ and a lot of tools (eg. for generating documentation) can work with it.
Code flow

the following one is probably a matter of taste, but it feels like you're overusing while True: loops in places where you can rely on other variables you are "waiting" for. For example av_select() - I'd use the aw_switch variable as a condition of the loop this way:

def av_select():
    av_switch = None
    while av_switch not in ('1', '2'):
        av_switch = input('Please enter \'1\' for Video and \'2\' for Audio\n')

    # outside the while loop, av_switch finally matches '1' or '2'
    return av_switch


Script that counts quarters, dimes, nickels, and pennies I am learning Python and I wrote a script that counts how many coins you would need for an amount in dollars. I was wondering if I could make any improvements to it.

def change():
    amnt = float(input("Enter an amount in USD: "))
    quarters = divmod(amnt, 0.25)
    print("Quarters: ", quarters[0])
    amnt = round(quarters[1], 2)
    dimes = divmod(amnt, 0.10)
    print("Dimes: ", dimes[0])
    amnt = round(dimes[1], 2)
    nickels = divmod(amnt, 0.
    print("Nickels: ", nickels[0])
    amnt = round(nickels[1], 2)
    penny = divmod(amnt, 0.01)
    print("Pennies", penny[0])
change()

Separate input from processing.  If you want to test your method with a number of different values, you'll have to call change() multiple times, and enter in the value each time.  Instead, change the function to accept the amnt, and you can call it many times passing in the amount of cash as an argument:

def change(amnt):
    # ...

Working with tuples from divmod is awkward.  Python has deconstructing assignment, which will take a returned tuple an assign the members to separate variables:

def change(amnt):
    quarters, amnt = divmod(amnt, 0.25)
    print("Quarters: ", quarters)

    dimes, amnt = divmod(round(amnt, 2), 0.10)
    print("Dimes: ", dimes)

For the last operation, you don't use the remainder, so the "throw-away" variable _ can be used for it:

    pennies, _ = divmod(round(amnt, 2), 0.01)
    print("Pennies: ", pennies)

If you import this script into another program, you probably don't want the script to immediately run; rather you just want the change(amnt) function to be defined so this other program can call it.  This is done by adding a "guard" at the end of the script, which only runs the code if the script is invoked directly:

if __name__ == '__main__':
    amnt = float(input("Enter an amount in USD: "))
    change(amnt)

In addition to separating input from processing, you might want to separate the processing from the output:

def change(amnt):
    quarters, amnt = divmod(amnt, 0.25)
    dimes,    amnt = divmod(amnt, 0.10)
    nickels,  amnt = divmod(amnt, 0.05)
    pennies = round(amnt / 0.01, 0)

    return list(map(int, [quarters, dimes, nickels, pennies]))

if __name__ == '__main__':
    amnt = float(input("Enter an amount in USD: "))
    quarters, dimes, nickels, pennies = change(amnt)
    print("{} quarters, {} dimes, {} nickels, {} pennies".format(
          quarters, dimes, nickels, pennies))

Despite attempts to fix rounding errors with things like round(amnt,2), calling change(0.85) returns [3, 0, 1, 5], showing that there wasn't quite enough change to make 2 nickels, but after removing 1 nickel, approximately 5 pennies remained.  This is caused by floating point math.

We can avoid these issues by switching to integer math, based on the number of pennies:

def change(amnt):
    pennies = round(amnt * 100)   # Convert from dollars to pennies

    quarters, pennies = divmod(pennies, 25)
    dimes,    pennies = divmod(pennies, 10)
    nickels,  pennies = divmod(pennies, 5)

    return quarters, dimes, nickels, pennies

if __name__ == '__main__':
    amnt = float(input("Enter an amount in USD: "))
    quarters, dimes, nickels, pennies = change(amnt)
    print("{} quarters, {} dimes, {} nickels, {} pennies".format(
          quarters, dimes, nickels, pennies))

As mentioned in the comments by @Ilmari, round(...) with only a single argument will round the value to the nearest whole number and return an integer value.  When integer values are used with divmod, results are also integers, so switching from dollars-and-cents to integer pennies eliminates the ugly rounding issues.


Reducing duplicated code when using many Netlogo Link Breeds Background

I am using many link breeds in Netlogo. I created various getter/setter and lookup functions for each of these breeds.

Issue

I need to increase my links from 5 different breeds to 44 breeds. And I'm sure copy and pasting these lines of code is not the best approach. If this was a Bash-Shell I would use Indirect Parameter Expansion to reduce the amount of code.

How can I increase the number of link breeds without duplicating so many lines of code?

Code

Declaration

undirected-link-breed [infection-activities infect-activity]
undirected-link-breed [exposure-activities exposure-activity]
undirected-link-breed [watch-activities watch-activity]
undirected-link-breed [mutation-activities mutation-activity]
undirected-link-breed [spillover-activities spillover-activity]
undirected-link-breed [unknown-activities unknown-activity]

Instantiation

infection-activities-own[
  activity-size
  activity-history
]
exposure-activities-own[
  activity-size
  activity-history
]
watch-activities-own[
  activity-size
  activity-history
]
mutation-activities-own[
  activity-size
  activity-history
]
spillover-activities-own[
  activity-size
  activity-history
]
unknown-activities-own[
  activity-size
  activity-history
]

Some Sample Getters Functions

to-report get-activity-from-field-surveillance
  let activity-breed [breed] of one-of links
  report get-activity-breed-common-name activity-breed
end

to-report get-activity-breed-common-name [layer]
  if layer = infection-activities [report "infection" ]
  if layer = exposure-activities [report "exposure" ]
  if layer = watch-activities [report "watch" ]
  if layer = mutation-activities [report "mutation" ]
  if layer = spillover-activities [report "spillover" ]
end

to-report get-activity [layer]
  set num-edges num-edges + 1
  if layer = "infection" [report infection-activities ]
  if layer = "exposure" [report exposure-activities ]
  if layer = "watch" [report watch-activities ]
  if layer = "mutation" [report mutation-activities ]
  if layer = "spillover" [report spillover-activities ]
  report "ACTIVITY not found"
end

to-report get-activity-breed-for-link [layer selected-agent selected-vector]
  let vector-who get-vector-who selected-vector
  let agent-who get-agent-who selected-agent
  set num-edges num-edges + 1
  if layer = "infection" [report infect-activity vector-who agent-who]
  if layer = "exposure" [report exposure-activity vector-who agent-who]
  if layer = "watch" [report watch-activity vector-who agent-who]
  if layer = "mutation" [report mutation-activity vector-who agent-who]
  if layer = "spillover" [report spillover-activity vector-who agent-who]
  report "ACTIVITY not found"
end

to-report get-activity-color [layer]
  if layer = "infection" [report blue ]
  if layer = "exposure" [report 113 ]
  if layer = "watch" [report brown ]
  if layer = "mutation" [report yellow ]
  if layer = "spillover" [report green ]
  report "ACTIVITY not found"
end

As far as breeds go, I don't know of any other way to build up the amount other than by copying and pasting. Of course, you can automate the creation in another language and then copy that into NetLogo, but in NetLogo itself I don't think there's any way.

As far as instantiation goes, you can trim down to the more generic links-own and all breeds will inherit those variables:

links-own [
  activity-size
  activity-history
]

For the different getter functions, I wonder if you could make use of dictionaries with the table extension. For example, using your get-activity [layer] reporter as a guide, build an example dictionary that has the key-value pairs defined in your if statements. Assuming this header info:

extensions [ table ]

globals [
  layer-activity-dict
  layer-color-dict
]

undirected-link-breed [infection-activities infect-activity]
undirected-link-breed [exposure-activities exposure-activity]
undirected-link-breed [watch-activities watch-activity]

Start by building the toy dictionary and creating some turtles and a link:

to setup
  ca
  ; build the activity dictionary
  set layer-activity-dict table:make
  let keys [ "infection" "exposure" "watch" ]
  let vals [ "infection-activities" "exposure-activities" "watch-activities" ]
  ( foreach keys vals [ [ k v ] ->
    table:put layer-activity-dict k v
    ] )

  ; build turtles and some links
  crt 10 [ setxy random-xcor random-ycor ]
  ask n-of 5 turtles [
    create-watch-activity-with one-of other turtles
  ]
  ask n-of 5 turtles [
    create-infect-activity-with one-of other turtles
  ]
  reset-ticks
end

To make use of the dictionary, try something like this reporter, which uses runresult to parse the text from the dictionary entry:

to-report get-activity [layer] ; link-reporter
  report runresult ( table:get layer-activity-dict layer )
end

Testing:

to go
  print get-activity "infection"
  print "Does get-activity return a string?"
  print is-string? get-activity "infection"
  print "Does get-activity return an agentset?"
  print is-agentset? get-activity "infection"

  ask get-activity "infection" [
    set color red
  ]
  ask get-activity "watch" [
    set color blue
  ]
end

Note that the reporter structure will vary depending on what your dictionary actually contains. For example, for color there is no runresult needed:

to setup-2
  ca
  ; build the color dictionary
  set layer-color-dict table:make
  let keys [ "infection" "exposure" "watch" ]
  let vals [ blue 113 brown ]
  ( foreach keys vals [ [ k v ] ->
    table:put layer-color-dict k v
    ] )

  print get-color "infection"
end

to-report get-color [layer] ; link-reporter
  report table:get layer-color-dict layer
end


Append multiple children to a node at once This my attempt at an answer for a question over on SO: Append multiple items in JavaScript

I'd like to know if I've made any mistakes as far as writing idiomatic ES6 JavaScript (mainly regarding convention and performance).

While satisfying, I'm pretty sure adding this behavior directly to Node.prototype is probably the wrong approach. I'm also concerned that maybe it should handle other inputs (like a NodeList?) and not just arrays as a single argument.

Node.prototype.appendChildren = function() {
  let children = [...arguments];

  if (
    children.length == 1 &&
    Object.prototype.toString.call(children[0]) === "[object Array]"
  ) {
    children = children[0];
  }

  const documentFragment = document.createDocumentFragment();
  children.forEach(c => documentFragment.appendChild(c));
  this.appendChild(documentFragment);
};

foo.appendChildren(bar1, bar2, bar3);
foo.appendChildren([bar1, bar2, bar3]);

First the code

Some changes to improve the function.

Node.prototype.appendChildren = function() {
  let children = [...arguments];  // old school

ES6 way

 Node.prototype.appendChildren = function(...children) {

The following is too complicated

  if (
    children.length == 1 &&
    Object.prototype.toString.call(children[0]) === "[object Array]"
  ) {
    children = children[0];
  }

To check for an array

if(Array.isArray(children[0])){...

But you are better of doing that in the loop, then you can handle arrays of arrays

  const documentFragment = document.createDocumentFragment();
  children.forEach(c => documentFragment.appendChild(c));

becomes

  const documentFragment = document.createDocumentFragment();
  children.forEach(child => {
      if (Array.isArray(child)) {
          child.forEach(child => documentFragment.appendChild(child))
      } else {
          documentFragment.appendChild(child);
      }
  });
  this.appendChild(documentFragment);

  // will handle
  foo.appendChildren(bar, [bar1, bar2, bar3]);

The design.

There are some that will argue that adding to a built in prototype is inviting disaster, others will argue its a great way to extend the functionality of the API.

As I see it there are two types of environments.

You share the page with a variety of 3rd party scripts, adverts and what not.
The page is yours alone and no other code, but yours, will ever see that page.

Case 1

For case 1 you should avoid adding to the prototype, as you can not trust it. Someone else may have already added that name to the prototype and you clobber their code, or they may run after you and they clobber yours.

If you must do it, first check

if(typeof Node.prototype.appendChildren === "undefined"){

}else{
     // someone already has added something with that name
}

And if you add it lock it so it can not be over written

Object.defineProperty(Node.prototype, 'appendChildren ', {
     writable : false,    // dont let them over write it
     enumerable : true,   // dont hide it
     configurable : false,   // dont let them change the config so that they
                             // can over write it
     value : function(...children) {... the rest of the function
});

That with the other check ensures you don't step on toes, and your code is safe as well. Though what to do if that function is already taken is an issue that is best solved by not adding to the prototype and just creating a function

function appendChildrenToNode(node, ...children){
  const documentFragment = document.createDocumentFragment();
  children.forEach(child => {
      if (Array.isArray(child)) {
          child.forEach(child => documentFragment.appendChild(child))
      } else {
          documentFragment.appendChild(child);
      }
  });
  node.appendChild(documentFragment);
}

Case 2

For case 2. Do what you want its safe, but be sure you dont forget about it and use it on a public page. In other words do the first case setup to be safe.


The Functional FizzBuzz Kata I run a coding dojo at work. For one session I'm to showing that you can use a kata to get into a new language.

I'm using the FizzBuzz Kata to to show F# (and JavaScript) because it is short.  The problem is I am very new to F# and find myself devolving to objects instead of being functional.

Thus, I would like to see better ways of doing this (using nunit):

namespace FizzBuzzFSharp
    open NUnit.Framework

    module FizzBuzzer =
        let FizzBuzz number =
            match number with
            | i when i % 3 = 0 && i % 5 = 0 -> "FizzBuzz"
            | i when i % 5 = 0 -> "Buzz"
            | i when i % 3 = 0 -> "Fizz"
            | _ -> number.ToString()

        [<Test>]
        [<TestCase(1, "1")>]
        [<TestCase(2, "2")>]
        [<TestCase(3, "Fizz")>]
        [<TestCase(4, "4")>]
        [<TestCase(5, "Buzz")>]
        [<TestCase(6, "Fizz")>]
        [<TestCase(7, "7")>]
        [<TestCase(8, "8")>]
        [<TestCase(9, "Fizz")>]
        [<TestCase(10, "Buzz")>]
        [<TestCase(11, "11")>]
        [<TestCase(12, "Fizz")>]
        [<TestCase(13, "13")>]
        [<TestCase(14, "14")>]
        [<TestCase(15, "FizzBuzz")>]
        [<TestCase(30, "FizzBuzz")>]
        let WhenNumber_ThenResult(number, expected) =
            Assert.AreEqual(expected, FizzBuzz(number));

I think your implementation is fine. I might write it like this, because I find it more idiomatic in F# to avoid guards (when clauses) in pattern matching where a guard free implementation is equally concise.

let fizzbuzz x =
    match (x % 5, x % 3) with
    | (0, 0) -> "FizzBuzz"
    | (0, _) -> "Buzz"
    | (_, 0) -> "Fizz"
    | _ -> string x

[1..100] |> List.map fizzbuzz |> List.iter (printfn "%s")

There are ways you can make this more elaborate / extensible but while that can be an interesting exercise I don't think they're really an improvement over a simple straightforward implementation like this.


Finding the floor value in an array Problem:

  Given an sorted array of N distinct integers, find floor value of input 'key'. Say, A = {1, 2, 3, 5, 6, 8, 9, 10} and key = 7, we should return 6 as outcome.

How can I optimize my solution?

     private static int CalclualteFloorValue(int[] _sortedarray, int l, int r, int key)
        {
            if (_sortedarray[l] == key)
                return -1;

            int m = -1;

            while (l<=r)
            {
                 m = (r+l)/ 2;

                if (_sortedarray[m] == key)
                {
                    return  _sortedarray[m - 1];

                }
                else if (_sortedarray[m] < key)
                {
                    l = m+1;
                }
                else
                {
                    r = m-1;
                }
            }

            return _sortedarray[m];

        }

  int _floorValue = CalclualteFloorValue(_sortedarray, 0, _sortedarray.Length - 1,7);

In terms of speed, your solution is about as optimized as it can get.  In terms of readability, your solution has much room for improvement.  I prefer readability over speed.  You should try to make your code as readable as possible, then, if you have performance issues, search for bottlenecks using a profiler and optimize them.  The method body of CalclualteFloorValue can be made into a one-liner, which happens to be slower but is much more readable.

private static int CalclualteFloorValue(int[] sortedArray, int key)
{
    return sortedArray.Last(x => x < key);
}

If you wish to keep the speed of your original solution and increase readability, there are many improvements you can make:

Have a consistent parameter naming convention.  _sortedarray has an underscore, but the other parameter names don't.  I prefer to only use underscores for variables declared within classes but not within methods.
_sortedarray should be lowerCamelCase.  That is, sortedArray.
If l will always be zero and r will always be array length - 1, declare them in the method body.  Don't make the method caller do unnecessary work.
Use descriptive variable names instead of single letters.

The beginning of the method will now look like this:

private static int CalclualteFloorValue(int[] sortedArray, int key)
{
    int left = 0;
    int right = sortedArray.Length - 1;


Find and rename all png files I have such problem : find all png files to disk and rename them as I did:

static void DirSearch(string sDir)
{
    try
    {
        var images = Directory.GetFiles(sDir, "*.png");
        images.ToList().AsParallel().WithDegreeOfParallelism(100).ForAll(dd =>
       Rename(dd)
   );

    }
    catch { }

    try
    {
        foreach (string path in Directory.GetDirectories(sDir))
                DirSearch(path);
    }
    catch (System.Exception excpt)
    {
        Console.WriteLine(excpt.Message);
    }

}

Actually a question : whether it is optimized enough ? Need as quickly as possible to process all png files.

First of all you should rename your function, it doesn't perform just a search then DirSearch() is not appropriate. What about, for example, RenameAllPngFiles()? More context may suggest a better name, rename how? According to which rules? That should be clear.

Parameter names are usually camelCase and they're not prefixed, sDir should then be searchFolder or even simply path.

You do not validate inputs. An I/O error or an invalid argument because calling logic is broken are completely different and they should be handled differently.

public static void RenameAllPngFiles(string path)
{
    if (path == null)
        throw new ArgumentNullException(nameof(path));

    if (String.IsNullOrEmpty(path))
        throw new ArgumentException("Path cannot be empty.", nameof(path));

    // ...
}

Note that a null argument and and empty argument are different (eventually path may even be empty to use current directory) and as such they should handled separately. First test is to throw ArgumentNullException instead of a generic ArgumentException if path is null. Second test may be rewritten to path.Length == 0 but I find IsNullOrEmpty() much more explicative then I accept the price of the redundant null-checking (at least until I'll need IsNullOrWhiteSpace()).

We're now at the core of your function. First of all drop ToList() because you do not need it.

GetFiles() search all files and then you can start processing. If folder is big or you will change to include sub-directories then application will lag until operation is completed, you should experiment using Path.EnumerateFiles() which iterates file by file while search proceeds.

100 I/O parallel operations might be too much, you will find a balance only with real-world testing but a reasonable value may be around 4 (especially because each operation is very short and threading overhead may be too high). Not to mention that hardly  .NET scheduler will really start so many threads for your loop then probably you will not note any difference.

Do not catch Exception but appropriate exceptions you know may happen: IOException and UnauthorizedAccessException, anything else is a programming error that should be fixed during development. Also your second catch block is useless because function itself will never throw (you catch and swallow everything). Do not discard the whole batch if one operation fails.

var images = Directory.GetFiles(path, "*.png", SearchOption.AllDirectories)
    .AsParallel()
    .WithDegreeOfParallelism(MaxParallelIOOperations)
    .ForAll(TryRenameImage);

That's all, note that you can pass function directly without wrapping inside (another) anonymous delegate. SearchOption.AllDirectories already search inside all sub-directories. If you do not have circular hard-links then it's way faster than your recursive implementation. If you need it to handle fine granted access permissions then code will be much more complex than this and you should definitely catch relevant exceptions (Directory.EnumerateFiles() will easily let you handle this problems case-by-case, see this SO post).

Change your Rename() function to handle I/O errors:

private static void TryRenameImage(string path)
{
    try
    {
        // Your logic
    }
    catch (IOException e)
    {
        // You may retry few times after a small delay,
        // file might be in use elsewhere.
    }
    catch (UnauthorizedAccessException e)
    {
        // Log?
    }
}

See also IOException: The process cannot access the file 'file path' because it is being used by another process.

If you write errors to console then you should use System.Console.Error instead of System.Console. stdout is for program output and stderr is for its errors.


Optimize jquery plugin "on" method extension I would like feedback on how to optimize this jQuery plugin used for triggering change event on any DOM element.
This plugin doesn't use a timer to check for DOM changes but instead extends some jQuery native functions.
I'm mostly concerned by the .on() method extension because I'm sure there is better way of doing this.

jsFiddle to test

;(function ($) {
    var fctsToObserve = {
        append: [$.fn.append, 'self'],
        prepend: [$.fn.prepend, 'self'],
        remove: [$.fn.remove, 'parent'],
        before: [$.fn.before, 'parent'],
        after: [$.fn.after, 'parent']
    }, fctsObserveKeys = '';
    $.each(fctsToObserve, function (key, element) {
        fctsObserveKeys += "hasChanged." + key + " ";
    });
    var oOn = $.fn.on;
    $.fn.on = function () {
        if (arguments[0].indexOf('hasChanged') != -1) arguments[0] += " " + fctsObserveKeys;
        return oOn.apply(this, arguments);
    };
    $.fn.hasChanged = function (types, data, fn) {
        return this.on(fctsObserveKeys, types, null, data, fn);
    };
    $.extend($, {
        observeMethods: function (namespace) {
            var namespace = namespace ? "." + namespace : "";
            var _len = $.fn.length;
            delete $.fn.length;
            $.each(fctsToObserve, function (key) {
                var _pre = this;
                $.fn[key] = function () {
                    var target = _pre[1] === 'self' ? this : this.parent(),
                        ret = _pre[0].apply(this, arguments);
                    target.trigger("hasChanged." + key + namespace, arguments);
                    return ret;
                };
            });
            $.fn.length = _len;
        }
    });
    $.observeMethods()
})(jQuery);

I dont know if there is a better way to do it, but it seems quite smart and minimally intrusive.

From a once over style perspective:

function (key, element) <- since you dont use element, you can remove it from the signature
This

$.each(fctsToObserve, function (key, element) {
  fctsObserveKeys += "hasChanged." + key + " ";
});

would have been better served with Array.reduce, not sure if you want to support browsers that don't have Array.reduce
Naming.. Any code reviewer would be remiss if they did not point that you abbreviate your variables too much which makes the code needlessly harder to understand
In the if in $.fn.on = function () { you should use curly braces or a least a newline.
There is no point in declaring var namespace here:

observeMethods: function (namespace) {
    var namespace = namespace ? "." + namespace : "";

just go for

observeMethods: function (namespace) {
    namespace = namespace ? "." + namespace : "";

Comments, some of the more trickier concepts could have used a line of comment


IsImageUrl extension method written in F# I'm a newbie to F# and so I've written a small extension method library to practice the language.  The extension method attempts to determine if a url string is for an image.  I wrote the code as an extension method so that it can be used as an extension or static method in other .net languages, namely C#.

The code first checks the url's extension to see if a guess can be made about its content.  If no guess can be made, then the image is requested and the content type in the response's header is checked.

Any advice, refactorings, or criticisms welcome.
Thanks!

namespace IsImageUrlDotNet
open System.Net
open System
open System.IO

[<System.Runtime.CompilerServices.Extension>]
module IsImageUrlDotNetLib =
    let ImageFileExtensions = ["png"; "jpg"; "gif"; "raw";"bmp"; "svg"; "jpeg"; "psd";]
    let NonImageFileExtensions = ["exe"; "pdf"; "html"; "htm"; "txt"; "mp3"; "wav"; "odp";]

    let private hasAFileExtensionInList(fileExtensionList:string list) (url:string) =
      if Path.HasExtension url = false then
         false
      else
         let urlSplitList = url.ToLower().Split '.'
         let lastExtension = urlSplitList.[urlSplitList.Length - 1]
         List.contains lastExtension fileExtensionList

    let private hasAnImageFileExtension(url:string) =
      hasAFileExtensionInList ImageFileExtensions url

    let private hasAnNonImageFileExtension(url:string) =
      hasAFileExtensionInList NonImageFileExtensions url

    let private requestUrlAndCheckIfImage(url:string) =
        let req = WebRequest.Create(Uri(url))
        use resp = req.GetResponse()
        let contentType = resp.ContentType
        if contentType.Contains("text/html") then
           false
        else
            contentType.Contains("image")

    [<System.Runtime.CompilerServices.Extension>]
    let IsImageUrl(opt:string) =
      match opt with
        | null | "" -> false
        | url when Uri.IsWellFormedUriString(url, UriKind.RelativeOrAbsolute) = false -> false
        | url when hasAnImageFileExtension url -> true
        | url when hasAnNonImageFileExtension url -> false
        | _ -> requestUrlAndCheckIfImage opt

Sample Test (C#):

[TestMethod]
public void ShouldBeAbleToIdentifyACommonImageUrl()
{
    const string CommonImageUrl = "https://www.google.com/images/branding/googlelogo/1x/googlelogo_color_272x92dp.png";

    var isImage = CommonImageUrl.IsImageUrl();

    Assert.IsTrue(isImage);
}

1.
F# has a good system of type inference. You don't need to specify the type explicitly, the compiler in many cases will do it for you.
You can read more about it here.

2.
As @JoelMueller said in comment, F# there's such an amazing opportunity as partial application.

  The idea of partial application is that if you fix the first N
  parameters of the function, you get a function of the remaining
  parameters

Taken from Partial application

So, you can rewrite the functions hasAnImageFileExtension, hasAnNonImageFileExtension:

let private hasAnImageFileExtension =
  hasAFileExtensionInList ImageFileExtensions

let private hasAnNonImageFileExtension =
  hasAFileExtensionInList NonImageFileExtensions

3.
Several very small comments:

a) In hasAFileExtensionInList instead urlSplitList.[urlSplitList.Length - 1] you can use Seq.last

b) for me, construction like:

if Path.HasExtension url = false then
         false
      else ...

better, rerwite as:

  if Path.HasExtension url then
     ...
  else
     false

4.
In function IsImageUrl it is better to use if-then-else instead pattern matching

5.
Question.

Why not use Path.GetExtension ?

When string not valid will throw the exception. It is better to provide this option.

I don't post a new version of the code, as I plan to edit the answer.


Pick what maintenance check required for game I have a Python script that plays Airline Manager 2 (on Facebook) for me. So far it can depart flights and buy fuel when the price is good. I would like the program to also automatically plan maintenance for aircraft. The following code will only be run when a check is due (one of the four checks will have the value 0.)

When an A-Check is due (hours till A-Check == 0,) the A-Check must be performed. Same goes for other checks. The twist is this: B-Checks also cover A, C covers A & B, and a D-Check covers all other checks. If an A-Check is required, I would like a B-Check to be performed if the B would run out before the A is needed again. There is no point in doing a B-Check if 2 hours later a C-Check would need to be done. In that case I might as well just do a C-Check (which also covers the B-Check.)

Examples

A-Check required; B-Check has 600 hours before another check is required; perform an A-Check.
The above sentence can be written as A,B,C,D=0,600,2000,10000 -> A-Check (C & D-Checks are set to their maximum values so they don't affect the result.)
A-Check required; B-Check has 200 hours left; perform a B-Check (because the B-Check would need to be done in another 200 hours anyway, so the A-Check would be covered now, then in another 200 hours with the B-Check, wasting 300 hours of A-Check.)
The above sentence can be written as A,B,C,D=0,200,2000,10000 -> B-Check (C & D-Checks are set to their maximum values so they don't affect the result.)
A,B,C,D=200,0,1300,9500 -> B-Check, which also covers the A-Check
A,B,C,D=200,0,700,9500 -> C-Check, because the C-Check would need to be done before the next B-Check (if a B-Check was performed.)

Rules

A-Checks should be performed every 500 hours
B-Checks should be performed every 1,000 hours and also covers A-Checks
C-Checks should be performed every 2,000 hours and also covers both A-Checks and B-Checks
D-Checks should be performed every 10,000 hours and covers all other checks as well

The code I have currently (that works):

The program takes input of the current hours left on each check (a, b, c & d.) It prints output of what check should be performed (a, b, c & d.)

# Input (hours till each check)
a = 0
b = 590
c = 400
d = 9900

amax = 500
bmax = 1000
cmax = 2000
dmax = 10000

if a == 0:
    if d - amax < 0:
        print "d"
    elif c - amax < 0:
        print "c"
    elif b - amax < 0:
        print "b"
    else:
        print "a"

elif b == 0:
    if d - bmax < 0:
        print "d"
    elif c - bmax < 0:
        print "c"
    else:
        print "b"

elif c == 0:
    if d - cmax < 0:
        print "d"
    else:
        print "c"

elif d == 0:
    print "d"

There is a lot of repetition in this code. I'm looking for a simpler version that does not compromise on readability.

You can get rid of your duplications by using loops. This comes without any performance hit in this case. Again, the limits have to be defined outside of the function, but this function can deal with a variable number of arguments, just like the numpy solution below.

limits = amax, bmax, cmax, dmax

def get_check_loop(*args):
    # assert len(limits) == len(args)
    for x, limit in zip(args, limits):
        if x == 0:
            i = len(args) - 1
            for t in reversed(args):
                if t - limit < 0:
                    return i
                i -= 1
    return -1

I had to use the slightly ugly manual decreasing of the inner loop variable, because enumerate is not an iterable, at least not as understood by reversed.

Ok, I found a numpy solution for this. It is slower than your approach, but shorter. Specifically, it is about 50 times slower (~10\$\mu\$s instead of ~200ns per call, on my machine).

First, I took your code and put it into a function. I left the max values outside of it. (This makes it slightly slower, because looking up global variables is slower. On the other hand, re-defining variables every call is also costly. In the end the two seem to cancel out.) I also changed the code to return numeric values (because that is easier for the numpy code I wrote). You can always use it as an index into the list ["a", "b", "c", "d", None], where the last entry is for the case that none of the values is zero, which I also included:

amax = 500
bmax = 1000
cmax = 2000
dmax = 10000

def get_check(a, b, c, d):
    if a == 0:
        if d - amax < 0:
            return 3
        elif c - amax < 0:
            return 2
        elif b - amax < 0:
            return 1
        else:
            return 0
    elif b == 0:
        if d - bmax < 0:
            return 3
        elif c - bmax < 0:
            return 2
        else:
            return 1
    elif c == 0:
        if d - cmax < 0:
            return 3
        else:
            return 2
    elif d == 0:
        return 3
    else:
        return -1

For the numpy approach we need to realize that if one of the variables is zero, then we need to subtract that variables max interval from all arguments. For example, if x = a, b, c, d = 351, 0, 304, 1500, then we need to have a look at x - bmax < 0 and choose the index of the last one that is below zero.

In code this is:

import numpy as np

bounds = np.array([500, 1000, 2000, 10000])

def argmax_last(x):
    """
    Returns the last occurrence of the maximum value in x.

    From http://stackoverflow.com/a/8768734/4042267
    """
    return len(x) - np.argmax(x[::-1]) - 1

def get_check_np(*v):
    if any(x == 0 for x in v):
        return argmax_last(v - bounds[np.argmin(v)] < 0)
    return -1

np.argmax (np.argmin) returns the index of the first maximum (minimum) in the passed array. argmax_last returns the last index where the value is the maximal value in that array. Examples:

>>> max([4, 1, 2, 4, 3, 2])
4
>>> np.argmax([4, 1, 2, 4, 3, 2])
0
>>> argmax_last([4, 1, 2, 4, 3, 2])
3

Note that the numpy function is more easily extendable (it only cares that bounds is at least as long as the position of the zero in the input arguments).


Helper method to find VisualChildren of multiple Types I created an helper method to return a List of UIElements where the returned  elements have to match with one of the Types which I pass to the method.

First I used this method to get all the children of one type:

public static IEnumerable<T> FindVisualChildren<T>(DependencyObject dependencyObj) where T : DependencyObject
{
    if (dependencyObj != null)
    {
        for (int i = 0; i < VisualTreeHelper.GetChildrenCount(dependencyObj); i++)
        {
            DependencyObject child = VisualTreeHelper.GetChild(dependencyObj, i);
            if (child != null && child is T)
            {
                yield return (T)child;
            }

            foreach (T childOfChild in FindVisualChildren<T>(child))
            {
                yield return childOfChild;
            }
        }
    }
}

I called this method like:

var textBoxes = UIHelper.FindVisualChildren<TextBox>(currentView);

But I need to find find elements of multiple types, so I rewrote the helper method to accept a List of Types.

public static IEnumerable<UIElement> FindVisualChildrenOfMultipleTypes(List<Type> types, DependencyObject dependencyObj)
{
    if (dependencyObj != null)
    {
        for (int i = 0; i < VisualTreeHelper.GetChildrenCount(dependencyObj); i++)
        {
            DependencyObject child = VisualTreeHelper.GetChild(dependencyObj, i);
            if (child != null && types.Contains(child.GetType()))
            {
                yield return child as UIElement;
            }

            foreach (var childOfChild in FindVisualChildrenOfMultipleTypes(types, child))
            {
                yield return childOfChild;
            }
        }
    }
}

Called like:

List<Type> types= new List<Type>()
{
    typeof(CheckBox),
    typeof(RadioButton),
    typeof(TextBox)
};
var children = UIHelper.FindVisualChildrenOfMultipleTypes(types, currentView);

This works, but I think there a some improvements possible. For example the signature of my method, I think Generics can be used in the method signature, but I'm not very experienced with Generics, so advice on this would be nice. I'm also wondering if the performance of the method can be improved.

SRP

Your current soulution violates the Single Responsibility Principle because it does two things:

it enumerates all children and
it checks if a child is of the specified type

This can and should be splitted into two methods that you can implements as  extensions.

The Children extension will only know how to get all children, nothing more. This means I removed the type checking and I reduced nesting by inverting the first condition.

public static IEnumerable<UIElement> Children(this DependencyObject dependencyObj)
{
    if (dependencyObj == null)
    {
        yield break;
    }

    for (int i = 0; i < VisualTreeHelper.GetChildrenCount(dependencyObj); i++)
    {
        var child = VisualTreeHelper.GetChild(dependencyObj, i);
        if (child != null)
        {
            yield return child as UIElement;
        }

        foreach (var childOfChild in child.Children())
        {
            yield return childOfChild;
        }
    }
}

The second extension OfTypes will be your filter. Here I changed the order of the parameters and decorated the last one with params which will allow you to easier specify the types without having to create an extra list for them.

public static IEnumerable<UIElement> OfTypes(this IEnumerable<UIElement> uiElements, params Type[] types)
{
    return uiElements.Where(x => types.Contains(x.GetType()));
}

Example:

var result = currentView.Children().OfTypes(
    typeof(CheckBox),
    typeof(RadioButton),
    typeof(TextBox)
);

The separation of those two concerns allows you to reuse the Children extension for other purposes should you need it somewhere else.

FindVisualChildrenOfMultipleTypes

By using extensions like the two I've just suggested, you can get rid of the very long method name because you splitted it in multiple parts.


jQuery plugin to count characters I wrote my first jQuery plugin. It counts characters in a similar way to what StackExchange uses for comment entries. You can see it in action with this fiddle.

I feel that it's messy, but I can't explain why. One of the default options is defined outside the defaults hash because it uses a self-reference, and the method for flashing the text seems out of place. I'm also a little worried about how it handles form submission, and its prevention when the minimum number of characters is not met.

I'm not very fluent with jQuery so I expect this to be atrocious. I would love to hear some feedback as to why, or how it could be improved.

The form is submitted via ajax, using Rails' unobtrusive JS.

(function ($) {
    "use strict";

    $.fn.counter = function (options) {
        var defaults = {
            minimumSize: 15,
            minimumWarning: " more to go...",
            maximumSize: 25,
            maximumWarning: " characters remaining...",
            warningSize: 20,
            targetClass: '.help-block'
        };

        defaults.defaultText = "Enter at least " + defaults.minimumSize + " characters.";
        options = $.extend(defaults, options);

        function count(elem) {
            var size = elem.val().length, target = elem.siblings(options.targetClass);
            if (size === 0) {
                target.html(options.defaultText);
            } else if (size < options.minimumSize) {
                target.html((options.minimumSize - size) + options.minimumWarning);
            } else if (size >= options.minimumSize && size < options.warningSize) {
                target.html('&nbsp;');
            } else if (size >= options.warningSize && size < options.maximumSize) {
                target.html((options.maximumSize - size) + options.maximumWarning);
            } else if (size >= options.maximumSize) {
                elem.val(elem.val().substring(0, options.maximumSize));
                target.html("0" + options.maximumWarning);
            }
        }

        this.each(function () {
            var elem = $(this);
            count(elem);
            elem.keyup(function () {
                count(elem);
            });
            elem.closest('form').submit(function () {
                if (elem.val().length < options.minimumSize) {
                    $(this).find(options.targetClass).fadeOut('fast').fadeIn('fast').fadeOut('fast').fadeIn('fast');
                    return false;
                }
            });
            return elem;
        });
    };
})(jQuery);

I suggest storing your defaults in a location that can be reached by the developers, and automatically filling in the minsize if minsize is included in the default text. For example,

(function ($) {
    "use strict";

    $.fn.counter = function (options) {
        options = $.extend($.fn.counter.defaults, options);

        function defaultText() {
            return options.defaultText.replace(/minsize/ig,options.minimumSize);
        }
        function count(elem) {
            var size = elem.val().length, target = elem.siblings(options.targetClass);
            if (size === 0) {
                target.html(defaultText());
            } else if (size < options.minimumSize) {
                target.html((options.minimumSize - size) + options.minimumWarning);
            } else if (size >= options.minimumSize && size < options.warningSize) {
                target.html('&nbsp;');
            } else if (size >= options.warningSize && size < options.maximumSize) {
                target.html((options.maximumSize - size) + options.maximumWarning);
            } else if (size >= options.maximumSize) {
                elem.val(elem.val().substring(0, options.maximumSize));
                target.html("0" + options.maximumWarning);
            }
        }

        this.each(function () {
            var elem = $(this);
            count(elem);
            elem.keyup(function () {
                count(elem);
            });
            elem.closest('form').submit(function () {
                if (elem.val().length < options.minimumSize) {
                    $(this).find(options.targetClass).fadeOut('fast').fadeIn('fast').fadeOut('fast').fadeIn('fast');
                    return false;
                }
            });
            return elem;
        });
    };

    $.fn.counter.defaults = {
        minimumSize: 15,
        minimumWarning: ' more to go...',
        maximumSize: 25,
        maximumWarning: ' characters remaining...',
        warningSize: 20,
        targetClass: '.help-block',
        defaultText: 'Enter at least minsize characters.'
    };
})(jQuery);


Directory watcher and notifier for files added or removed I have written a piece of code that notifies if something is added or removed from a folder location:

from PyQt4 import QtGui
from PyQt4 import QtCore
import sys
import os

def xor(lst1, lst2):
    """ returns a tuple of items of item not in either of lists
    """
    x = lst2 if len(lst2) > len(lst1) else lst1
    y = lst1 if len(lst1) < len(lst2) else lst2
    return tuple(item for item in x if item not in y)

class MyMainWindow(QtGui.QMainWindow):
    def __init__(self, pathToWatch=None, parent=None):
        super(MyMainWindow, self).__init__(parent)
        self.setupUi()
        self._pathToWatch = pathToWatch or "/Users/ppl/Desktop/testFolder"
        self._initialContent = os.listdir(self._pathToWatch)
        self._fileSysWatcher    = QtCore.QFileSystemWatcher()
        self._fileSysWatcher.addPath(self._pathToWatch)
        self.connectSignals()

    def connectSignals(self):
        QtCore.QObject.connect(self._fileSysWatcher, QtCore.SIGNAL("directoryChanged(QString)"),
            self,      QtCore.SLOT("slotDirChanged(QString)"))

    def setupUi(self):
        self.label = QtGui.QLabel("Watching folder")
        self.setCentralWidget(self.label)
        self.setWindowTitle("Detect Dir Change")

    @QtCore.pyqtSlot("QString")
    def slotDirChanged(self, path):
        newContent = ''.join(xor(os.listdir(path), self._initialContent))

        self._initialContent = os.listdir(path)
        msg = ""
        if newContent not in self._initialContent:
            msg = "removed: %s" % newContent
        else:
            msg = "added: %s" %  newContent
        self.label.setText("Detected Directory Change!! \n %s" % msg)

def main():
    app         = QtGui.QApplication(sys.argv)
    window      = MyMainWindow()
    window.show()
    window.raise_()

    return sys.exit(app.exec_())
if __name__ == '__main__':
  main()

I have used QFileSystemWatcher that notifies that the change happened and then I recheck the content to compare with the difference.

This code compares the size of the lists twice:

x = lst2 if len(lst2) > len(lst1) else lst1
y = lst1 if len(lst1) < len(lst2) else lst2

It would be better to do that only once:

longer, shorter = (lst2, lst1) if len(lst2) > len(lst1) else (lst1, lst2)

Notice that I also used better names instead of the meaningless x, y.

With this change the function becomes:

def xor(lst1, lst2):
    """ returns a tuple of items of item not in either of lists
    """
    longer, shorter = (lst2, lst1) if len(lst2) > len(lst1) else (lst1, lst2)
    return tuple(item for item in longer if item not in shorter)

It might also help to turn the shorter list into a set for faster lookups:

    longer, shorter = (lst2, set(lst1)) if len(lst2) > len(lst1) else (lst1, set(lst2))

However, at this point, I realize the method doesn't do what the comment says it does:
it returns the items of the longer list that are not in the shorter list,
which is not the same as items in neither lists (can be fewer than that).

It would be better to move "/Users/ppl/Desktop/testFolder" into a constant at the top of the file, rather than buried inside a class constructor.

It's recommended to follow PEP8, the Python coding style guide.

For example there are several coding style violations here:

def main():
    app         = QtGui.QApplication(sys.argv)
    window      = MyMainWindow()
    window.show()
    window.raise_()

    return sys.exit(app.exec_())
if __name__ == '__main__':
  main()

The recommended writing style would be:

def main():
    app = QtGui.QApplication(sys.argv)
    window = MyMainWindow()
    window.show()
    window.raise_()

    return sys.exit(app.exec_())

if __name__ == '__main__':
    main()


Organize if else block Background
I have a code that basically caches file. The way it works is it will first check if the file exists on the local drive and if it does if there is a flag passed called validate_file_size then it will do another check to see if the size of the file on local drive equals the size of the file on S3. if they equal we assume the local file is the most recent version and if they don't then we need to download the latest version.
CODE
  def cache_file(self, s3_path_of_file: str, local_path: Path, file_name: str, validate_file_size: bool = True) -> None:
        bucket, key = s3.deconstruct_s3_url(f"{s3_path_of_file}/{file_name}")
        is_file_size_valid = True
        if not (local_path / file_name).exists():
            os.makedirs(local_path, exist_ok=True)
            try:
                s3_client().download_file(bucket, key, f"{local_path}/{file_name}")
                os.chmod(local_path / file_name, S_IREAD | S_IRGRP | S_IROTH)
            except botocore.exceptions.ClientError as e:
                if e.response["Error"]["Code"] == "404":
                    raise FileNotFoundError
        elif validate_file_size:
            is_file_size_valid = self._is_file_size_equal(
                s3_path_of_file, local_path, file_name
            )
            if not is_file_size_valid:
                try:
                    s3_client().download_file(bucket, key, f"{local_path}/{file_name}")
                    os.chmod(local_path / file_name, S_IREAD | S_IRGRP | S_IROTH)
                except botocore.exceptions.ClientError as e:
                    if e.response["Error"]["Code"] == "404":
                        raise FileNotFoundError
            else:
                logger.info("Cached File is Valid!")

ISSUE
I am wondering if there is a better way to organize my if else statements so my code is cleaner and less redundant. I am not particularly happy about this block in particular that is repeated twice.
 try:
     s3_client().download_file(bucket, key, f"{local_path}/{file_name}")
     os.chmod(local_path / file_name, S_IREAD | S_IRGRP | S_IROTH)
 except botocore.exceptions.ClientError as e:
     if e.response["Error"]["Code"] == "404":
         raise FileNotFoundError

One obvious choice is to take it out as a separate function but I am almost wondering if i can avoid that and organize my if else statement in a way where i don't really repeat this try except block twice.

I'm not a Python programmer, but the way to not repeat the code without creating a function is to do the download checking first have flags set by that checking one for creating a new file path and one for doing the download. Figure out everything that needs to be done first and then do the download and path creation as necessary.
pseudo code:
if not local path or file:
    create_local_path_flag = True

if file size not valid:
    file_out_of_date_flag = True

if create_local_path_flag or file_out_of_date_flag:
    if create_local_path_flag:
        create_local_path()
    download_file()


Finding the perfect numbers between 1 - 1000 This code was made to find all the perfect numbers between 1 and 1000. A perfect number, is a number where all of it's divisors add up to that number.

For example:

6 is divisible by 3,2, and 1 and3+2+1 = 6. Therefore 6 is a perfect number.

public class Perfect {
    public static void main(String[] args) {
        final int LIMIT = 1000;
        boolean isPerfect = false;
        int i;
        for(i = 2; i < LIMIT; i++) {
            isPerfect = isNumPerfect(i);
            if(isPerfect) {
                System.out.println(i + " is a perfect number");
            }
        }
    }
    public static boolean isNumPerfect(int i) {
        boolean isPerfect = false;
        int sum = 1;
        int x;
        for(x = 2; x <= i / 2; x++) {
            if(i % x == 0) {
                sum += x;
            }
        }
        if(sum == i) {
            isPerfect = true;
        }
        return isPerfect;
    }
}

What are some ways that I can improve on this code? More specifically, ways to shorten it.

If you're trying to shorten it, there are only 3 perfect numbers up to 1000: just list 6, 28, and 496.

If you're trying to speed it up, an array of 1000 numbers is small enough to stay reasonably cache-resident: you can compute all divisor sums together faster than you can compute each of them one at a time.  This is similar to how you can sieve a long interval for primes much faster than you can test each number individually for primality.  Code fragment:

for(int i=1; i<=n/2; i++) {
  for(int x=2*i; x<=n; x+=i) {
    divisor_sum[x] += i;
  }
}

The length of the inner loop is O(n/i), which gets small very quickly, making the total cost O(n log n), whereas your double loop is O(n^2).


Amanda's Relationship Tips app, with scroll performance issues I am having some major issues with scrolling in my active app "Amanda's Relationship Tips"

Under the User Advice tab:

I'm using the Parse Database as a backend to pull user data.  This lag is presenting a huge problem with Overall User Experience and I want to solve it, but I'm coming up short.  I think (90% sure) that it's totally related with pulling the user images from parse.  Here is the following code that is in the app currently:

//-------------------------------------------------------------------------------------------------------
- (PFTableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(NSIndexPath *)indexPath object: (PFObject *)object
//-------------------------------------------------------------------------------------------------------
{
    static NSString *myWallTableIdentifier = @"UserPostsCell";
    WallCell *cell = [tableView dequeueReusableCellWithIdentifier:myWallTableIdentifier];
    if (cell == nil)
    {
        cell = [[WallCell alloc] initWithStyle:UITableViewCellStyleDefault reuseIdentifier:myWallTableIdentifier];
    }

    //////////////////////////CONFIGURE THE CELL//////////////////////////

    PFUser * user = object[@"User"];
    [user fetchIfNeeded];

    ////USER NAME//////////////////////////////////////
    UILabel *cUser = (UILabel*) [cell viewWithTag:105];
    cUser.text = [object objectForKey:@"Post_Author"];

    if (cUser.text == nil) {
        cUser.text = [NSString stringWithFormat:@"Anonymous User"];
    }

    //User's Profile Picture//
    profilePictureAttributes *thumbnailImageView = (profilePictureAttributes*)[cell viewWithTag:106];
    [AFDownload start:user[PF_USER_PICTUREURL] complete:^(NSString *path, NSError *error, BOOL network)
     {
         if (error == nil)
         {
             thumbnailImageView.image = [[UIImage alloc] initWithContentsOfFile:path];
             NSLog(@"No Error with PictureURL");
         }
         else
         {
             thumbnailImageView.file = user[PF_USER_PICTURE];
             [thumbnailImageView loadInBackground];
             NSLog(@"Error with PictureURL trying to pull Parse File");
         }
     }];

    ////DATE LABEL (WHEN CREATED)/////////////////////////
    UILabel *dateLabel = (UILabel*) [cell viewWithTag:102];
    NSDate *parseDate = object.createdAt;
    NSString *timeAgoFormattedDate = [parseDate formattedAsTimeAgo];
    [dateLabel setText:[NSString stringWithFormat:@"%@", timeAgoFormattedDate]];
    NSLog(@"%@", timeAgoFormattedDate);

    ////DESCRIPTION TEXT OF POST/////////////////////////////////
    UITextView *postLabel = (UITextView*) [cell viewWithTag:103];
    postLabel.text = [object objectForKey:@"body"];

    ////COMMENT BUTTON/////////////////////////////////////////////////////
    CommentButton *commentButton = (CommentButton*) [cell viewWithTag:222];
    commentButton.index = indexPath.row;
    [commentButton addTarget:self action:@selector(commentSegueWithButton:) forControlEvents:UIControlEventTouchUpInside];

    ////////////////////////////
    [cell configureCell:object];
    return cell;
}

I have been told before that using tags for my cell's is a terrible idea, and to be honest, that's the only way I know of how to reference the make-up of my cell.

I get the warning in the console that a long-running operation is being executed on the main thread. When I check instruments, the lag is 2999MS on the main thread.  I would greatly appreciate any help!

What is this?

//////////////////////////CONFIGURE THE CELL//////////////////////////

////USER NAME//////////////////////////////////////

//User's Profile Picture//

////DATE LABEL (WHEN CREATED)/////////////////////////

////DESCRIPTION TEXT OF POST/////////////////////////////////

////COMMENT BUTTON/////////////////////////////////////////////////////

////////////////////////////

It's good to break your code into small, digestable chunks, but when you're using comments like this to segment them out, you're doing it wrong.

If you'd just create some small, reusable, testable methods, you wouldn't need comments like this.  The method names themselves will document every thing that's happening here.

 UILabel *cUser = (UILabel*) [cell viewWithTag:105];

 profilePictureAttributes *thumbnailImageView = (profilePictureAttributes*)[cell viewWithTag:106];

 UILabel *dateLabel = (UILabel*) [cell viewWithTag:102];

 UITextView *postLabel = (UITextView*) [cell viewWithTag:103];

 CommentButton *commentButton = (CommentButton*) [cell viewWithTag:222];

This sort of code is extraordinarily prone to error.  We've taken the time to create our WallCell class, so why add the proper methods for setting our cell up correctly?

Seems like your WallCell class should have exposed the following methods:

- (void)setPostAuthor:(NSString *)author;
- (void)setProfilePicture:(UIImage *)profilePicture;
- (void)setPostDate:(NSDate *)date;
- (void)setPostMessage:(NSString *)message;
- (void)addTarget:(id)target forAction:(SEL)action withIndex:(NSInteger)index;

Although... I might argue that it should be even simpler.  You should replace the first four methods with:

- (void)setPost:(WallPost *)post;

Where the WallPost class looks something like this:

@property User *postAuthor;
@property NSDate *postDate;
@property NSString *message;

And the User class looks something like this:

@property NSString *name;
@property UIImage *profilePicture;

(although, arguably, that last one is maybe just the URL to the picture)


Project Euler # 49 Prime permutations in Python The arithmetic sequence, 1487, 4817, 8147, in which each of the terms increases by 3330, is unusual
in two ways: (i) each of the three terms are prime, and, (ii) each of the 4-digit numbers are
permutations of one another.

There are no arithmetic sequences made up of three 1-, 2-, or 3-digit primes, exhibiting this
property, but there is one other 4-digit increasing sequence.

What 12-digit number do you form by concatenating the three terms in this sequence?

from time import time

def sieve(n):
    """generates primes up to n."""
    s = [True] * (n + 1)
    for p in range(2, n):
        if s[p]:
            yield p
            for i in range(p * p, n, p):
                s[i] = False

def is_permutation(n1, n2):
    """returns True if n1 is permutation of n2"""
    to_str_1 = str(n1)
    to_str_2 = str(n2)
    if n1 == n2:
        return False
    to_str_1_digits = {digit: to_str_1.count(digit) for digit in to_str_1}
    to_str_2_digits = {digit: to_str_2.count(digit) for digit in to_str_2}
    if not to_str_1_digits == to_str_2_digits:
        return False
    return True

def get_permutations(n):
    """generates tuples of 3 permutations each within range n."""
    primes = set(sieve(n))
    for prime1 in primes:
        for prime2 in primes:
            if is_permutation(prime1, prime2):
                for prime3 in primes:
                    if is_permutation(prime3, prime1) and is_permutation(prime3, prime2):
                        yield prime1, prime2, prime3

def check_subtraction(n):
    """checks permutations within range n for subtraction rules.
    returns valid permutations."""
    permutations = get_permutations(n)
    for x, y, z in permutations:
        if abs(x - y) == abs(y - z):
            return x, y, z

if __name__ == '__main__':
    start_time = time()
    x, y, z = sorted(check_subtraction(10000))
    print(str(x) + str(y) + str(z))
    print(f'Time: {time() - start_time} seconds.')

Micro optimisations in is_permutation

In is_permutation, you could call the str functions after checking whether n1 == n2.

Also, you could simplify the end of the function with a single return to_str_1_digits == to_str_2_digits.

Taking into account these comments and shortening a bit the variable names, we get:

def is_permutation(n1, n2):
    """returns True if n1 is permutation of n2"""
    if n1 == n2:
        return False
    str1 = str(n1)
    str2 = str(n2)
    digits1 = {digit: str1.count(digit) for digit in str1}
    digits2 = {digit: str2.count(digit) for digit in str2}
    return digits1 == digits2

Improving the algorithm

We generate a set of primes and iterate over it with nested loops. A faster way would be to be able to group primes which correspond to the same permutation.

An efficient way to do it would be to use a dictionnary mapping a hashable value to the list of primes being permutations of each other. That value should be chosen to be some kind of signature which would be the same if and only if values are permutations. For instance, we could use the string representation of prime p sorted alphabetically: ''.join(sorted(str(p))).

Once they are grouped, we can easily get the different permutations:

def get_permutations(n):
    """generates tuples of 3 permutations each within range n."""
    permut_primes = dict()
    for p in sieve(n):
        permut_primes.setdefault(''.join(sorted(str(p))), []).append(p)
    for perms in permut_primes.values():
        for a, b, c in itertools.combinations(perms, 3):
            assert c > b > a
            yield a, b, c

This also needs a minor change in check_substraction to ensure we do not find the already provided result:

def check_subtraction(n):
    """checks permutations within range n for subtraction rules.
    returns valid permutations."""
    permutations = get_permutations(n)
    for x, y, z in permutations:
        if abs(x - y) == abs(y - z) and x != 1487:
            return x, y, z

The corresponding code is more than 500 times faster than the original code.


Longest Consecutive Sequence This task is taken from Leetcode -

  Given an unsorted array of integers, find the length of the longest
  consecutive elements sequence.

  Your algorithm should run in \$O(n)\$ complexity.

  Example:

Input: [100, 4, 200, 1, 3, 2]

Output: 4

/** Explanation: The longest consecutive elements sequence is [1, 2, 3, 4]. Therefore its length is 4. */

My solution

/**
 * @param {number[]} nums
 * @return {number}
 */
var longestConsecutive = function(nums) {
  const set = new Set(nums);
  let longestStreak = 1;
  let max = 0;
  set.forEach(x => {
    if (!set.has(x - 1)) {
      let num = x;
      while(set.has(++num)) { ++longestStreak; }
      max = Math.max(max, longestStreak);
      longestStreak = 1;
    }
  });
  return max;
};

Time complexity is \$O(2n)\$ which is still in the range of \$O(n)\$ time complexity.

num is only used within the while loop and not outside of it.
A for loop is more appropriate for such pattern.

The solution looks as if you forgot to perform a final refactoring to clean things up.
The function starting with let longestStreak = 1; looks like a good idea,
as the exercise is looking for the longest streak, and at this point,
my expectation is that this variable will be probably returned at the end.
But no, to my surprise it is reset to 1 in the forEach,
and at the end the function returns max.

All that to say, it would make more sense to use a local length variable inside the forEach, and update longestStreak when a higher value is found,
and return it at the end.

var longestConsecutive = function(nums) {
  const set = new Set(nums);
  let longestStreak = 0;
  set.forEach(x => {
    if (!set.has(x - 1)) {
      let length = 1;
      for (let num = x + 1; set.has(num); num++) { ++length; }
      longestStreak = Math.max(longestStreak, length);
    }
  });
  return longestStreak;
};


Code for calculating number of trailing zeroes after factorial While trying my hands at the beginner codechef problem I came across this . So basically doing that problem with Dynamic programming is an overkill but why not try that and get some DP practice.

Nice, I almost solved that problem in minutes but the problem is this takes too much memory , and ends up with a Segmentation Fault upon entering a big integer , I solved this problem by running a  ulimit -s unlimited in my local machine but that wont make the codechef judge happy.

Here is the code:

#include <iostream>
#include <vector>
#include <algorithm>

int getZeroes(long long n){
    if(n>0){
        if(n%5 == 0){
            long long a = n/5;
            int b = 1;
            while(a/5){
                if(a%5 == 0){
                    b++;
                }else{
                    break;
                }
                a = a/5;
            }
            return (b+getZeroes(n-1));
        }else{
            return (getZeroes(n-1));
        }
    }
}

int main(){
    int n;
    std::cin >> n;
    std::vector<int> x;
    for(int i=0;i<n;i++){
        long long cases;
        std::cin >> cases;
        x.push_back(getZeroes(cases));
    }

    for(int itr : x){
        std::cout << itr << std::endl;
    }

return 0;
}

Upon some research I found most recommend to declare the items in a heap rather than stack , but that's somewhat confusing to me.  Can anyone help me to find out what can I do to overcome this?

Code Review
All return paths from a function must return  value. Leaving a function that has a non void type without a return is undefined behavior (google nasal dragons).
int getZeroes(long long n){
    if(n>0){
       // STUFF
       return stuff;
    }
    // BUT HERE there is no return.
    // Any negative value or zero causes undefined behavior.
}

This while loop seems overly cumbersome with a break;
        while(a%5){
            if(a%5 == 0){  // will this ever be true.
                // STUFF   // Since it is just after a test that checks the opposite?
            }else{
                break;
            }
            a = a/5;
        }

This looks exactly the same both sides of the else:
        return (b+getZeroes(n-1));
    }else{
        return (getZeroes(n-1));

The only difference is b (which just needs to be made zero for the second case then) you can yank it out of the conditional.
Why store the results?
x.push_back(getZeroes(cases));

The only thing you do with them is print them. May as well just print them as you generate them!


Union Find implementation I am trying to complete this challenge. The user should enter a sequence of instructions, = to link two numbers, and ? to query whether two integers are linked. For example,

? 1 2
= 1 5
= 2 5
? 1 2

should produce

no
yes

I have tried following the algorithm given here to the letter, but I keep getting a judgment of "time limit exceeded". It's very infuriating as the difficultly rating for the problem suggests it should be quite easy. I have tried modifying the code so that redundant links (e.g. = 1 2 where 1 is never queried nor later linked to another number) are ignored, but it still isn't fast enough.

#include <iostream>
#include <vector>

using namespace std;

int root(int a, vector<int> & parent) {
    int b = parent[a];
    return b == a ? a : parent[a] = root(b, parent);
}

void link(int a1, int a2, vector<int> & parent, vector<int> & rk) {
    int root1 = root(a1, parent);
    int root2 = root(a2, parent);
    if (root1 == root2)
        return;
    int rk1 = rk[root1];
    int rk2 = rk[root2];
    if (rk1 < rk2) {
        parent[root1] = root2;
    } else if (rk2 < rk1) {
        parent[root2] = root1;
    } else {
        parent[root1] = root2;
        rk[root2]++;
    }
}

int main() {
    int n, q;
    cin >> n >> q;
    vector<int> parent;
    for (int i = 0; i < n; i++)
        parent.push_back(i);
    vector<int> rk(n, 0);
    for (int i = 0; i < q; i++) {
        string str;
        int a, b;
        cin >> str >> a >> b;
        if (str == "?")
            cout << (a == b || root(a, parent) == root(b, parent) ? "yes" : "no") << endl;
        else
            link(a, b, parent, rk);
    }
    return 0;
}

Consider an iterative path compression design for root. The recursive version uses more memory for stack frames. Typical iterative versions follow the path one at a time. The path can be followed two at a time since the end loops back to itself. The root function could be refactored to be something along the lines of:

int root(int a, vector<int> &parent) {
    while (parent[a] != a)
      a = parent[a] = parent[parent[a]];
    return a;
}

Plus, consider adding a second vector to keep track of the sizes of each group. This will allow for more intelligent selecting of representatives during link. Something along the lines of the following:

void link(int i, int j, vector<int> &parent, vector<int> &sizes) {
  i = root(i, parent);
  j = root(j, parent);
  if (i == j) return;
  if (sizes[i] < sizes[j]) {
    parent[i] = parent[j];
    sizes[j] += sizes[i];
  } else {
    parent[j] = parent[i];
    sizes[i] += sizes[j];
  }
}

See this post for more information.

Consider allocating the vector using the constructor, resize, or reserve. Then initial values can be set using iota (ref).  For example,

vector<int> parent(n);
iota(begin(parent), end(parent), 0);

Consider using printf and scanf over cin and cout. Also, if you are going to use cin and cout, consider including ios_base::sync_with_stdio(false); and cin.tie(NULL);. See this question for more information.


Handler for default values in a CLI flag utility library in C I am developing a very basic CLI flag utility library in C. The following code is a part of the main translation unit containing all the relevant functions, and is located in the global scope.

struct pair {
    char name[100];
    char defaultValue[100];
};

static struct pair *defaults = NULL;

void setDefault(const char *flagName, const char *defaultValue) {
    if (defaults == NULL) {
        defaults = malloc(10 * sizeof(struct pair));
    }

    static short pos = 0;
    if(pos % 10 == 0) {
        defaults = realloc(defaults, (pos + 10) * sizeof(struct pair));
    }

    strncpy((defaults + pos)->name, flagName, 100);
    strncpy((defaults + pos)->defaultValue, defaultValue, 100);

    pos++;
}

static void clearDefaults(void) {
    free(defaults);
}

To clue you in, the idea is that the user calls setDefault with 2 strings, the first of which represents a flag name, and the second which represents the default value of that (should it not have been passed to the program). The pair is stored in memory and is later used by other library functions.

My question is centered around the storage of flagName:defaultValue pairs, which are represented using the pair struct. Right now I'm using a global static variable defaults which stores a pointer to a dynamically allocated array of said structs. It's static, therefore I had to initially set it to NULL, and "initialize" it only after the first call of setDefaults. I feel that this creates unnecessary clutter as other library functions have to test defaults for NULL before trying to search it for a specific pair.

That is my first concern, is there a better way to initialize a global static variable?

The second is my implementation of dynamic reallocation in setDefault. It initially allocates space for 10 pairs, and once the limit is reached it allocated space for 10 more and so on. I've tested the implementation with Valgrind and a bunch of calls to setDefault and there were no memory leaks or other errors.

Is this a good way to do it? Can anything be improved? I'm very much a beginner at C so any feedback is welcome.

Yes, there are things to improve:

if (defaults == NULL) {
    defaults = malloc(10 * sizeof(struct pair));
}

static short pos = 0;
if(pos % 10 == 0) {
    defaults = realloc(defaults, (pos + 10) * sizeof(struct pair));
}

That allocates space for 10 elements, and then re-allocates space for ten elements. What a waste of time.

Additionally, avoid sizeof(TYPE). sizeof expr is safer, and will be obviously correct.

Next, allocating memory is known to fail sometimes. So, check for that.

And finally, rework setDefault() and clearDefaults() so the latter actually only clears the defaults.

bool setDefault(const char *flagName, const char *defaultValue) {
    static short pos = 0;
    if (!defaults) pos = 0;
    if (pos % 10 == 0) {
        struct pair* temp = realloc(defaults, (pos + 10) * sizeof *temp);
        if (!temp) return false;
        defaults = temp;
    }
    ...
}

static void clearDefaults(void) {
    free(defaults);
    defaults = NULL;
}

Are you sure you want to use strncpy()? You know that's not really a string-function, but a zero-padding function?

Another interesting question is how the other functions should know how many elements are in the list. I have no idea, design something.

Also, consider being more dynamic, and just saving two pointers to independently-allocated strings in the pair-element.

And if having defaults starts as a null-pointer-value really disturbs you, just initialize it to oint to a statically-allocated null-object.