Print limited previous lines from a file This is a question from careercup.com. Given a word w and a number k.Find k words in the file occurring before occurrence of w.Assume that the average word size is m in the file
eg.
aaa
bbb
ccc
booking
alpha
beta
gamma

for k=3 and w = booking
the output should be [aaa,bbb,ccc,booking]
similarly for k =2 and w = beta
output should be [booking,alpha,beta]
Assume that the file size can grow very large
and try to get solution with space complexity lesser than O(n)

package com.careercup;

import java.io.BufferedReader;
import java.io.FileNotFoundException;
import java.io.FileReader;
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;

public class LimitedLengthQueue {

    private List<String> innerStore;
    private int size;
    private static final String FILENAME="C:/filepath.txt.txt";
    public LimitedLengthQueue(int size) {
        this.innerStore = new ArrayList<>();
        this.size = size;
    }

    boolean add(String s){
        if(innerStore.size() == size){
            innerStore.remove(0);
            innerStore.add(s);
            return true;
        }
        innerStore.add(s);
        return true;
    }

    public static void main(String[] args) throws FileNotFoundException, IOException  {
        LimitedLengthQueue inputQueue = new LimitedLengthQueue(Integer.parseInt(args[0]));
        String key = args[1];

        try( FileReader fr = new FileReader(FILENAME);
             BufferedReader br = new BufferedReader(fr)){

            String input = null;

            while( (input = br.readLine()) != null){

                if(key.equals(input.trim())){
                    inputQueue.innerStore.forEach(System.out::println);
                    System.out.println(input);
                    break;
                }
                inputQueue.add(input);
             }
        }
    }
}

private int size;

I would find this more consistent with other Java classes if it called this capacity rather than size.

    private int capacity;

The size is typically the number of elements currently in the data structure.

        this.innerStore = new ArrayList<>();

This could be

        innerStore = new ArrayList<>(capacity);

While you can use the this. there (perhaps to make it obvious that it is a field), you don't need to do so as it is not ambiguous as to which innerStore is involved.

Passing the capacity (or size) initially tells it how big the structure will grow.  This saves having to do multiple allocations.

You could also do some extra work and use an array here.  Of course, with an array, you have to manually keep track of where you are in the array.  For a queue, you would normally use the array in circular fashion, overwriting the oldest entry and returning to the beginning after reaching the end.

    boolean add(String s){
        if(innerStore.size() == size){
            innerStore.remove(0);
            innerStore.add(s);
            return true;
        }
        innerStore.add(s);
        return true;
    }

This could be just

    boolean add(String s) {
        while (innerStore.size() >= capacity) {
            innerStore.remove(0);
        }

        return innerStore.add(s);
    }

The other way, if you passed the maximum capacity, it would stop enforcing it.

You don't need to add and return twice.  Once is plenty.

You shouldn't always return true.  If the internal add fails, you should return false.  Returning the result of the internal add accomplishes this exactly.


Finding if the tree is balanced or not Can someone please review my code and let me know if there are some bugs or possible improvements?

    /**
 * Definition for a binary tree node.
 * public class TreeNode {
 *     int val;
 *     TreeNode left;
 *     TreeNode right;
 *     TreeNode(int x) { val = x; }
 * }
 */
public class Solution {
    //return true if the root node is null or there is only root node.
    public boolean isBalanced(TreeNode root) {
        if (root == null || (root != null && root.left == null && root.right == null)) {
            return true;
        } else {
            int balancedRight = 0;
            int balancedLeft = 0;
            if (root.left != null) {
                balancedLeft = findHeight(root.left);
            }
            if (root.right != null) {
                balancedRight = findHeight(root.right);
            }

            return Math.abs(balancedLeft - balancedRight) <= 1 ? true : false;
        }

    }
//find the height of the tree
    public int findHeight(TreeNode root) {
        if (root == null) {
            return 0;
        } else if (root.left == null && root.right == null) {
            return 1;
        } else if (root.left == null && root.right != null) {
            return 1 + findHeight(root.right);
        } else if (root.left != null && root.right == null) {
            return 1 + findHeight(root.left);
        } else {
            return Math.max(1+findHeight(root.left), 1+findHeight(root.right));
        }
    }
}

Definition of balanced

The usual definition of a balanced binary tree:

The left and right subtrees' heights differ by at most one, AND
The left subtree is balanced, AND
The right subtree is balanced

Your algorithm only checks the level of the root, comparing the height of the left and right branches of the root. It doesn't go deeper, so the left or right subtrees may be unbalanced.

Unnecessary conditions

This condition can be simplified:

