Sorting file lines in natural order I'm new to Java and have been through some tutorials and I'm in the next step of checking how I'm doing now. Is there a better way of doing this?

import java.io.BufferedReader;
import java.io.BufferedWriter;
import java.io.FileNotFoundException;
import java.io.FileReader;
import java.io.FileWriter;
import java.io.IOException;
import java.util.Collections;
import java.util.Date;
import java.util.Vector;

import org.apache.log4j.helpers.FileWatchdog;

/**
*
* Simple class that reads in a file - sorts its lines in natural order,
* writes it back out to the given filename and adds a timestamp of when it was sorted
*
* Program will be called with:
*
* java TestClass <input_filename> <output_filename>
*
*/

public class TestClass {

public static void main(String [] args) throws RuntimeException{
    boolean OVERWRITE_FILE = true;
    //get filename from args
    String inputFilename = args[0];
    String outputFilename = args[1];

    //check usage
    if(args.length != 2){
        System.out.println("Incorrect number of args");
        System.out.println("Correct usage is:");
        System.out.println(" java TestClass <input_filename> <output_filename> ");
        System.exit(-1);
    }

    //read in file into collection
    try {
        BufferedReader in = new BufferedReader(new FileReader(inputFilename));
        Vector fileContents = new Vector<Object>();
        while(in.ready()){
            fileContents.add(in.readLine());
        }

        //sort the collection in natural order
        Collections.sort(fileContents);

        //add timestamp
        fileContents.add("File Sorted on: ");
        fileContents.add(new Date());

        //write outputFile
        BufferedWriter out = new BufferedWriter(new FileWriter(outputFilename, OVERWRITE_FILE));
        for(int i = 0; i < fileContents.size(); i++){
            out.write((String)fileContents.get(i) + "\n");
        }
        //close output file
        out.close();

    } catch (IOException e) {
        System.out.println("Error while performing IO");
    }
}
}

Welcome to Code Review.

Before reviewing your code, it needs to be fixed:

First, make it work...

Usage is never printed

Your program is supposed to print the usage instructions if the wrong number of parameters ist supplied. Check what happens if you run it without any parameters:

  Exception in thread "main"

java.lang.ArrayIndexOutOfBoundsException: 0
at codereview.TestClass.main(TestClass.java:30)

To fix this, you need to check the length of args before accessing args[0] and args[1].
Failure because of unnecessary cast
If the correct number of arguments is supplied, your program will fail with the following output:

  Exception in thread "main"

java.lang.ClassCastException: java.util.Date cannot be cast to java.lang.String
at codereview.TestClass.main(TestClass.java:59)

This is because you inserted a Date object into your fileContents Vector. Please thoroughly study generics as a better understanding of this important topic would have prevented the error.

For now, just remove the cast (String)fileContents.get(i) and test your code. A complete solution is given later in this answer. No, really run your code now. You'll see that there is another problem:
Sorting is not in the natural order

You are sorting a list of Strings. This means that the sort order will always be alphabetical (1,3,10,12 would be sorted as 1,10,12,3) even though your documentation comments state that the lines are sorted in their natural order. You can't promise to do that without knowing what kind of input they contain. As it currently stands, you should just change your comments.
You are appending to the end of the output file
The signature of the FileWriter constructor is:

public FileWriter(String fileName, boolean append) throws IOException

So your OVERWRITE_FILE is a lie. Rename it to APPEND_FILE and set it to false (a cleaner, completely different solution will be given below).

... then, clean it up.

General issues

You have several unused imports. Remove them.
RuntimeException is unchecked, so you should remove throws RuntimeException from main.
Your error messages are unhelpful. Error performing IO does not tell the user what went wrong.

Code structure

Readability, maintainability, reusability: you should strive to achieve these three goals in your code (apart from correctness, of course, which goes above all else). One problem with your code is that everything happens directly inside main. You should delegate the bulk of your work into helper methods:

public static void main(String[] args) {
    try
    {
        validateNumberOfArguments(args.length);
        String inputFilename = args[0];
        String outputFilename = args[1];
        List<String> lines = readLinesFrom(inputFilename);
        sortAndAddTimestampTo(lines);
        writeLinesTo(outputFilename, lines);
    }
    catch (IOException e){
        exitBecause("Error accessing file", e);
    }
}

This gives a nice overview of what your code does - similar to article headings in a newspaper or chapters in a book.

Here's the implementation. Your main has been split up into concise methods. Each of them does one thing, so they are easy to understand.

private static void validateNumberOfArguments(int length) {
    if (length != 2) {
        exitBecause("Incorrect number of args", "Correct usage is:",
        "java TestClass <input_filename> <output_filename>");
    }
}

private static List<String> readLinesFrom(String path) throws IOException {
    return Files.readAllLines(getPath(path), StandardCharsets.UTF_8);
}

private static void sortAndAddTimestampTo(List<String> lines) {
    Collections.sort(lines);
    lines.add("File sorted on: ");
    lines.add(new Date().toString());
}

private static void writeLinesTo(String path, List<String> lines) throws IOException {
    Files.write(getPath(path), lines, StandardCharsets.UTF_8);
}

private static Path getPath(String fileName) {
    return new File(fileName).toPath();
}

private static void exitBecause(Object... reasons) {
    for (Object reason : reasons) {
        System.out.println(reason);
    }
    System.exit(-1);
}

Required imports (JRE7):

import java.io.File;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Collections;
import java.util.Date;
import java.util.List;

The error messages will now be a bit more helpful:

Error accessing file
java.nio.file.NoSuchFileException: input.txt

or

Error accessing file
java.io.IOException: The process cannot access the file because it is being used by
another process


Simple C++ Test class with rule of 5 It has been a long time I used c++.
So I brushing over concepts for interview preparation.
Can you please help me with code review?
If you have any observations/bugs/alternatives please point out.
For better formatting, I have shared code here feel free to open issues. :)
Thanks a ton in advance.

#include <cstring>
#include <memory>
#include <utility>

class Test {
public:
    //Default constructor
    Test() = default;

    //Copy constructor
    Test(const Test& other) : m_size(other.m_size), m_buffer(new char[m_size]()) {
        strncpy(m_buffer.get(), other.m_buffer.get(), m_size);
    }

    //Parametarized constructor
    Test(std::size_t len, const char* data) : m_size(len), m_buffer(new char[len]()) {
        strncpy(m_buffer.get(), data, m_size);
    }

    //Move constructor
    Test(Test&& other) noexcept {
        Test temp(other);
        swap(*this, temp);
    }

    //Assignment operator
    Test& operator=(Test other) {
        if(this != &other) {
            swap(*this, other);
        }
        return *this;
    }

    //Move assignment operator
    Test& operator=(Test&& other) noexcept {
        if(this != &other) {
            Test temp(other);
            swap(*this, temp);
        }
        return *this;
    }

    //Destructor
    ~Test() {
        m_size = 0;
        m_buffer = nullptr;
    }
private:
    std::size_t m_size = 0;
    std::unique_ptr<char[]> m_buffer = nullptr;

    static void swap(Test& first, Test& second) noexcept{
        using std::swap;
        swap(first.m_size, second.m_size);
        swap(first.m_buffer, second.m_buffer);
    }
};

Observations
Your rule of 5 implementation is basically wrong. You are implement copy operations for both copy and move (which means you may as well not have the move operations).
Your destructor is not doing any real work and thus adding unneeded code.
Your swap function does not work as expected as it can not be used with  Koenig lookup (ADL) and thus in most standard library would be replaced with std::swap resulting in a sub optimal swap.
Details
Here I don't see the need for creating a temporary.
    //Move constructor
    Test(Test&& other) noexcept {
        Test temp(other);
        swap(*this, temp);
    }

By creating the temporary your move is basically just as expensive as your copy. The whole point of the move is that it is cheap.
I would simply swap the current with other.
    //Move constructor
    Test(Test&& other) noexcept {
        swap(other);
    }

After the move construction the content of other has no guaranteed state (only that it is valid). Since you define m_size and m_buffer to auto initialize the current this is in a valid state and can simply be swapped with other.
    // Note I usually define a `noexcept` swap method.

    void swap(Test& other) noexcept {
        using std::swap;
        swap(m_size,   other. m_size);
        swap(m_buffer, other. m_buffer);
    }

I also define swap() with two parameters as a free standing function that simply calls the swap method. This is because the compiler can using Koenig lookup (ADL) to find the correct swap function.
    void swap(Test& lhs, Test& rhs) {
        lhs.swap(rhs);
    }

The Copy assignment is sub optimal.
    //Assignment operator
    Test& operator=(Test other) {
        if(this != &other) {
            swap(*this, other);
        }
        return *this;
    }

Since the parameter other is created via a copy construction it is guaranteed not to be the same as this. Thus the test if(this != &other) is just a pessimization as it is not needed.

Again you perform a copy during the move assignment.
    //Move assignment operator
    Test& operator=(Test&& other) noexcept {
        if(this != &other) {
            Test temp(other);
            swap(*this, temp);
        }
        return *this;
    }

This defeats the purpose of the move (as it is supposed to be cheaper than a copy). You can simply swap this and other as they should both be valid.
    Test& operator=(Test&& other) noexcept {
        swap(other);
        return *this;
    }

No need to check for self assignment as this is a pessimization of the normal more common situation. Even of they are the same object the swap will work correctly.

The destructor is useless:
    //Destructor
    ~Test() {
        m_size = 0;
        m_buffer = nullptr;
    }

The member: m_buffer will correctly clean up the memory.

Making this a static member is not doing you any favors as it prevents Koenig lookup (ADL).
    static void swap(Test& first, Test& second) noexcept{
        using std::swap;
        swap(first.m_size, second.m_size);
        swap(first.m_buffer, second.m_buffer);
    }

Make this a free standing function:
namespace PL
{

class TestStatic
{
    public:
        static void swap(TestStatic& lhs, TestStatic& rhs) noexcept
        {}
};

class TestFreeStand
{
    public:
};
void swap(TestFreeStand& lhs, TestFreeStand& rhs) noexcept
{}

}

int main()
{
    PL::TestStatic      staticA;
    PL::TestStatic      staticB;

    swap(staticA, staticB);

    PL::TestFreeStand   freeStandA;
    PL::TestFreeStand   freeStandB;

    swap(freeStandA, freeStandB);
}

Notice the only compilation error here is: swap(staticA, staticB);
This means that if you add using std::swap; this code will now compile but it will not use your static void swap() method it will use std::swap which makes a copy of the object. The point of writing your own version of swap is that you are providing an optimization over the standard swap.
Better Implementation
#include <cstring>
#include <memory>
#include <utility>

namespace Testing
{

    class Test
    {
        public:
            // Default constructor
            Test() = default;

