Applying dynamic filters to a SQL query using Python conditional expressions I wrote this piece of code to eliminate the need for multiple if then statements based on the filters from the incoming REST request.

If the request has category in it, then the query has to be filtered by category, the same for product_location. I wrote a single statement instead of having to check for the filters and have multiple queries.

If the product_category is equal to All Category, then a filter need not be applied, etc.

query = \
    'select AVG(stock_date - order_date) as c,product_category from data_table_data where stock_date between :s1 and :s2 ' \
    + (('and product_category = :category ' if category
       != 'All Category' else '')) + (('and product_location = :loc '
         if store_location != 'All Stores' else '')) \
    + 'group by product_category'
data = engine.execute(text(query), s1=startDate, s2=endDate,
                      category=category, loc=store_location).fetchall()

The code in the post seems fine. I think it would be easier to read if you kept the lines to 79 columns or less, put each conditional in its own statement, and made the field names match the query parameter names, like this:

query = """
    SELECT AVG(stock_date - order_date) AS c, product_category
    FROM data_table_data
    WHERE stock_date BETWEEN :stock_date_start AND :stock_date_end
"""
if category != 'All Category':
    query += "AND product_category = :product_category "
if store_location != 'All Stores':
    query += "AND product_location = :product_location "
query += "GROUP BY product_category"
params = dict(stock_date_start=startDate,
              stock_date_end=endDate,
              product_category=category,
              product_location=store_location)
data = engine.execute(text(query), **params).fetchall()

Alternatively, you could push the conditional down into the SQL, and then the query would be constant:

QUERY = """
    SELECT AVG(stock_date - order_date) AS c, product_category
    FROM data_table_data
    WHERE stock_date BETWEEN :s1 AND :s2
    AND product_category IN ('All Category', :product_category)
    AND product_location IN ('All Stores', :product_location)
    GROUP BY product_category
"""


C program with 10 options I have a C program where the user can select from 10 different options. I've already split the core functionality of each option into different methods, but my main() function is still quite long. The rest of the code in main() involves getting user input and formatting strings to the display so that the program is clearer and looks nice.
E.g. my code is similar to:
int actionOneFunction(char *name) {
    /* Action one logic */
}

int actionTwoFunction(FILE *fptr) {
    /* Action two logic */
}

void getUserInput(char *display_msg, char *input_variable, int variable_size) {
    printf("%s", display_msg);
    fgets(input_variable, variable_size, stdin);
    strtok(input_variable, "\n"); /* Remove newline added by fgets */
}

void displayStartingText() {
    /*
        A bunch of printf statements
    */
}

int main() {
    int result
    int action;

    displayStartingText();

    printf("Select the action you would like to carry out: ");
    scanf("%d", &action);
    getChar(); /* Remove newline generated by scanf from buffer */

    if (action < 0 || action > 10) {
        printf("Invalid action selected.\n");
        return 1;
    }

    if (action == 1) {
        char name[255];

        printf("------------\n");
        printf(" Action One \n");
        printf("------------\n");

        getUserInput("Enter the name: ", name, sizeof(name));
        result = actionOneFunction(name);
        if (result != 0) {
            return errno;
        }
        printf("Action one was successful!\n");
    }
    else if (action == 2) {
        FILE *fptr;
        char name[255];
        char *content;

        printf("------------\n");
        printf(" Action Two \n");
        printf("------------\n");

        getUserInput("Enter the file name: ", name, sizeof(name));

        fptr = openFile(name, "r");
        content = actionTwoFunction(fptr);
        fclose(fptr);

        printf("Content: \n", content);
    }
    else if (action == 3) {
        /* ... */
    }

    /*
        ...
    */

    else if (action == 10) {
        /* ... */
    }

    return 0;

}

With all ten options and their functionality defined, my main() becomes rather long. Would it be better to put each of the if blocks into their own functions?

I can't be very detailed here because I haven't worked in C lately. But basically, yes, you should move more of the code out of the if statements into the breakout functions.
Specifically, each break-out function should have the same signature (type/arity). Based on the examples you've given so far, these appear to be quite open-ended effectful subroutines, each comparable to a mini main, so they might as well have signature ()->int. Or maybe there'll be enough structure to their behavior that you can be more specific than that, IDK.
If you're going to print out the fancy headers, abstract that away into a helper function.
Anyway, you might consider even getting rid of the if itself, in favor of a lookup-table of functions. Exactly how to implement that will depend a bit on the context; it's not something I've ever done in C.


Leetcode - First Unique Character in a String Given a string, find the first non-repeating character in it and
  return it's index. If it doesn't exist, return -1.

Examples:

s = "leetcode"
return 0.

s = "loveleetcode",
return 2.

  Note: You may assume the string contain only lowercase letters.

My solution

public class FirstUniqueCharacter {

    public int firstUniqChar(String s) {
        LinkedHashMap<Character, Integer> map = new LinkedHashMap<>();

        for (char c : s.toCharArray()) {
            if (Objects.isNull(map.get(c))) {
                map.put(c, 1);
            } else {
                int count = (Integer) map.get(c);
                map.put(c, ++count);
            }
        }

        for (Map.Entry e : map.entrySet()) {
            int value = (int) e.getValue();
            Character c = (Character) e.getKey();
            if (value == 1) {
                return s.indexOf(c);
            }
        }

        return -1;
    }
}

The statistics shows it’s still slower than 60% of the solutions submitted in terms of speed and space. How can I improve it?  Any comments regarding code readibility are most welcome.

You don’t need Objects.isNull(...).  Simply using != null would suffice, and avoids an extra function call so should be faster (unless the compiler can optimize the call out).

Your loop at the end is using raw types.  You should use