if (root == null || (root != null && root.left == null && root.right == null)) {

You can drop the root != null, because it will always be true.
This is exactly the same:

if (root == null || (root.left == null && root.right == null)) {

This chain of conditions has the same issue,
with some unnecessary conditions that can be eliminated:

if (root == null) {
    return 0;
} else if (root.left == null && root.right == null) {
    return 1;
} else if (root.left == null && root.right != null) {
    return 1 + findHeight(root.right);
} else if (root.left != null && root.right == null) {
    return 1 + findHeight(root.left);
} else {

Can be simplified to:

if (root == null) {
    return 0;
} else if (root.left == null && root.right == null) {
    return 1;
} else if (root.left == null) {
    return 1 + findHeight(root.right);
} else if (root.right == null) {
    return 1 + findHeight(root.left);
} else {

Using boolean expressions

You can use boolean expressions directly. For example like this:

return Math.abs(balancedLeft - balancedRight) <= 1;

Other simplifications

Instead of this:

int balancedRight = 0;
int balancedLeft = 0;
if (root.left != null) {
    balancedLeft = findHeight(root.left);
}
if (root.right != null) {
    balancedRight = findHeight(root.right);
}

Since findHeight immediately does a check for the case of root == null,
you could simplify the above as:

int balancedRight = findHeight(root.left);
int balancedLeft = findHeight(root.right);

Another thing, instead of adding a constant in both sides of a Math.max:

return Math.max(1+findHeight(root.left), 1+findHeight(root.right));

You could do just once:

return 1 + Math.max(findHeight(root.left), findHeight(root.right));

On closer look, you could simplify the entire findHeight method to this:

public int findHeight(TreeNode root) {
    if (root == null) {
        return 0;
    }
    return 1 + Math.max(findHeight(root.left), findHeight(root.right));
}

Naming

Instead balancedLeft and balancedRight,
leftHeight and rightHeight would have made a lot more sense.


Sum of integers in an interval Here is my solution to a series of problems in JS fundamentals. I've made it pass but I know this could be more efficient. I'm not looking for anyone to rewrite the code, but just asking what methods should be used when writing similar code in the future.
The goal is basically to input two parameters, both have to be positive numbers and typeof numbers otherwise return ERROR. Also form an array that adds said two integers and all numbers between them. For example: if you enter 1,4 the answer would compute 1 + 2 + 3 + 4 = 10, and return 10.
const sumAll = function(lowEnd, highEnd) {
    let total = [];
    let sum = 0;
    let first = lowEnd;
    let second = highEnd;
    if (typeof first === 'number' && typeof second === 'number') {
        if (lowEnd >= 0 && highEnd >= 0) {
            if (lowEnd <= highEnd){
                for (let i = lowEnd; i <= highEnd; i++) {
                    total.push(i);
                }
            }
            else {
                for (let i = highEnd; i <= lowEnd; i++) {
                    total.push(i);
                }
            }
            for (let i = 0; i < total.length; i++) {
                sum += total[i];
            };
            return sum;
        }
        else if (lowEnd < 0 || highEnd < 0)
            return 'ERROR';
    }
    else {
        return 'ERROR';
    }
}

and here are the tests,
const sumAll = require('./sumAll')

describe('sumAll', function() {
  it('sums numbers within the range', function() {
    expect(sumAll(1, 4)).toEqual(10);
  });
  it('works with large numbers', function() {
    expect(sumAll(1, 4000)).toEqual(8002000);
  });
  it('works with larger number first', function() {
    expect(sumAll(123, 1)).toEqual(7626);
  });
  it('returns ERROR with negative numbers', function() {
    expect(sumAll(-10, 4)).toEqual('ERROR');
  });
  it('returns ERROR with non-number parameters', function() {
    expect(sumAll(10, "90")).toEqual('ERROR');
  });
  it('returns ERROR with non-number parameters', function() {
    expect(sumAll(10, [90, 1])).toEqual('ERROR');
  });
});

A good addition is to add a check at the start of your function to check if your parameters are correct. This way you don't need extra if statements which helps for readability.

const isValidNum = (n) => typeof n === "number" && n >= 0;

const sumAll = function(lowEnd, highEnd) {
  if (!isValidNum(lowEnd) || !isValidNum(highEnd)) return "ERROR";

  let total = [];
  let sum = 0;

  if (lowEnd <= highEnd) {
    for (let i = lowEnd; i <= highEnd; i++) {
      total.push(i);
    }
  } else {
    for (let i = highEnd; i <= lowEnd; i++) {
      total.push(i);
    }
  }

  for (let i = 0; i < total.length; i++) {
    sum += total[i];
  };

  return sum;
}

console.log(sumAll(1, 4));
console.log(sumAll(4, 1));
console.log(sumAll("1", 4));
console.log(sumAll(1, "4"));
console.log(sumAll(1, -4));
console.log(sumAll(-1, -4));

Another option is to check for the highest and lowest number via Math.min() and Math.max(). This way the order of input doesn't matter and you don't have to write the loop twice.

const isValidNum = (n) => typeof n === "number" && n >= 0;

const sumAll = function(lowEnd, highEnd) {
  if (!isValidNum(lowEnd) || !isValidNum(highEnd)) return "ERROR";

  let total = [];
  let sum = 0;

  const lowest = Math.min(lowEnd, highEnd);
  const highest = Math.max(lowEnd, highEnd);

  for (let i = lowest; i <= highest; i++) {
    total.push(i);
  }

  for (let i = 0; i < total.length; i++) {
    sum += total[i];
  };

  return sum;
}

console.log(sumAll(1, 4));
console.log(sumAll(4, 1));
console.log(sumAll("1", 4));
console.log(sumAll(1, "4"));
console.log(sumAll(1, -4));
console.log(sumAll(-1, -4));

At last you could add a formula to calculate the total sum of a range of numbers. Which removes the need of loops. (More info about the formula here)

const isValidNum = (n) => typeof n === "number" && n >= 0;

const sumRange = (low, high) => ((high - low + 1) * (low + high)) / 2;

const sumAll = function(lowEnd, highEnd) {
  if (!isValidNum(lowEnd) || !isValidNum(highEnd)) return "ERROR";

  const lowest = Math.min(lowEnd, highEnd);
  const highest = Math.max(lowEnd, highEnd);

  return sumRange(lowest, highest);
}

console.log(sumAll(1, 4));
console.log(sumAll(4, 1));
console.log(sumAll("1", 4));
console.log(sumAll(1, "4"));
console.log(sumAll(1, -4));
console.log(sumAll(-1, -4));

Total revision with comments:

// Return true if "n" is positive and of type number, else return false
const isValidNum = (n) => typeof n === "number" && n >= 0;

// Formula for adding a range of numbers
const sumRange = (low, high) => ((high - low + 1) * (low + high)) / 2;

function sumAll(lowEnd, highEnd) {
  // Return "ERROR" if arguments are invalid
  if (!isValidNum(lowEnd) || !isValidNum(highEnd)) return "ERROR";

  // Find highest and lowest number
  const lowest = Math.min(lowEnd, highEnd);
  const highest = Math.max(lowEnd, highEnd);

  // Return added numbers
  return sumRange(lowest, highest);
}

console.log(sumAll(1, 4));
console.log(sumAll(4, 1));
console.log(sumAll("1", 4));
console.log(sumAll(1, "4"));
console.log(sumAll(1, -4));
console.log(sumAll(-1, -4));

PS: I really like the way your tests are written. nicely clean and concise.


Elegant way of processing an "options" parameter In this constructor function I'm assigning everything to the proper variables. For the third parameter it accepts an options argument that contains the optional settings.

To prevent a Cannot read property x of undefined. error I'm always checking this variable by steps, since the options variable can contain both complex objects and primitive data types.

I'm wondering if there is a more elegant way I could write this?

Constructor

function LiveDate(timeUrl, element, options) {
    this.timeUrl = timeUrl;
    this.element = element;

    this.format = options && options.format ? options.format : LiveDate.formats.ISO8601;
    this.offset = options && options.offset ? options.offset : 0;

    this.weekdayNames = {
        long: options && options.weekdayNames &&  options.weekdayNames.long ? options.weekdayNames.long : [
            'Sunday',
            'Monday',
            'Tuesday',
            'Wednesday',
            'Thursday',
            'Friday',
            'Saturday'
        ],
        short: options && options.weekdayNames && options.weekdayNames.short ? options.weekdayNames.short : [
            'Sun',
            'Mon',
            'Tue',
            'Wed',
            'Thu',
            'Fri',
            'Sat'
        ]
    };

    this.monthNames = {
        long: options && options.monthNames && options.monthNames.long ? options.monthNames.long : [
            'January',
            'February',
            'March',
            'April',
            'May',
            'June',
            'July',
            'August',
            'September',
            'October',
            'November',
            'December'
        ],
        short: options && options.monthNames && options.monthNames.short ? options.monthNames.short : [
            'Jan',
            'Feb',
            'Mar',
            'Apr',
            'May',
            'Jun',
            'Jul',
            'Aug',
            'Sep',
            'Oct',
            'Nov',
            'Dec'
        ]
    };

    this.start();
}

You could do something like this to clean things up a little:

options = options || {};

// Set to Default values first
this.weekdayNames = {
  "long": [...],
  "short": [...]
};

// Then check to see if we need to override
if ( options.weekdayNames ){
  for ( var key in options.weekdayNames ){
    this.weekdayNames[ key ] = options.weekdayNames[ key ];
  }
}

Furthermore, something like lodash's defaults function https://lodash.com/docs#defaults may be of use.


Split all images over x amount of pages with $_GET I would like to receive some feedback on the code I am currently using to display x amount of images on my website per
$success = true;
$page = array_key_exists('page', $_GET) ? $_GET['page'] : 1;

$all_pictures = glob('crossles/*.jpg');

$total_images = 0;
foreach ($all_pictures as $img_total) {
    $total_images++;
}

$images_per_page = 30;

$max_pages = ceil($total_images / $images_per_page);

if ($page <= -1) {
    $success = false;
}

if ($page > $max_pages) {
    $success = false;
}
if ($success === false) {
    echo "

No pictures to be displayed.

";
}

I know that $_GET is not the safest way of doing this but since it is only to retrieve images I don't think it is that big of an issue.
It would be nice if I could get some feedback to implement it to my code if needed.

$total_images = 0;
foreach ($all_pictures as $img_total) {
    $total_images++;
}

This could be just

$total_images = ($all_pictures === false) ? 0 : count($all_pictures);

or

if ($all_pictures === false) {
    $total_images = 0;
    $success === false;
} else {
    $total_images = count($all_pictures);
}

The glob function returns an array, so you can just do the normal array operations on it.  You only have to check for an error.  I'm not quite sure what your original code would have done if there was an error.  You might want to check later so you can say something like "Oops, there was an error looking for images!" rather than "No images".

if ($page <= -1) {

This is a weird way to write this.  First, what would you do if page were set to -.5 here?  Second, what does a page value of 0 mean?  You are defaulting to 1, so it would seem that you are 1-indexed.  So why accept anything less than 1?  Consider

if ($page < 1) {

That eliminates all the invalid values below the range.

For much the same reason,

$page = array_key_exists('page', $_GET) ? $_GET['page'] : 1;

should probably be

$page = array_key_exists('page', $_GET) ? intval($_GET['page']) : 1;

Get parameters can be user input, so you should sanitize them to what you expect.

    echo "

No pictures to be displayed.

";

This is an odd way to write a string in PHP.  Using double quotes indicates that there is a value to interpolate in it.  But you don't have one here.  This could just as well be

    echo '

No pictures to be displayed.

';

or

?>

No pictures to be displayed.

<?php

Consider moving the whole thing into a function or even a class.

class Page_Handler {

    private $image_count;

    private $success = true;

    private $all_pictures;

    private $max_page_number;

    private $page;

    private $images_per_page = 30;

    function __construct($path = 'crossles/*.jpg') {
        $this->all_pictures = glob($path);
        $this->success = $this->parse();
    }

    private function parse() {
        if ($this->all_pictures === false) {
            return false;
        }

        $this->image_count = count($this->all_pictures);
        $this->max_page_number = ceil($this->image_count / $this->images_per_page);

        $this->page = array_key_exists('page', $_GET) ? $_GET['page'] : 1;
        if ($this->page < 1 || $this->page > $this->max_page_number) {
            return false;
        }

        return true;
    }

There would also be getters and possibly other helper methods.  I'll leave those as an exercise for the reader (mostly because I'm lazy).  Note that you could differentiate between a failure to load the images and an invalid page.  Those are two separate problems and you may want to message them differently, as one is a server error and the other is user error.  Page number out of range is often a problem that the user can fix.

I changed the names of some of the variables.  I generally use a system where plural names indicate collections and singular names indicate scalars.  You don't have to follow the same system, but it's the one I use when writing code.

One of the differences here is that the parse code doesn't have to keep processing after an error.  At the first error, it returns failure.


Equalized PRNG number distribution - Part 2 A few days ago, I've made a code to equally distribute a set of numbers and retrieve them randomly. Today, I've rewritten the code (most of it, at least). I've followed most of the advice and tried to keep it clean, but also as fast as possible. The new code has a few more comments than the last one (they were needed) and the code suffered a huge simplification.

( function ( window ) {

    window.RNG = function ( min, max ) {

        var nan = function ( val ) {
                /*
                 * Checks if the value is an usable number or NaN or Infinity
                 * When dividing by 1, it will convert the value to a number or NaN
                 * NaN values return false when compared with themselves ( NaN != NaN returns true)
                 *
                 * The 2nd part of the verification will check if the value is infinite
                 * Infinity will return Infinity if you sum or subtract 1
                 * E.g.: Infinity - 1 == Infinity --> returns true
                 */
                return ( ( val / 1 ) != ( val / 1 ) ) || ( val - 1 == val );
            },
            //will contain all the available numbers
            avail = [];

        if ( nan( min ) )
        {
            throw new TypeError( 'Expecting Number or NumericString, got ' + ( min != min ? 'NaN' : ( min - 1 == min ? 'Infinity' : typeof min) ) );
        }
        if ( arguments.length > 1 )
        {
            if ( nan( max ) )
            {
                throw new TypeError( 'Expecting Number or NumericString as the second parameter, got ' + typeof max );
            }
            /*
             * converts the numbers to integers ( n>>=0 -> n = n >> 0)
             * and check if the minimum is higher than the maximum,
             * throwing an exception if that is the case
             */
            else if ( ( min >>= 0 ) >= ( max >>= 0 ) )
            {
                throw new RangeError( 'The 2nd parameter should be higher than the first' );
            }
            else if ( ( max - min ) > 268435455 )
            {
                //big numbers crash hard, so we must stop them before
                throw new RangeError( 'The difference between ' + max + ' and ' + min + ' is higher than 268435455' );
            }
        }
        else
        {
            max = min >> 0;
            min = 0;
        }

        return {
            valueOf: function ( ) {

                if( !avail.length )
                {
                    for( var i = 0; i <= max - min; i++ )
                    {
                        avail[i] = min + i;
                    }
                }

                if( avail.length > 1 )
                {
                    //returns a random element between 0 and avail.length (excluded)
                    return avail.splice( ( window.Math.random() * avail.length ) >> 0, 1)[0];
                }
                else
                {
                    return avail.splice( 0, 1 )[0];
                }

            },
            toString: function ( ) {
                return this.valueOf() + '';
            }
        };

    };

} )( Function('return this')( ) );

To use it, simply create a new object:

var random_number = new RNG( 50 ); //same as 0,50

Then, you use this to generate numbers, with direct access:

alert( random_number ); //should alert a number between 0 and 50

This also works for negative numbers but it won't work for decimal numbers.

In terms of readability and performance, what else should I improve?

If you notice, on the answer to the question, one of the pieces of advice was to avoid using:

else if( ( min = min >> 0 ) >= ( max = max >> 0 ) )

I've completely ignored it to keep my code a little DRY and to improve performance.

Input validation

You shouldn't just change the user input if it is wrong. If a user calls var random_number = new RNG( 10, 19.9 ); they will expect to get number between 10 and 19.9. But that's not what they will get, because their assumption is wrong (your random number generator only generates integers). A user should be informed about this, instead of just silently correcting their wrong assumption.

Regarding I've completely ignored it to keep my code a little DRY and to improve performance: Are you sure that there is a performance problem? Because it doesn't seem like it (instantiating a new RNG is something that probably isn't done all that often), and the code is quite hard to read because it does too many things (changing data and comparing data), and it uses tricks (shifting instead of casting), which is generally not recommended, as it hurts readability.

Array Generation

This:

                for( var i = 0; i <= max - min; i++ )
                {
                    avail[i] = min + i;
                }

Would be a more natural and easier to read written as this (you can immediately see if min and max are inclusive or not:

                for( var i = min; i <= max; i++ )
                {
                    avail[i] = i;
                }

Comments

It's good that you added comments (although using a bit less tricky code would probably reduce the need for some comments in the first place).

But this comment is a bit misleading: //returns a random element between 0 and avail.length (excluded): The next line also changes the array, something the comment doesn't mention at all. I would rewrite it as removes and returns random array element (you don't need the range, as its from the complete array, which makes sense).


jQuery navbar scrollTop position I have this code, but I not the best to javascript/jQuery so are there any out there who can optimize this code?

$(function(){
    var pos = $("header").offset().top;

    $(document).scroll(function(){
        if($(this).scrollTop() - 10 > pos)
        {
           $('header').addClass("filled");
        } else {
           $('header').removeClass("filled");
        }
    });
});

I would write this like:

$(function(){

  var header = $('header');
  var threshold = header.offset().top;

  $(document).scroll(function(){
      header.toggleClass('filled', $(this).scrollTop() - 10 > threshold);
  });

});


Python script using distutils to copy files on a Mac I have a Python script I have written to copy files to a mounted Windows SMB share on a Mac.

import os
import distutils.core
# Create a local site for the mount to reside
directory = "/Users/username/share"
if not os.path.exists(directory): os.makedirs(directory)
# Mount the Windows smb share
os.system("mount_smbfs //service.account:password@server.domain.com/share ~/share")
# set source and destination
fromDirectory1 = "/Volumes/Macintosh HD/folder1/folder2/folder3"
fromDirectory2 = "/Volumes/Macintosh HD/folder4/folder5/folder6"
fromDirectory3 = "/Volumes/Macintosh HD2/folder1/folder2"
fromDirectory4 = "/Volumes/Macintosh HD2/folder1/folder3/folder4"
toDirectory1 = "/Users/username/share/folder3"
toDirectory2 = "/Users/username/share/folder6"
toDirectory3 = "/Users/username/share/folder2"
toDirectory4 = "/Users/username/share/folder4"
#Do the copying
distutils.dir_util.copy_tree(fromDirectory1, toDirectory1)
distutils.dir_util.copy_tree(fromDirectory2, toDirectory2)
distutils.dir_util.copy_tree(fromDirectory3, toDirectory3)
distutils.dir_util.copy_tree(fromDirectory4, toDirectory4)
#Unmount the share
os.system("umount ~/share")

I understand the script is verbose but I wrote it this way to # out lines to problem solve. Can you suggest a cleaner way to write it?

Any insights gratefully received.

Version 2:

import os
import distutils.core

# Create a local site for the mount to reside
directory = "/Users/username/share"
if not os.path.exists(directory): os.makedirs(directory)

# Mount the Windows smb share
os.system("mount_smbfs //service.account:password@server.domain.com/share ~/share")

# set source and destination
copy_jobs = []
copy_jobs.append({"from": "/Volumes/Macintosh HD/folder1/folder2/folder3", "to": "/Users/username/share/folder3"})
copy_jobs.append({"from": "/Volumes/Macintosh HD/folder4/folder5/folder6", "to": "/Users/username/share/folder6"})
copy_jobs.append({"from": "/Volumes/Macintosh HD2/folder1/folder2", "to": "/Users/username/share/folder2"})
copy_jobs.append({"from": "/Volumes/Macintosh HD2/folder1/folder3/folder4", "to": "/Users/username/share/folder4"})

#Do the copying
for job in copy_jobs:
    distutils.dir_util.copy_tree(job["from"], job["to"])

# Unmount the Windows smb share
os.system("umount ~/share")

The code looks OK. A few specific improvements can be made.

Readability

import os
import distutils.core

Learn about PEP8, you should have two empty lines between imports and the rest of the code. More generally, you should give more structure to your code to make it more readable. Empty lines are one way to achieve that. For example, one empty line before each comment could be nice.

# Create a local site for the mount to reside
directory = "/Users/username/share"
if not os.path.exists(directory): os.makedirs(directory)

PEP8 comment: prefer indenting ifs to make clear they are ifs.

"Security"

# Mount the Windows smb share
os.system("mount_smbfs //service.account:password@server.domain.com/share ~/share")

Don't store account and password in your source file. You could move them to a configuration file (that would not be tracked if you used git or svn) or retrieve them from the environment (but then they would show up in your shell logs).

Don't Repeat Yourself

# set source and destination
fromDirectory1 = "/Volumes/Macintosh HD/folder1/folder2/folder3"
fromDirectory2 = "/Volumes/Macintosh HD/folder4/folder5/folder6"
fromDirectory3 = "/Volumes/Macintosh HD2/folder1/folder2"
fromDirectory4 = "/Volumes/Macintosh HD2/folder1/folder3/folder4"
toDirectory1 = "/Users/username/share/folder3"
toDirectory2 = "/Users/username/share/folder6"
toDirectory3 = "/Users/username/share/folder2"
toDirectory4 = "/Users/username/share/folder4"

Variables name var1, var2 and so on indicate that you should use a list of pairs or a dictionary!

directories = [
    ("/Volumes/Macintosh HD/folder1/folder2/folder3", "/Users/username/share/folder3"),
    ("/Volumes/Macintosh HD/folder4/folder5/folder6", "/Users/username/share/folder6"),
]

#Do the copying
for from, to in directories:
    distutils.dir_util.copy_tree(from, to)

#Unmount the share
os.system("umount ~/share")

A note about the 80-char limit: in this case, the directories list is more readable if you don't enforce the rule.

Recovering from failures

What happens when mounting works but not copying? You should still try to unmount your folder.

try:
    # mount
    # copy
finally:
    # umount

You can also decide to catch exceptions if you know what to do about them.


Rewriting this function to use `promises` instead of `async: false` Using parse.com's javascript API, I would like to get a variable number or images from img tags on a page and save those images in the currently logged in user's user object.

To my knowledge, parse cannot store image files directly from a url so I am using ajax to send each url to a php page that converts it and returns a base64 encoded string

Of course, left unchecked, the code would continue to run and save the user object before the php page returns the encoded images.

As a quick and dirty solution I set async: false which does work but of course give the warning: synchronous XMLHttpRequest on the main thread is deprecated because of its detrimental effects to the end user's experience.

How can I re-write the below to use promises instead of async: false?

Or, should I gather up all of the image urls, send them in one ajax call, encode each, send them all back, and move the save user code into the success function of the ajax call?

function accountCreation(){

        var user = Parse.User.current(); // get the currently logged in user object

        // loop through each image element
        $('.images').each(function(index, element) {
              // first we'll need to get the image source
              var src = $(element).attr('src');
              // alone, the source is just a url and not of much use to us
              // we'll need to take the source and create a base64 encoded string from it
              // we'll use an ajax call to a small php script (shown below) to do this
              // it might be better to gather up each src url and send them all in one ajax call??
              var encodedImg;
              try {
                $.ajax({
                   type: "POST",
                   url: "encodeImgToBase64.php", // you may need to adjust this path depending on where you store the file in relation to this file
                   data: 'url='+src,
                   success: function(data){
                     encodedImg = data; // encodedImg is now a base64 encoded string
                     // if I did send all the image urls at once and return all the encoded strings in one response, I guess I could just move the rest of the below code into this success function and be done with it
                   },
                   async: false
                   // note the use of `async: false` here
                   // this works but gives the warning:
                   // synchronous XMLHttpRequest on the main thread is deprecated because of its detrimental effects to the end user's experience.
                   // I know I should be using a promise here but Im not sure how to incorporate it
                 });
                var file = new Parse.File("photo.jpg", { base64: encodedImg }); // this part actually saves the image file to parse
                user.set("image"+index,file); // set this image to the corosponding column on our object
              } catch (e) {
                console.log('failed to create file, src= '+src+', encoded img='+encodedImg)
              }
        });
        // save the user object
        user.save(null,{
          success: function(user) {
              // do success stuff...
          },
          error: function(user, error) {
              // do fail stuff...
          }
        });
 }

Here is the php code from encodeImgToBase64.php

<?php
if (isset($_POST['url']) ){
    echo base64_encode(file_get_contents($_POST['url']));
}else{
    echo 'Error: No image to encode';
}
?>

I don't know Parse() myself, but this should show you the general idea for using promises with jQuery Ajax:

function accountCreation(){
    var user = Parse.User.current(); // get the currently logged in user object

    // loop through each image element
    var promises = $('.images').map(function(index, element) {
        var src = $(element).attr('src');
        return $.ajax({
            type: "POST",
            url: "encodeImgToBase64.php", // you may need to adjust this path depending on where you store the file in relation to this file
            data: 'url='+src,
        });
    });
    $.when.apply($, promises).then(function() {
        // arguments[0][0] is first result
        // arguments[1][0] is second result and so on
        for (var i = 0; i < arguments.length; i++) {
            var file = new Parse.File("photo.jpg", { base64: arguments[i][0] }); // this part actually saves the image file to parse
            user.set("image" + i, file); // set this image to the corosponding column on our object
        }
        // save the user object
        user.save(null,{
          success: function(user) {
              // do success stuff...
          },
          error: function(user, error) {
              // do fail stuff...
          }
        });
    });
 }

Here's a description of how the code works:

$('.images').map() cycles through all the images, calling a custom function for each item.  The return value from the custom callback is added to an array and the final result of the .map() call is an array of all the returned values.

return $.ajax() returns from the .map() callback, the promise that $.ajax() returns.  So, when the .map() is done, it will have an array of ajax promises.

$.when.apply($, promises) passes the array of promises to $.when() which then waits until all the promises are resolved (e.g. all the ajax calls have completed) before calling the .then() handler.

In the .then() handler, the results of all the ajax calls are passed as arguments.  Each argument is an array of length three.  [0] in each array is the returned data from the ajax call.  [1] is the status from the ajax call, [2] is the jqXHR object from the ajax call.  In this case, you just need [0] because we already know the status.

So, then .then() handler cycles through all the arguments object passed to the .then() handler with one array passed as an argument for each ajax call that was executed and the [0] in each of those arrays is the ajax result.

Then, after processing all the ajax results, user.save() is called.

FYI, when looking at the Parse documentation, user.save() looks like it also returns a promise and new Parse.File() looks like it has a .save() method (not sure if that needs to be called).


Cleaning up code and making better loops for soccer shoot-out game I'll start by saying I've just started with programming, and Ruby is my first language. I've made a little soccer shoot-out game. I'm having problems with the end of the code looping back to where I want it.

If there's a tie, I want it to keep the same score and players, but add another round of shooting. Should I do this by creating an array to store the points?

If there's anything else I can simplify, I'd really appreciate your input.

class Shoot
def initialize
    shoot
end

def shoot
    @kick = 1 + rand(6)
end

def kick
    @kick
end
end

response = ''
until response == 'N'
play = "Have another shoot-out? Put Y or N."

puts "Player 1's name is:"
player_1_name = gets.chomp

puts "Player 2's name is?"
player_2_name = gets.chomp

puts "#{player_1_name} and #{player_2_name}, are you ready to play sudden death   (ENTER)?!"
gets.chomp

p1_score = 0
p2_score = 0
result_1 = 0
result_2 = 0

until result_1 >= 1 || result_2 >= 1
  puts "#{player_1_name}, please take your kick (ENTER)."
  gets.chomp!
  shot_1 = Shoot.new.kick
  result_1 = shot_1*0.2
  puts "#{player_1_name} takes the shot, and it's looking good..."
  puts "(ENTER)"
  gets.chomp

  if result_1 >= 1
      puts "...And it's a goal!!!"
      p1_score += 1
  else
      puts "...Oh! So close, but it's no good!!!"
  end

  puts "#{player_2_name}, please take your kick (ENTER)."
  gets.chomp!

  shot_2 = Shoot.new.kick
  result_2 = shot_2*0.2
      puts "#{player_2_name} takes the shot, and it's looking good..."
  puts "(ENTER)"
  gets.chomp

  if result_2 >= 1
      puts "...And it's a goal!!!"
      p2_score += 1
  else
      puts "...Oh! So close, but it's no good!!!"
  end

puts "#{player_1_name}: #{p1_score}"
puts "#{player_2_name}: #{p2_score}"

if p1_score > p2_score
    puts "#{player_1_name}, you're the winner! Congratulations!"
elsif p1_score < p2_score
    puts "#{player_2_name}, you're the winner! Congratulations!"
else
    puts "Looks like we've gotta go another round!"
    return
end
end

puts play
response = gets.chomp
end

Not a bad start. However here's a few things that immediately pops out when I look at the code.

Avoid duplication: The code for both players to kick is exactly the same. You should try to move that into a function instead of writing it out twice.
Make a player class: By moving all the code for a player into a class, you avoid having to keep each variable twice. Instead of shot_1/shot_2, result_1/result_2 etc, you could have player[0].shot, player[0].result etc.
Make a game class: While you're at it, why not also turn the game itself into a class. I think this would solve your looping problem all by itself.

Something like this:

class Game
  def initialize
    # set up for a new game
  end

  def play_round
    # play one round of the game
  end
end


Parabola calculations I'm trying to make a small parabola calculator just for fun. Is this code good?

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

#define _X_UNTIL_Y(q,w)          \
    while (argv[1][q] != w) {    \
        q++;                     \
    }

#define _ATOI(s,d,f,g)           \
    strncpy(s, argv[1]+f, g);    \
    d = atoi(s)

int main(int argc, char const *argv[]) {
    int i=0, j, l;
    float D, a=1, b=1, c=1, r, k;
    char A[5], B[5], C[5], d;

    if(argv[1][0] == 'x');
    else{
        _X_UNTIL_Y(i,'x');
        _ATOI(A,a,0,i);
    }
    if(argv[1][0] == '-' && argv[1][1] == 'x') a=-1;

    j = i += 3;
    _X_UNTIL_Y(j,'x');
    if(argv[1][j-1] == '+' || argv[1][j-1] == '-');
    else{
        _ATOI(B,b,i,j);
    }
    if(argv[1][j-1] == '-') b=-b;

    l = j += 1;
    _X_UNTIL_Y(l,'=');
    _ATOI(C,c,j,l);

    D = pow(b,2)-4*a*c;

    if(D < 0){
        d = 'n';
        printf("Parabola doesn't have real roots.\n");
    }else if(D == 0){
        d = 's';
        printf("Parabola has one real root.\n");
    }else if(D > 0){
        d = 'p';
        printf("Parabola has two real roots.\n");
    }else
        return -1;

    r = -(b/(2*a));
    k = -(D/(4*a));

    a < 0 ? printf("Parabola look downwards.\n") : printf("Parabola look upwards.\n");
    printf("Vertex: %.1f,%.1f\n", r, k);
    printf("Axis of symmtery: %.1f\n", r);
    printf("y-intercept: %.1f\n", c);

    if(d=='n');
    else if(d=='s')
        printf("x-intercept: %.1f\n", r);
    else if(d=='p'){
        float x1, x2;
        x1=(-b + sqrt(D)) / (2*a);
        x2=(-b - sqrt(D)) / (2*a);
        printf("x-intercepts: %.1f, %.1f\n", x1, x2);
    }
    return 0;
}

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

Avoid macros

The macros _X_UNTIL_Y and _ATOI do not help with readability. I'd advise not using macros like those, preferring functions instead.

Use more descriptive variable names

All of the single-letter variable names make it difficult to decipher the algorithm.  More meaningful variable names make the code easier to read and maintain.

Break up the code into smaller functions

Rather than having everything in one long main function, it would be easier to read and maintain if each discrete step were its own function.

Think of the user

If the user runs the code with no arguments, he or she is rewarded with a segfault and crash.  It would be much nicer instead to prompt the user with a string describing how the program is intended to be used instead.

Use idiomatic C

Lines like these:

if(argv[1][0] == 'x');
else{

are quite odd.  It would make it much easier to read and understand if it were written instead like this:

if(argv[1][0] != 'x') {

Add comments

Code like this is difficult enough to read for the reasons listed above.  Just a few well-placed comments about why the code is doing what it's doing would greatly aid readers of the code to understand your intent.

Eliminate return 0 at the end of main

Since C99, the compiler automatically generates the code corresponding to return 0 at the end of main so there is no need to explicitly write it.


Contacts programming challenge I successfully solved a problem on Hackerrank called Contacts, but I'm curious if my code in C++ looks good to C++ developers. If not, how it should be improved.
Below is the short description of the problem and then the code itself.
Problem

Add name, where name is a string denoting a contact name. This must store name as a new contact in the application.
Find partial, where partial is a string denoting a partial name to search the application for. It must count the number of contacts starting with partial and print the count on a new line.

So, given sequential add and find operations, perform each operation in order.
Solution
Use a Trie like structure to solve the problem. All the children of a node have a common prefix of the contact associated with that parent node, and the root is associated with the empty string.
Code
struct node {
    uint32_t count = 0;
    std::unordered_map<char, std::unique_ptr<node>> childrenMap;
};

/*
 * Complete the contacts function below.
 */
std::vector<uint32_t> contacts(std::vector<std::vector<std::string>>& queries) {
    std::vector<uint32_t> results;

    node root;
    for (const auto& row : queries) {
        const auto operation = row[0];
        const auto name = row[1];

        if (operation == "add") {
            auto* parent = &root;
            for (auto& ch : name) {
                auto& childrenMap = parent->childrenMap;
                if (!childrenMap.count(ch)) {
                    childrenMap.emplace(ch, std::make_unique<node>());
                }
                childrenMap[ch]->count++;
                parent = childrenMap[ch].get();
            }
        } else {
            auto* parent = &root;
            auto result = 0;
            for (auto& ch : name) {
                auto& childrenMap = parent->childrenMap;
                if (!childrenMap.count(ch)) {
                    result = 0;
                    break;
                }
                parent = childrenMap[ch].get();
                result = childrenMap[ch]->count;
            }
            results.push_back(result);
        }
    }

    return results;
}

The Trie approach might be overkill for this question.  I think that a plain (ordered) std::set (or std::multiset) would be adequate.  The set's lower_bound() member takes you directly to the first entry with the specified substring.  From there, we can either count linearly, or use lower_bound again with a new key created by adding 1 to the last character, then just return the iterator difference.
I'd certainly consider writing separate add() and find() functions instead of plonking all the code in that big if/else.  If nothing else, it makes the unit tests easier to write.


Multiply a square matrix with only the diagonal and antidiagonal elements different from zero in vector So I have a square matrix with the diagonal and antidiagonal elements different from zero in a vector
vector<int> matrix = {1,2,3,4,5,6,7,8};

The matrix:
1 0 0 5
0 2 6 0
0 7 3 0
8 0 0 4

I try to multiply this with another matrix
xmatrix is the class for matrices
I tried like this:
void xmatrix::operator*(const xmatrix& mat)
{
    if(matrixSize != mat.getmatrixSize()) throw "Diff size";
    int x;
    for(int i = 0;i < matrixSize;i++){
        for(int j = 0;j < matrixSize;j++){
            x = 0;
            for(int k = 0;k < matrixSize;k++){
                x += getElement(i,k) * mat.getElement(k,j);
            }
            setElementMul(i,j,x);
        }
    }
}

The var and func names clearly says it all
ATM this code works fine with the first half of the matrix (multiply with itself)
41 0 0 225
0 46 294 0
0 35 219 0
40 0 0 216

what other ways can work fine or how should i edit this code

I believe your issue is that you're altering your matrix (the multiplicand) as you are performing the multiplication. This is why part of the result is correct while the other part is incorrect. For example, the top-right cell (which should be 25) is evaluated from \$ 41*5+5*4=225\$ where the 41 (which should be 1) is used because you've prematurely set that cell from an earlier iteration.
A simple way to fix this could be to use an intermediary matrix or buffer during the calculation. However, if your xmatrix class represents a matrix that can only be diagonal, anti-diagonal, or both, then you can use the properties of diagonal matrices to optimize and simplify your class.
If this is indeed the case, then you can simply represent the entire matrix by its diagonal and anti-diagonal components:
vector<int> diagonal;
vector<int> antidiag;

For the anti-diagonal component, I'll refer to your example and use a top-right to bottom-left orientation (so the top-right cell will be first and the bottom-left cell will be last).
We can now optimize the multiplication operation by noting a few properties of diagonal matrices. Consider a diagonal matrix \$ A_x \$ and an anti-diagonal matrix \$ A_y \$, both of size \$ n \$. Let these be the anti-diagonal and diagonal components of an \$ n*n \$ matrix \$ A \$. We can then represent \$ A \$ such that \$ A = A_x+A_y \$. However, this representation requires special care when \$ n \$ is odd because of the common middle element. When such a case occurs, we'll absorb the middle element into the diagonal component and zero out the middle element in the anti-diagonal component. So now we have our multiplicand matrix \$ A \$, we use a similar notation to represent the multiplier matrix \$ B \$ such that \$ B = B_x+B_y \$. Using a few properties of matrices, we can show that the multiplication of these matrices is a sum of diagonal and anti-diagonal components:
$$
AB=(A_x+A_y)(B_x+B_y)=A_xB_x+A_xB_y+A_yB_x+A_yB_y\\=(A_xB_x+A_yB_y)+(A_xB_y+A_yB_x)\\=(\text{[diagonal]})+(\text{[anti-diagonal]})
$$
Now that we've worked through the math, we can use this result to implement an optimized multiplication algorithm with a time complexity of \$ O(n) \$.
#include <iostream>
#include <vector>

class XMatrix
{
public:
    XMatrix(const std::vector<int>& diagonal, const std::vector<int>& antidiag)
     : diagonal(diagonal), antidiag(antidiag), size(diagonal.size())
    {
        if (diagonal.size() != antidiag.size())
            throw;

        if (this->size % 2 != 0) {
            this->antidiag[this->size / 2] = 0;
        }
    }
/*
...
*/

    XMatrix operator*(const XMatrix& m)
    {
        if (this->size != m.size)
            throw;

        std::vector<int> result_diagonal(this->size);
        std::vector<int> result_antidiag(this->size);

        for (int i = 0; i < this->size; ++i) {
            result_diagonal[i]
             = this->diagonal[i] * m.diagonal[i] +
               this->antidiag[i] * m.antidiag[m.size-i-1];
            result_antidiag[i]
             = this->diagonal[i] * m.antidiag[i] +
               this->antidiag[i] * m.diagonal[m.size-i-1];
        }

        return XMatrix(result_diagonal, result_antidiag);
    }

    void print() const
    {
        for (int row = 0; row < this->size; ++row) {
            for (int col = 0; col < this->size; ++col) {
                std::cout << '\t';
                if (row == col)
                    std::cout << this->diagonal[row];
                else if (row == (this->size - col - 1))
                    std::cout << this->antidiag[row];
                else
                    std::cout << '0';
            }
            std::cout << '\n';
        }
    }

private:
    int size;
    std::vector<int> diagonal;
    std::vector<int> antidiag;
};

int main(int argc, char *argv[])
{
    XMatrix matrix_A({1,2,3,4},{5,6,7,8});
    XMatrix matrix_B = matrix_A * matrix_A;
    matrix_B.print();

    return 0;
}

I've altered some things compared to your original code for the purpose of illustrating everything within this response. This may be a bit more than you were asking for regarding why your matrix multiplication was only half working (if that's all you were concerned about then feel free to disregard everything after the first two paragraphs), but in any case I hope I was able to help.


Inserting customer care database analysis detail records I have written this linq-to-sql query to insert a bunch of customer care database analysis detail records:

DataClasses1DataContext behzad = new DataClasses1DataContext(conn);
            var query = (from p in behzad.Customer_Care_Database_Analysis_TEMPs
                         select p).ToArray();

            for (int i = 0; i < query.Count(); i++)
            {
                Customer_Care_Database_Analysis_DETAIL custom_detail = new Customer_Care_Database_Analysis_DETAIL();
                custom_detail.code_faaliyat = query[i].code_faaliyat.Trim();
                custom_detail.code_markaz = query[i].code_markaz.Trim();
                custom_detail.code_switch = query[i].code_switch.Trim();
                custom_detail.ellate_xata = query[i].ellate_xata.Trim();
                custom_detail.name_faaliyat = query[i].name_faaliyat.Trim();
                custom_detail.name_markaz = query[i].name_markaz.Trim();
                custom_detail.name_switch = query[i].name_Switch.Trim();
                custom_detail.noe_moshtarak = query[i].haq_hoq.Trim();
                custom_detail.shomare_tel = query[i].phone_number.Trim();

                behzad.Customer_Care_Database_Analysis_DETAILs.InsertOnSubmit(custom_detail);
                behzad.SubmitChanges();

            }

The code works, but it can take a long time - more than 10 minutes for my data.

How can I speed this up?

With ToArray() you create an in-memory copy of all queried data. If you have 10m records this is incredibly expensive (both for CPU and memory). Avoid that and fetch data from DB each time you use it without needless copies. See also this question post for further details.

First change it to:

var query = from p in behzad.Customer_Care_Database_Analysis_TEMPs select p;

Your query does nothing then you may simply drop it if you don't need to perform any filtering.
Instead of for use foreach and do not (re)count collection each time. Even if LINQ should be smart enough (it was an array and it may cast and directly access .Length property) it's useless (see also Eric Lippert's answer about purpose) and with a pure enumeration it'll hit database with SELECT COUNT(*) ... multiple times.

foreach (var temp in behzad.Customer_Care_Database_Analysis_TEMPs)
{
    var detail = new Customer_Care_Database_Analysis_DETAIL();
    detail.code_faaliyat = temp.code_faaliyat.Trim();
    detail.code_markaz = temp.code_markaz.Trim();

    // And so on...

    behzad.Customer_Care_Database_Analysis_DETAILs.InsertOnSubmit(detail);
    behzad.SubmitChanges();
}

Now you should move SubmitChanges() outside your loop, this is 2nd big step to improve performance. Start searching from this question but there are many (better) posts out there about this topic. See also this question.

foreach (var temp in behzad.Customer_Care_Database_Analysis_TEMPs)
{
    var detail = new Customer_Care_Database_Analysis_DETAIL();
    detail.code_faaliyat = temp.code_faaliyat.Trim();
    detail.code_markaz = temp.code_markaz.Trim();

    // And so on...

    behzad.Customer_Care_Database_Analysis_DETAILs.InsertOnSubmit(detail);
}

behzad.SubmitChanges();

Performance impact of removing multiple array item deferencing and of introducing enumeration is absolutely negligible in this case.
Note that you may also partition your inputs. This is something you should profile for your specific case but you may get better performance working on batches of input data.

You may find many good implementations of Partition() function (also on Stack Overflow, for example or one of linked duplicates) then here I just provide pseudo-code for that:

const int batchSize = 100;
var batches = Partition(behzad.Customer_Care_Database_Analysis_TEMPs, batchSize);

foreach (var batch in batches)
{
    foreach (var temp in partition)
    {
        var detail = new Customer_Care_Database_Analysis_DETAIL();
        detail.code_faaliyat = temp.code_faaliyat.Trim();
        detail.code_markaz = temp.code_markaz.Trim();

        // And so on...

        behzad.Customer_Care_Database_Analysis_DETAILs.InsertOnSubmit(detail);
    }

    behzad.SubmitChanges();
}


Fast line-by-line file reader This is designed for a high performance complex log analyzer. Very simple idea: read a file line-by-line as fast as possible.

I would appreciate any hints what should/could be improved in this code.

GitHub

FastLineReader.h

/* Copyright (c) 2015 Simon Toth kontakt@simontoth.cz
 * Lincensed under the MIT license: http://opensource.org/licenses/MIT
 */

#ifndef FASTLINEREADER_H
#define FASTLINEREADER_H

// STD C++
#include <iosfwd>

/** Quick line-by-line parser of text files for POSIX/Linux
 *
 *  This function provides a fast line parser with a callback model.
 *
 * @param filename file to be parsed
 * @param callback function that will be called for each line
 * @returns 0 on success, -1 if file could not be opened
 **/
int fastLineParser(const char * const filename, void (*callback)(const char * const, const char * const));

#endif // FASTLINEREADER_H

FastLineReader.cpp

#include "FastLineReader.h"

// POSIX
#include <sys/types.h>
#include <sys/mman.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <unistd.h>
#include <errno.h>

// C++ STD
#include <iostream>
#include <cstring>
using namespace std;

int fastLineParser(const char * const filename, void (*callback)(const char * const, const char * const))
{
    int fd = open(filename, O_RDONLY); // open file
    if (fd == -1)
    {
        cerr << "Could not open \"" << filename << "\" for reading (" << strerror(errno) << ")." << endl;
        return -1;
    }

    struct stat fs;
    if (fstat(fd, &fs) == -1)
    {
        cerr << "Could not stat \"" << filename << "\" for reading (" << strerror(errno) << ")." << endl;
        close(fd);
        return -1;
    }

    posix_fadvise(fd,0,0,1); // announce the desire to sequentialy read this file
    // silent error handling - weak error

    char *buf = static_cast<char*>(mmap(0, static_cast<size_t>(fs.st_size), PROT_READ, MAP_SHARED, fd, 0));
    if (buf == MAP_FAILED)
    {
        cerr << "Could not memory map file \"" << filename << "\" (" << strerror(errno) << ")." << endl;
        close(fd);
        return -1;
    }

    char *buff_end = buf + fs.st_size;
    char *begin = buf, *end = NULL;

    // search for newline in the remainder in the file
    while ((end = static_cast<char*>(memchr(begin,'\n',static_cast<size_t>(buff_end-begin)))) != NULL)
    {
        callback(begin,end);

        if (end != buff_end)
            begin = end+1;
        else
            break;
    }

    // enable if you are working with malformed text files, proper text file needs to end with a newline
#ifdef MALFORMED_TEXFILE
    callback(begin,buff_end);
#endif

    munmap(buf, static_cast<size_t>(fs.st_size));
    // silent error handling - weak error

    close(fd);
    return 0;
}

posix_fadvise conveniently provides a POSIX_FADV_SEQUENTIAL macro. Use it instead of a magic 1 and a comment.
The client doesn't know in advance whether the file is malformed or not. Better detect a malformed text file in run time:

    if (begin != buf_end)

A Bugs section of posix_fadvise man page says that

  In kernels before 2.6.6, if len was specified as 0, then this was interpreted literally as "zero bytes", rather than as meaning "all bytes through to the end of the file".

Since you already know the file size, better be safe and call it with fs.st_size instead of 0.
Move #include <iosfwd> to the cpp file. The client code doesn't need it.
fastLineParser can be used in C code; just declare it as extern "C"
I see no reason to use C++ here at all. However, if you do so, do not use namespace std.
Finally, do you have any evidence that this is faster than fgets?


Finding the Nth occurrence of character in string I wrote an SQL Server function which returns the substring before the Nth occurrence of a character.

For example:

SELECT dbo.fn_getFirstNthSentence('.', 'hello world.It.is.raining.today', 3)

returns 'hello world.It.Is.' as a result.

The function I wrote looks dirty and slow so I want to optimize it. Any advice to make it clean is appreciated.

CREATE FUNCTION fn_getFirstNthSentence
(
  @TargetStr VARCHAR(MAX) ,
  @SearchedStr VARCHAR(8000) ,
  @Occurrence INT
)
RETURNS varchar(MAX)
AS
BEGIN

    DECLARE @pos INT ,
        @counter INT ,
        @ret INT;

    SET @pos = CHARINDEX(@TargetStr, @SearchedStr);

    IF ( @pos = 0 )
        RETURN @SearchedStr

    SET @counter = 1;

    IF @Occurrence = 1
        SET @ret = @pos;

    ELSE
        BEGIN

            WHILE ( @counter < @Occurrence )
                BEGIN

                    IF(LEN(@SearchedStr) < @pos + 1)
                        RETURN @SearchedStr

                    SELECT  @ret = CHARINDEX(@TargetStr, @SearchedStr,
                                             @pos + 1);
                    IF(@ret = 0)
                        RETURN @SearchedStr
                    SET @counter = @counter + 1;
                    SET @pos = @ret;
                END;
        END;
    RETURN LEFT(@SearchedStr, @ret)
END;

It's not necessary to set the return string @ret all the time. You can first look for occurrences of @TargetStr with an incrementing start_location parameter of the CHARINDEX function. Only after no (new) occurrences are found, or when the required max number of occurrences are found is it time to set the return string:

CREATE FUNCTION fn_getFirstNthSentence
(
  @TargetStr VARCHAR(MAX) ,
  @SearchedStr VARCHAR(8000) ,
  @Occurrence INT
)
RETURNS varchar(MAX)
AS
BEGIN
    DECLARE @counter INT = 0
          , @index INT = 0
          , @newIndex INT
          , @end BIT = 0;

    WHILE(@end = 0 AND @counter < @occurrence)
    BEGIN
        SET @counter = @counter + 1;
        SET @newIndex = CHARINDEX(@TargetStr, @SearchedStr, @index + 1);
        IF (@newIndex > 0)
            SET @index = @newIndex;
        ELSE
            SET @end = 1;
    END
    IF @index > 0 SET @index = @index - 1;
    RETURN LEFT(@SearchedStr, @index)
END

Note that this returns an empty string if no occurrences are found. I think that's more correct for a function that returns "everything until the nth occurrence of x".


Receiving an language string from file I'm receiving an language string out of an PHP file which returns an array like this:

DIR_LANG\en\home.php:

return array(

    'welcome_string'    => 'welcome @ home! '

);

And my Lang class:

<?php

class Lang
{

    private static $langArray = array();
    private static $lang = DEFAULT_LANG;

    public static function get($langString)
    {
        if (isset(self::$langArray[self::$lang]) && isset(self::$langArray[self::$lang][$langString])) {
            return self::$langArray[self::$lang][$langString];
        } else {
            $langUsed = self::$lang;
            self::receiveLang($langString);

            if (!isset(self::$langArray[$langUsed][$langString])) {
                if ($langUsed != DEFAULT_LANG) {
                    $langUsed = DEFAULT_LANG;
                    self::receiveLanguage($langString, DEFAULT_LANG);
                } else {
                    $langUsed = FALLBACK_LANG;
                    self::receiveLanguage($langString, FALLBACK_LANG);
                }
            }
            return (isset(self::$langArray[$langUsed][$langString]) ? self::$langArray[$langUsed][$langString] : $langString);
        }
    }

    private static function receiveLang($langString)
    {
        $langStringExp = explode('.', $langString);
        $usedLang = self::$lang;
        $langFile = DIR_LANG . $usedLang . '/' . $langStringExp[0] . '.php';

        if (!file_exists($langFile)) {
            $usedLang = ($usedLang != DEFAULT_LANG) ? DEFAULT_LANG : FALLBACK_LANG;
            $langFile = DIR_LANG . $usedLang . '/' . $langStringExp[0] . '.php';

            if (!file_exists($langFile)) {
                return;
            }
        }
        $langFileContents = require_once $langFile;

        foreach ($langFileContents as $key => $value) {
            self::$langArray[$usedLang][$langStringExp[0] . '.' . $key] = $value;
        }
    }

    private static function receiveLanguage($langString, $language)
    {
        $langStringExp = explode('.', $langString);
        $langFile = DIR_LANG . $language . '/' . $langStringExp[0] . '.php';

        if (!file_exists($langFile)) {
            return;
        }

        $langFileContents = require_once $langFile;

        foreach ($langFileContents as $key => $value) {
            self::$langArray[$language][$langStringExp[0] . '.' . $key] = $value;
        }
    }

    public static function setLang($newLang)
    {
        self::$lang = $newLang;
    }
}

And somewhere in my view, I receive the language like this:

echo Lang::get('home.welcome_string');

The constants are defined in an config.php. Here's an stripdown of it:

define('DIR_LANG', 'app/language/');
define('DEFAULT_LANG', 'nl');
define('FALLBACK_LANG', 'en');

Although this do work, it feels a bit bloated. What could I do to improve this, or is this the best way (I can't believe it)

Before presenting my solution, I would like to quickly review your code. The first thing that popped into my mind was the extensive use of the keyword static, which I will address later together with your use of constants.

When looking over your code I have a hard time determining which variables and methods are different. All your variables start with the same characters. Your methods receiveLang() and receiveLanguage() easily creates confusion. What is it one does which the other don't? You also has a "silly assignment" where you write: $langUsed = self::$lang;. Since self::$lang is never changed you could use the static property and save a local variable. With that said I think your overall code looks good and your indentation is good. You could give more space and blank lines to increase readability though.

Pro Tip:

When using isset() you can reduce some of your conditionals to half the length. The difference between array_key_exists() and isset() is that isset() keeps going through keys even with multi-dimensional arrays. This means you could as an example reduce your first if condition in get() from:

if (isset(self::$langArray[self::$lang]) && isset(self::$langArray[self::$lang][$langString])) {...

to:

if(isset(self::$langArray[self::$lang][$langString])) {...

Now I would like to ask a question: why is your class completely static?. I know it's easier to interact with, but it also make testing your class harder.

I like your idea of using a file for each supported languages as it keeps concerns clear. But your use of constants that are defined outside your class, but is required lights some red warning signals to me. Consider me (another developer) or yourself in six months using this class. I could deduce that I need to provide a default path to languages files, but the default languages as well as the fallback language itself isn't so evident.

Considering your language class would only be used in a single view (right?), I would make this an instance class and provide the required constant values as parameters during construction to a __construct() method. Now if I would use your class I have to provide the required information. You would also have the benefit of (in some weird situation) having more than one language in your view as each instance would specify its preferred language.

Making the class an instance of course makes it more complicated to pass around the same instance, but it should be possible with a simple dependency manager.

I have written an alternative implementation of your class. It also implements a fallback language. The difference is that the solution I have written loads the fallback language first and then overwrites any language keys from the selected language. This makes the method for retrieving a language string much easier.

class Language {

    /**
     * @var array $language
     */
    private $language = [];

    /**
     * Create a Language instance.
     *
     * @param string $path     An absolute directory path to language files
     * @param string $used     A string declaring the language to use.
     * @param string $fallback A string declaring the default language.
     */
    public function __construct($path, $use, $default)
    {
        $this->language = $this->prepare($path, $use, $default);
    }

    /**
     * Determine if a language string exists.
     *
     * @param string $key
     *
     * @return bool
     */
    public function exists($key)
    {
        return array_key_exists($key, $this->language);
    }

    /**
     * Fetches the value of a language string associated with
     * the specified key.
     *
     * @param string $key
     *
     * @return string
     */
    public function fetch($key)
    {
        if($this->exists($key)) {
            return $this->language[$key];
        }

        /*
         * Throw an exception (preferred) or trigger an error/warning here; its your choice.
         *
         * I return an empty string to keep the return type consistent. You
         * can return NULL, but you would have to check the returned value
         * each time.
         */
        return '';
    }

    /**
     * Prepares the class by loading specified languages and
     * combining them into a single associative array.
     *
     * @param string $path
     * @param string $specified
     * @param string $fallback
     *
     * @return array
     */
    private function prepare($path, $use, $fallback)
    {
        /*
         * Load entire fallback language contents.
         */
        $language  = $this->load($path, $fallback);
        $overwrite = $this->load($path, $use);

        /*
         * I merge the language file recursively. The fallback language is
         * overwritten by the specified language.
         */
        return array_merge_recursive($language, $overwrite);
    }

    /**
     * Loads a language file and validates the returned contents
     * to an array.
     *
     * @param string $path
     * @param string $language
     *
     * @return array
     */
    private function load($path, $language)
    {
        /*
         * Build language filename.
         * An example format is: root/resources/languages/en.php
         */
        $filename = rtrim($path, '/') . '/' . strtolower($language) . '.php';

        if(!is_file($filename) || !is_readable($filename)) {
            throw new \RuntimeException("Cannot open language file: {$filename}");
        }

        $contents = require $filename;

        if(!is_array($contents)) {
            throw new \LogicException('Invalid language file structure. Expected an array.');
        }

        return $contents;
    }

}

This is a very crude implementation. You could easily build upon this, but the basics are there. You could even increase testability even more by extrating the load() and prepare() methods into a separate class. This class should only be responsible for loading your language files. You would then pass the loaded language as a parameter into the constructor

public function __construct(array $language) {...

Now consider testing this. You can now pass an array without loading any files and still test your class easily.

$array = [
    'welcome' => 'Welcome to this awesome site!'
];

$language = new Language($array);

I hope this can help you, happy coding!


Auto-generated generic data layer I'm trying to make a reusable data layer that is mostly generated from a database via T4 templates. On top of this data layer, I'd like to have WebAPI endpoints for each repository, with most of the basic CRUD operations generated/from a base class. The generics are somewhat getting out of hand, and I don't know if I'm trying to build too much of a swiss army knife here.

Starting at the bottom, I have an entity class (some bits truncated):

public class Entity<T>
{
    public T Id { get; set; }
}

A T4 template runs a SQL statement to pull out database schema, and spits out a file with a bunch of classes along the lines of:

public partial class Visit : Entity<int>
{
   //Fields on the table
}

Generated along side these entity classes are corresponding repositories, all implementing this interface:

public interface IRepository<TEntity, in TKey> where TEntity: Entity<TKey>
{
    Task<IList<TEntity>> GetAll();
    // Other Methods
}

These get filled out with proper SQL statements for the action. Specific repositories can have additional methods as needed.

This bit I'm happy with, it's easy to understand, and the code generators for it aren't complicated. Where it starts getting complicated is when I move up into the WebAPI Controllers.

All data access is handled from a Unit of Work, which handles connection and transaction lifetimes. Currently, there are individual properties for each repository, and this is done via a T4 template:

public partial class RulesWorkUnit : ITransactionWorkUnit
{
    private readonly Lazy<VisitRepository> _Visit;
    public RulesWorkUnit(DbConnectionFactory connectionFactory)
    {
        _Visit = new Lazy<VisitRepository>(() => new VisitRepository(_conn, _tran));
    }

    public VisitRepository Visit { get { return _Visit.Value; } }
}

This part I also like, and is what I've been doing for a bit for data access, and works well for the cases where I'm coding by hand.

Now, here's where it starts going off of the rails a bit. I have a base API Controller class. This needs to know which repository to create, so that goes in as a generic parameter. This parameter needs to go down to the Work Unit so it knows which repository to return, that results in this code added to the work unit:

//Member variable
private Dictionary<Type, Func<dynamic>> _repositoryTypes;

//Added to constructor
_repositoryTypes[typeof(VisitRepository)] = () => new VisitRepository(_conn, _tran);

//New Method
public TRepository GetRepository<TRepository, TEntity, TKey>()
        where TRepository : class, IRepository<TEntity, TKey>
        where TEntity : Entity<TKey>
    {
        return _repositoryTypes[typeof (TRepository)] as TRepository;
    }

This is where the generic definitions start exploding. The base API controller is one worse, since it also needs to know the model to map to:

public abstract class RepositoryApiController<TRepository, TModel, TEntity, TKey> : ApiController
    where TRepository : class, IRepository<TEntity, TKey>
    where TEntity : Entity<TKey>

Which starts setting off my code smell alarms. I would like somehow to manage the boilerplate code for most of the basic API methods (they'll all have a Get All, Get By Id, Post, Put, Delete that have the same code, with the exception of the entity type and the return model type).

Is there a better way of designing this?

It looks like you're basically writing Entity Framework. If so...why not use Entity Framework?

It also looks like in addition to your rewriting of Entity Framework, you're writing the whole generic repository pattern + unit of work pattern that people like to add on top of it. I would strongly recommend against that. It ends up being a lot of extra code and very little or negative benefit, considering that EF/your ORM pretty much uses the repository pattern anyway (if you have a DataContext structured like EF's).

On top of all of that, you're also writing your own custom dependency injection/IOC container now with your static dictionary. The generic type problems you have are based on your unit of work problem. See my answer on this CR post for details.

Instead of writing everything from scratch, I would recommend using vanilla EF with no repository pattern, along with an already-existing IOC container, and wire up DI the plain old vanilla way the Web API site recommends or the way Mark Seemann recommends if you're opinionated about DI.


Updating WPF MainWindow from an async task Is this an acceptable approach to update WPF MainWindow from an async task, using events thrown from a public static class?

In MainWindow.cs, I subscribe to  UI.TaskCompleted event
In UI.cs, I subscribe to AsyncTaskActions.TaskCompleted event

Using this pattern, AsyncTaskActions.TaskCompleted is raised when an async task is completed.  UI catch the event and raise UI.TaskCompleted.  This way, the event is caught in MainWindow where I can use Displacher.Invoke to refresh the displayed page.  The end result is I get a page refresh when a task is completed and the task still runs asychronously.

Application design summary:

MainWindow: main window which can have several different page classes in content area within the main window.
Common.cs  public static class contains a number of common methods through the UI in the application.
AsyncTaskActions.cs - class with a number of async common methods (i.e. download file)

Code snippets:

    public partial class MainWindow
        public MainWindow()
        {
           UI.TaskCompleted += UI_TaskCompleted;
        }

        void UI_TaskCompleted(EventArgs e)
        {
           Dispatcher.Invoke(new Action(this.PageRefresh));
        }

       public void PageRefresh()
       {
           var page = ((ContentArea)).Content;
           if (page == null) return;

            //if (page.GetType().Name == "")
            switch (page.GetType().Name)
            {
                case "SearchPage":
                    ((SearchPage) page).SearchParts();
                    break;
                case "LegoPartPage":
                    ((LegoPartPage) page).LoadData();
                    break;
                case "LegoSetPage":
                    ((LegoSetPage) page).LoadData();
                    break;
                case "MainPage":
                    ((MainPage) page).LoadData();
                    break;
                case "MiniFigPage":
                    ((MiniFigPage) page).LoadData();
                    break;
            }
        }
   }

    public static class UI
    {
       public delegate void TaskComplete(EventArgs e);
       public static event TaskComplete TaskCompleted;

        public static async Task<int> DownloadPriceSummaryAsync(String itemNo, Int64 colorId)
        {
            var wAsyncTaskClasses = new AsyncTaskClasses();
            wAsyncTaskClasses.TaskCompleted += wAsyncTaskClasses_TaskCompleted;
            Task<HtmlDocument> task = wAsyncTaskClasses.DownloadPriceGuide(string.Format(Common.BrickLinkPrice, itemNo, colorId), itemNo, colorId);
            return await  wAsyncTaskClasses.ParsePriceSummaryAsync(task, itemNo, colorId);
        }
 }

public class AsyncTaskActions
{
    public delegate void TaskComplete(object sender, EventArgs e);
    public event TaskComplete TaskCompleted;

    public async Task<int> ParsePriceSummaryAsync(Task<HtmlDocument> task, string itemNo, Int64 colorId)
    {
        return await Task.Run(() => ParsePriceSummary(task, itemNo, colorId));
    }

    public int ParsePriceSummary(Task<HtmlDocument> task, string itemNo, Int64 colorId)
    {

        if (null != TaskCompleted)
        {
            TaskCompleted(this, new EventArgs());
        }

        return recordCount;
    }

I would look into MVVM, then your async task will operate on the ViewModel and not the UI element. Then WPF will update the UI when it deem fit.

I like Caliburn.Micro for the MVVM parts

https://github.com/Caliburn-Micro


Find words in a word grid I made a program to find words in a word grid. It will find words that are horizontal, vertical, positive sloped and vertical sloped, all in both forwards and backwards.
Here is my code:
with open('grid.txt', 'r') as f:
    lst = [[a for a in l.strip() if a != ' ' and a!= '\n'] for l in f.readlines() if l] # Convert file to a nested list, with all whitespaces removed
    lst2 = [a[::-1] for a in zip(*lst)] # Create another nested list that is the first nested list rotated 90 degrees clockwise

height = len(lst) # Height of grid
width = len(lst[0]) # Width of grid

while True:

    word = input("Input your word: ") # Word to find

    # Find horizontal word
    for l in lst:
        w = ''.join(l)
        if word in w or word in w[::-1]:
            print(w)
            print('Horizontal!')

    # Find vertical word
    for l in lst2:
        w = ''.join(l)
        if word in w or word in w[::-1]:
            print(w)
            print('Vertical!')

    # Find Negative slope
    for i in range(width):
        w = ''
        for j in range(height):
            try:
                w += lst[j][i]
                i += 1
            except:
                break
        if word in w or word in w[::-1]:
            print(w)
            print('Negative Slope!')

    for i in range(height):
        w = ''
        for j in range(width):
            try:
                w += lst[i][j]
                i += 1
            except:
                break
        if word in w or word in w[::-1]:
            print(w)
            print('Negative Slope!')

    # Find positive slope
    for i in range(height):
        w = ''
        for j in range(width):
            try:
                w += lst2[j][i]
                i += 1
            except:
                break
        if word in w or word in w[::-1]:
            print(w)
            print('Positive Slope!')

    for i in range(width):
        w = ''
        for j in range(height):
            try:
                w += lst2[i][j]
                i += 1
            except:
                break
        if word in w or word in w[::-1]:
            print(w)
            print('Positive Slope!')

Here is my grid.txt:
n x b z t a ê s e y m t w u e r r
y s j q l n o i l f k j c y h t a
q x n e z d l g v r x y x a p u a
b b k à a y e g q i h u x b q e u
m h z x r é q m x m e l t i b t q
i k i g c j c h w l w i ï t p u o
i s o w u w m a p z a b w g d e p
i p s g u y n n j i j f o f n w l
l i u l ô i e j b e o t s e b c j
y m f v f e v h f c p r d m y v n
f w h v d t z y i x t e e u v n e
e w x e h l x n q m s n p z e f b
e j l v r x ç t d b i a z v d h z
c i i n s s i b s t o q o d d k x
z o f c a w b j b l m w k f e i s

I feel like my code is unnecessarily long for a rather simple task.
Are there any ways to shorten and improve it?

Catch invalid input
When I was trying out your code, I entered Cntrl + Z which crashed the program. Using Try and Except can prevent this from happening
try:
    word = input("Enter your word:")
except Exception:
    print("Invalid input!")

Also, I'd keep a check of len(word) == 0 since an empty input is dangerous too

Reading grid.txt
with open('grid.txt', 'r') as f:
    lst = [[a for a in l.strip() if a != ' ' and a!= '\n'] for l in f.readlines() if l]
    lst2 = [a[::-1] for a in zip(*lst)]

You can greatly simplify this part by using list.split()
with open('grid.txt', 'r') as f:
    lst = [line.split() for line in f.readlines()]
    lst2 = list( zip (*lst[::-1]) )

If you're worried about the '\n' at the end, the new lists using this method will not have it.

I'd suggest you keep the names of the lists as horizontal and vertical rather than lst and lst2 which don't mean much

Split work into functions
You should separate tasks into functions so that there isn't one huge block of code that does everything. Moreover, splitting work into functions makes it easy to maintain your code. What if your main game loop could look like this
def game_loop():
    while True:
        word = take_input()
        matches = find_matches(grid,word)

        if matches == None: print("No matches")
        else:
            for match in matches:
                print( "Match: " + match )

        if input("Do you want to play again? (y/n): ").lower() == 'n':
            break

An alternate approach
The reason your code isn't as efficient as the method I will just show you is that you repeatedly reverse the lists and use a lot of loops. There is a much better and cleaner way in which you need to go through the whole grid only once.

Read the .txt file into a list ( like we previously did for lst)
Start by going through each element in the 2-D list
If the element matches the first character of the entered word, check all 8 directions to complete the match

Implementaion


Comparing phone numbers across CSVs Python (continuation from Speeding up and fixing phone numbers from CSVs with Regex)

I'm pulling all of the phone numbers from all CSVs in two different directories, outputting them in a single simple format to two different files, and then comparing those two files for which numbers are in one but not the other.

I'm failing in speed (of execution), style, and results.  Here's what I was trying:

import csv
import re
import glob
import string

with open('Firstlist.csv', 'wb') as out:
    with open('Secondlist.csv', 'wb') as out2:
        with open('SecondnotinFirst.csv', 'wb') as out3:
            with open('FirstnotinSecond.csv', 'wb') as out4:
                seen = set()
                regex = re.compile(r'(\+?[2-9]\d{2}\)?[ -]?\d{3}[ -]?\d{4})')
                out_writer = csv.writer(out)
                out_writer.writerow([])
                csv_files = glob.glob('First\*.csv')
                for filename in csv_files:
                    with open(filename, 'rbU') as ifile:
                        read = csv.reader(ifile)
                        for row in read:
                            for column in row:
                                s1 = column.strip()
                                match = regex.search(s1)
                                if match:
                                    canonical_phone = re.sub(r'\D', '', match.group(0))
                                    if canonical_phone not in seen:
                                        seen.add(canonical_phone)
                for val in seen:
                    out_writer.writerow([val])

                seen2 = set()
                out_writer2 = csv.writer(out2)
                out_writer2.writerow([])
                csv_files2 = glob.glob('Second\*.csv')
                for filename in csv_files2:
                    with open(filename, 'rbU') as ifile2:
                        read2 = csv.reader(ifile2)
                        for row in read2:
                            for column in row:
                                s2 = column.strip()
                                match2 = regex.search(s2)
                                if match2:
                                    canonical_phone2 = re.sub(r'\D', '', match2.group(0))
                                    if canonical_phone2 not in seen2:
                                        seen2.add(canonical_phone2)

                for val in seen2:
                    out_writer2.writerow([val])

                out_writer3 = csv.writer(out3)
                for val in seen2 not in seen:
                    out_writer3.writerow([val])

                out_writer4 = csv.writer(out4)
                for val in seen not in seen2:
                    out_writer4.writerow([val])

Here's my 5 cents as non-python guy
Naming:
As soon as you begin numbering variable names, there should be alarm bells ringing... multiple loud alarm bells.
rename your out, out2, and the corresponding writers. Instead use descriptive names (maybe similar to your "filenames"): first_list, second_list, and so on. The same applies for your writers.
Also seen2 is not a good name.. What do you store in that variable? Name it after that: second_seen is definitely better, than seen2


PHP Template: Image file upload handler I'm trying to develop some templates for common PHP tasks I've been dealing with. One of which is a general image file upload handler.

So far I'm using the following reusable code which seems to be working fine without any noticeable bug:

<?php

    if ( !isset($_POST['submit']) ) {
        goto page_content;}

    if ( $_FILES['file_upload']['error']===4 ) {
        echo 'No file uploaded';
        goto page_content;}

    if ( $_FILES['file_upload']['error']===1 || $_FILES['file_upload']['error']===2 ) {
        echo 'File exceeds maximum size limit';
        goto page_content;}

    if ( $_FILES['file_upload']['error']!==0 ) {
        echo 'Failed to upload the file';
        goto page_content;}

    if ( !is_uploaded_file($_FILES['file_upload']['tmp_name']) ) {
        echo 'Failed to upload the file';
        goto page_content;}

    require_once('imageResize.php');
    $err = imageResize($_FILES['file_upload']['tmp_name'], 'random.png' );
    if ( $err !== 0 ) {
        echo 'Invalid image format';
        goto page_content;}

    echo 'Image uploaded successfully';

    page_content:
?>
<form action="filename.php" method="POST" enctype="multipart/form-data">

    <input type="hidden" name="MAX_FILE_SIZE" value="1000000">
    <input type="file" name="file_upload" accept="image/*">
    <input type="submit" name="submit">

</form>

Additional file imageResize.php:

<?php
    // image resize
    function imageResize($source, $target){

        $size = getimagesize($source);
        if ($size === false) {return 1;} // invalid image format

        $sourceImg = @imagecreatefromstring(@file_get_contents($source));
        if ($sourceImg === false) {return 2;} //invalid image format

        $width = imagesx($sourceImg);
        $height = imagesy($sourceImg);
        $sidelenght = min($width,$height);
        $targetImg = imagecreatetruecolor(100, 100);
        imagecopyresampled($targetImg, $sourceImg, 0, 0, ($width-$sidelenght)/2, ($height-$sidelenght)/2, 100, 100, $sidelenght, $sidelenght);
        imagedestroy($sourceImg);
        imagepng($targetImg, $target);
        imagedestroy($targetImg);

        return 0;
    }
?>

Some main characteristics of this code are:

provides messages for the most common errors that can happened during the upload process
it allows the client to upload an image file up to 1Mb size
resizes all images to a standard 100x100 px size
save all images to a standard PNG format

Questions

Is this code safe? Or are there any vulnerability that could be exploited by an malicious client? In this case, how can I solve it?
To avoid several nested if-then-else conditions (which can become hard to read), I'm currently using goto (which can become a bad control structure practice). Is there a better alternative?

DO NOT use GOTO. They have been criticised since 1960s.

Your indentation is broken for the first snippet. It is also inconsistent with your second snippet. Keep it consistent throughout a project. Try looking at some linters or follow a style guide.
Do not suppress your errors by using @. Let the function throw any and all errors it encounters. You should instead make use of try-catch blocks.
While you could replace the error lookups by error code to a switch-case block, I'd suggest that you use an associated array (or a hashtable) to keep a mapping from ERROR_CODE => "ERROR MESSAGE". This will go in a separate errors.php maybe, and referenced as needed.
Define the target image resolution as a constant, instead of placing magic numbers.
You could also have a client side javascript snippet for checking file size, so that users do not waste their bandwidth and time waiting for upload process.


How would you improve this object model design to get around Covariance Issues with ObjectModel.Collection? Consider the following simple relationship.

Code

[DataContract(Name = "Wheel")]
public class Wheel { }

[DataContract(Name = "OffRoadWheel")]
public class OffRoadWheel : Wheel { }

[DataContract(Name = "RoadVehicle")]
public class RoadVehicle
{
    public Collection<Wheel> Wheels { get; private set; }

    public RoadVehicle()
    {
        Wheels = new Collection<Wheel>();
    }
}

[DataContract(Name = "Truck")]
public class Truck : RoadVehicle
{
    public Collection<OffRoadWheel> OffRoadWheels { get; private set; }

    public Truck()
    {
        OffRoadWheels = new Collection<OffRoadWheel>();
    }
}

The Collections are configured as Read-Only and initiated in the class constructor as per FXCop recommendation CAS2227. http://msdn.microsoft.com/en-us/library/ms182327(VS.80).aspx

Now I know all the rules about Covariance & Contravariance, and the varying capabilities of using Collections vs. Lists vs. Enumerables (vs. their Interface Equivalents). However the above object model creates a number of competing difficulties.

If you create a truck with 4 Off-Road Wheels, down cast it to a RoadVehicle, and then iterate the Wheels collection... it's empty.

One way around this would be to create a public new Collection<Wheel> Wheels property on the Truck class which on-the-fly casts the contents of the OffRoadWheel collection, but this breaks behaviour if you go to add or remove on that collection.

Another problem is serialization, you can alter the above to store and duplicate the inner Wheels and OffRoadWheels properties, but then you'd end up complete serializing both if you wanted to serialize Truck

Any other suggestions on how to make this more Developer Friendly object model where things happen as you'd expect them to?

I think the model here is broken. If you have a RoadVehicle that's actually Truck, the interface lets you to add a normal Wheel to it, which is wrong.

One way to make it work is to make the collection immutable (e.g. IEnumerable<T>). This can be a reasonable solution if you're okay with setting the wheels only in the constructor.


"Counter Game" Powers of 2 Game I have been working on this hackerrank problem:

  Louise and Richard have developed a numbers game. They pick a number (1 ≤ n < 264)
  and check to see if it is a power of 2. If it is, they divide it by 2.
  If not, they reduce it by the next lower number which is a power of 2.
  Whoever reduces the number to 1 wins the game. Louise always starts.

  Given an initial value, determine who wins the game.

  As an example, let the initial value n = 132. It's Louise's turn so
  she first determines that 132 is not a power of 2. The next lower
  power of is 128, so she subtracts that from 132 and passes 4 to
  Richard. 4 is a power of 2, so Richard divides it by 2 and passes 2 to
  Louise. Likewise, 2 is a power so she divides it by 2 and reaches 1.
  She wins the game.

  If they initially set counter to 1, Richard wins. Louise cannot make a
  move so she loses.

My question is, could somebody verify/help me with the current time complexity of this? And given this time complexity, what are some ways I could reduce it?

I have tried to solve the time complexity, but it is quite confusing for me. What I have so far is: it takes O(log(N)) time to "reach" a given value N, since you are going up by factors of 2. Here I believe the worst case scenario is every number you land on is a power of two, since whenever you have a number that isn't a power of two, subtracting the highest power of two below it will reduce it by more than you would halving it. With that in mind, you have to "reach" a given value n log(M) times, where M is the first value for N.

So we have Log(N) to reach a value N, and you have to do this log(M) times. The part that confuses me is that the value N halves each time. I don't know how to approach the rest of this. Any help with this would be greatly appreciated.

What are some improvements to my code I could make?

static String counterGame(long n) {
    int turn = 0;
    long current = 2;
    long previous = 2;
    if (n == 1) return "Louise";
    while (true) {
        if (current == n) {
            n /= 2;
            if (n == 1) {
                break;
            } else {
                turn ^= 1;
                current = 2;
                previous = 2;
            }
        } else if (current < n) {
            previous = current;
            current *= 2;
            continue;
        } else {
            n = n - previous;
            if (n == 1) {
                break;
            }
            turn ^= 1;
            current = 2;
            previous = 2;
        }
    }
    return (turn == 1) ? "Richard" : "Louise";
}

This can actually be solved with a super efficient 1 line of code, but the explanation is long :)
(and there are a few other things I should point out first)

Your code will fail with any input bigger than \$2^{62}\$ because you never overshoot this number by multiplying by 2 a power of 2 (in your code: current). This is because a signed long has only enough bits to represent powers of 2 up to \$2^{62}\$ (you could have used it as an unsigned variable to have one extra bit, but it still wouldn't be enough to avoid this problem with the biggest numbers that the challenge says are expected input), multiplying it by 2 will cause the left most bit to become 1, which turns it into a big negative number, and multiplying it by 2 again will make it 0 which will then stay 0 no matter how much you multiply it, so your code will enter an infinite loop.

Once you detect that a number is a power of 2 you shouldn't reset previous and current to 2 because a power of 2 divided by 2 is still a power of 2, so you know you can divide current by 2 together with n.

A thing you could use to your advantage is the way powers of 2 look in binary code:

power | number | binary | even/odd power
2^0   = 1      = 00001  | even
2^1   = 2      = 00010  | odd
2^2   = 4      = 00100  | even
2^3   = 8      = 01000  | odd
2^4   = 16     = 10000  | even

Notice how all bits are 0 except one, the position of this 1 bit tells you if you need an odd or even number of divisions (turns in the game) to reach 1.

long filter = 0b101010101010101010101010101010101010101010101010101010101010101L;
boolean oddPower = (n & filter) == 0;

As soon as you reach a power of 2, this check is enough to tell you who's gonna win. An odd number of turns left means that the player whose turn it is now will win, otherwise the other player will win.

We can go further with looking at the bits and bypass the need to find the next lower power of 2 to divide n by it if n is not a power of 2. As you already know, any power of 2 is represented by a single 1 bit and lots of 0 bits. Now let's take a look at a few numbers represented in binary code:

3 = 0011
4 = 0100
5 = 0101
6 = 0110
7 = 0111
8 = 1000

When the number isn't a power of 2, according to the game you need to remove the next lower power of 2. In binary it's gonna look like removing the left most bit (you can look at the numbers above to confirm this). Given our knowledge and solutions so far, we can rewrite the rules of the game to simple binary operations:

if ( number is a power of 2 ) then:
    use the check from the previous part of my answer to figure out the winner.
else:
    turn the most left 1 bit to 0.

This means the number of turns until we reach a power of 2 and find the answer depends on how many 1-bits are in the number, and knowing if this number is odd or even - combined with the answer to whether the power you reach is odd or even - is all you really need to know to predict the outcome of the game!

Let's test this solution: here's what you see when you track the changes in a number as the game is played, in binary:

1000000110100
0000000110100
0000000010100
0000000000100
0000000000010
0000000000001

There are 4 1s (even number) but you don't remove the last 1 bit so ignore it and say there are 3 1s to be removed (odd number) and the first power of 2 you reach is 4 (even) so overall there will be an odd number of turns taken in the whole game, which you can see is true.

Here is the most optimized code I could possibly write to solve this:

static String counterGame(long n){
    boolean richardWins = true;

    while((n & 1) == 0){
        richardWins = !richardWins;
        n >>>= 1;
    }
    while(n != 1){
        if((n & 1) == 1)
            richardWins = !richardWins;
        n >>>= 1;
    }

    return richardWins ? "Richard" : "Louise";
}

This code looks at each bit once, so the time complexity of this code is O(log(N)) where N is the input number, and log(N) is the worst case scenario for the number of bits needed to represent the input number in binary.

Judging by the fact that the exception to the rules (where Richard wins when Louise gets the 1 at the start of the game) emerges naturally with this solution if you don't add an if statement at the start to take care of it, I'd say this is what the author of this challenge expected to be the best possible answer.

Bonus

There are processor instructions for counting 1 bits and trailing zeros, so you could actually do the whole thing in one line instead of manually looping through the bits, and using these processor instructions is faster:

static String counterGame(long n){
    return ((Long.numberOfTrailingZeros(n) & 1) == 1) ^ ((Long.bitCount(n) & 1) == 1) ? "Richard" : "Louise";
}

EDIT: a shorter code suggested by @JS1:

  By subtracting one, it turns all the trailing zeroes into trailing ones, so that they can be counted by bitCount.

static String counterGame(long n){
    return ((Long.bitCount(n-1) & 1) == 0) ? "Richard" : "Louise";
}