            // Parametrized constructor
            Test(std::size_t len, const char* data)
                : m_size(len)
                , m_buffer(new char[len]())
            {
                // Should validate the input here.
                // It is the only time you get outside input.
                // Potentially you could throw here I choose
                // not too (because I can see it as a valid input)
                // Your use case may be different.
                if (data) {
                    strncpy(m_buffer.get(), data, m_size);
                }
            }

            // Copy constructor
            Test(const Test& other)
                : m_size(other.m_size)
                , m_buffer(new char[m_size]())
            {
                strncpy(m_buffer.get(), other.m_buffer.get(), m_size);
            }

            // Copy Assignment operator
            Test& operator=(Test other)
            {
                swap(other);
                return *this;
            }

            // Move constructor
            Test(Test&& other) noexcept
            {
                swap(other);
            }

            // Move assignment operator
            Test& operator=(Test&& other) noexcept
            {
                swap(other);
                return *this;
            }

            void swap(Test& rhs) noexcept
            {
                using std::swap;
                swap(m_size,    rhs.m_size);
                swap(m_buffer,  rhs.m_buffer);
            }
        private:
            std::size_t m_size = 0;
            std::unique_ptr<char[]> m_buffer = nullptr;
    };

    void swap(Test& lhs, Test& rhs) noexcept
    {
        lhs.swap(rhs);
    }
}


JS Procedural Hangman I wrote a procedural version of Hangman (very basic) and would appreciate some feedback on the following points:

Is the basic logic OK? Any obvious improvements?
Any details or matters of style I need to address?
Var vs Const vs Let - I'm probably all over the place with this. My basic approach was "If it needs to be global, use var, if it's not going to change, use Const and if it is going to change but is not global, use Let."

I'm aware that a procedural approach may not be the best, but I wanted to focus on it for this program.

HTML:

<button id="play">Play</button>

JS:

var secretWord;
var playing = false;
var wrongGuesses;
var limit = 4;
var solutionSoFar;

const words = [
    "fish",
    "cat",
    "chicken",
    "freedom",
    "turnip"
];

function displayBoard() {
    console.log( solutionSoFar.join( " " ) );
}

function playAgainDialogue() {
    if ( confirm( "Play again?" ) ) {
        console.log( "" );
        play();
    }
    console.log( "Bye!" );
}

function play() {

    playing = true;

    // Set secret word
    secretWord = words[ Math.floor( Math.random() * words.length ) ];

    wrongGuesses = 0;

    // initialize solution
    const numLetters = secretWord.length;
    solutionSoFar = new Array( numLetters ).fill( "_" );

    // Display instructions
    console.log( "Guess one letter at a time to reveal the secret word." );

    while ( playing ) {
        displayBoard();
        playerTurn();
    }

}

function playerTurn() {

    // Out of turns
    if ( wrongGuesses >= limit ) {
        console.log( "You have used all your guesses. Game Over." );
        playing = false;
        playAgainDialogue();
        return;
    }

    // Player wins
    if ( solutionSoFar.join( "" ) === secretWord ) {
        console.log( "Yay, you guessed it!" );
        playing = false;
        playAgainDialogue();
        return;
    }

    // get player guess
    let remainingGuesses = limit - wrongGuesses;
    let playerGuess = prompt( "Guess a letter. You have " + remainingGuesses + " remaining guesses" ).toLowerCase();
    if ( secretWord.indexOf( playerGuess ) === -1 ) {
        console.log( "That letter in not in the secret word." );
        wrongGuesses += 1;
        return;
    }

    // Successful guess
    console.log( "Well done." );
    solutionSoFar.forEach( function( val, index ) {
        if ( secretWord.split( "" )[ index ] === playerGuess ) {
            solutionSoFar[ index ] = playerGuess;
        }
    } );
}

document.getElementById("play").addEventListener("click", play);

Logic

Is the basic logic OK? Any obvious improvements?

Well does the game work? If so then the basic logic must be OK.
Is there room for improvement, yes.
The code is tangled mess of roles and states.

You have 4 functions that manage 7 distinct game states.
There is some repeated code (win and lose).
You have magic values in the code (text prompts and displays)
The whole thing runs in a continuous blocking state which is VERY bad for browser based code (you can not exit until end game)
There is no checking for repeated character guesses, or valid character guesses.
You randomly pick words, not checking if a new game is using an already used word.

Example
The example shows some alternative approaches to creating game like state machines.
Note that I do not use the console, prompt, or confirm, thus the game needs an additional wait state to wait for input.
Nor do I vet input for repeats, or bad characters.
update I have fixed the inplay state to only allow start when not inplay
Some points.

Uses finite state model to control the game.
Settings out of the code and grouped in one place to allow easy changes.
Encapsulate the game in a singleton (to keep the global scope clean)
Though I do not use prompt and confirm, the state changes use setTimeout to change between states, this stops the game from blocking the page.
States are pedantic but there roles are clear and without ambiguity.

"use strict";  // Always add this to the top of your code.

/* Start game is external state request */
document.getElementById("play").addEventListener("click", ()=> {
    hangman.state = "start";
})

const hangman = (()=> {
    const settings = {
        limit : 4,
        words : ["fish","cat","chicken","freedom","turnip"],
        text : {
            start : "Guess one letter at a time to reveal the secret word.",
            prompt : "Guess a letter. You have ## remaining guesses", // ## is replaced with remaining guesses
            good : "Well done.",
            bad : "That letter in not in the secret word.",
            win : "Yay, you guessed it!",
            lose : "You have used all your guesses. Game Over.",
            again : "Play again Y/[N] ",
            gameOver : "You have used up all the words. GAME OVER!",
            exit : "Bye!",
        }
    };

    const display = text => log(settings.text[text] ? settings.text[text] : text);
    var word, player, inplay, wrongGuesses;

    function start() {
        if(settings.words.length > 0){
            word = settings.words.splice(Math.random() * settings.words.length | 0, 1)[0];
            player = "";
            wrongGuesses = 0;
            display("start");
            display("Cheat >>> " + word + " <<<<");
            return states.guess;
        }
        return states.gameOver;
    }

    function getPlayerGuess() {
        const gotKey = (key) => {
          const guess = key.toLowerCase();
          player += guess;
          game.state = word.indexOf( guess ) === -1 ? states.badGuess : states.goodGuess;
        }
        var remaining = settings.limit - wrongGuesses;
        const guess = prompt(settings.text.prompt.replace("##", remaining), gotKey);
        return states.wait;
    }

    function checkPlayer(){
        var wordDisplay = "";
        for (const char of word) { wordDisplay += player.indexOf(char) > -1 ? char : "_" }
        display(wordDisplay);
        return wordDisplay === word ? states.win : states.guess;
    }

    const states = { // All game states named
        start : 1,
        guess : 2,
        badGuess : 3,
        goodGuess : 4,
        win : 5,
        lose : 6,
        playAgain : 7,
        gameOver : 8,
        wait : 9,
    };

    const game = {  // assign property game.state to change state
        set state(value) {
            if(! inplay){
                if(value === "start") {
                    value = states.start;
                } else {
                    return;
                }
            }
            inplay = true; // prevents game restart while in play
            var nextState; // if left undefined then game ends
            switch(value) {
                case states.wait:
                   return;
                case states.playAgain:
                    confirm(settings.text.again, result => {
                       game.state = result ? states.start : states.exit;
                    });
                    nextState = states.wait;
                    break;
                case states.start:
                    nextState = start();
                    break;
                case states.guess:
                    nextState = getPlayerGuess();
                    break;
                case states.goodGuess:
                    display("good");
                    nextState = checkPlayer();
                    break;
                case states.badGuess:
                    display("bad")
                    wrongGuesses += 1;
                    nextState = wrongGuesses === settings.limit ? states.lose : states.guess;
                    break;
                case states.win:
                    display("win");
                    nextState = states.playAgain;
                    break;
                case states.lose:
                    display("lose");
                    nextState = states.playAgain;
                    break;
                case states.gameOver:
                    display("gameOver");
                    break;
                default :
                    return;
            }
            if (nextState) { setTimeout(() => game.state = nextState, 0) }
            else {
                display("exit");
                inplay = false;
            }
        }
    };
    return game;
})();