for(Map.Entry<Character, Integer> e : map.entrySey()) {

instead, for type safety.  Your castings then are unnecessary.

There is no need to fetch the character c = e.getKey() unless value == 1 is true; you can more that inside the if for a minor performance gain.

Counting the character occurrences is slightly dangerous: you could overflow an Integer, or even a Long with a long enough string.  Simply flagging the character as “seen once” or “more than once” avoids the counting overflow bug.

The map.put(c, 1) call returns the previous value stored in the map, or null if no value was stored.  Instead of fetching the value, testing whether it was present or not, and then storing another value, why not store & fetch in one operation?  Then, if a value was already present, you can flag it as occurring twice (or more).

if( map.put(c, 1) != null )
    map.put(c, 2);

Since you are no longer counting, you don’t need a LinkedHashMap<Character, Integer>.  You just have 3 states: not seen, seen once, and seen more than once.  A Boolean can cover this.  Boolean.TRUE is seen once (unique), Boolean.FALSE is seen more than once, and not present (null) is never seen.

LinkedHashMap<Character, Boolean> unique = new LinkedHashMap<>();

for(char c: ...) {
    if (unique.put(c, Boolean.TRUE) != null)
        unique.put(c, Boolean.FALSE);
}

We’ve saved a tiny bit of space, since we only have two Boolean objects, instead of several (possibly interned) Integer objects.  Much more importantly, we’ve avoided auto boxing, so this should be much faster.

We are still wasting time storing both Boolean.TRUE and Boolean.FALSE successively on the third and subsequent occurrences of any character.

If we always store Boolean.FALSE, then on the first occurrence null will be returned.  We can detect that and overwrite it with Boolean.TRUE instead, so the exceptional first occurrence has 2 map put operations, but subsequent occurrences only use 1 map put operation, for better performance.

    if (unique.put(c, Boolean.FALSE) == null)
        unique.put(c, Boolean.TRUE);

To truly gain speed and reduce memory usage, avoid clunky HashSet<> memory structures, and store the data yourself.  You are told you can assume only lowercase letters are used, so you can use a new byte[26] array for “not seen”, “seen once”, and “seen multiple times” storage.  And use a new char[26] array to maintain encounter order of “first seen” characters.

You can use a bit more memory and store the index of the first seen characters in a new int[26], so you can avoid the linear s.indexOf(c) search at the end.  You could even use 0 for not seen, index+1 for seen once, and -1 for seen multiple times, and avoid the new byte[26] flag storage.


Detecting which properties have been changed from two objects I have the following code. Providing two objects cmd and this.xxx.properties, script detects the only property changed and mutate the input object (cmd).

Script works fine but I would like to know if I could optimize or improve my code for speed.

    _deltaProperties: function (cmd) {

        var dataBefore = this.xxx.properties,
            newData = {};
        for (var prop in cmd.properties) {
            var newValue = cmd.properties[prop],
                oldValue = dataBefore[prop];
            if (newValue !== oldValue) {
                newData[prop] = cmd.properties[prop];
            }
        }
        cmd.properties = newData;
        return cmd;
    },

I did a bit of testing on jsperf which seems to indicate that it would be marginally faster for you to delete the properties you don't need rather than create a new object and replace it. Obviously, your mileage may vary. My test was objects with 6 properties, 3 of which were different.

EDIT : Delete is faster on IE. The new object version is faster on Chrome and FF.

_deltaProperties: function (cmd) {
    var prop;
    for (prop in cmd.properties) {
        if (cmd.properties[prop] === this.xxx.properties[prop]) {
            delete cmd.properties[prop];
        }
    }
    return cmd;
},

If you decided to keep your current code, there are some improvements you could make in terms of styling:

// I personally dislike underscores in variable names
deltaProperties: function (cmd) {
    // JavaScript only has function scope, a lot of developers define all
    // variables at the top of the function - can help with subtle scope bugs
    var dataBefore = this.xxx.properties,
        newData = {},
        prop,
        newValue,
        oldValue;

    for (prop in cmd.properties) {
        newValue = cmd.properties[prop],
        oldValue = dataBefore[prop];
        if (newValue !== oldValue) {
            // Use newValue rather than accessing member again
            newData[prop] = newValue;
        }
    }
    cmd.properties = newData;
    return cmd;
},


Optimize triplet summation I need to compute the summation of:

$$\left\lfloor\frac{\text{sum of triplets}}{\text{product of triplets}}\right\rfloor$$

from integers of an array.

Input:

5

1 2 1 7 3

Output:

6

Explanation:

  The following triplets exists in the given array:

  1 2 1

  1 2 7

  1 2 3

  1 1 7

  1 1 3

  1 7 3

  2 1 7

  2 1 3

  2 7 3

  1 7 3

  Considering these triplets from the sample input:

  1 2 1 contributes 2 [floor{(1+2+1)/(1*2*1)} = floor{4/2}=2]

  1 2 3 contributes 1

  1 1 7 contributes 1

  1 1 3 contributes 1

  2 1 3 contributes 1

  All other triplets contribute 0 to the sum.

  Hence the answer is (2+1+1+1+1)=6.

What I tried has complexity \$O(n^3)\$:

#include <iostream>
#include <cmath>
using namespace std;

int main()
{
    long t,n[300005],sum=0,mul=1,i,j,k,res=0;
    cin >> t;

    for(i=0;i<t;i++)
        cin >>n[i];

    for(i=0;i<t-2;i++)
    for(j=i+1;j<t-1;j++)
    for(k=j+1;k<t;k++)
    {
        sum = n[i]+n[j]+n[k];
        mul = n[i]*n[j]*n[k];
        res += floor(sum/mul);
    }

    cout << res << endl;
    return 0;
}

Is there any hint of better optimization?

Can be done in \$O(n)\$ time

You can do better than \$O(n^3)\$ if you think about what kinds of triplets give nonzero contributions.  First of all, any triplet that does not contain a 1 will contribute zero to the final answer.  Next, if you examine all the triplets that contain 1, there are 5 types of triplets that contribute nonzero amounts to the answer:

1 1 1 contributes 3
1 1 2 contributes 2
1 1 X contributes 1, where X > 2
1 2 2 contributes 1
1 2 3 contributes 1

So you can get an \$O(n)\$ solution by counting all the 1s 2s and 3s in the input.  Then you can compute the number of each of the 5 types of triplets above by using simple combinatorics.

Sample \$O(n)\$ solution

Here is a sample solution based on the above.  Notice I removed the entire input array because all that is needed is the counts:

#include <iostream>

int main()
{
    long n   = 0;
    long c1  = 0;
    long c2  = 0;
    long c3  = 0;
    long res = 0;

    std::cin >> n;
    for(long i=0;i<n;i++) {
        long num;
        std::cin >> num;
        if (num == 1)
            c1++;
        else if (num == 2)
            c2++;
        else if (num == 3)
            c3++;
    }

    // Count all the {1, 1, 1} triplets.  There are (c1 choose 3),
    // each contributing 3:
    res += ((c1 * (c1-1) * (c1-2)) / 6) * 3;

    // Count all the {1, 1, 2} triplets.  There are (c1 choose 2) * c2
    // combinations, each contributing 2:
    res += ((c1 * (c1-1)) / 2) * c2 * 2;

    // Count all the {1, 1, X} triplets, where X > 2.  There are
    // (c1 choose 2) * (n - c1 - c2) combinations, each contributing 1:
    res += ((c1 * (c1-1)) / 2) * (n - c1 - c2);

    // Count all the {1, 2, 2} triplets.  There are c1 * (c2 choose 2)
    // combinations, each contributing 1:
    res += c1 * ((c2 * (c2-1)) / 2);

    // Count all the {1, 2, 3} triplets.  There are c1 * c2 * c3
    // combinations, each contributing 1:
    res += c1 * c2 * c3;

    std::cout << res << std::endl;
    return 0;
}

Of course, this program can be optimized even further by combining some of the expressions, but I wrote it in the way that most clearly demonstrates where each part of the answer came from.


Recursive max int in array I would like a review for this simple recursive algorithm which returns the largest int in an array of integers. I would like feedback on the transfer of state via the third parameter specifically. Please also note 2 things:

It must be recursive
This is not for production usage, but rather recursion practice

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

int max_val(int arr[], int length, int curr_max)
{
    if(length <= 0)
        return curr_max;

    if(curr_max < *arr)
    {
        curr_max = *arr;
    }
    arr++;
    return max_val(arr,length-1,curr_max);
}

int main()
{
    int arr[] = {2,73,39,5,8,62,9};
    printf("Max: %d\n", max_val(arr,7,0));
    return EXIT_SUCCESS;
}

Never omit "optional" braces, as you did for your first if statement.  That's a bad habit that will eventually lead to a coding accident.  If you feel compelled to omit braces, then at least put the whole statement on the same line.

You should avoid unnecessary mutation, as you did with arr++.  This is  especially important with an exercise in recursion: it is one of the principles of functional-programming.

The curr_max parameter is annoying and unnecessary.  It also leads to a bug with negative array members, when a poorly selected initial value is chosen.  (You should use INT_MIN from <limits.h> instead of 0.)  Furthermore, your base case is questionable: is the maximum of a zero-length array well defined?

Suggested solution

#include <assert.h>

int max_val(int arr[], size_t length)
{
    assert(length > 0);
    if (length == 1) return arr[0];
    int max_tail = max_val(arr + 1, length - 1);
    return arr[0] > max_tail ? arr[0] : max_tail;
}


Python bot to answer mathematical questions for a remote server The MSP Tech Club puts on an annual CTF, available here. I've worked on a solution but was wondering if there's a better way to write it or if I have any particular bad habits showing themselves?

The challenge is to write a script to connect to a server and answer math problems presented to it, it looks like this:

Answering 500 questions in a small enough timeframe reveals this:

I've left the netcat server in the code to allow for testing of changes. Since the CTF is public facing I don't think this poses an issue but feel free to remove if I'm breaking any S/O rules.

#!/usr/bin/python3

import socket
import re

if __name__ == '__main__':
    client = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
    client.connect(('195.154.53.62', 1337))

    while True:
        data = b''
        while True:
            chunk = client.recv(4096)
            data += chunk
            if len(chunk) < 4096:
                break

        # our flag contains ALEXCTF, once it's revealed print recevied data and exit
        if 'ALEXCTF' in data.decode('utf-8'):
            print(data.decode('utf-8'))
            break

        # \d+ matches a digit (equal to [0-9])
        # .{3} matches any  character, except line terminators exactly three times
        m = re.search('\d+.{3}\d+', data.decode('utf-8'))
        expression = m.group(0)

        #properly handle division
        if '/' in expression:
            expression = expression.replace('/', '//')

        result = eval(expression)

        #print results to screen to see script progress
        print(expression + ' = ' + str(result))

        #encode and transfer
        data = str(result).encode('utf-8') + b'\n'
        client.send(data)

My main concern is the use of eval() without proper validation (well, the regular expression extraction puts some limits onto the possible expression values, but still it can be exploitable). I would either switch to tools like numexpr (there are some other options at this topic), or, taking into account the simplicity of the possible expressions, map the operator strings into actual operators, something like:

import operator

OPERATIONS = {
    '+': operator.add,
    '-': operator.sub,
    '*': operator.mul,
    '/': operator.floordiv  # // operator
}
match = re.search(r'(\d+)\s*([-+*/])\s*(\d+)', data.decode('utf-8'))
if not match:
    raise ValueError("Invalid expression string")

operand1, operation, operand2 = match.groups()

if operation not in OPERATIONS:
    raise ValueError("Invalid operation '%s'" % operation)

result = OPERATIONS[operation](float(operand1), float(operand2))

I'm not sure about converting to floats, may be you need Decimals.

Some other notes:

it is recommended to use raw strings for regular expression strings
you can pre-compile the regular expression before going into the loop (even though the Python's regex engine is smart enough to cache the compiled patterns implicitly, see this topic for more information)
error handling - check if the "match" is not None before calling .group() on it (see how I've done it above)
fix the typo - "recevied" vs "received"
comments style - according to PEP8, there has to be a single whitespace after the # character


Printing multiple triangles I am new to Python and only just started learning about functions. I stumbled upon a question which recommends using function to print triangles in such a format:

Enter height: 5

         /\
        /  \
       /    \
      /      \
     /________\
    /\        /\
   /  \      /  \
  /    \    /    \
 /      \  /      \
/________\/________\

Enter height: 2

   /\
  /__\
 /\  /\
/__\/__\

Although I have gone through and successfully coded for the requirements. I feel though there needs to be a better approach. I was wondering if someone could review my code and point me in the right direction as to how I might improve on it.

This is what I currently have:

import sys

def triangle(height):
    print()
    max = (height * 2) - 1
    mid = 0
    while max > height:
        statement = " " * max + "/" + " " * mid + "\\"
        print(statement)
        max -= 1
        mid += 2
    statement = " " * max + "/" + "_" * mid + "\\"
    max -= 1
    print(statement)
    small = 0
    while max > 0:
        statement = " " * max + "/" + " " * small + "\\" + " " * mid + "/" + " " * small + "\\"
        print(statement)
        mid -= 2
        max -= 1
        small += 2
    statement = " " * max + "/" + "_" * small + "\\" + " " * mid + "/" + "_" * small + "\\"
    print(statement)
    pass

if __name__ == "__main__":
    userInput = input("Enter height: ")
    try:
        userInput = int(userInput)
    except ValueError:
        sys.exit("\nInvalid height.")
    if userInput < 2 or userInput > 20:
        sys.exit("\nInvalid height.")
    triangle(userInput)

I am curious as to there is a way to only code for one triangle and print multiple.

Your code seems to be working fine, but I have some suggestions:

Make sure that no line is longer than 80 characters
Instead of exiting when input is <2 or >20, instead prompt for a new input (your code also works for input 1, but not for 0)
Instead of the function handling the printing make it return the statement string
Avoid having variables with the names of builtins, e.g. max
The last pass doesn't add any functionality

To further improve your solution, I would do one of two things:

Continue with the same logic, but reduce the cluttering

I would definitely change from using while loops to using for loops. With a bit of math, the function can be reduced to:

def create_triangle(n):
    triangle = ""
    for i in range(n):
        triangle += ' '*(2*n-i-1) + '/' + ' _'[i==n-1]*2*i + '\\' + "\n"
    for i in range(n, 2*n):
        triangle += ' '*(2*n-i-1) + '/' + ' _'[i==2*n-1]*(2*i-2*n) + '\\'
        triangle += ' '*(4*n-2*i-2) + '/' + ' _'[i==2*n-1]*(2*i-2*n) + '\\'
        triangle += '\n'
    return triangle

Most of it is similar to what you have in your code, the only new thing being ' _'[i==n-1]*2*i. This statement could be hard to interpret, but the only thing it does is to print _ when i==n-1, and a space otherwise.

Change to a more general approach for these kinds of problems

If you want to solve more complex problems related to creating patterns with strings, I'd instead use an array based solution. A function like this:

def create_print_array(n):
    arr = [['*' for i in range(n)] for j in range(n)]
    return '\n'.join([''.join(row) for row in arr])

returns a pattern like this:

*****
*****
*****
*****
*****

for input n = 5. Since we have a 2d matrix, we can directly manipulate the values to form our desired output. With a bit of math again, the function can be written as:

def create_triangle_array(n):
    # create a 2D array filled with spaces
    arr = [[' ' for i in range(4*n)] for j in range(2*n)]
    # create the outer triangle legs
    for i in range(2*n):
        arr[i][2*n-i-1] = '/'
        arr[i][2*n+i] = '\\'
    # create the inner triangle legs
    for i in range(n, 2*n):
        arr[i][i] = '\\'
        arr[i][4*n-i-1] = '/'
    # create the bases for all three triangles
    for i in range(2*n-2):
        arr[n-1][n+1+i] = '_'
        arr[2*n-1][2*n+1+i] = '_'
        arr[2*n-1][1+i] = '_'
    # join the array into a string
    return '\n'.join([''.join(row) for row in arr])

Both of these approaches give the same form on the output, but it should be noted that the second approach gives some extra spaces on the end of each line, to make each line equally long. Both approaches also work for input >= 0 (I left out error-handling for input < 0 since you already covered that in your code).

As a final note, I'd say that your code looks very good for a beginner. It is easy to follow, and you have covered potential wrong input from the user.


Python and computing checksums I have this module which computes checksums from a list of files in a given directory.
The problem is that my is_changed_file is long and ugly, but my attempts in refactoring failed, so I would like some other point of views...

import hashlib
import logging

# if available use the faster cPickle module
try:
    import cPickle as pickle
except ImportError:
    import pickle

from os import path

class DirectoryChecksum(object):
    """Manage the checksums of the given files in a directory
    """

    def __init__(self, directory, to_hash):
        self.directory = directory
        self.to_hash = to_hash
        self.checksum_file = path.join(self.directory, '.checksums')
        self.checks = self._compute()
        self.logger = logging.getLogger("checksum(%s): " % self.directory)

    def _abs_path(self, filename):
        return path.join(self.directory, filename)

    def _get_checksum(self, filename):
        content = open(self._abs_path(filename)).read()
        return hashlib.md5(content).hexdigest()

    def _compute(self):
        """Compute all the checksums for the files to hash
        """
        dic = {}
        for f in self.to_hash:
            if self._file_exists(f):
                dic[f] = self._get_checksum(f)
        return dic

    def _file_exists(self, filename):
        return path.isfile(self._abs_path(filename))

    def is_changed(self):
        """True if any of the files to hash has been changed
        """
        return any(self.is_file_changed(x) for x in self.to_hash)

    #FIXME: refactor this mess, there is also a bug which impacts on
    #the airbus. eggs, so probably something to do with the path
    def is_file_changed(self, filename):
        """Return true if the given file was changed
        """
        if not self._has_checksum():
            self.logger.debug("no checksum is available yet, creating it")
            self.write_checksums()
            return True

        stored_checks = self.load_checksums()

        if not self._file_exists(filename):
            if filename in stored_checks:
                self.logger.debug("file %s has been removed" % filename)
                # what if it existed before and now it doesn't??
                return True
            else:
                return False

        checksum = self._get_checksum(filename)
        if filename in stored_checks:
            # if the file is already there but the values are changed
            # then we also need to rewrite the stored checksums
            if stored_checks[filename] != checksum:
                self.write_checksums()
                return True
            else:
                return False
        else:
            # this means that there is a new file to hash, just do it again
            self.write_checksums()
            return True

    def _has_checksum(self):
        return path.isfile(self.checksum_file)

    def load_checksums(self):
        """Load the checksum file, returning the dictionary stored
        """
        return pickle.load(open(self.checksum_file))

    def write_checksums(self):
        """Write to output (potentially overwriting) the computed checksum dictionary
        """
        self.logger.debug("writing the checksums to %s" % self.checksum_file)
        return pickle.dump(self.checks, open(self.checksum_file, 'w'))

Probably there is no need to refactor the method as it represents quite clear decision tree. Breaking it down into smaller methods will cause readability problems.

Just make sure you have unit tests covering each outcome and that there are no cases, which the code doesn't handle.

If you definitely want to refactor, I propose to add a method like this:

def _check_log_writesums(cond, logmessage, rewrite=True):
    if cond:
        if logmessage:
            self.logger.debug(logmessage)
        if rewrite:
            self.write_checksums()
        return True
    return False

Then you can call it like this:

    if filename in stored_checks:
        return self._check_log_writesums(stored_checks[filename] != checksum,
                                         "",
                                         rewrite=True)

Maybe the name can be better (_update_checksums or _do_write_checksums).


Script for automating regression testing I am writing a simple script to automate my regression testing. I am new to bash scripting and my goal is achieve the most efficient and clean code. I have written a simple function to allow me to select which subject I would like to test. This will loop until I select the exit option. Again, this is quite simple; however I am looking for some feedback on how I can improve.

BIO=biology

showMenu () {
    echo "What subject would like to regression test?"
    echo "1) Physics"
    echo "2) Sociology"
    echo "3) Biology"
    echo "4) Quit"
}

while [ 0 ]
do
    showMenu
    read CHOICE
    case "$CHOICE" in

    "1")
        read SUBJECT
        SUBJECT=physics
        echo "LOG: REGRESSION TESTING $SUBJECT" $SUBJECT
        GENERATE_CSS=`lessc /home/developer/oer.exports/css/ccap-$BIO.less > /home/developer/oer.exports/css/ccap-$BIO.css`
        echo "LOG: CSS GENERATION COMPLETE"
        GENERATE_PDF=`python collectiondbk2pdf.py -v -s ccap-$BIO -d ./test-ccap/col$SUBJECT $SUBJECT.pdf -t tempdir/$SUBJECT`
        echo "LOG: GENERATING $SUBJECT PDF WITH $BIO CSS STYLE"
        ;;
    "2")
            read SUBJECT
        SUBJECT=sociology
        echo "LOG: REGRESSION TESTING SOCIOLOGY" $SUBJECT
        GENERATE_CSS=`lessc /home/developer/oer.exports/css/ccap-$BIO.less > /home/developer/oer.exports/css/ccap-$BIO.css`
        echo "LOG: CSS GENERATION COMPLETE"
        GENERATE_PDF=`python collectiondbk2pdf.py -v -s ccap-$BIO -d ./test-ccap/col$SUBJECT $SUBJECT.pdf -t tempdir/$SUBJECT`
        echo "LOG: GENERATING $SUBJECT PDF WITH $BIO CSS STYLE"
        ;;
    "3")
        echo "Biology"
        DIFF=`diffpdf -a --debug=2 /tmp/pdf/diff bio.pdf bio_slicer.pdf`
        echo "LOG: DIFF DEV vs STAGING NOW AVAILABLE FOR PDF VIEW. "
        ;;
    "4")
        echo "Exiting now..."
        exit
        ;;

    esac
done

while : is a standard idiom for "loop forever" in bash. The colon (:) is a synonym for true.
Prefer $(command) to `command` ("backticks"). The former allows you to nest; the latter does not.
Though in your case, you don't need the backticks at all. These are for capturing the output of the command, which you don't use anywhere.
It isn't clear why you have the read SUBJECT inside each case -- you throw away the value in the very next statement.
Don't do this: lessc ccap-$BIO.less > ccap-$BIO.css. (It's a classic mistake in shell scripting; everyone's been bit by it.) This will nuke the file -- it will be truncated by the shell, overwriting the contents before lessc has the chance to read it.

Aside from your code: if your goal is to regenerate the output files whenever one of the input files has changed, you should consider learning make. With a 10-20 line makefile, you can automatically regenerate whichever of the output files need updating whenever one of the input files change.


Implementing min of 3 numbers in Mic-1 code I have the following algorithm to find a minimum of three input numbers:

min(a,b,c):
   x := a
   if b < x then x := b
   if c < x then x := c
   return x
end min(a,b,c)

I'm trying to implement a Mic-1 micro code following the algorithms above:

OP1, OP2, OP3 = any 16 bit 2s complement value

OPRES: 0
      .LOC 50
main: LODD OP1: push
      LODD OP2: push
      LODD OP3: push
      CALL min:
      INSP 2
      STOD OPRES
      HALT
min:  LODL 1; OP1
      SUBL 2; OP2 - OP1
      JPOS op1small
      SUBL 3; OP3 - OP1
      JPOS op1small
op1small:
      LODL 1
      RETN ; OP1

I'm pretty new to Mic-1 and would like to get any input on this Mic-1 code above. Is there a better or shorter way I can find a min of three numbers in Mic-1?

Min... ish

  Is there a better or shorter way I can find a min of three numbers in Mic-1?

Maybe, but first you should be finding the min of three numbers because right now, that's not what is happening.

This is what your current assembly code looks like translated to a pseudo-code:

ac = OP1
ac -= OP2
if ac > 0, return OP1
ac -= 3
if ac > 0, return OP1
return OP1

Try following this through with some example numbers. Still don't understand where you are going wrong? Look at this example again with some mo

; OP1 = 3, OP2 = 2, OP3 = 1
ac = 3
ac -= 2 ; ac is now 1
; ac is > 0, so jump passes and OP1 is returned when OP3 should be instead

See it now? This was probably overlooked due to the fact that you did not thoroughly test your code. It is important to test even the most extreme cases to make sure that your code is cleaned.

Now we need to re-write your code. First off, let's break down your code: instead of trying to find the min of just three numbers, why not try to find the min of two numbers? Then, this subroutine could be used in other min-based subroutines.

I think that the min subroutine would now look like this:

min2:
    LODL 1 ; ac = OP1
    SUBL 2 ; ac -= OP2
    JPOS min2_op2small ; if ac > 0

    LODL 1
    RETN
op2small:
    LODL 2
    RETN

If you are having trouble understanding how this works, try plugging in some values in your head. Here is a run down of what it is doing

Store OP1 in the accumulator.
Subtract OP2 from the accumulator.
If the accumulator is negative, then return OP1.
If the accumulator is positive, then return OP2.

Now that we have this basic min subroutine, we can the subroutine for finding the min of 3 numbers.

Using our handy-dandy min2 sub, this new subroutine can simply use the min2 sub to determine what is the greater of the three values:

Find the min2 of OP1 and OP2
Compare the lesser one with OP3
Return the lesser one.

Your current code already follows this structure. However, as show above, it doesn't quite do what it seems.

Comments

You seem slightly confused about how this "architecture" works, judging by your comments. In particular, these ones:

  SUBL 2; OP2 - OP1
  ...
  SUBL 3; OP3 - OP1

No; you aren't subtracting 1st parameter from the 2nd and 3rd ones here. In reality, these two statements are the equivalent of

  \$(OP1-OP2)-OP3\$

Why is it like this and not how you have indicated it? Here is what your code is doing, minus the conditionals:

Load the accumulator with OP1
Subtract OP2 from the accumulator.
Subtract OP3 from the accumulator.

The accumulator is a register that is used to hold a single value that is often the result of arithmetic. More simple instruction sets (such as this one) force any arithmetic to be done with one of the operands being the accumulator.

See what the difference is? Your comments seemed to act as if there were no conditional whatsoever. More accurate comments would be:

SUBL 2; ac = OP1 - OP2
...
SUBL 3; ac = (OP1 - OP2) - OP3


Lorentz Attractor visualization using functional programming I have been working on this Lorenz Attractor visualization for the past day. I am currently also trying to change my coding style into a more functional programming one.

I find it quite hard, to be honest, especially the "Only use pure functions." rule. That mostly means no side effects and functions that perform 1 small task. The "No side effect." rule is, of course, impossible while drawing on the HTML canvas. So I am looking for performance optimization and code style tips for FP in general.

I also should like to pass the 3 global variables (x, y, z) as parameters when calling the update function ("No global variables allowed.") however I have had no luck so far. I can't find a way to keep them from resetting while using parameters. Unlike the draw function that calls itself. Any help is welcome, thanks for your time.

https://jsfiddle.net/9r8qcgnh/3/