/*========================================================================*/
/*========== unrelated code to handle input and display ==================*/
var prompting;
var keyVal;
document.addEventListener("keyup",e=>{
    if(prompting){
        keyVal = e.key;
        prompting = false;
    }
})
function prompt(text,cb){
    var ticker = 0;
    const line = log(text );
    prompting = true;
    function tick(){
        if(prompting){
            line.textContent = text + (ticker++ & 1 ? "?" : ".");
            setTimeout(tick, 250);
        }else{
            line.textContent = text + "! " + keyVal;
            cb(keyVal[0]);
        }
    }
    tick();
}
function confirm(text, cb){
   prompt(text,(key)=>{ cb(key.toLowerCase() === "y") });
}
function log(text){
    var line;
    if(textContainer.children.length > 7){ line = textContainer.removeChild(textContainer.querySelector(".text")) }
    else{ line = Object.assign(document.createElement("div"),{className : "text"}) }
    textContainer.appendChild(line)
    line.textContent = text;
    return line;
}
body {
   background : black;
   font-size : 16px;
   color : #0f0;
.text {

}
<input type="button" id="play" value="play hangman">
<code id="textContainer">
</code>

Style

Any details or matters of style I need to address?

The style is generally good. Adding spaces after opening and before closing parentheses is a little unusual console.log( solutionSoFar.join( " " ) ) it is by no means bad. The most important aspect of code style is that you use it consistently, which you have done.
Comments should be kept to a minimum, they constitute un-vetted noise. None of your comments are needed, the code is self explanatory.
Variable declaration type

var vs const vs let - ... My basic approach was "If it needs to be global, use var, if it's not going to change, use const and if it is going to change but is not global, use let."

const
OK first const it will be the most used type of variable declaration in any good code. You used it correctly for words but missed using it for limit.
For solutionSoFar which holds an array you assign a new array each time you need. There is nothing wrong with that. But it could also have been a const and it would be declared as const solutionSoFar = []; and solutionSoFar.length = secretWord.length;' followed by solutionSoFar.fill("_");`
Doing it this way lets you keep additional references to the same array, and reduces the overall memory management overhead (in this case negligible overhead)
var V let
Using either in global scope makes no difference apart from when you can access them.
var is function scope and hoisted. The global scope can be thought of as the global function and var has scope to everything within that call. It is also hoisted to the top of the code and thus accessible at any time (however it may not yet contain a value).
let is block scoped and is not hoisted. Again the global scope like a function is contained in a block and thus let is accessible from anywhere but only after its been declared or it will throw an error (Warning let and const can not be accessed before they have been declared) .
Which to use?
The argument that block scope reduces bugs is non sequitur.
What you use is up to you. But var for global and let for functions makes no sense. I use the rule that if declared in function scope (and not a const) it should be a var
eg
function foo() {
    var funcScope;  // this is correct for function scope
    let blockScope; // This could / (should in my book) be function scope

    if (true) {
        var blockVar; // this is function scoped and should be at the top
                      // of the function
        blockScope = 0;    // Reference error blockScope is not defined
                           // The next line recreates the variable for this
                           // scope putting the name in a temporary dead zone
                           // This is a run time error and will not be
                           // thrown when parsing even in strict mode.

        let blockScope; // this is correct and is a new instance, not the
                        // same as the previouse declared blockScope
    }
}


Reverse Integer in JavaScript Problem Description:

  Given a 32-bit signed integer, reverse digits of an integer in
  JavaScript.

  Example 1:

  Input: 123
  Output: 321

  Example 2:

  Input: -123
  Output: -321

  Example 3:

  Input: 120
  Output: 21

  Note: It should return zero when it reaches out of limit [−2 ^31,  2 ^31 − 1]

My implementation

var reverse = function (x) {
  var minRange = Math.pow(-2, 31)
  var maxRange = Math.pow(2, 31) - 1

  var isNegative = false;
  if (x < 0) {
    isNegative = true;
    x = (x - x - x);
  }
  var result = Number(x.toString().split('').reverse().join(''));
  (isNegative) ? result = (result - result - result) : result;
  if (result < minRange || result > maxRange) {
    return 0
  } else {
    return result;
  }

};

Please help to improve.

-- adding this as an answer because I can't comment.

The answer referenced in ggorlen's comment, can be improved by remarking that a (negative number % 10) is a negative number, so there is no need for sign checking.

const reverse = val => {
    let res = 0;
    const Base = 10;
    while (val) {
        res = res * Base + (val % Base);
        val = (val / Base) | 0;
    }
    return (res | 0) == res ? res : 0;
}

Tests:

reverse(1) === 1;
reverse(-1) === -1
reverse(0) === 0
reverse(Math.pow(2,31) - 1) === 0
reverse(Math.pow(-2,31)) === 0
reverse(1463847412) === 2147483641
reverse(1463847413) === 0

By the way, what's the reasoning behind "x = (x - x - x)"? x-x evaluates to zero. so that's just x = - x.


Adding ranks to objects in an array This works, and I personally find it elegant and kind of "neat," but then I worry it's a bit too tricky. Comments welcome:

function (reports) {
    reports.forEach(function (report, i) {
        report.upPercentage = report.upTime / report.totalTime;
        report.rank = i; // temporary storage in a convenient location
    });
    _.sortBy(reports, 'upPercentage').forEach(function (report, i) {
        reports[report.rank].rank = i; // the 'trick'
    });
    return reports;
}

The first loop goes through the array of reports, and calculates the upPercentage for each report. This part should arguably be done on the backend but it isn't and it isn't going to be. Not my call.

The first loop also assigns each element's own position in the array to its rank. This is not the report's actual rank, it's just data I need for the second loop.

The second loop is iterating over a sorted copy of the reports array (it is important that the original sorting of reports be maintained), and uses the element's original position, stored in rank, to find it in the original, unsorted array. It then assigns the element's position in the sorted array to the rank of the element in the original, unsorted array.

Thus rank ends up being equal to the number of reports that have a lower upPercentage than does the given report.

Forgive me if I misunderstand it, but in the second loop isn't the report and reports[report.rank] refer the same object?

Because if so, then the following simpler code would produce the same result:

function (reports) {
    reports.forEach(function (report, i) {
        report.upPercentage = reports.upTime / reports.totalTime;
    });
    _.sortBy(reports, 'upPercentage').forEach(function (report, i) {
        report.rank = i;
    });
    return reports;
}


Ask the user a number between 2 and 20 Here is my attempt:

#include <stdio.h>

int main()
{
    int number = -1;
    printf("Enter a number in range [2,20]\n");
    scanf("%d", &number);
    while (number < 2 || 20 < number)
    {
        while (getchar() != '\n')
        {
        }
        printf("Try again\n");
        scanf("%d", &number);
    }
    printf("Your number was: %d", number);
    return 0;
}

But the solution from the professor is the following:

#include<stdio.h>

int main(){
    int n;
    while(scanf("%d",&n)==0 || n <2 || n >20){
        while ( getchar () != ’\n’);
        printf( "Wrong. Try again\n");
    }
    printf ( "%n\n",n);
    return 0;
}

Both approaches seem to work fine but is there a benefit to his method? Is it possible that scanf changes variable even though it couldn't read in a correct number. In that case checking the result of the scanf function would protect the program from accepting a wrong answer.

Prof's transcribed code has weaknesses

#include<stdio.h>

int main(){
    int n;
    while(scanf("%d",&n)==0 || n <2 || n >20){
        while ( getchar () != ’\n’);
        printf( "Wrong. Try again\n");
    }
    printf ( "%n\n",n);
    return 0;
}

Infinity loop

When input is closed after a scanf("%d",&n) == 0, getchar() can eventual return EOF before '\n' and thus while ( getchar () != ’\n’); never ends.

Smart quotes

’\n’ vs. '\n'.  Transcription problem or is that in the original?  In any case, the posted code was certainly not compiled.

Invalid specifier

"%n" does not print the int value.  It is undefined behavior as "%n" matches an int *.

    //printf ( "%n\n",n);
    printf ( "%d\n",n);

Requirement weakness

"Ask the user a number between 2 and 20" is a classic ambiguity.  Depending on perspective are 2 and 20 acceptable or not?  Better to provide more detail

"Ask the user a number, inclusively, between 2 and 20" or the like.

OP code has issues too.

#include <stdio.h>

int main()
{
    int number = -1;
    printf("Enter a number in range [2,20]\n");
    scanf("%d", &number);
    while (number < 2 || 20 < number)
    {
        while (getchar() != '\n')
        {
        }
        printf("Try again\n");
        scanf("%d", &number);
    }
    printf("Your number was: %d", number);
    return 0;
}

Failure to check input return value

scanf("%d", &number); may have failed due to non-numeric text input.  Check the result.

// scanf("%d", &number);
if (scanf("%d", &number) == 1) Success();
else Fail();

Infinite loop

OP's code suffers same potential infinite loop on end-of-file

// while (getchar() != '\n') { }
int ch;
while ((ch = getchar()) != '\n' && ch != EOF) { }

Also check scanf() return value.

Functional difference

With non-numeric input, Prof's code tries again.  OP's code, which does not evaluate the return value of scanf(), technically leads to trouble on rare input error.  On input error, the value of number is indeterminate. See comment

The 2 locations to take user input is simply twice the amount of code that needs checking.  Better to take input from 1 call.  Production code would have a get_int() to handle this.  Something like getint()

  but is there a benefit to his method?

OP's' code does have an informative prompt.

Else not much, both need work.  Perhaps

// untested code
#include<stdio.h>
#define MIN_N 2
#define MAX_N 20

int main(void) {
  int cnt;
  int n;
  printf("Enter a number in range [%d,%d]\n", MIN_N, MAX_N);
  while((cnt = scanf("%d",&n)) == 0 || ((cnt == 1) && (n < MIN_N || n > MAX_N))) {
    int ch;
    while ((ch = getchar ()) != '\n' && ch != EOF) {
      ;
    }
    printf( "Wrong input. Try again\n");
  }
  if (cnt == 1) printf ( "%d\n",n);
  return 0;
}

Better code would read a line into a string and process the input with strtol().


Specialized version of the cross correlation function I am writing a specialized version of the cross correlation function as used in neuroscience. The function below is supposed to take a time series data and ask how many of its values fall in specified bins. My function xcorr works but is horrifically slow even. A test data set with 1000 points (and so 0.5*(1000*999) intervals) distributed over 400 bins takes almost ten minutes.

Bottleneck

The bottleneck is in the line counts = array([sum .... I assume it is there because each iteration of the foreach loop searches through the entire vector diffs, which is of length len(first)**2.

def xcorr(first,second,dt=0.001,window=0.2,savename=None):
    length = min(len(first),len(second))
    diffs = array([first[i]-second[j] for i in xrange(length) for j in xrange(length)])
    bins = arange(-(int(window/dt)),int(window/dt))
    counts = array[sum((diffs>((i-.5)*dt)) & (diffs<((i+.5)*dt))) for i in bins]
    counts -= len(first)**2*dt/float(max(first[-1],second[-1])) #Normalization
    if savename:
        cPickle.dump((bins*dt,counts),open(savename,'wb'))
    return (bins,counts)

Here's a crack at my own question that uses built-in functions from NumPy

Update

The function chain bincount(digitize(data,bins)) is equivalent to histogram, which allows for a more succinct expression.

 def xcorr(first,second,dt=0.001,window=0.2,savename=None):
      length = min(len(first),len(second))
      diffs = array([first[i]-second[j] for i in xrange(length) for j in xrange(length)])
      counts,edges = histogram(diffs,bins=arange(-window,window,dt))
      counts -= length**2*dt/float(first[length])
      if savename:
           cPickle.dump((counts,edges[:-1]),open(savename,'wb'))
      return (counts,edges[:-1])

The indexing in the return statement comes because I want to ignore the edge bins.


Let's play some Swift Poker Before you can do any sort of card game, you must first write some code to define your deck of cards.

One thing I've noticed in looking at some of Apple's Swift interfaces is that they very much so like to use extensions.  This is probably good.  In the big picture long term, it's a really good way of allowing ever-expanding classes while actually keeping individual source code files quite tidy.

I also wanted to try something out with Swift enums and iterating through every value in them...

The Basics

enum Suit

enum Suit: String {
    case Clubs = "Clubs"
    case Diamonds = "Diamonds"
    case Hearts = "Hearts"
    case Spades = "Spades"
    case __EXHAUST = ""
}

enum Rank

enum Rank: String {
    case Ace = "Ace"
    case Two = "Two"
    case Three = "Three"
    case Four = "Four"
    case Five = "Five"
    case Six = "Six"
    case Seven = "Seven"
    case Eight = "Eight"
    case Nine = "Nine"
    case Ten = "Ten"
    case Jack = "Jack"
    case Queen = "Queen"
    case King = "King"
    case __EXHAUST = ""
}

struct Card

struct Card {
    let suit: Suit
    let rank: Rank
}

The Extensions

Suit: FowardIndexType

extension Suit: ForwardIndexType {
    func successor() -> Suit {
        switch self {
        case .Clubs: return .Diamonds
        case .Diamonds: return .Hearts
        case .Hearts: return .Spades
        case .Spades: return .__EXHAUST
        case .__EXHAUST: return .__EXHAUST
        }
    }
}

Rank: ForwardIndexType

extension Rank: ForwardIndexType {
    func successor() -> Rank {
        switch self {
        case .Ace: return .Two
        case .Two: return .Three
        case .Three: return .Four
        case .Four: return .Five
        case .Five: return .Six
        case .Six: return .Seven
        case .Seven: return .Eight
        case .Eight: return .Nine
        case .Nine: return .Ten
        case .Ten: return .Jack
        case .Jack: return .Queen
        case .Queen: return .King
        case .King: return .__EXHAUST
        case .__EXHAUST: return .__EXHAUST
        }
    }
}

Card: Printable, DebugPrintable

extension Card: Printable, DebugPrintable {
    var description: String {
        get {
            return self.rank.rawValue + " of " + self.suit.rawValue
        }
    }
    var debugDescription: String {
        get {
            return self.rank.rawValue + " of " + self.suit.rawValue
        }
    }
}

Array

extension Array {
    mutating func shuffle() {
        for i in 0..<(count - 1) {
            let j = Int(arc4random_uniform(UInt32(count - i))) + i
            swap(&self[i], &self[j])
        }
    }
}

Int

extension Int {
    func repeat(function: () -> ()) {
        for _ in 1...self {
            function()
        }
    }
}

Example Usage

Constructing the Deck

var deck: [Card] = Array<Card>()
for suit in Suit.Clubs...Suit.Spades {
    for rank in Rank.Ace...Rank.King {
        let newCard = Card(suit: suit, rank: rank)
        deck.append(newCard)
    }
}

Shuffling the Deck

3.repeat({deck.shuffle()})

Printing the entire Deck

for card in deck {
    println(card)
}

Conforming to the ForwardIndexType protocol is what allows the ... and ..< syntax for loops.  The unfortunate downside is that for an enum, we must include one extra value in order to actually iterate through the entire enumeration, per this StackOverflow question.  This seems like a bug that will hopefully be fixed.

I think this is a good foundation for a deck of playing cards in Swift.

Including Jokers might prove difficult.  I'm not sure whether it could be done very from this structure.  And working with extensions means that as we add additional functionality to a card or the enums, we can do it cleanly in new source code files without mucking around with this file.

Bearing in mind that I don't know much Swift...

Your Rank enum could save you a lot of typing if you simply made it a plain old autoincrementing integer enumeration:

enum Rank: Int {
    case Ace = 1, Two, Three, Four, Five, Six, Seven,
    Eight, Nine, Ten, Jack, Queen, King, __EXHAUST
}

extension Rank: ForwardIndexType {
    func successor() -> Rank {
        switch self {
        case .__EXHAUST: return .__EXHAUST
        default: return Rank(rawValue: self.rawValue + 1)!
        }
    }
}

extension Rank: Printable, DebugPrintable {
    var description: String {
        get {
            switch (self) {
            case .Ace: return "Ace"
            case .Jack: return "Jack"
            case .Queen: return "Queen"
            case .King: return "King"
            default: return self.rawValue.description
            }
        }
    }
    var debugDescription: String {
        get { return self.description }
    }
}

for i in Rank.Ace ... .King {
    print(i)
}

When testing out this code, please notice that the Printable protocol doesn't work in Playgrounds at the moment.

Also, of course, shuffling the deck 3 times doesn't make the order of the cards any "more random" than shuffling the deck once. But you probably know that, since you went to the bother of implementing copying a correct shuffling algorithm.

To add jokers, the most "C-like" approach would be to add Suit.Joker and Rank.Joker and leave Card alone.
The most "functional-programming" approach would be to change Card from a struct to an enum with a special case for jokers:

enum Card {
    case Regular(Rank, Suit)
    case Joker
}

var c = Card.Regular(.Ace, .Spades)
var j = Card.Joker


Perfect number Calculation Perfect numbers are strictly positive whole numbers of which the sum of their positive divisors equals a multiple of the number itself. An example of a 3-perfect number is 120. The sum of all divisors of 120, namely 1+2+3+4+5+6+8+10+12+15+20+24+30+40+60+120 = 360 equals three times 120.

My code does work, but it's not really efficient (if you have 2 very large number, it will take a very long time). Is there a more elegant way to solve this problem?

public class Main {

    public static void main(String[] args) {
        int number1 = 10;
        int number2 = 50;
        boolean found = false;
        int perfectNumber = 0;
        int multiplier = 0;
        for(int i = number1; i<=number2 && !found;i++){
            if(i%2==0){
                int somDivisors = lookSomeDivisors(i);
                if(somDivisors%i==0){
                    perfectNumber = i;
                    multiplier = somDivisors/i;
                    found=true;
                }
            }
        }
        System.out.println("Number1: " + number1 + " Number2: " + number2);
        System.out.println("PerfectNumber: " + perfectNumber + " Multiplier: " + multiplier);
    }

    private static int lookSomeDivisors(int number) {
        int som = 0;
        for(int i=1;i<=number;i++){
            if(number%i==0){
                som+= i;
            }
        }return som;
    }
}

Well, if you are interested in number-theoretical solutions, take a look at the wiki page referenced in one of the comments to your question. It does not apply directly to your problem,  but you can lift some useful insights from it.

As to your "brute force" solution, first of all, note, that, when looking for divisors you only need to test values up to Math.sqrt(number), there is no need to go higher, because everything you can find there can be obtained easier by diving number by one of the divisors found so far.
Further, if a number is odd, it cannot have any even divisors, so once a test for number % 2 fails, there is no need to test 4 or 6 or 8 ... Same goes for other divisors: once you test and fail for number % 3, you can exclude 6, 9, 12 etc.

What this boils down to is that to find all divisors of a number you only need to test its divisibility by every prime number between 1 and Math.sqrt(number). If you find and hardcode a large enough list of first N primes, your program can be much faster, at least, for input numbers up to the last of your primes squared.


Recursively listing files in Python I'm a complete amateur at python.  Other than hello world programs, this is the first thing I've ever done in Python.

I cannot find a good way to make os.walk function the way I want it to so I worked on my own function for it.

I'm curious which would be the best way to do something.

import os
def atDirList(startDir, maxDepth=0, minDepth=0, curDepth=0):
    output = []
    curDir = []
    curDir = os.listdir(startDir)
    if curDepth >= minDepth:
        for item in curDir:
            fullItem = os.path.join(startDir,item)
            if os.path.isfile(fullItem):
                output.append(fullItem)
    if curDepth+1 <= maxDepth:
        for item in curDir:
            fullItem = os.path.join(startDir,item)
            if os.path.isdir(fullItem):
                output = output+atDirList(fullItem,maxDepth,minDepth,curDepth+1)
    return output

print(atDirList('/music/main'))

or

import os
def atDirList(startDir, maxDepth=0, minDepth=0, curDepth=0):
    output = []
    curDir = []
    curDir = os.listdir(startDir)
    if curDepth >= minDepth:
        for item in curDir:
            fullItem = os.path.join(startDir,item)
            if os.path.isfile(fullItem) and curDepth >= minDepth:
                output.append(fullItem)
            elif os.path.isdir(fullItem) and curDepth+1 <= maxDepth:
                output = output+atDirList(fullItem,maxDepth,minDepth,curDepth+1)
    return output

print(atDirList('/music/main'))

Basically of those, is checking the if once, and traversing the list twice best or is traversing the list once, but checking the if on each entry best?

In Python you usually want to avoid having a recursive solution, because of the maximum recursion depth. Here this will probably not be a problem, because it is 1000 by default (so unless you directory hierarchy is 1000 levels deep, you are fine).

However, I would at least make your functions generators. They have the advantage that you don't need to generate the whole list in one go, but one element at a time. For this, Python 3 has even an additional nice feature, yield from. Note that you will have to call list on the result if you need the whole list at once (which negates any positive effect of using a generator), but that you can directly iterate over the output of a generator

So, your second function would become:

def atDirList(startDir, maxDepth=0, minDepth=0, curDepth=0):
    if curDepth >= minDepth:
        for item in os.listdir(startDir):
            fullItem = os.path.join(startDir, item)
            try:
                if os.path.isfile(fullItem) and curDepth >= minDepth:
                    yield fullItem
                elif os.path.isdir(fullItem) and curDepth + 1 <= maxDepth:
                    yield from atDirList(fullItem, maxDepth, minDepth, curDepth + 1)
            except OSError:
                continue

Note that I also added a try..except block to skip over files for which you don't have enough permissions to check if they are a file or directory.

As an alternative solution, I would propose to use glob, which basically shell expands a string like "/home/graipher/*/*" to a list of all files and directories matching this pattern. This has two caveats, first we still need to filter out the files from the directories (easily achieved with filter) and second, this ignores files which start with a . (hidden files).

import glob
import os

def glob_list(start, max_depth=0, min_depth=0):
    # start out at least `min_depth` levels deep
    current_dir = os.path.join(start, *"*" * min_depth)
    for depth in range(min_depth, max_depth + 1):
        # go one level deeper
        current_dir = os.path.join(current_dir, "*")
        # print(current_dir)
        yield from filter(os.path.isfile, glob.iglob(current_dir))

if __name__ == "__main__":
    for file_name in glob_list(os.path.expanduser("~"), max_depth=2, min_depth=1):
        print(file_name)

Here I changed the parameter names to conform to Python's official style-guide, PEP8, by using lower_case instead of camelCase.

The *"*" * min_depth part is probably slightly complicated, because of all the stars. Let's break it down:

An iterable (like a list or a string) multiplied with an integer is just the iterable repeated n times:

>>> "a" * 3
"aaa"
>>> min_depth = 2
>>> "*" * min_depth
"**"

A * can be used to unpack a tuple into function arguments, like in this function that takes an arbitrary number of parameters:

>>> def f(*args):
...  print(args)
...
>>> f(*"aaa")
('a', 'a', 'a')
>>> f(*"**")
('*', '*')

Putting this together, this just joins the base path with min_depth + 1 levels of stars:

>>> os.path.join("/home/graipher", *"*" * min_depth)
'/home/graipher/*/*'

Possible bug:

After having written this alternative implementation I noticed some strange behavior of your (second, I did not check the first) function. When minDepth is greater than zero, you would not expect any files from the base directory. However, they are still included (note that you need to start with a curDepth = minDepth, otherwise your script will never run).

This is what I would write with the additional constraints of having to return a list and possibly have a simple pattern (simple enough to be expressed with wildcard characters or an additional filter function):

def glob_list(start, max_depth=0, min_depth=0, pattern="*", func=os.path.isfile):
    output = []
    for depth in range(min_depth, max_depth + 1):
        path = os.path.join(start, *("*" * depth), pattern)
        output.extend(filter(func, glob.iglob(path)))
    return output


Determining the number of days from today Today I've got a pretty simple snippet to be reviewed. The function is extremely simple, as all it does is retrieve some entities from Core Data and then compares the dueDate attribute to today. I'm here asking about two questions:

Is this code reliable?
Is this the most efficient way (less lines of code) to accomplish this task?

The output will be separated into different arrays as seen by the comments inside the various if statements. The categories include:

Greater than One Month
One Month
One Week
Tomorrow
Today

func fetchAssignments() {

    let appDelegate =
    UIApplication.sharedApplication().delegate as! AppDelegate

    let managedContext = appDelegate.managedObjectContext!

    let fetchRequest = NSFetchRequest(entityName:"Assignment")

    var error: NSError?

    let fetchedResults =
    managedContext.executeFetchRequest(fetchRequest,
    error: &error) as! [NSManagedObject]?

    if let results = fetchedResults {
        for item in results {
            let dueDate = item.valueForKey("dueDate") as! NSDate
            let todayDate = NSDate()

            let calendar: NSCalendar = NSCalendar(calendarIdentifier: NSCalendarIdentifierGregorian)!
            let calendarComponents: NSDateComponents = calendar.components(NSCalendarUnit.CalendarUnitYear | NSCalendarUnit.CalendarUnitMonth | NSCalendarUnit.CalendarUnitDay, fromDate: todayDate, toDate: dueDate, options: .allZeros)
            println("Days: " + calendarComponents.day.description + " Months: " + calendarComponents.month.description)
            if calendarComponents.month > 0 && calendarComponents.day > 0 {
                // Greater Than One Month
                return
            } else if calendarComponents.day > 7 {
                // One Month
                return
            } else if calendarComponents.day > 1 {
                // One Week
                return
            } else if calendarComponents.day == 1 {
                // Tomorrow
                return
            } else {
                // Today
                return
            }
        }
    } else {
        println("Could not fetch \(error), \(error!.userInfo)")
    }
}

Let's start with the main error that I see: You compute the timespan between the "current date" and the "due date". If the current date is today at 11am, and the due date is tomorrow at 10am then the difference
is 0 days and 23 hours, to this is reported as "today".

This is probably not what you wanted, therefore todayDate must be
set to the start of the current day. This is easily done with

let todayDate = calendar.startOfDayForDate(NSDate())

Then todayDate and calendar should be computed only once before the
loop starts, and not for each loop iteration.

There are also problems in the analysis of the computed date components:

if calendarComponents.month > 0 && calendarComponents.day > 0 { ...

What if the difference is exactly 2 (or more) month and 0 days?

} else if calendarComponents.day > 7 {

What if the difference is exactly one month and 0 days?

The first two if-conditions should therefore be

if calendarComponents.month >= 2 || (calendarComponents.month == 1 && calendarComponents.day > 0) {
    // More than a month
} else if calendarComponents.month == 1 || calendarComponents.day > 7 {
    // Between 8 days and one month
} else if ...

Other things that can be improved:

If you create a NSManagedObject subclass for your entity then
you can use the property accessors instead of valueForKey():

let fetchedResults = managedContext.executeFetchRequest(fetchRequest,
    error: &error) as! [Assignment]?

// ... later ...
let dueDate = item.dueDate

The advantage is that you don't need the cast as! NSDate anymore
and the compiler can check for correct types.

Some type annotations are not necessary, e.g.

let calendarComponents: NSDateComponents = calendar.components(NSCalendarUnit.CalendarUnitYear | NSCalendarUnit.CalendarUnitMonth | NSCalendarUnit.CalendarUnitDay,
    fromDate: todayDate, toDate: dueDate, options: .allZeros)

can be simplified to

let calendarComponents = calendar.components(.CalendarUnitYear | .CalendarUnitMonth | .CalendarUnitDay,
    fromDate: todayDate, toDate: dueDate, options: nil)

And

println("Days: " + calendarComponents.day.description + " Months: " + calendarComponents.month.description)

can be simplified with string interpolation:

println("Days: \(calendarComponents.day) Months: \(calendarComponents.month)")


Speeding up I/O bound python program I have the following program. I have 90 million domain names split between 87 files. I need to resolve the IP of all. Right now I feel like this is super slow.
import multiprocessing
import fileinput
import socket
from datetime import datetime

socket.setdefaulttimeout(2)

def worker(i):
    filename = (str(i).zfill(len(str(111))))
    buffer_array = []
    for line in fileinput.input(f"./data/{filename}"):
        domain = '.'.join( list( reversed( line.split('\t')[1].split('.') ) ) )
        try:
            ip_list = socket.gethostbyname(domain)
            buffer_array.append(ip_list)
            if(len(buffer_array) == 1000):
                with open(f"./parsed_data/{i}", "a+") as save_file:
                    save_file.write("\n".join(buffer_array))
                    buffer_array = []
                    print(f"{datetime.now()} -- WRITING: worker {i}")
                    save_file.close()

        except Exception as e:
            pass

    with open(f"./parsed_data/{i}", "a+") as save_file:
        save_file.write("\n".join(buffer_array))
        print(f"{datetime.now()} -- WRITING ***FINAL***: worker {i}")
        save_file.close()

if __name__ == '__main__':
    jobs = []
    for i in range(87):
        p = multiprocessing.Process(target=worker, args=(i,))
        jobs.append(p)
        p.start()

In node, I was able to do 1,000,000 on a 2017 macbook pro in about 12 hours without using workers. Right now this accomplished 39 million in 32 hours. My hope was I could make it do it's thing in 12 hours (1m per worker in in 12 hours) on a AWS T3aXL
Does anyone have a bit faster way of doing this?

Minor Python cleanup
First, a bunch of little stuff:
This -
(str(i).zfill(len(str(111))))

does not need outer parens. Also, why the acrobatics to get "3"? Just declare a MAX_DIGITS = 3.
This -
'.'.join( list( reversed( line.split('\t')[1].split('.') ) ) )

should not be creating an inner list. The output of reversed is iterable.
This -
if(len(buffer_array) == 1000):

does not need outer parens; you're not in (C/Java/etc).
This -
save_file.close()

needs to be deleted both places that it appears. There's an implicit close() from your use of a with.
This -
    except Exception as e:
        pass

is a bad idea. Log or output the exception.
87 deserves a named constant.
Broad performance
gethostbyname is probably slow (as indicated in the comments). Consider running a local caching DNS resolution service to eliminate the first network hop.


Places memoized service via Google Maps I made a service which saves places to localStorage after the user clicks the left mouse button on the map. When the user clicks the right mouse button on the marker, that marker is removed from localStorage.

var map,
    markersAll = [];

function initMap() {
  map = new google.maps.Map(document.getElementById('map'), {
    zoom: 4,
    center: {lat: -25.363, lng: 131.044}
  });

  renderMarkers();

  map.addListener('click', function(e) {
    var lat = e.latLng.lat();
    var lng = e.latLng.lng();
    generateMarker(lat, lng);
    //console.log(lat, lng);
  });
}

function getMarkersFromLocalStorage() {
  return localStorage.markers ? JSON.parse(localStorage.markers) : [];
};

function addMarkerToLocalStorage(lat, lng) {
  var markers = getMarkersFromLocalStorage();
  markers.push({lat, lng});
  localStorage.markers = JSON.stringify(markers);
};

function removeMarkerFromLocalStorage(lat, lng) {
  var markers = getMarkersFromLocalStorage(),
      newMarkers = [];

  markers.forEach(function(marker) {
    if(marker.lat != lat && marker.lng != lng) {
      newMarkers.push({
        lat: marker.lat,
        lng: marker.lng
      });
    }
  });

  localStorage.markers = JSON.stringify(newMarkers);
};

function clearMarkers() {
  markersAll.forEach((marker) => {
    marker.setMap(null);
  });
};

function renderMarkers() {
  var markers = getMarkersFromLocalStorage();

  markers.forEach(function(marker) {
    //console.log(marker.lat, marker.lng);
    generateMarker(marker.lat, marker.lng);
  });
};

function generateMarker(lat, lng) {
  var marker = new google.maps.Marker({
    position: {lat: lat, lng: lng}
  });
  marker.setMap(map);
  markersAll.push(marker);
  addMarkerToLocalStorage(lat, lng);

  marker.addListener('rightclick', function(e) {
    var lat = e.latLng.lat();
    var lng = e.latLng.lng();
    //console.log('right click', lat, lng);
    clearMarkers();
    removeMarkerFromLocalStorage(lat, lng);
    renderMarkers();
  });

  return marker;
};
html, body {
  height: 100%;
  margin: 0;
  padding: 0;
}

#map {
  height: 100%;
}

The code looks like typical code that uses the Google Maps API. Nice work serializing the markers in localstorage.

I did notice that there doesn't appear to be any mechanism to prevent the user from adding one marker multiple times. You may need to change your data structure to one that associates keys with values (e.g. a (hash) Map, or use an object instead of an array).

Related to preventing markers from appearing multiple times in the array, there appears to be a bug: when the user removes a marker, all other markers get duplicated because after removeMarkersFromLocalStorage() is called, the call to renderMarkers() will call generateMarker() for each marker, which in turn adds the marker to the list again.

I would recommend that instead of adding a listener for rightclick to each marker, try to add the listener to the map. It doesn't appear that the callback run on the rightclick event utilizes any properties of the marker - just the latitude and longitude of the event. It may be the case that the Google Maps API and/or modern browsers have handled it but in general, one cause of memory leaks in Javascript is adding an event listener to a DOM element and then having that element removed. For more information on that topic , read 4 Types of Memory Leaks in JavaScript and How to Get Rid Of Them - especially the section under the heading 2: Forgotten timers or callbacks.


Secured Database Insert I've been working on a safe inserting method for a while, and I think I've found a pretty secure way to do this. Would you CRitters mind taking a look at it and maybe give me some tips to improve my script?

function dbRowInsert($table, $data) {
   require_once('../config.inc.php');

     $buildData = null;
     $countLoop = 1;

     foreach($data as $field) {
          $sep = ($countLoop!=count($data) ? ',' : '') ;
      if((int)$field == $field) {
        $buildData .= (int)$field . $sep;
      } else {
        $buildData .= '"' .mysqli_real_escape_string($conn, (string)$field) . '"' . $sep;
      }
      $countLoop++;
     }

   $fields = array_keys($data);

   mysqli_query($conn, "INSERT INTO" . $table . "(`" . implode('`, `', $fields) . "`)
                        VALUES('" . $buildData . "')");
}

It looks like you're trying to create a generic INSERT function to put any data into the given table based on an array, securely.

Since you're using mysqli_ a better approach might be to use bound parameters.

It's not really "good practice" to require your config file in the function (I'm assuming it'll be pulled in anyway), it would be better to pass your $conn variable (which I assume is a mysqli object) into the function.

I'd also structure $data so that it's an associative array of fields to values, for example (as a sort of theoretical person insert). This means your values are bound to the field names within that array which makes a foreach loop less error prone.

$data = array(
    'forename' => "Fred",
    'surname' => "Bloggs",
    'email' => "fred.bloggs@example.com"
);

You'd have to have done any validation checks beforehand of course (making sure that the email is valid for instance).

Now your function could be written:

function dbRowInsert($conn, $table, array $data) {
    $fieldInsert = "";
    $valueInsert = "";
    $valueTypes = "";

    //array for bound parameters
    $paramData = array();

    //loop through the fields
    foreach($data as $fieldName => &$fieldData) {

        //SQL inserts
        $fieldInsert .= "{$table}.{$fieldName}, ";
        $valueInsert .= "?, ";

        //determine the data types for the bound parameters
        if(is_float($fieldData)) { $valueTypes .= "d"; }
        elseif(is_int($fieldData)) { $valueTypes .= "i"; }
        else { $valueTypes .= "s"; }

        //push the field data to the bound parameters array (by reference)
        $paramData[] = &$fieldData;
    }

    //trim the trailing ", " that will have been left
    $fieldInsert = substr($fieldInsert, 0, -2);
    $valueInsert = substr($valueInsert, 0, -2);

    //prepend the bound parameters array with the data types string
    array_unshift($paramData, $valueTypes);

    //prepare the statement
    $stmt = $conn->prepare("INSERT INTO {$table} ({$fieldInsert}) VALUES ({$valueInsert})");

    //error trapping
    if(!$stmt) { throw new Exception("Failed to prepare statement - ERR: {$conn->error}", E_USER_ERROR); }

    //call_user_func_array wouldn't be necessary with PDO as
    //you could bind parameters one at a time in the loop
    call_user_func_array(array($stmt, 'bind_param'), $paramData);

    //execute ... and relax
    $stmt->execute();
}

It looks a bit longer but is has the following advantages:

it uses a prepared statement
the parameters are bound by type so it'll not be attempting to
escape integers as strings for instance

The biggest problem is that it's using mysqli_ which doesn't have a nice way to bind parameters (or values) to the statement when you have an unknown number of parameters to bind ... hence the ugly call_user_func_array usage.

This could easily be mitigated by using PDO rather than mysqli_ though.


Check if all leaves are at same level Check if all leaves are at same level. This question is attributed to geek for geeks. Looking for code-review, optimization and best practices.

public class LeavesLevel<T> {

    private TreeNode<T> root;

    public LeavesLevel(List<T> items) {
        create (items);
    }

    private void create (List<T> items) {
        root = new TreeNode<>(items.get(0));

        final Queue<TreeNode<T>> queue = new LinkedList<TreeNode<T>>();
        queue.add(root);

        final int half = items.size() / 2;

        for (int i = 0; i < half; i++) {
            if (items.get(i) != null) {
                final TreeNode<T> current = queue.poll();
                final int left = 2 * i + 1;
                final int right = 2 * i + 2;

                if (items.get(left) != null) {
                    current.left = new TreeNode<T>(items.get(left));
                    queue.add(current.left);
                }
                if (right < items.size() && items.get(right) != null) {
                    current.right = new TreeNode<T>(items.get(right));
                    queue.add(current.right);
                }
            }
        }
    }

    private static class TreeNode<T> {
        private TreeNode<T> left;
        private T item;
        private TreeNode<T> right;

        TreeNode(T item) {
            this.item = item;
        }
    }

    public boolean refurbish() {
        return recurse (root, 0,  new NodeDepth(-1));
    }

    private static class NodeDepth {
        private int depth;
        NodeDepth(int depth) {
            this.depth = depth;
        }
    }

    public boolean recurse(TreeNode<T> node, int leafLevel, NodeDepth nodeDepth) {
        if (node == null) {
            return true;
        }

        if (node.left == null && node.right == null) {
           if (nodeDepth.depth == -1) {
               nodeDepth.depth = leafLevel;
               return true; // if tree contains a single leaf node, then
           }
           return leafLevel == nodeDepth.depth;
        }

        return recurse (node.left, leafLevel + 1, nodeDepth) && recurse (node.right, leafLevel + 1, nodeDepth);
    }
}

public class LeafLevelTest {

    @Test
    public void test1() {
        /**
         *
         *           12
                   /    \
                 5       7
               /          \
              3             1
         *
         */
        LeavesLevel<Integer> ll1 = new LeavesLevel<>(Arrays.asList(1, 5, 7, 3, null, null, 1));
        assertTrue(ll1.refurbish());
    }

    @Test
    public void test2() {
        /**
         *       12
                /    \
              5       7
             /
            3
         *
         */
        LeavesLevel<Integer> ll2 = new LeavesLevel<>(Arrays.asList(1, 5, 7, 3));
        assertFalse(ll2.refurbish());
    }

    @Test
    public void test3() {
        /**
         *
         *
                    12
                    /
                  5
                /   \
               3     9
              /      /
             1      2
         *
         *
         */
        LeavesLevel<Integer> ll3 = new LeavesLevel<>(Arrays.asList(1, 5, null, 3, 9, null, null, 1, null, 2));
        assertTrue(ll3.refurbish());
    }

}

Code reuse

You've asked a few dozen questions about tree algorithms, many of which contain very similar code.  Copy-and-paste programming is an antipattern; copy-and-pasting with modifications here and there is even worse.  By now, you should have developed a library of reusable classes.

That means decomposing your classes to obey the Single Responsibility Principle.  There should be a TreeNode<T> class that acts as a generic node with two child pointers, and possibly a create(List<? extends T>) convenience method.

public class TreeNode<T> {
    public TreeNode<T> left, right;
    public final T item;

    public TreeNode(T item) {
        this.item = item;
    }

    public static <T> TreeNode<T> createTree(List<? extends T> items) {
        TreeNode<T> root = new TreeNode<>(items.get(0));

        final Queue<TreeNode<T>> queue = new LinkedList<TreeNode<T>>();
        queue.add(root);

        final int half = items.size() / 2;

        for (int i = 0; i < half; i++) {
            if (items.get(i) != null) {
                final TreeNode<T> current = queue.poll();
                final int left = 2 * i + 1;
                final int right = 2 * i + 2;

                if (items.get(left) != null) {
                    current.left = new TreeNode<T>(items.get(left));
                    queue.add(current.left);
                }
                if (right < items.size() && items.get(right) != null) {
                    current.right = new TreeNode<T>(items.get(right));
                    queue.add(current.right);
                }
            }
        }
        return root;
    }
}

There should be a TreePredicate<T> interface for algorithms that test a tree and return a true/false result.

interface TreePredicate<T> {
    boolean test(TreeNode<T> root);
}

Algorithms that implement the TreePredicate<T> interface should be in classes that are named as nouns, as with all Java classes.

Algorithm

Your use of recursion is weird.

The methods are poorly named: refurbish() is unintuitive; recurse() is too generic.  Furthermore, recurse() looks like a helper function that should not have been made public.

You shouldn't use mutation with recursion.  A key advantage of recursion is that immutable stack values make it easy to extend local reasoning to the large scale.  Impure functions with side-effects make such analysis difficult.

It should be clear what the invariants are when you recurse.  See the example JavaDoc below.

public class UniformLeafDepthPredicate<T> implements TreePredicate<T> {

    public boolean test(TreeNode<T> root) {
        return allLeavesDepth(root) >= 0;
    }

    /**
     * Returns the depth of this node, if the paths to all of its descendant
     * leaf nodes are all of the same length, or -1 if the height is
     * non-uniform.  The depth of a leaf node is 1.
     */
    private int allLeavesDepth(TreeNode<T> node) {
        if (node == null) return 0;

        int lDepth = allLeavesDepth(node.left);
        if (lDepth < 0) return -1;          // LHS has non-uniform depth
        int rDepth = allLeavesDepth(node.right);
        if (rDepth < 0) return -1;          // RHS has non-uniform depth

        if (lDepth == 0) return 1 + rDepth; // Only R child; RHS has uniform depth
        if (rDepth == 0) return 1 + lDepth; // Only L child; LHS has uniform depth

        return (lDepth == rDepth) ? 1 + lDepth : -1;
    }
}

By following these guidelines for recursion, the code is greatly simplified.

Unit tests

I don't know why you labelled the root node as "12" in your comments.  It's inconsistent with your actual tests.

I suggest adding one more test, because you currently have no test in which a child node has non-uniform depth.

public class LeafLevelTest {

    private TreePredicate<Integer> tester = new UniformLeafDepthPredicate<>();

    …

    @Test
    public void test4() {
        /*
                   1
                  /
                 5
                / \
               3   9
              /
             4
         */
        TreeNode<Integer> ll4 = TreeNode.createTree(Arrays.asList(1, 5, null, 3, 9, null, null, 4));
        assertFalse(tester.test(ll4));
    }
}


Asking for a name and showing the initials I've seen code that basically does the same thing, but i'd like some input on this. Basically what i did wrong or not good enough heh :).

#include <stdio.h>
#include <String.h>
#include <stdlib.h>
#include <ctype.h>
char *getName() {
    char name[100];
    int length;
    char *holder;
    printf("Tell me your name!\n");
    fgets(name,100,stdin);

    length = strlen(name)+1;

    holder = (char*)malloc(length*sizeof(char));
    strcpy(holder, name);

    printf("The name is: %s", holder);

    return holder;

}
char getInitials(char *name) {
    int length,i,j = 0;

    char *initials;

    length = strlen(name);

    initials = (char*)malloc(length*sizeof(char));

    for(i = 0;i < length;i++){
        if(i == 0)    {
            initials[j] = toupper(name[i]);
            j++;
      }
        else if(name[i] == ' '){
            initials[j] = toupper(name[i+1]);
            j++;

        }
    }
    initials[j] = '\0';
    printf("The initials are: %s",initials);
    free(initials);

}
int main() {
    char *name = getName();
    getInitials(name);
}

White Space

As was mentioned in the comments by @Aluan Haddad, your initial code was missing white space between things like operators and operands.  As it stands, your code is still missing standard white space, such as a blank line between methods, a blank line between the end of your #include section and the start of the code.  It makes your code feel a bit cramped.

Const

If you're not going to be modifying the contents of variables passed into functions you should be declaring the parameters as const.  This helps the client to know that it is safe for example to pass a string literal into your getInitials function.

Users are unreliable

Even well users with good intentions make mistakes.  You're not capturing all of those mistakes with your current code.  What happens if the user puts two spaces between parts of their name?

Cleanup

In getName, you malloc up a buffer which you never clean up.  It may be that you've intentionally done this because you know that it will be cleaned up anyway when the program exits.  Alternately it may be that you've forgotten to do it.  Generally I'd suggest always cleaning up after your malloc's because that way whoever is reading the code knows you've thought about it.

Naming

Some of your names are good and appropriate (name, initials), others are less descriptive (holder, j).  Good naming makes your code a lot easier to follow.

Live up to your promises

getInitials says that it returns a char.  It doesn't.

Be consistent

getName prompts the user, outputs what they tell the program, then returns the value.  getInitials on the other hand parses a string, outputs the initials and then doesn't return anything.  The prefix get really suggests that the methods should be doing a similar level of task.

Separate out user interactions

getInitials could be written in such a way that it took in a name and returned a string containing the initials, then it could be called from future programs that had different user interfaces.  At the moment, it has a printf in it which makes this more challenging.  Try to isolate your user interactions (inputs/outputs) from your main logic (string processing in this case).

Allocate how much?

getInitials allocates sufficient memory to store every character from the persons name.  Is this really required?  Since you've declared a buffer for the persons name as 100, should this really be a constant that can be reused so that getInitials doesn't need to malloc, it can just have a local buffer?

Multiple declarations on a single line

I don't really like them.  I find that they make it hard to locate variables so I would avoid this:

int length,i,j = 0;

Unnecessary casting

You should avoid unnecessary casting.  Since malloc returns a void*, you don't need to cast it when assigning it to a char*.  You can simply do:

initials = malloc(length);


Display text on video screen The code displays a video that will play a video and then display some text on the screen at specific locations on the video screen.

The code works but I would like to see if I could change somethings to make it better and how I would do that. You should be able to just copy the code into a web browser and it should work fine.

<!doctype html>
<html>
<head>
<script type="text/javascript">
window.addEventListener('load', eventWindowLoaded, false);
var videoElement;
var videoDiv;

function eventWindowLoaded() {
    videoElement = document.createElement("video");
    videoDiv     = document.createElement('div');

    document.body.appendChild(videoDiv);
    videoDiv.appendChild(videoElement);
    videoDiv.setAttribute("style", "display:none;");
    var videoType = supportedVideoFormat(videoElement);

    videoElement.addEventListener("canplaythrough",videoLoaded,false);
    videoElement.setAttribute("src", "muirbeach." + videoType);
}

function supportedVideoFormat(video) {
    var returnExtension = "";
    if (video.canPlayType("video/webm") =="probably" || video.canPlayType("video/webm") == "maybe") {
        returnExtension = "webm";
    } else if(video.canPlayType("video/mp4") == "probably" || video.canPlayType("video/mp4") == "maybe") {
        returnExtension = "mp4";
    } else if(video.canPlayType("video/ogg") =="probably" || video.canPlayType("video/ogg") == "maybe") {
        returnExtension = "ogg";
    }
    return returnExtension;
}

function canvasSupport () {
    return Modernizr.canvas;
}

function videoLoaded() {
    canvasApp();
}

function canvasApp() {
    function  drawScreen () {
        //Background and redraw background screen
        context.fillStyle = '#ffffaa';
        context.fillRect(0,0,theCanvas.width,theCanvas.height);
        context.strokeStyle = '#000000';
        context.strokeRect(5,5,theCanvas.width-10,theCanvas.height-10);

        context.drawImage(videoElement,85,30);

        context.fillStyle = "#000000";
        context.font      = "10px sans";
        context.fillText  ("Duration:"     + videoElement.duration,10,280);
        context.fillText  ("Current time:" + videoElement.currentTime,260,280);
        context.fillText  ("Loop: "        + videoElement.loop,10,290);
        context.fillText  ("Autoplay: "    + videoElement.autoplay,80,290);
        context.fillText  ("Muted: "       + videoElement.muted,160,290);
        context.fillText  ("Controls: "    + videoElement.controls,240,290);
        context.fillText  ("Volume: "      + videoElement.volume,320,290);

        //Display Message
        for (var i =0; i < messages.length ; i++) {
            var tempMessage = messages[i];
            if (videoElement.currentTime > tempMessage.time) {
                context.font = "bold 14px sans";
                context.fillStyle    = "#FFFF00";
                context.fillText  (tempMessage.message,  tempMessage.x                                                                 ,
                            tempMessage.y);
            }
        }
    }

    var messages = new Array();
    messages[0] = {time:7,message:"", x:0 ,y:0};
    messages[1] = {time:1,message:"I love David Mathews Band!", x:90 ,y:200};
    messages[2] = {time:4,message:"Look At Those Waves!", x:240 ,y:240};
    messages[3] = {time:8,message:"Look At Those Rocks!", x:100 ,y:100};

    var theCanvas = document.getElementById('canvasOne');
    var context   = theCanvas.getContext('2d');
    videoElement.play();

    function gameLoop() {
        window.setTimeout(gameLoop, 20);
        drawScreen();
    }

    gameLoop();
}
</script>

</head>
<body>
<div style="position: absolute; top: 50px; left: 50px;">
       <canvas id="canvasOne" width="500" height="300"></canvas>
</div>

</body>
</html>

Interesting code,

you are skirting the rules of 'code you wrote', but I still think review of this code is valuable. In essence this code is fine, there are some style changes you might want to consider.

I am guessing part of your goal is to have a "one page has it all" HTML page, but you should really have that JavaScript in separate script file
You create a video and a div tag with JavaScript, I would have these elements already in the HTML
You are not using canvasSupport anywhere
videoLoaded is a one liner that calls another functions without parameters, I would kill that function and do

videoElement.addEventListener("canplaythrough",canvasApp,false);

It is considered better to have one single comma separated var statement:

var videoElement;
    videoDiv;

I am a fan of early returns, I would write supportedVideoFormat like this:

function supportedVideoFormat(video) {
    if (video.canPlayType("video/webm") =="probably" || video.canPlayType("video/webm") == "maybe") {
        return "webm";
    }
    if(video.canPlayType("video/mp4") == "probably" || video.canPlayType("video/mp4") == "maybe") {
        return "mp4";
    }
    if(video.canPlayType("video/ogg") =="probably" || video.canPlayType("video/ogg") == "maybe") {
        return "ogg";
    }
    return "";
}

But probably I would then get irritated by the repeated calls for the same property, so I would probably shortcut those, it would also help with the horizontal stretching:

function supportedVideoFormatExtesion(video) {
   var supportsWebM = video.canPlayType("video/webm"),
       supportsMP4  = video.canPlayType("video/mp4"),
       supportsOgg  = video.canPlayType("video/ogg");
    if (  supportsWebM =="probably" || supportsWebM == "maybe") {
        return "Webm";
    }
    if( supportsMP4 == "probably" || supportsMP4 == "maybe") {
        return "mp4";
    }
    if( supportsOgg =="probably" || supportsOgg == "maybe") {
        return "ogg";
    }
    return "";
}

This would irritate me as well, at this point it is blindingly obvious that you are trying to map a video format string to an extension string.. The above code is not the best approach to do that..

function supportedVideoFormatExtesion(video) {
   var formats    = ['video/webm','video/mp4','video/ogg'],
       extensions = ['Webm','mp4','ogg'],
       support;
   for( var i = 0 ; i < formats.length ; i++ ){
     support = video.canPlayType( formats[i] );
     if( support == 'probably' || support == 'maybe' )
     {
       return extensions[i];
     }
   }
   return "";
}

Now I can support multiple versions, and adapt easily to future changes.
In canvasApp, I would put the var statements on top. It looks odd to see you access theCanvas without it being assigned. It works, because of hoisting, but that doesnt mean you have to put all the declarations at the bottom.
The messages code looks odd, I will just say that var messages = []; is preferred over var messages = new Array(); and that personally I would write

var messages = [
    {time:7,message:"", x:0 ,y:0},
    {time:1,message:"I love David Mathews Band!", x:90 ,y:200},
    {time:4,message:"Look At Those Waves!", x:240 ,y:240},
    {time:8,message:"Look At Those Rocks!", x:100 ,y:100}
]


x86 assembly function that determines whether an array is continuous I wrote an x86 assembly function using MASM that tests for whether or not an array is continuous.  That is to say, if an array with length n that starts with the value k, all the subsequent entries in the array are k+1, k+2, ... k+n.

I am interested in becoming better at x86 and x86-64 assembly and I'm wondering if I could get some feedback on the quality of:

The algorithm itself
Comments - am I too thorough? Not thorough enough?
Label names - are they typically what you would expect handwritten x86 labels to look like?
The part at the end where I set eax in three different cases then jump to a final pop esi/ret instruction - this felt rather ugly to me, but I couldn't think of a better way to do it

Assembly code:

.386
.MODEL FLAT, C
.CODE
; Checks if an array if integers is continuously increasing.
; I.e. if an array of length n starts with element k, all
; the elements in the array are k, k+1, k+2, k+3... k+n
; Uses:
;   ESI - Loop counter (when loop >= length, done looping)
;   ECX - Data counter (compare for equality with k, k+1, etc...)
;   EDX - Pointer to start of array
; Params:
;   ESP+4 - Array pointer
;   ESP+8 - Array length
; Because of the PUSH ESI instruction at the start of the function,
; the value relative to ESP increases by 4 when they are accessed
; Returns:
; In EAX:
;   1 - If the array is continuous
;   0 - If the array is not continuous
;  -1 - If the function was passed an invalid (<1) length
iscontinuous PROC
    ; Preserve ESI
    push    esi
    ; Check for valid (>=1) length
    mov esi, dword ptr [esp+12]
    cmp esi, 1
    jl  invalid_length
    ; Load data array pointer into EDX and deref into ECX (for first number in data)
    mov edx, dword ptr [esp+8]
    mov ecx, dword ptr [edx]
    ; Set EAX as loop counter (start at 0 and keep going till EAX=ESI)
    xor eax, eax
continuity_loop:
    ; Compare array element with current position of counter (pointer to array + offset * sizeof(int))
    cmp dword ptr [edx+eax*4], ecx
    jne not_continuous
    inc     ecx
    inc eax

    cmp eax, esi
    jl  continuity_loop

; If we reached here without branching into not_continuous, we are good to go; return 1 indicating success
    mov eax, 1
    jmp end_iscontinuous
not_continuous:
    xor eax, eax
    jmp end_iscontinuous
invalid_length:
    mov eax, -1
end_iscontinuous:
    pop esi
    ret
iscontinuous ENDP
END

C code for test driver:

#include <stdio.h>

int __cdecl iscontinuous(int *data, int len);

int __cdecl wmain(int argc, wchar_t *argv[])
{
    int test[20] = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 13, 14, 15, 16, 17, 18, 19, 20 };
    wprintf_s(L"Test 1 (expected pass): test[0..11] = %d\nTest 2 (expected fail): test[0..20] = %d\nTest 3 (expected pass): test[13..20] = %d\n",
        iscontinuous(test, 11),
        iscontinuous(test, 20),
        iscontinuous(test+12, 7)
    );
    return 0;
}

Program output:

Test 1 (expected pass): test[0..11] = 1
Test 2 (expected fail): test[0..20] = 0
Test 3 (expected pass): test[13..20] = 1

My thoughts, in no particular order:

I'm ok with the comments.  Some things seem obvious to me, but I'd rather have an unneeded obvious comment than have something that ISN'T obvious (to me) lacking a comment.
The label names seem fine.
You might want to include the C prototype as a comment in the asm code.  If someone wants to use this code, being able to copy/paste this into their C code might be valuable.
You might want to special case handling the first array entry.  Not much point in comparing the value you just read with the memory location you just read it from.  Maybe inc edx right before continuity_loop and move the inc ecx to right after?
How about changing the return codes so that -1 indicates success, 0 indicates bad length, and >0 indicates the mismatched index?  This lets you return more useful information than just 'failed'.  Also, it helps with how often you have to set eax (hint: move the xor eax, eax up after the push; then eax already contains the 'right' return value in 2/3 cases).
Spacing problem on inc     ecx?

Note I haven't tried the things I'm suggesting.  Sometimes things look good in abstract, but don't work out as neatly when you actually try to write them.

There might be some way to tighten up continuity_loop, but (other than moving the inc ecx) I'm not seeing it right now.  If you end up making the other changes, I can give it a second look.


Reversing a 64 bit Integer Example: 00010111 -> 11101000

public long reverse(long x) {
    long r = 0;
    int i = 0;
    while (x > 0) {
        int bit = 0x0001 & x;
        r += bit << (63 - i);
        x >>= 1;
        i++;
    }
    return r;
}

Can someone review my code and provide any comments or suggest a better way of doing this?

This code does not work, right?

What you have is an infinite loop, because x >> 1; does not do what you think it does.

There are a number of things wrong in here....

if the input long value is negative, you will never reverse it (it's < 0).
you should read up on the zero-shifting operator >>>
you will need to do all the 64 bits of shift otherwise the you will only reverse the set bits .... i.e. 0x00000000000001 should reverse to 0x8000000000000000 and not 0x00000000000001

The code you probably want will look more like:

public long reverse(long x) {
    long r = 0;
    for (int i = 63; i >= 0; i--) {
        r |= ((x >>> i) & 0x1L) << (63 - i);
    }
    return r;
}

Of course, you could always do:

public long reverse(long x) {
    return Long.reverse(x);
}


Detect locale with PHP I am developing a multilingual site, so i needed some sort of a function to detect user locale, as well as not to scare away search engines, so I wrote this kind of a method to detect locale.
The priority is as following:

We look if user has active language in $_SESSION;
If not, we look in the database to detect user preference;
If nothing in the database, we look on the domain name and try to detect locale from domain name and superglobal $_SERVER;
If we failed, we look at user's browser locale ($_SERVER again) and try to detect from it.
If we are totally a failure, we fallback to English.

This method is called on every page of a website.

public static function detect_locale() {
    if(isset($_SESSION["lang"])){
        $lang = $_SESSION["lang"];
        return $lang;
    }
    if(isset($_SESSION["id"])){
        $user = new User($_SESSION["id"]);
        $lang = $user->get_locale();
        return $lang;
    }
    if($_SERVER["HTTP_HOST"] == "ru.mysupersite.com" || $_SERVER["HTTP_HOST"] == "www.mysupersite.ru"){
        $lang = "ru";
        return $lang;
    }
    if(isset($_SERVER["HTTP_ACCEPT_LANGUAGE"])){
        $lang = substr($_SERVER["HTTP_ACCEPT_LANGUAGE"], 0, 2);
        return $lang;
    }
    $lang = "en";
    return $lang;
}

I see it as a pretty straightforward solution, but something in the back of my mind says that there might be some flaws. Are there any or am I just paranoid?

Thank you!

Your basic hierarchy seems fairly solid.  There is just one thing - HTTP_ACCEPT_LANGUAGE is not so simple.  I wrote an answer on it here.  What follows is the relevant code from that:

// Parse the Accept-Language according to:
// http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.4
preg_match_all(
   '/([a-z]{1,8})' .       // M1 - First part of language e.g en
   '(-[a-z]{1,8})*\s*' .   // M2 -other parts of language e.g -us
   // Optional quality factor M3 ;q=, M4 - Quality Factor
   '(;\s*q\s*=\s*((1(\.0{0,3}))|(0(\.[0-9]{0,3}))))?/i',
   $_SERVER['HTTP_ACCEPT_LANGUAGE'],
   $langParse);

$langs = $langParse[1]; // M1 - First part of language
$quals = $langParse[4]; // M4 - Quality Factor

$numLanguages = count($langs);
$langArr = array();

for ($num = 0; $num < $numLanguages; $num++)
{
   $newLang = strtoupper($langs[$num]);
   $newQual = isset($quals[$num]) ?
      (empty($quals[$num]) ? 1.0 : floatval($quals[$num])) : 0.0;

   // Choose whether to upgrade or set the quality factor for the
   // primary language.
   $langArr[$newLang] = (isset($langArr[$newLang])) ?
      max($langArr[$newLang], $newQual) : $newQual;
}

// sort list based on value
// langArr will now be an array like: array('EN' => 1, 'ES' => 0.5)
arsort($langArr, SORT_NUMERIC);

// The languages the client accepts in order of preference.
$acceptedLanguages = array_keys($langArr);

// Set the most preferred language that we have a translation for.
foreach ($acceptedLanguages as $preferredLanguage)
{
    if (in_array($preferredLanguage, $websiteLanguages))
    {
       $_SESSION['lang'] = $preferredLanguage;
       return $preferredLanguage;
    }
}

You would need to define website languages or just set the most preferred language.

Also as a very minor point I would change:

$lang = xxx;
return $lang;

To:

return xxx;


simple shared pointer implementation in single threaded environment Update: The code below has some obvious problems, and it is not allowed to revise code in the original question, so I have to post a new question, the revised code is here.

std::shared_ptr has the potential problem that two pointer point to the same object but to different control blocks. And its atomic operation is pure overhead in a single threaded environment. So I wrote the following version to avoid these problems. This implementation also has the advantage that an empty pointer is the same size as a void *.

template <typename T>
struct ThreadUnsafeSharedPtr {

private:
    struct Pair {
        T data;
        size_t cnt;
        template <typename ... Args>
        Pair(Args && ... args): data(std::forward<Args>(args)...), cnt(1) {}
    };
    Pair *ptr;

public:
    template <typename ... Args>
    static ThreadUnsafeSharedPtr make(Args && ... args) {
        ThreadUnsafeSharedPtr p;
        p.ptr = new Pair(std::forward<Args>(args)...);
        return p;
    }

    ThreadUnsafeSharedPtr() : ptr(nullptr) {}

    ThreadUnsafeSharedPtr(const ThreadUnsafeSharedPtr & other) : ptr(other.ptr) {
        if (ptr && &other != this) {
            ++(ptr->cnt);
        }
    }
    ThreadUnsafeSharedPtr & operator=(const ThreadUnsafeSharedPtr & other) {
        ptr = other.ptr;
        if (ptr && &other != this) {
            ++(ptr->cnt);
        }
        return *this;
    }
    ThreadUnsafeSharedPtr(ThreadUnsafeSharedPtr && other) : ptr(other.ptr) {
        if (&other != this) {
            other.ptr = nullptr;
        }
    }
    ThreadUnsafeSharedPtr & operator=(ThreadUnsafeSharedPtr && other) {
        if (&other != this) {
            ptr = other.ptr;
            other.ptr = nullptr;
        }
        return *this;
    }

    ~ThreadUnsafeSharedPtr() {
        if (ptr) {
            if(--ptr->cnt == 0) {
                delete ptr;
            }
        }
    }
};

Your implementation misses to implement pointer semantics in the form of an overloaded operator->() and operator*().

Isn't that the whole point of a pointer surrogate?


Unbreakable average program I'm creating a program that averages four numbers - three from a file and one from user input. Simple enough. But the catch is, the professor is going to be deliberately trying to break the program.

I'd like advice in any regard, but remember the focus is on robustness.

import java.util.*;
import java.io.*;

public class average {
    public static void main (String[] args) throws IOException{
        double[] nums = new double[4];
        nums = inputHandler();
        System.out.println("Average " + findAverage(nums));

        System.out.println("End Program.");
    }

    public static double findAverage(double[] nums){
        double average = 0.0;
        final double N = 4.0;

        for(int i = 0; i < N; i++){
            average += nums[i] / N;
        }

        return average;
    }

    public static double[] inputHandler() throws IOException{
        Scanner input = new Scanner(System.in);
        double[] nums = new double[4];
        double[] fileNums = new double[3];

        // get first three numbers from file
        System.out.print("Enter the name of the file which contains the first three numbers: ");
        fileNums = fileInput(input.nextLine());
        System.out.println();

        // copy numbers from fileNums[] to nums[]
        for(int i = 0; i < 3; i++){
            nums[i] = fileNums[i];
        }

        //get last number from user input
        System.out.print("Enter the fourth number: ");
        nums[3] = userInput(input.nextLine());
        System.out.println();

        input.close();

        return nums;
    }

    public static double[] fileInput(String fileName) throws IOException{
        double[] nums = new double[3];

        System.out.println();

        File file = new File(fileName);

        if(!file.isFile()){
            System.err.println("ERROR: File does not exist.");
            System.exit(-1);
        }

        System.out.println(fileName + ":");

        Scanner input = new Scanner(file);
        String cur = "";

        for(int i = 0; i < 3; i++){

            if(!input.hasNext()){
                System.err.println("ERROR: File does not contain enough numbers.");
                System.exit(-1);
            }

            cur = input.next();

            System.out.println("Number " + (i + 1) + ": " + cur);

            if(cur.matches("-?\\d+(\\.\\d+)?")){ //regex ensures numeric input
                if(Double.parseDouble(cur) < 1.7e308){
                    nums[i] = Double.parseDouble(cur);
                }
                else{
                    System.err.println("ERROR: Number too large");
                    System.exit(-1);
                }
            }
            else{
                System.err.println("ERROR: Non numeric input. Please check your file and try again.");
                System.exit(-1);
            }
        }

        input.close();

        return nums;
    }

    public static double userInput(String userInput){
        double num = 0;

        if(userInput.matches("-?\\d+(\\.\\d+)?")){ //regex ensures input is numeric
            num = Double.parseDouble(userInput);
        }
        else{
            System.err.println("ERROR: Non numeric input. Please check your file and try again.");
            System.exit(-1);
        }

        return num;
    }
}

First, use try/catch blocks around parts that can throw exceptions.  For example, what happens when an IOException is thrown by the file, perhaps because the data is corrupted and the file exists, but cannot be opened?

Second, I don't know what constitutes failure, but an app-closing failure is failure in my book:

if(!file.isFile()){
    System.err.println("ERROR: File does not exist.");
    System.exit(-1);
}

Third, @SirPython is right - use the built-in method for reading your number.  There are built-in functions provided for reading both doubles and ints, whichever your program needs.

Fourth, this will not necessarily help robustness, split your method logic up.  You are doing a lot of printing in your fileInput() method.  Do your printing in a dedicated method.  Do your input in a dedicated method.  Feel free to throw an exception when exceptional behavior happens, but be sure to handle the exception.  Split your logic up into bite-sized components and implement them separately - this will make reuse easy.

Fifth, recover from your errors.  If you need, start the entire input processes over, but do not close the program.  Closing the program because of an error will probably be counted as an error, and is unacceptable in many programs.