{
  const canvas = document.querySelector('#canvas')
  const ctx = canvas.getContext('2d')
  ctx.fillStyle = 'rgb(255, 255, 255)'
  ctx.translate(500, 400)
  ctx.scale(10.5, 10.5)

  let x = 0.01
  let y = 0.0
  let z = 0.0

  function update () {
    const a = 10.0
    const b = 28.0
    const c = 8.0 / 3.0
    const dt = 0.015
    const dx = a * (y - x) * dt
    const dy = (x * (b - z) - y) * dt
    const dz = (x * y - c * z) * dt
    x = x + dx
    y = y + dy
    z = z + dz
    return { x: x, y: y, z: z }
  }

  function draw (hue) {
    const { x, y } = update()
    hue = hue || 0
    hue >= 240 ? (hue = 0) : hue++
    ctx.fillStyle = `hsl(${hue},100%, 50%)`
    ctx.fillRect(x, y, 0.1, 0.1)
    setTimeout(() => {
      return draw(hue)
    }, 5)
  }

  draw()
}

I don't know anything about Lorentz Attractors or functional programming, but I have a few things I want to mention.

getElementById is faster than querySelector

const canvas = document.getElementById('canvas')

You can use shorthand property names in your draw function.

return { x, y, z };

You can give parameters a default value

function draw (hue = 0) {
  //...
}

and remove this line

hue = hue || 0

I don't know if it's deliberate because you have something against purple, but the hue goes to 360.

Use semicolon at the end of your lines. Technically, JavaScript can run without them, but it just... looks wrong. If anything, do it to make it a habit, since it's gonna be a problem if you code something in another language later.

It seems more appropriate to use setInterval instead of setTimeout here. You will have to handle the hue differently in that case. Making it global is an option, but not good if you are trying to avoid those. Another option could be a generator.

function draw (hue = 0) {
  const { x, y } = update();
  ctx.fillStyle = `hsl(${hue},100%, 50%)`;
  ctx.fillRect(x, y, 0.1, 0.1);
}

const hueGenerator = (function*() {
  let hue = 0;
  while(true) {
    hue %= 360;
    yield hue++;
  }
})();

setInterval(() => {
  draw(hueGenerator.next().value);
}, 5)


Hangman Game - Python Anything I can improve?

def main():
    word = 'nedim'
    hidden_word = list('_'*len(word))
    wrong_guess, amount_of_letters, guesses, done = 0, 0, 4, False

    while(wrong_guess != 5 and not done):
        guess = raw_input("Guess a letter: ")
        if len(guess) == 1 and guess.isalpha():
            while(guess in hidden_word):
                if guess in hidden_word:
                    print "Letter already exists!"
                    guess = raw_input('Try again: ')
            for letter in word:
                if letter == guess:
                    hidden_word[word.index(guess)] = guess
                    print ''.join(hidden_word)
                    if ''.join(hidden_word) == word:
                        print "\n ######## \n You won! \n ######## \n"
                        done = True
            if guess not in word:
                wrong_guess += 1
                if(guesses-wrong_guess < 0):
                    print "You have no chances left. Loser!"
                else:
                    print "You have ",guesses-wrong_guess," chances left."
        else:
            print "INVALID GUESS! \nRULES: \n\t > One guess at a time \n\t > Only english alphabet \n\t > Only Letters"

if __name__ == '__main__':
    main()

I see a number of things that may help you improve your code.

Eliminate unused variables

Unused variables are a sign of poor code quality, so eliminating them should be a priority.  In this code, amount_of_letters is set to 0 but never actually used.

Use better function names

The name main might be find if this were a C program, but there is no requirement or advantage to using the name main in a Python program.  Instead, I'd name it something like hangman.

Fix bug #1

At some point, if one is unsuccessful in guessing the word, the program will display the message:

You have  0  chances left.
Guess a letter:

If I have 0 chances left, why am I being asked to guess again?

Fix bug #2

If the word contains multiple copies of the same letter, such as "maxim", if the user guesses a, x and m in that order, they'll see this:

Guess a letter: m
max__
max__

Use a list comprehension

You might not have studied Python long enough to learn about them, but I'd use a list comprehension here.  Instead of the for letter in word loop, I'd write this:

hidden_word = [guess if word[i]==guess else ch for i, ch in enumerate(hidden_word)]

This looks complex, perhaps, but it's actually pretty easy to understand once you've mastered list comprehensions.

Set the case for all letters

At the moment, if the user guesses 'M' but the word is 'nadim', the program will count that as a bad guess.  In the traditional hangman game, no distinction is made between capital and lowercase letters.  You might consider handling that for your program, such as by forcing all letters to be lowercase for example.

Use variables consistently

The guesses variable looks like it's supposed to be the maximum number of guesses allowed, but then we also see a hard-coded 5 in the program at the top of the loop.  Choose one (I'd prefer a named constant such as max_guesses) and stick with it.


Sorting a list of IComparables I have a DateTime class (my own, not System.DateTime) which implements the IComparable interface.

Here is a method that takes a List<DateTime> and sorts it:

private List<DateTime> SortDateTimes(List<DateTime> dateTimes)
{
    for (int i = 0; i < dateTimes.Count; i++)
    {
        for (int j = 0; j < dateTimes.Count; j++)
        {
            if (dateTimes[i] < dateTimes[j])
            {
                var temp = dateTimes[i];
                dateTimes[i] = dateTimes[j];
                dateTimes[j] = temp;
            }
        }
    }

    return dateTimes;
}

I believe using foreach loops instead of for is suitable here, but having the following variable names drives me crazy. :D

foreach(var dateTime1 in dateTimes)
{
    foreach(var dateTime2 in datetimes)
    ...

What are your thoughts?
Thanks!

Prologue

  I have a DateTime class (my own, not System.DateTime)

You could have just pasted the code for that class. Since you are asking a question about sorting, and your DateTime class implements IComparable, its content could have been pivotal for a proper review.

Parodos

  Here is a method that takes a List<DateTime> and sorts it:

You have just told us your class implements IComparable, but you don't show us how you'd use its functionality. I would have expected to to see dateTimes[i].CompareTo(dateTimes[j]) but instead we get dateTimes[i] < dateTimes[j]. This means you have created an operator overload on < and are not sharing this crucial code with us.

Furthermore, why would you roll out your own sorting algorithm if built-in functionality is available? As suggested in the comments by VisualMelon, you could just call dateTimes.Sort();

Exodus

  I believe using foreach loops instead of for is suitable here, but having the following variable names drives me crazy

To get things even more confusing, you ask about a syntactic change to the algorithm rather than going about how to properly sort or how to correctly implement IComparable.

Finishing off, in the for (dateTime1 in dateTimes) approach, you cannot change dateTimes[i] = dateTimes[j]; as it will throw System.InvalidOperationException: Collection was modified; enumeration operation may not execute. rendering your suggested approach unusable.

Cliffhanger

As a challenge, I let you search how to prevent this exception.


Optimize python script for speed I am working on the following script for python2.7 which "works" on small files.

I ran a sample on an input file of 188kB and it took approx. 1.15 min to complete. However, I need to process a 5GB file using this script and I did the math, it will take 11.48 years to finish it the way it is now.

sample input1

aba_transit_number  com
abaca   plt|sub|sub|sub
abacus  art|art
abalone anm
abamp   qud

sample input2

zoonosis-n  of+n-j+n-the-development-n
zoonosis-n  of+n-j+n-the-j-collection-n 1
zoonosis-n  of+n-j+n-the-j-success-n    1

Can someone provide me insight on how to optimize my script for computation speed??

    #!/usr/bin/python
    # -*- coding: utf-8 -*-

    from __future__ import division
    from collections import defaultdict, Counter
    import codecs
    import random

    mapping = dict()

#### takes as input a file with the following input1:

    with codecs.open ("input1", "rb", "utf-8") as oSenseFile:
        for line in oSenseFile:
            concept, conceptClass = line.split()
            mapping[concept + '-n'] = conceptClass

    lemmas = set()

#### takes as input2 a file with the following format

    with codecs.open('input2', "rb", "utf-8") as oIndexFile:
        for line in oIndexFile:
            lemma = line.split()[0]
            if lemma in mapping.keys():
                lemmas.add(lemma)

### randomly splits input2 into 2 files -- 80% and 20%
# -- and prints the 20% directly  into out 2 for the other 80%
# --- it matches each 1st column in input2 with the first column in input 1
# -- if it is a match - it replaces it with the corresponding value in Col2 of  Input1
# --- if there is more than one volume in Col2 of Input 1
# -- it prints all of the possible combinations and divides the freq (Col4 in Input2)
# by the number of values present

        training_lemmas = random.sample(lemmas, int(len(lemmas) * 0.8))

    classFreqs = defaultdict(lambda: Counter())

    with codecs.open('out1', 'wb', 'utf-8') as testOutfile:
        with codecs.open('input2', "rb", "utf-8") as oIndexFile:
            for line in oIndexFile:
                lemmaTAR, slot, filler, freq = line.split()
                if lemmaTAR in training_lemmas:
                    senses = mapping[lemmaTAR].split(u'|')
                    for sense in senses:
                        classFreqs[sense][tuple([slot, filler])] += int(freq) / len(senses)
                elif lemmaTAR in lemmas:
                    testOutfile.write(line)

    with codecs.open('out2', 'wb', 'utf-8') as oOutFile:
        for sense in sorted(classFreqs.keys()):
            for slotfill in classFreqs[sense].keys():
                 string_slotfill = '\t'.join(list(slotfill))
                 outstring = '\t'.join([sense, string_slotfill, str(classFreqs[sense][slotfill])])
                 oOutFile.write(outstring + '\n')

Remove all usages of the keys method. Note, this was already mentioned in the comments to your question, but it seems to have mostly done the trick for your problem.


PHP function to evaluate comparisons between two values, with debug logging The following method works fine but it is rather verbose:

private function performComparison($element, $operator, $value_allowed, $value_entered)
{
    switch($operator):
        case '>':
            $result = ($value_entered >  $value_allowed) ? true : false;
            break;
        case '>=':
            $result = ($value_entered >= $value_allowed) ? true : false;
            break;
        case '<':
            $result = ($value_entered <  $value_allowed) ? true : false;
            break;
        case '<=':
            $result = ($value_entered <= $value_allowed) ? true : false;
            break;
        case '=':
            $result = ($value_entered == $value_allowed) ? true : false;
            break;
    endswitch;
        $dateformat = 'n/j/Y';
        $debug = ['element' => $element, 'operator' => $operator, 'value allowed' => is_a($value_allowed, 'DateTime') ? $value_allowed->format($dateformat) : $value_allowed, 'value entered' => is_a($value_entered, 'DateTime') ? $value_entered->format($dateformat) : $value_entered, 'result' => ($result === false) ? 'false' : 'true'];
        echo '
'.print_r($debug,1).'
';
    return $result;
}

Note that the 3 lines of code just before the return $result; line are very helpful for debugging in the Development phase so I'd like to keep those.

In an attempt to make a more concise version of this method I came up with the following:

private function performComparison($element, $operator, $value_allowed, $value_entered)
{
    $comparisonFunctions = [
        '>' => function($value_allowed, $value_entered) { return ($value_entered >  $value_allowed) ? true : false; },
        '>='    => function($value_allowed, $value_entered) { return ($value_entered >= $value_allowed) ? true : false; },
        '<' => function($value_allowed, $value_entered) { return ($value_entered <  $value_allowed) ? true : false; },
        '<='    => function($value_allowed, $value_entered) { return ($value_entered <= $value_allowed) ? true : false; },
        '=' => function($value_allowed, $value_entered) { return ($value_entered == $value_allowed) ? true : false; }
    ];

    $result = array_key_exists($operator, $comparisonFunctions) ? $comparisonFunctions[$operator]($value_allowed, $value_entered) : null;
        $dateformat = 'n/j/Y';
        $debug = ['element' => $element, 'operator' => $operator, 'value allowed' => is_a($value_allowed, 'DateTime') ? $value_allowed->format($dateformat) : $value_allowed, 'value entered' => is_a($value_entered, 'DateTime') ? $value_entered->format($dateformat) : $value_entered, 'result' => ($result === false) ? 'false' : 'true'];
        echo '
'.print_r($debug,1).'
';
    return $result;
}

This version also works fine but I'm wondering if there are any ways it could be condensed further.

given your function's purpose is to return the calculation result, I would return the expression result right away, especially given return makes break unnecessary as well

private function performComparison($operator, $value_allowed, $value_entered)
{
    switch($operator) {
        case '>':
            return ($value_entered >  $value_allowed);
        case '>=':
            return ($value_entered >= $value_allowed);
        case '<':
            return ($value_entered <  $value_allowed);
        case '<=':
            return ($value_entered <= $value_allowed);
        case '=':
            return ($value_entered == $value_allowed);
        default:
            throw new InvalidArgumentException("Invalid operator");
    }
}

The code above is both concise and readable, it takes one no problem to gist the function's meaning and purpose.

Whereas any debugging should be done elsewhere. If you need any debugging for this function, create another one encapsulating the current function's call.

    private function debugPerformComparison($element, $operator, $value_allowed, $value_entered)
    {
        $result = $this->performComparison($operator, $value_allowed, $value_entered);
        $dateformat = 'n/j/Y';
        $debug = [
            'element' => $element,
            'operator' => $operator,
            'value allowed' => is_a($value_allowed, 'DateTime') ? $value_allowed->format($dateformat) : $value_allowed,
            'value entered' => is_a($value_entered, 'DateTime') ? $value_entered->format($dateformat) : $value_entered,
            'result' => $result,
        ];
        echo '<pre>'.var_export($debug,1).'</pre>';
        return $result;
    }

Regarding anonymous function-based version, a concise version shouldn't be less readable in the first place. Any code improvements should improve the readability, by means of removing unnecessary parts, not worsen it. And the said version is extremely hard to read.


Optimize this Scheme-written chess engine module I'd like to know how to optimize this by shadowing symbols 'lo and 'hi inside nested function f. I guess CPS conversion would solve this but how?

;; chess engine - early development

;; Alist: (cons 'piece xy-coordinates)
(define whites) (define blacks)

(begin
  (define setup
    (let ([lo `(p0 p1 p2 p3 p4 p5 p6 p7)]
            [hi `(r0 n0 b0 q k b1 n1 r1)])
      (define f
        (let ([xs (iota 8)])
          (lambda (ls y)
            (map (lambda (i x) (cons i (cons x y))) ls xs))))
      (lambda (l h) (concatenate (f lo l) (f hi h)))))
  (set! whites (setup 1 0)) (set! blacks (setup 6 7)))

Your question and code are both confusing to me, but I'll give it a shot.

(define whites) doesn't compile with my standard definition of define.  define usually needs to bind the identifier to something, even if it is just '() or #f.  Since it seems like they (blacks and whites) are meant to hold a-lists, I've initialized them to '().

I don't know what dialect of scheme you're running, but you may be able to drop the outer begin form, and your concatenate function is usually called append in most schemes.  Finally, you may have a list* function so to replace (cons i (cons x y)) with (list* i x y)

Next, in order to call setup, it wasn't immediately clear that setup was a function that took two arguments, the first being an integer intended to be the row to place the pieces onto, and the second being the row to set the pawns.  I would make setup into a thunk since the initial placement of the pieces is pretty intrinsic to chess unless you're planning to support rectangular boards or something.  Also, being a thunk emphasizes that it's being called for its side effect, and not it's value.  You could really bring home that point by calling the function setup! instead.

I would give more descriptive names to your identifiers.  I would create a function that put a list of pieces together with their coordinates, called make-row, and a function that setup both the pieces and pawns given the rows to place them on, called setup-color

With regard to your concern about optimizing: readability and extensibility are usually more important than a few instructions.  There are exceptions, and those exceptions will kill performance if you don't optimize when they occur.  There are no rules about where performance bottlenecks lie, except for they are usually not where you think they are ;-).  That's why it is important to MEASURE your performance as you go.  The bottlenecks of most chess engines are usually NOT in the method used to setup the board.  They are usually in the search methods, so when you get there, time spent optimizing will yield a much better ROI.

Also, when you go to optimize, before starting down the path of flipping your program inside -out to CPS it, consider easier wins first, like using a vector instead of a list as your primary data structure.

As for shadowing symbols, it seems you may have a misunderstanding.  Shadowing doesn't buy you anything in performance.  It's not the identifier that takes up space, it's whatever that identifier is bound to.  When you shadow a variable, the old value still has to be stored for when it is unshadowed later.

In fact, shadowing variables can actually be harmful to performance in some cases.  Since you can shadow primitive operations like +, the compiler is unable to safely inline those operations.  If you can tell the compiler that you promise not to shadow +, it can inline addition operations.  If your implementation has a decent module system, wrapping you code in modules may help the compiler make these deductions, otherwise there may be flags to encourage inlining.

Here is my suggested refactoring.

;; Alist: (cons 'piece xy-coordinates)
(define whites '())
(define blacks '())

;;Set!s whites and blacks to the initial chessboard setup
(define (setup!)
  (define (make-row pieces row)
    (map (lambda (piece x) (list* piece x row))
     pieces
     (iota 8)))

  (define (setup-color pawn-row piece-row)
    (let ((pawns '(p0 p1 p2 p3 p4 p5 p6 p7))
          (pieces '(r0 n0 b0 q k b1 n1 r1)))
      (append
       (make-row pawns pawn-row)
       (make-row pieces piece-row))))

  (set! whites (setup-color 1 0))
  (set! blacks (setup-color 6 7)))


Appropriate insertion sorting algorithm I am learning data structures, and have written an algorithm for insertion sort after learning from some sources. But I am confused which implementation is more correct or appropriate.
Code #1 This implementation takes an element and sorts itself with all the elements to its left, one by one, like bubble sort I think. I am just sorting and I think I can not say I am inserting an element to its correct location.

public static int[] insertionSortAsc(int[] elemArr){
    int len = elemArr.length;
    int temp;
    for (int i=0; i<= (len-2); i++){
        for (int j=(i+1); j>0; j--){
            if (elemArr[j] < elemArr[j-1]){
                //swap
                temp = elemArr[j];
                elemArr[j] = elemArr[j-1];
                elemArr[j-1] = temp;
            }
        }
    }
    return elemArr;
}

Code #2 This implementation takes an element into a 'temp' variable, compares it with left elements in the array and sorts them by comparing with the 'temp' element. When sorting stops, which means that the left side is sorted corresponding the 'temp' element, then this is element is put into that last location.

public static int[] insertionSort(int[] elements) {
        for (int i=1; i<elements.length; i++) {
            int j, elem_i = elements[i];
            for (j=i; j>0 && elements[j-1] > elem_i; j--) {
                elements[j] = elements[j-1];
            }
            elements[j] = elem_i;
        }
        return elements;
    }

I also have another idea though I have not written the code which I think matches my thought about the meaning of insertion sort.

Create an array, sortArr, of same size as that of input array.
Find the minimum element, and put that to sortArr[0]. If more elements with same value, put them into successive locations and maintain the count, means the index to which value is filled into sortArr. Put this minimum element to some tempMin variable.
Now check for another element in the original array which is just greater than tempMin, if found, replace the value of tempMin with this value, and then copy this to sortArr according to the filling count variable, increase the count thereby.
Repeat steps 2 and 3, until filling count variable reaches array length.
Exit.

I might be wrong but that is my thought. Please suggest algorithm modifications or my thinking if I am wrong.

Bubble or Insert?

#1 and #2 are both implementations of insertion sort (*).  The way you insert the element is not so important: whether you copy one by one, use arraycopy, or bubble it down is up to implementation.  I'd imagine arraycopy to be the best for larger inputs.

#3 would be closer to selection sort, a relative of insertion sort, though there's no advantage in allocating a new array. (Or maybe I misunderstand.)

Both build a sorted part in the array, guaranteeing that at step i, array[0..i] is sorted.  Both do their sorting in-line, not requiring extra array allocations.  And both select an element from the unsorted part and then insert it.  The difference between insertion and selection is in how they select the next element to be inserted.

Insertion sort doesn't care which element is next, so it selects whichever.

Selection sort specifically select the littlest element from the rest of the array.  This makes it more costly, but adds the additional guarantee that not only is array[0..i] in order, but it also contains the littlest elements of the entire array.

(*) Though #1 is related to bubble sort in a way his parents won't talk about.

Optimise?

A strong point of insertion sort is that it performs very well for input that is already (mostly) sorted.

#1 unconditionally runs in quadratic time: i:[0..len-2], j:[i+1..0], which is in the order of n² - n steps.

#2 is smarter about it, and doesn't run back comparing if it won't need to.  This means it has the potential to run in close to linear time.  Specifically, it will be in the order of n + d*n steps, with d being the number of elements out of order.

#1 can be 'fixed' with a simple addition:

for (...) {
    for (...) {
        if (cond) {
            swap;
        } else {
            break; // <--
        }
    }
}

... or, depending on your style preference:

for (...) {
    for (... && cond) { // <-- as in #2
        if (cond) {
            swap;
        }
    }
}

... after which it's pretty much the same as #2.


Returning the lowest and highest number based on input This simple console program prompts the users for a number and returns the lowest and highest numbers from their inputs. It is difficult to add more user inputs, though. How can I make this easier to maintain and extend?

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace ConsoleApplication1
{
    class Program
    {
        static void Main(string[] args)
        {
            int num1 = 0, num2 = 0, num3 = 0;

            Console.WriteLine("lowest");

            Console.WriteLine("input 1st number");
            num1 = int.Parse(Console.ReadLine());

            Console.WriteLine("input 2nd number");
            num2 = int.Parse(Console.ReadLine());

            Console.WriteLine("input 3rd number");
            num3 = int.Parse(Console.ReadLine());

           Console.WriteLine("input 4th number");
            num4 = int.Parse(Console.ReadLine());

               Console.WriteLine("input 5th number");
            num5 = int.Parse(Console.ReadLine());

            Console.Clear();

            int lowest = ((num1 < num2 ? num1: num2) < num3 ?
                (num1 < num2 ? num1 : num2) : num3);
            int highest = ((num1 > num2 ? num1: num2) > num3 ?
                (num1 > num2 ?  num1 : num2)  : num3) ;

            Console.WriteLine(" lowest is {0} ", lowest);
            Console.WriteLine(" highest is {0} ", highest);
            Console.ReadLine();
        }
    }
}

If you are only worried about showing Min and Max, and nothing else is the desired result then why not use an immediate calculation approach like this? It surely does not store unnecessary inputs -

using System;

namespace ConsoleApplication1
{
    class Program
    {
        static void Main(string[] args)
        {
            int max = Int32.MinValue, min = Int32.MaxValue;
            int value = 0;

            while (int.TryParse(Console.ReadLine(), out value))
            {
                max = Math.Max(max, value);
                min = Math.Min(min, value);

                Console.WriteLine(" lowest is {0} ", min);
                Console.WriteLine(" highest is {0} ", max);
            }
        }
    }
}

This program will print min and max, until a non-digit character is given.


Find filenames with 4 digits which are not divisible by 20 and delete them I have a set of files in folder my_dir, which is not the folder where my Python script resides. Sample content of my_dir:

  foo.bar
  ba.bar
  moog_0001.h5
  moog_0007.h5
  moog_0020.h5
  moog_0027.h5
  moog_0032.h5
  moog_0039.h5
  moog_0041.h5
  moog_0053.h5
  moog_0060.h5

I need to:

  find the files with extension .h5
  for each such file, extract the 4 digits in the filename which follow the underscore _. Note: if a file has the extension .h5, its filename always contains a substring _dddd. There can be other digit groups in the full pathname of the file, but none starts with a _, followed by 4 digits, and ends with a .
  if the resulting integer is not divisble by 20, delete the corresponding file

According to these rules, after running the script the content of the folder my_dir must be:

  foo.bar
  ba.bar
  moog_0020.h5
  moog_0060.h5

My solution:

import os
import re

MY_DIR = "/tmp/logs/20190519T1032"

root, dirs, files = next(os.walk(MY_DIR, topdown=True))
files = [ os.path.join(root, f) for f in files ]
print(files)

files = [ file for file in files if file.endswith(".h5") ]

for file in files:
    match = re.search(r'_\d{4}', file).group(0)
    match = match[1:]
    digits = int(match)
    if digits % 20 != 0:
        print("remove file " + file )
        os.remove(file)
    else:
        print("skip file " + file)

Any suggestions? I was told to put the content of the for block in a function, in order to substitute the for block with a list comprehension, but I don't know if it would be significantly faster (the number of files is O(103), tops). Also, the resulting function wouldn't do only one thing: at the very least, it would extract the 4 digits and delete those files for  which the corresponding integer is not divisible by 20. I think that most functions should only one thing.

Requirements

The requirements are somewhat imprecise because they rely on untold assumptions about the filenames we should expect. Before implementing anything, we should try to think about the different inputs we can have and how we should handle them.

In our case, this could correspond to:

what if the regexp does not match (no underscore or less than 3 numbers) ?
what if we have more than 4 numbers ? Should we consider only the first 4 ?
what if the pattern appears more than once ?

In order to tests tests, I've defined the following list of file names from my invention.

files = [
    '.h5',
    '_.h5',
    'foo_.h5',
    'foo_123.h5',
    'foo_1234.h5',
    'foo_1240.h5',
    'foo_12345.h5',
    'foo_12340.h5',
    'foo_12340.h5',
    'foo_12400.h5',
    'foo_12403.h5',
    'foo_123_bar.h5',
    'foo_1234_bar.h5',
    'foo_1240_bar.h5',
    'foo_12345_bar.h5',
    'foo_12340_bar.h5',
    'foo_12400_bar.h5',
    'foo_12403_bar.h5',
    'foo_1234_bar_1240.h5',
    'foo_1240_bar_1234.h5',
]

From here, changes in the code may be considered wrong as they could change the way the code behaves (on some of the inputs above) but I do not know what the expected behavior is.

Improving the code

The first thing we could do is try to be more robust when the pattern does not match.

Usually after a call to re.search, the next step is "if match" (or "if match is None").

Taking this chance to define variables with better names ("digits" instead of re-using "match" for the string of 4 digits, "n" instead of "digits" from the corresponding integer), we'd have something like:

for f in files:
    match = re.search(r'_\d{4}', f)
    if match is not None:
        digits = match.group(0)[1:]
        n = int(digits)
        if n % 20 != 0:
            print("remove file " + f + " (" + digits + ")")
            continue
    print("skip file " + f)

Removing the need for modulo

Division by 20 is simple enough so that the corresponding logic can be moved into the regexp.

Disclaimer: This may not correspond to something we usually want to do but it is fun and interesting so let's do it anyway :)

A number is divisible by 20 if and only if:

last digits is a 0
the digit before that is divisible by 2

We could write:

for f in files:
    match = re.search(r'_\d\d[02468]0', f)
    if match is None:
        print("remove file " + f)
    else:
        print("skip file " + f)

Organisation

It would indeed be worth defining small functions to make your code easier to understand. We could imagine having a function "def file_must_be_deleted(filename)" return a boolean.


Change whether file is executable This is a cocoa app that adds a right click menu option to change whether a file is executable. It works by executing the system command chmod on each of the files. But I want to know if there is a better way to change the permission of a file, such as a cocoa function that can change the permission of a file.

What are some other ways that I can improve my code?

#import "AppDelegate.h"

@implementation AppDelegate

- (void)applicationDidFinishLaunching:(NSNotification *)aNotification
{
    // Insert code here to initialize your application
    [NSApp setServicesProvider:self];
    NSUpdateDynamicServices();
}
void runSystemCommand(NSString *cmd)
{
    NSTask *task = [[NSTask alloc] init];
    [task setLaunchPath:@"/bin/chmod"];
    if ([[NSFileManager defaultManager] isExecutableFileAtPath:cmd]) {
        //NSLog(@"is executable \n");
        [task setArguments:@[ @"-x", cmd]];
        [task launch];

    } else {
        //NSLog(@"is not executable \n");
        [task setArguments:@[ @"+x", cmd]];
        [task launch];

    }
}
- (void)handleServices:(NSPasteboard *)pboard
              userData:(NSString *)userData
                 error:(NSString **)error {
    if([[pboard types] containsObject:NSFilenamesPboardType]){
        NSArray* fileArray=[pboard propertyListForType:NSFilenamesPboardType];
        int i;
        int count;
        count = [fileArray count];
        for (i = 0; i < count; i++){
            //NSLog (@"%@", [fileArray objectAtIndex: i]);
            runSystemCommand([fileArray objectAtIndex: i]);
        }
    }
}

@end

Here is the repository for the complete project.

The relevant NSFileManager call, in Foundation, is - setAttributes:ofItemAtPath:error:, and it is the same in iOS as in Mac OS X.  (It's funny that you already call - isExecutableFileAtPath: in the same NSFileManager class.)  Even if you weren't familiar with Foundation, you would still be better off calling the POSIX function chmod(2) rather than the chmod(1) command.

You've neglected to do any kind of error handling on the /bin/chmod task.  The operation could easily fail if, for example, the user did not have permission to modify the file.

It's misleading to name the parameter of runSystemCommand() cmd, as it really only contains the path to the file whose permissions are to be toggled.


Bash script that searches for directories with files called package.json and install dependencies I'm relatively familiar with the command line, but I've usually reached for other languages rather than bash for more full on scripts. It works, but I'm wondering if anything isn't really idiomatic. I've never used bash arrays before, and I've never had to write such an involved find command so I'm just wondering if I did all that fine. Also just overall is there a shorter way to do this?

.

#!/bin/bash

set -e

# The directories to search in
# Currently, lib + tools, and whatever command line args are provided
DIRS=(lib tools $@)
for i in "${!DIRS[@]}";
do
  DIRS[i]="$(pwd)/${DIRS[i]}"
done

# Get absolute path of directories with a package.json file and install dependencies
while IFS='' read -r line; do
  echo "Installing dependencies for $line"
  cd "$line" && yarn
done < <(
  find "${DIRS[@]}" -iname 'package.json' -not -path '*/node_modules/*' -exec dirname {} \;
)

# Do current directory
echo "Installing dependencies for current directory"
yarn

Working with $@

If some command line arguments contain spaces this won't work:

DIRS=(lib tools $@)

In 99% of use cases you probably want to double quote $@.

Unnecessary work

It's unnecessary to execute $(pwd) in a loop if the directory doesn't change in between cycles:

  DIRS[i]="$(pwd)/${DIRS[i]}"

A simple fix is using $PWD instead, which will not incur command execution:

  DIRS[i]="$PWD/${DIRS[i]}"

Building arrays

This is not very idiomatic:

DIRS=(lib tools $@)
for i in "${!DIRS[@]}";
do
  DIRS[i]="$(pwd)/${DIRS[i]}"
done

It would be better like this:

DIRS=()
for i in lib tools "$@"
do
  DIRS+=("$PWD/$i")
done

cd in a sub-shell

This works because you made sure the paths are absolute paths:

while IFS='' read -r line; do
  echo "Installing dependencies for $line"
  cd "$line" && yarn
done < <(
  find "${DIRS[@]}" -iname 'package.json' -not -path '*/node_modules/*' -exec dirname {} \;
)

If the paths had been relative, this wouldn't work.
That's why you built DIRS, a considerable extra effort.
There's a way to make this safe even with relative paths,
by wrapping cd && yarn in a sub-shell:

while IFS='' read -r line; do
  echo "Installing dependencies for $line"
  (cd "$line" && yarn)
done < <(
  find lib tools "$@" -iname 'package.json' -not -path '*/node_modules/*' -exec dirname {} \;
)

Without DIRS, the script can be shorter and simpler.

find ... -execdir

If your version of find supports the -execdir option,
and if you don't mind not printing the "Installing dependencies for ...",
then you can further simplify the script:

#!/bin/bash

set -e

# Install dependencies in directories with a package.json file
find lib tools "$@" -iname 'package.json' -not -path '*/node_modules/*' \
  -execdir yarn \;

# Do current directory
echo "Installing dependencies for current directory"
yarn

If you really do want to print that informational line,
that's possible, though not very sexy:

find lib tools "$@" -iname 'package.json' -not -path '*/node_modules/*' \
  -exec printf Installing dependencies for \; -execdir pwd \; -execdir yarn \;


Daemonize a process under Microsoft Windows I wrote a simple tool to allow to daemonize any process under Microsoft Windows.

The goal is to push any process into the background with no questions asked. That includes GUI applications, which means that the main window will not be created...or at least not shown. For console applications it will simply detach the process from the current console and not open another one. Usage is simple:

daemonize.exe PROGRAM [ARGUMENTS]

So let's cut to the code:

#include <stdio.h>
#include <string.h>

#include <windows.h>

// The important things to note and know:
//
//  * Only works with ASCII paths and filenames.
//  * Quotes all arguments regardless of content.
//  * SW_HIDE in STARTUPINFO is needed for GUI applications
//  * DETACHED_PROCESS in CreateProcessA is needed for command line applications
//

int main(int argc, char *argv[]) {
    if (argc <= 1) {
        printf("daemonize.exe COMMAND [ARGUMENTS]");
        return 1;
    }

    char* command = argv[1];
    char* arguments = NULL;

    if (argc > 2) {
        // We only need to concatenate arguments if there
        // actually are arguments.
        int idx;

        // Calculate the length of all arguments
        size_t argLength = 0;
        for (idx = 1; idx < argc; idx++) {
            argLength += strlen(argv[idx] + 2 + 1); // One for the space, two for the quotes.
        }
        argLength--; // Strip the last space.

        // Reserve some memory and NULL the new string.
        //The + 1 is the space for the last null character.
        arguments = (char*)malloc(sizeof(char) * (argLength + 1));
        arguments[0] = 0;

        // Now concatenate the arguments.
        for (idx = 1; idx < argc; idx++) {
            strcat(arguments, "\"");
            strcat(arguments, argv[idx]);
            strcat(arguments, "\"");
            if (idx < argc - 1) {
                strcat(arguments, " ");
            }
        }
    }

    STARTUPINFO startInfo;
    PROCESS_INFORMATION process;

    ZeroMemory(&startInfo, sizeof(startInfo));
    ZeroMemory(&process, sizeof(process));

    startInfo.cb = sizeof(startInfo);

    // Tell the system to use/honor the ShowWindow flags.
    startInfo.dwFlags = STARTF_USESHOWWINDOW;
    // Tell the system that the main window of the process should be hidden.
    startInfo.wShowWindow = SW_HIDE;

    if (!CreateProcessA(
            command,            // application name
            arguments,            // command line arguments
            NULL,                // process attributes
            NULL,                // thread attributes
            FALSE,                // inherit (file) handles
            // Detach the process from the current console.
            DETACHED_PROCESS,   // creation flags
            NULL,                // environment
            NULL,                // current directory
            &startInfo,            // startup info
            &process)            // process information
        ) {

        printf("Creation of the process failed, trying to fetch error message...\n");

        long lastError = GetLastError();
        char* message = NULL;

        FormatMessageA(
            FORMAT_MESSAGE_ALLOCATE_BUFFER |
            FORMAT_MESSAGE_ARGUMENT_ARRAY |
            FORMAT_MESSAGE_FROM_SYSTEM |
            FORMAT_MESSAGE_IGNORE_INSERTS, // Flags
            NULL,               // source
            lastError,          // message identifier
            0,                  // language id
            (LPSTR)&message,   // message buffer
            0,                  // size of the message
            NULL                // arguments
        );

        printf(message);

        return 1;
    }

    return 0;
}

It looks good: very good, assuming it works (i.e. has the run-time behaviour that you want: I don't know the effect of calling CreateProcess in that way).

You don't free(arguments) but that doesn't matter because memory will be reclaimed by the O/S when the process exits. Similarly, in theory I'd expect an API call to release message.

I don't see why you need the (LPSTR) cast. Using a cast can hide compiler warnings/errors, which should be fixed instead of their causing errors at run-time.

Instead of int idx; it's better to not define a local variable until the moment at which you initialize it: instead you could say, for (int idx = 1; idx < argc; idx++).

Also, shouldn't it be for (int idx = 2; idx < argc; idx++)? Because, idx=1 is the 'command' parameter, not the first 'argument' parameter.

If when you say "C/C++" you mean "C++", then it's more conventional these days to use std::string instead of C-style strings (but this is an unusually short program; perhaps although I don't know why you want to avoid the hassle of linking to the STL, especially if you know enough to avoid the many pitfalls of C-style string manipulation, which you seem to have).

For example, using std::string you could write the following, which is simpler than the string-manipulation code you have at the moment:

std::string s;
for (int idx = 1; idx < argc; idx++)
{
    s += "\"" + argv[idx] + "\"";
    if (idx < argc - 1) {
        s += " ";
    }
}
char* arguments = strdup(s.c_str());

The reason for strdup in the above is that MSDN says ...

  The Unicode version of this function, CreateProcessW, can modify the contents of this string. Therefore, this parameter cannot be a pointer to read-only memory (such as a const variable or a literal string). If this parameter is a constant string, the function may cause an access violation.

... which helps to explain why arguments must be char* not const char*. Sometimes you can ignore these (the windows API sometimes doesn't say its parameters are const when in fact they are), but doing so may be unsafe bad practice (so I use strdup above -- note that strdup'ed memory should be free'ed).


Binary search large tree in C I tested creating 1 billion (1,000,000,000) nodes and lookups are fast. You can ask for a node by node_id.

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

typedef struct node {
    int node_id;
    int data;
    char *name;
    struct node *left;
    struct node *right;
} node;

node *newNode(int data, char *name, int node_id) {
    node *new_node = malloc(sizeof(node));
    new_node->data = data;
    new_node->name = name;
    new_node->node_id = node_id;
    new_node->right = new_node->left = NULL;
    return new_node;
}

node *insert_node(node *root, int data, int node_id, char *name) {
    if (root == NULL)
        return newNode(data, name, node_id);
    else {
        node *cur;
        if (node_id < root->node_id) {
            cur = insert_node(root->left, data, node_id, name);
            root->left = cur;
        } else if (node_id > root->node_id) {
            cur = insert_node(root->right, data, node_id, name);
            root->right = cur;
        }
    }
    return root;
}

node *find_node_data(node *root, int node_id) {
    if (root == NULL)
        return NULL;
    else if (root->node_id == node_id)
        return root;
    else if (root->node_id > node_id)
        return find_node_data(root->left, node_id);
    else
        return find_node_data(root->right, node_id);
}

void print(node *np) {
    if (np) {
        print(np->left);
        printf("(%d, %d, %s)", np->node_id, np->data, np->name);
        print(np->right);
    }
}

int main() {
    int T = 1000000000; //test case 1000000000 nodes
    int data, node_id;
    //printf("Input number of nodes:");
    //scanf("%d", &T);
    node *root = NULL;
    srand(time(NULL));
    while (T-- > 0) {
        //printf("Input data. %d:", T);
        //scanf("%d %d", &data, &node_id);
        int r = rand() % 1000000000;
        int r2 = rand() % 1000000000;
        root = insert_node(root, r, r2, "Rosencrantz");
    }
    //print(root);
    printf("\n");
    printf("Find data at node:");
    scanf("%d", &T);
    printf("node data %d %s", find_node_data(root, T)->data, find_node_data(root, T)->name);
    return 0;
}

node *cur is not necessary. A c idiom is

if (node_id < root->node_id) {
    root->left = insert_node(root->left, data, node_id, name);

find_node_data is unnecessarily recursive. The compiler may understand that it is a tail recursion, and optimize it out, but it always better to be explicit:

    while (root) {
        if (root->node_id == node_id) {
            break;
        }
        root = root->node_id > node_id? root->left: root->right;
    }
    return root;

Note the insertion may also be made iterative, although much less elegant.
The line

printf("node data %d %s", find_node_data(root, T)->data, find_node_data(root, T)->name);

looks really strange. Why do you call find_node_data twice, and why don't you call print, which you are already defined?


Find the oldest person by Lithuanian National Identification Number I made a program which parses the NIDs from kodai.txt file and calculates the oldest woman and man and outputs their DOB.

#include <iostream>
#include <fstream>
#include "math.h"
using namespace std;

enum Genders {
    Man, Woman
};

struct Person {
    Genders gender;
    int dobYear;
    int dobMonth;
    int dobDay;
    long code;
};

Person parsePerson(long kodas) {
    Person person;

    kodas = kodas / 10000;
    int firstNumber = kodas / 1000000;

    switch(firstNumber) {
        case 1:
            person.gender = Genders::Man;
            person.dobYear = 1800;
            break;
        case 2:
            person.gender = Genders::Woman;
            person.dobYear = 1800;
            break;
        case 3:
            person.gender = Genders::Man;
            person.dobYear = 1900;
            break;
        case 4:
            person.gender = Genders::Woman;
            person.dobYear = 1900;
            break;
        case 5:
            person.gender = Genders::Man;
            person.dobYear = 2000;
            break;
        case 6:
            person.gender = Genders::Woman;
            person.dobYear = 2000;
            break;
    }

    kodas = kodas % 1000000;
    int years = kodas / 10000;

    person.dobYear += years;

    kodas = kodas % 10000;

    person.dobMonth = kodas / 100;

    kodas = kodas % 100;

    person.dobDay = kodas;

    cout << person.dobDay << endl;
    return person;
}

bool comparePerson (Person p1, Person p2) {
    if (p1.dobYear < p2.dobYear) {
        return true;
    }

    if (p1.dobYear == p2.dobYear) {
        if (p1.dobMonth < p2.dobMonth) {
            return true;
        }

        if (p1.dobMonth == p2.dobMonth) {
            if (p1.dobDay < p2.dobDay) {
                return true;
            }
        }
    }

    return false;
}

int main () {
    ifstream kodai("kodai.txt");

    Person oldestMan;
    Person oldestWoman;

    oldestMan.dobYear = 9999;
    oldestMan.dobMonth = 9999;
    oldestMan.dobDay = 9999;

    oldestWoman.dobYear = 9999;
    oldestWoman.dobMonth = 9999;
    oldestWoman.dobDay = 9999;

    long code;

    while (kodai >> code) {
        Person person = parsePerson(code);

        if (person.gender == Genders::Woman) {
            if (comparePerson(person, oldestWoman)) {
                oldestWoman = person;
            }
        } else if (person.gender == Genders::Man) {
            if (comparePerson(person, oldestMan)) {
                oldestMan = person;
            }
        }

    }

    cout << "Oldest man: " << oldestMan.dobYear << "-" << oldestMan.dobMonth << "-" << oldestMan.dobDay << endl;
    cout << "Oldest woman: " << oldestWoman.dobYear << "-" << oldestWoman.dobMonth << "-" << oldestWoman.dobDay << endl;
}

Example kodai.txt:

33309240064
40212220064
50211020064
10001010064
20001010064

Can you give me any suggestions or problems that you might see?

Headers

Prefer <cmath> to <math.h> when writing new C++ code.  It seems it's not required in this program, so prefer to omit it instead!

<fstream> is only required for the test program - I'll move it to after the function.  (In fact, I'll use a string stream instead, to make the test program self-contained).

Avoid using namespace std;, especially in headers - there's many answers here on Code Review that explain why it's harmful.

Types

Enumeration types are normally named as singular, unless they are intended to be combined as bitmask values.  That's a useful convention to maintain, so let's rename Genders to Gender.  We never want Gender to implicitly convert to and from integer values, so we can make it a strong enumeration, using enum class.

We can improve the cohesion of Person by observing that all its properties are derived from the ID number.  Instead of requiring client code to populate all fields, we can supply a constructor which accepts the ID number and populates the whole structure from it.

We might even choose to store only the number (to reduce the size of Person) and compute the derived properties when needed.  That would be an appropriate strategy if we had a much larger number of input lines.

We can present a public interface that doesn't depend on our implementation choice:

class Person
{
public:
    Person(long kodas);
    Gender gender() const;
    int dobYear() const;
    int dobMonth() const;
    int dobDay() const;
    long code() const;
};

I also suggest making the comparison and formatting functions into members:

class Person
{
public:
    Person(long kodas);
    Gender gender() const;
    bool older_than(const Person& other) const;
    std::ostream& operator<<(std::ostream&) const;
};

(Note that the birth date no longer needs to be public).

Implementation

We don't need to split the code into year, month and day to compare - we can just sort the numbers directly (because 10000*year + 100*month + day will sort the same way).  If we did need to sort based on multiple fields, we could use std::make_tuple() or std::tie() to simplify the code.

When printing month and day, it's usual to zero-fill single-digit numbers.

We should check that we reached end-of-stream - if not, that indicates a read error, and we can't claim to have found the answer.

Modified code

#include <iosfwd>

enum class Gender {
    Man, Woman
};

class Person
{
    long kodas;
public:
    Person(long kodas);
    Gender gender() const;
    bool older_than(const Person& other) const;

    friend std::ostream& operator<<(std::ostream&, const Person&);

private:
    long remove_gender() const;
};

#include <iomanip>
#include <ostream>
#include <stdexcept>
Person::Person(long kodas)
    : kodas{kodas}
{
    if (kodas < 0)
        throw std::domain_error("negative ID");
}

Gender Person::gender() const
{
    auto firstDigit = kodas / 10000000000;
    return firstDigit % 2 ? Gender::Man : Gender::Woman;
}

bool Person::older_than(const Person& other) const
{
    return remove_gender() < other.remove_gender();
}

std::ostream& operator<<(std::ostream& os, const Person& p)
{
    auto n = p.remove_gender() / 10000;
    int century = 18 + n/2000000;
    n %= 1000000;

    int year = n / 10000;
    n %= 10000;

    int month = n / 100;
    int day = n % 100;

    // save stream settings
    auto fill = os.fill();

    return os << std::setfill('0')
              << century
              << std::setw(2) << year
              << '-'
              << std::setw(2) << month
              << '-'
              << std::setw(2) << day
              << std::setfill(fill);
}

long Person::remove_gender() const
{
    return gender() == Gender::Woman ? kodas - 10000000000 : kodas;
}

#include <sstream>
#include <iostream>
int main () {
    std::istringstream kodai(
                        "33309240064\n"
                        "40212220064\n"
                        "50211020064\n"
                        "10001010064\n"
                        "20001010064\n"
                        );

    Person oldestMan(99999999999);
    Person oldestWoman(89999999999);

    long code;
    while (kodai >> code) {
        Person person{code};
        switch (person.gender()) {
        case Gender::Woman:
            if (person.older_than(oldestWoman))
                oldestWoman = person;
            break;
        case Gender::Man:
            if (person.older_than(oldestMan))
                oldestMan = person;
            break;
        }
    }
    if (!kodai.eof())
        throw std::runtime_error("Failed to read whole file");

    std::cout << "Oldest man: " << oldestMan << std::endl;
    std::cout << "Oldest woman: " << oldestWoman << std::endl;
}


ConcurrentLinkedQueue to unmodifiable list I have a ConcurrentLinkedQueue where I put some mails in. Now I have a visual page where I want to see what's in the Queue, but of course I do not permit that they alter the Queue in any way.

I put a getter, but instead of the Queue, I return a List. We know that the mail does disappear while we are watching, the use case permits that a refresh of the screen is enough to see the new status.

Code at the moment:

private final ConcurrentLinkedQueue<MimeMessage> mailsToSend = new ConcurrentLinkedQueue<MimeMessage>();

public List<MimeMessage> getMailsToSend() {
    return Collections.unmodifiableList(Lists.newArrayList(mailsToSend.iterator()));
}

I know, it's a very small code fragment and much review isn't there, but if there are better ways to convert it to an unmodifiable list, I'd like to hear it.

Lists.newArrayList will allocate a new list (as the name indicates) so there is no reason to wrap it in a Collections.unmodifiableList.

What you have is a defensive copy. This won't allow propagation of changes back to the queue, but requires you to get a new list each time you want an update.

If you still want a queue backed view then you should return Collections.unmodifiableCollection:

public Collection<MimeMessage> getMailsToSend() {
    return Collections.unmodifiableCollection(mailsToSend);
}

This has the downside that only way to get all items from a generic Collection is using the iterator or the toArray methods. Though each time you call it on them you get an up to date view (with the same caveats of the same methods of the queue).


Implementing Transpose I'm implementing transpose in Haskell.

-- transpose [[1,2,3],[4,5,6],[7,8,9]]
-- [[1,4,7],[2,5,8],[3,6,9]]

Please give it a look. Note that I'm trying not to use any built-in functions (excluding map and ++):

transpose' :: [[a]] -> [[a]]
transpose' ys
   | null $ filter (not . null) ys = []
   | otherwise = [flatten' $ map head' ys] ++ transpose' (map tail' ys)

head' :: [a] -> [a]
head' []     = []
head' (x:xs) = [x]

tail' :: [a] -> [a]
tail' []     = []
tail' (x:xs) = xs

flatten' :: [[a]] -> [a]
flatten' as = foldl (\acc x -> acc ++ x)  [] as

Side note - I know that I could look at the Haskell source, but I appreciate the comments & thoughtful insights here.

I see a few problems, mostly similar to my remarks in a previous answer:

Your definition of head' is unconventional.  Its definition should be

head' :: [a] -> a
head (x:_) = x

Furthermore, your problematic definition of head' is making you define flatten'.  If you fix head' as suggested above, thenflatten' becomes unnecessary.
The definition of tail' is too complicated.  It could just be

tail' :: [a] -> [a]
tail (_:xs) = xs

For clarity, I would rename the parameter to transpose' from ys to rows.
The base case of transpose' is too complicated.  Also, it can be done using pattern matching.

transpose' [[]]    = []
transpose' [[], _] = []

Prefer x:xs to [x] ++ xs.
Scope your helper functions using where.

Here's what I came up with:

transpose' :: [[a]] -> [[a]]
transpose' [[]]    = []
transpose' [[], _] = []
transpose' rows    = (map head' rows) : transpose' (map tail' rows)
  where
    head' (x:_) = x
    tail' (_:xs) = xs


Remove vulnerabilities from query on public website I think that the following query is preventing against SQL injection, but what other measures do I need to take to ensure my queries are 100% safe from any malicious attacks?

$statement = $db->prepare(
    "INSERT INTO blogs (blogtitle, blogdesc, coverimage, userID, frontpage, tags)
    VALUES (:buildtitle, :builddesc, :buildcover, :userid, :frontpage, :addtags)"
);

if ($statement->execute(array(
    ':buildtitle' => $_POST['addbuildtitle'],
    ':builddesc' => $_POST['addbuilddesc'],
    ':buildcover' => $imagepath,
    ':userid' => $_POST['adduserid'],
    ':frontpage' => $frontpage,
    ':addtags' => $_POST['addtags']))
);

Though prepared statements do a pretty good job protecting your DB from injection attacks, it never hurts to perform some additional checks on the data you're receiving. For example: when a request is supposed to contain an email address, you might perform a SELECT query usign that email address, or insert it in your DB. If so, you should add this:

if (!filter_var($_POST['email'], FILTER_VALIDATE_EMAIL))
{
    exit();//redirect, optionally show error message
}

Same goes for any posted data, as one of the comments mentions: XSS attacks are one of the biggest threats on the web today. I've dealt with the matter in this answer.
The very least you could do is:

$execParam = array_map(
    'strip_tags'
    array(
        ':buildtitle' => $_POST['addbuildtitle'],
        ':builddesc' => $_POST['addbuilddesc'],
        ':buildcover' => $imagepath,
        ':userid' => $_POST['adduserid'],
        ':frontpage' => $frontpage,
        ':addtags' => $_POST['addtags']
    )
);
$statement->execute($execParam);

Here, I've just called strip_tags on each and every single item you're about to insert, to avoid some jokester posting

<script>location.href = 'http://malware-infested-site.org';</script>;

in the addbuilddesc field... because, if you don't filter those things out, people visiting your site won't be all too happy about that... and be honest: would you?

You can take this as far as you want. Since you're obviously using a traditional *SQL DB, you might use the table schema, to determine if the POST params are of the correct type and length:

//assume blogtitle is a VARCHAR(255) field:
':buildtitle' => sprintf('%255s',$_POST['addbuildtitle'])
//and userID is an unsigned int, 11 long
':userid' => sprintf("%11d",abs((int)$_POST['adduserid']))

This ensures your :buildtitle bind will be no longer than 255 chars. The userid value will not be negative, and will be an integer of max 11 digits long.
But to implement this would mean a huuuge mess of sprintf calls, so you could use dataModels for this (check pretty much all of my DB related answers on this site for details), and use getters and setters that use this sprintf function to format the data accordingly.
still, this will mean a lot of overhead for each and every query. Sadly, the tradoff will always be one of performance vs security. If security is all you're after, you'll also note that, in my other answers, I basically advise everybody who's trying to create a solid (as in strong, but also as in S.O.L.I.D.) query system, to have a look at doctrine. No need to reinvent the wheel...
Anyway, I'm going off course...

Now, as far as injection goes, prepared statements are pretty robust, provided you don't take it to new, absurd, extremes. I've seen people use variables instead of the DB name, just like I've seen them use POST params to fill in the table name. You don't seem to do that, and that's good... great even. Just a friendly warning: if ever you consider using variables in those parts of your query that should be static (SELECT fieldnames FROM tblNames), step back and rethink your approach, because that's where you may still introduce vulnerabilities to your system.

Basically, prepared statements work like this:

Pass a string, from which a statement is to be prepared. This is processed by the DB server as is.
The DB server parses, and "prepares" the query (optimization etc...)
Pass the arguments, with which the statement is to be executed. These values (binds) are sent via a different protocol and here's where they're being escaped correctly.

As you can see, the binds are what is being escaped and sent completely separatly, that's why prepared statements are safe. This also explains why the actual SELECT and FROM clauses in a prepared statement may not be gotten from user input. This part of the query isn't processed in the same way the binds are...

Other niggles:

At no point do you seem to be checking isset($_POST['adduserid']) and other post keys. You just seem to expect they're there. That might trigger notices, and slows your code down. Just isset() all those params.
You have a semi-colon at the end of your if statement. If you copy-pasted this code, then that's going to give you grief.
Ensure, especially with INSERT's to set the PDO::ERR_MODE correctly, and insert in transactions. In case something goes wrong, you can still use rollBack!

Here's a little sample of how that would look

$db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);//force PDO to throw on error
try
{
    $db->beginTransaction();//start here
    $stmt = $db->prepare($query);
    $stmt->execute($sanitizedInsertParamsArray);
    $db->commit();//make insert permament
}
catch(PDOExcpetion $e)
{
    $db->rollBack();//revert insert!
    exit($e->getMessage());
}

The code above will throw an instance of PDOException if the prepare, execute call or even the commit fails. As long as commit hasn't been called, the transaction is still pending (ie the data isn't really inserted yet).
So simply put, this snippet asks the DB: prepare this statement, then try to execute it with these values, if you did that, then save it. If any of these three steps failed, $db->rollBack() just tells the DB: OK, so it didn't work, forget about it... don't try and save half of the data, just forget everything.

Transactions are a good way to avoid orphaned data (as are FK's, btw). And they make for a safer query all together, too...