Simple jQuery drag event I would like some comments on my code and implementation for this simple piece of jQuery.

Basically it is 2 div boxes #side1 and #side2, in a container div called #frontimage, where #side2 can be swung out by dragging from right to left along #frontimage.

My jQuery code below continuously updates the CSS transform rotateY parameters for #side2 based on a mousemove event.

Here is a working example:

http://jsfiddle.net/Sr4zw/embedded/result/

And here is the complete code including HTML and CSS:

http://jsfiddle.net/Sr4zw/

$(document).ready(function() {

    var yDegrees = 0, slidingLength = 372, maxRotation = -45;

    $("#frontimage").on('mousedown', function( event ) {

        var originX = event.pageX;

        $("#frontimage").on("mousemove", function( event ) {

            yDegrees = yDegrees + (((originX - event.pageX) / slidingLength) * maxRotation);
            if (yDegrees < maxRotation) {
                yDegrees = maxRotation;
            };
            if (yDegrees > 0) {
                yDegrees = 0;
                };

            originX = event.pageX;

            $("#side2").css({
                '-webkit-transform': 'rotateY(' + yDegrees + 'deg)',
                '-moz-transform': 'rotateY(' + yDegrees + 'deg)',
                '-ms-transform': 'rotateY(' + yDegrees + 'deg)',
                '-o-transform': 'rotateY(' + yDegrees + 'deg)',
                'transform': 'rotateY(' + yDegrees + 'deg)'
            });
        });
    });

    $('html').on('mouseup', function() {

        $("#frontimage").off('mousemove');

    });
});

Some comments on my implementation:

yDegrees stores the current rotation degree.
slidingLength is the length (in pixels) of the mouse drag that changes yDegrees.

The semicolons after the closing braces are really unnecessary, and the indentation is a bit off here:

        if (yDegrees < maxRotation) {
            yDegrees = maxRotation;
        };
        if (yDegrees > 0) {
            yDegrees = 0;
            };

It would be better this way:

        if (yDegrees < maxRotation) {
            yDegrees = maxRotation;
        }
        if (yDegrees > 0) {
            yDegrees = 0;
        }

This could be written more compactly:

yDegrees = yDegrees + (((originX - event.pageX) / slidingLength) * maxRotation);

Like this:

yDegrees += (originX - event.pageX) / slidingLength * maxRotation;

That is, using += and removing unnecessary parentheses.


Replacing a for loop that iterates over an array with the functional programming equivalent How would I accomplish the same thing in a functional paradigm?

Player.prototype.d2 = function(ratingList, rdList) {
    var tempSum = 0;
    for (var i = 0; i < ratingList.length; i++) {
        var tempE = this.e(ratingList[i], rdList[i]);
        tempSum += Math.pow(this.g(rdList[i]), 2) * tempE * (1 - tempE);
    }
    return 1 / Math.pow(q, 2) * tempSum;
};

I think something like this will work for you.

Array.prototype.zip = function(other) {
    if (!Array.prototype.isPrototypeOf(other)) {
            throw new TypeError('Expecting an array dummy!');
    }
    if (this.length !== other.length) {
            throw new Error('Must be the same length!');
    }
    var r = [];
    for (var i = 0, length = this.length; i < length; i++) {
        r.push([this[i], other[i]]);
    }
    return r;
};

Player.prototype.d2 = function (ratingList, rdList) {
    var self = this;
    return 1 / Math.pow(q, 2) *
        ratingList
        .zip(rdList)
        .map(function (elem) {
            var tempE = self.e(elem[0], elem[1]);
            return Math.pow(self.g(elem[1]), 2) * tempE * (1 - tempE);
        })
        .reduce(function (acc, value) {
            return acc + value;
        });
};


Simplify code using array I am trying to simplify the amount of code required here using arrays, I feel like my foreach loop is still excessively long. Any advice would be appreciated.

$date_list = Array(
Array( 'age', "$birthday", 'now' ),
Array( 'membership', "$datejoined", 'now' ),
Array( 'promoted', "$lastpromo", 'now' ),
);

foreach ( $date_list as $date_set ) {
$var = $date_set[0];
$start = $date_set[1];
$end  = $date_set[2];
$datetime1 = date_create($start);
$datetime2 = date_create($end);
$interval = date_diff($datetime1, $datetime2);
if ( substr_count( $var, 'age' ) > 0 ){
    $age = $interval->format('%y');
}
elseif ( substr_count( $var, 'membership' ) > 0 ){
    $years = $interval->format('%y');
    $months = $interval->format('%m');
    $membership = floor(($years*12)+$months);
    if($membership > 1){
        $suffix = 'Months';
    }
    elseif($membership == 1){
        $suffix = 'Month';
    }
    else{
        $membership = "< 1";
        $suffix = 'Month';
    }
}
elseif ( substr_count( $var, 'promoted' ) > 0 ){
    $years = $interval->format('%y');
    $months = $interval->format('%m');
    $test = $interval->format('%a');
    $promoted = floor(($years*12)+$months);
    if($promoted > 1){
        $suffix = 'Months ago';
    }
    elseif($promoted == 1){
        $suffix = 'Month ago';
    }
    else{
        $promoted = "< 1";
        $suffix = 'ago';
    }
}

}

It's hard to suggest simplifications without a greater understanding of the problem being solved.  However, this is perhaps a bit more readable:

$dates = array(
    array('type' => 'age',        'start' => $birthday,   'stop' => 'now' ),
    array('type' => 'membership', 'start' => $datejoined, 'stop' => 'now' ),
    array('type' => 'promoted',   'start' => $lastpromo,  'stop' => 'now' ),
);

foreach($dates as $date)
{
    $start = date_create($date['start']);
    $stop  = date_create($date['stop']);
    $interval = date_diff($start, $stop);

    switch($date['type'])
    {
        case 'age':
            $age = $interval->format('%y');
            break;

        case 'membership':
            $years      = $interval->format('%y');
            $months     = $interval->format('%m');
            $membership = floor(($years*12)+$months);

            if($membership >  1) { $suffix = 'Months'; }
            if($membership == 1) { $suffix = 'Month'; }
            if($membership <  1) { $suffix = 'Month'; $membership = '< 1'; }
            break;

        case 'prompted':
            $years    = $interval->format('%y');
            $months   = $interval->format('%m');
            $test     = $interval->format('%a');
            $promoted = floor(($years*12)+$months);

            if ($promoted  > 1) { $suffix = 'Months ago'; }
            if ($promoted == 0) { $suffix = 'Month ago';  }
            if ($promoted  < 1) { $suffix = 'ago'; $promoted = '< 1'; }
            break;
    }
}

?


Binary search implementation Is there a better way to implement a binary search than I am in my code? My function is taking two two lists - first_booth_voters and second_booth_voters with second_booth_voters being sorted and first_booth_voters not sorted. I feel like I am making an unnecessary amount of comparisons.

from classes import VoterList

def fraud_detect_bin(first_booth_voters, second_booth_voters):
    fraud = VoterList()
    comparisons = 0

    for i in range(0, len(second_booth_voters)):
        item = second_booth_voters[i]
        first = 0
        last = len(second_booth_voters) - 1

        while first <= last:
            midpoint = (first + last) // 2

            if first_booth_voters[midpoint] == item:
            comparisons += 1
            fraud.append(first_booth_voters[midpoint])
            break
        else:
            if item < first_booth_voters[midpoint]:
                last = midpoint - 1
                comparisons += 2
            else:
                first = midpoint + 1
                comparisons += 2

    return fraud, comparisons

Don't use this, it's a redundant duplicate

for i in range(0, len(second_booth_voters)):
    item = second_booth_voters[i]

Instead use Python's for value in iterable syntax that lets you directly get the value of each list item for each iteration.

for item in second_booth_voters:

Also you should include a docstring.


Generate table of contents form HTML I've written a small bit of JavaScript which parses some HTML and generate a table of contents based on the various heading tags. The table of contents needs to be nested (E.g. H2 tags appear under H1).

My code is currently working well but I'm not sure the my method of storing all the headings is the best (it's been awhile since I've had to do much JS). The headings are being stored in a series of nested objects. The objects are made up of two properties (#title and #id) to store the heading title and anchor link. It can then contain incrementally numbered sub-heading objects.

A large portion of my code seems to be dealing with finding the last item in the object to append the next heading to. I don't know if there would be an easier way of doing this with nested arrays.

The script runs the parseDOM function passing it the DIV containing the report. Once that's complete the resulting object is passed to a series of functions which turns it into a series of ordered list elements.

I've stripped out the parts for generating the table of contents HTML, I can post it if you want it though.

var heading_selectors = [
 'H1',
 'H2',
 'H3',
 'H4',
 'H5',
 'H6',
];
function parseDOM(report) {
  // Generate selector from heading selectors.
  var selector = heading_selectors.join(',');
  var nav_hierarchy = {};
  var next_heading_id = 0;
  $(selector, report).each(function(index, heading_element) {
    var heading = $(heading_element);
    // Determine heading level.
    var tag_name = heading.get(0).nodeName;
    var level = heading_selectors.indexOf(tag_name);
    var title = heading.text();
    var id = next_heading_id;
    var id_name = id + '-' + level + '-' + title.replace(/\s+/g, '-').toLowerCase();
    heading.attr('id', id_name);

    // Create object to hold heading.
    var new_heading = {
      '#title': title,
      '#id': id_name,
    };

    var parent_item = lastHeadingAtLevel(nav_hierarchy, level);
    parent_item[id] = new_heading;

    // Increment heading id.
    next_heading_id++;
  });

  return nav_hierarchy;
}

/**
 * Returns the last heading at the specified level.
 *
 * @param object heading Heading to retrieve the last element of.
 * @param int level What level of heading to retrieve from.
 *
 * @return object Last heading at specified level.
 */
function lastHeadingAtLevel(heading, level) {
  if (level !== 0) {
    // Recurse through lists until you hit the required level.
    var last_item = getLastHeading(heading);
    last_item = lastHeadingAtLevel(last_item, level-1);
    return last_item;
  }
  // No recursion required.
  return heading;
}

/**
 * Return the last heading in the object (excludes properties beginning with #).
 *
 * @param object heading Heading to retrieve last heading of.
 *
 * @return object Last Heading of object.
 */
function getLastHeading(heading) {
  // Get the keys of the current heading.
  var keys = Object.keys(heading);
  // Default last_element to lowest value
  var last_element = -1;
  for (var i = 0; i < keys.length; i++) {
    if (keys[i].startsWith('#')) {
      continue;
    }
    if (parseInt(keys[i], 10) > last_element) {
      last_element = parseInt(keys[i], 10);
    }
  }

  // Return the last element added.
  return heading[last_element];
}

First, your indentation is half-indentation. It should be four spaces instead of two. Try putting your code into JSFiddle's TidyUp function to see how to better format.

You can improve the following block by swapping the logic and not initialising last_item:

You also don't need a === check, == will do for integers.

function lastHeadingAtLevel(heading, level) {
  if (level !== 0) {
    // Recurse through lists until you hit the required level.
    var last_item = getLastHeading(heading);
    last_item = lastHeadingAtLevel(last_item, level-1);
    return last_item;
  }
  // No recursion required.
  return heading;
}

by applying those changes, you get the following:

function lastHeadingAtLevel(heading, level) {
    if (level == 0) {
        // No recursion required.
        return heading;
    }
    // Recurse through lists until you hit the required level.
    var last_item = getLastHeading(heading);
    return lastHeadingAtLevel(last_item, level - 1);
}

Instead of string concatenation (can be slow), you should use an array join instead (faster):

var id_name = id + '-' + level + '-' + title.replace(/\s+/g, '-').toLowerCase();

into:

var id_name = [id, level, title.replace(/\s+/g, '-').toLowerCase()].join('-');

In the following block, keys is an array, so instead of a for loop, use a forEach loop:

  for (var i = 0; i < keys.length; i++) {
    if (keys[i].startsWith('#')) {
      continue;
    }
    if (parseInt(keys[i], 10) > last_element) {
      last_element = parseInt(keys[i], 10);
    }
  }

which could transform it into the following:

var lastElement = -1;
Object.keys(heading).forEach(function(key){
    if (key.startsWith('#')) {
        continue;
    }
    if (parseInt(key, 10) > lastElement){
        lastElement = parseInt(key, 10);
    }
});

However, in fact, you don't even need to do that.

Using a polyfill, you can access the last item:

if (!Object.prototype.last){
    Object.prototype.last = function(){
        return this[Object.keys(this)[Object.keys(this).length - 1]];
    };
};

Now just access it like: {'a': 1, 'b': 2, 'c': '3'}.last();


Smallest expression within brackets I would like to know if this is a good approach to finding out all the expressions within brackets. (I was actually asked to find out the smallest bracket expression, I have omitted the function to check for the smallest string and spaces within the expression).

s = "((a+b*d)*(x/2))*(1+(y+(x-2)/10))"
b = []
j = 0
b.append(j)
stack = []

for i in range(0, len(s)):
    if s[i] == '(':
        j = j + 1
        b.append(j)
        b[j] = ''
    elif s[i] == ')':
        b[j] = b[j] + ')'
        stack.append(b[j])
        j = j - 1

    # 0 is omitted to exclude characters outside of brackets
    for k in range(1, j + 1):
        b[k] = b[k] + s[i]

print(s)
print(stack)

Your code works, but is difficult to understand. Especially it is hard to understand what exactly is stored in b, this list contains both integers and strings. The name b also does not give a clue what it used for.

I would suggest a more straight forward method with more descriptive names for variables:

s = "((a+b*d)*(x/2))*(1+(y+(x-2)/10))"

opening_bracket_pos = []
stack = []

for i in range(0, len(s)):
    if s[i] == '(':
        opening_bracket_pos.append(i)
    elif s[i] == ')':
        start_pos = opening_bracket_pos.pop()
        stack.append(s[start_pos: i+1])

print(s)
print(stack)


Computing the number of permutations of n consecutive numbers I'm just starting to learn C++ and have been experimenting with various functions. Here is one that computes the number of permutations of n consecutive numbers as well as all possible permutations provided n. I'd love to receive any form of tips and criticisms on improving this function.

#include <string>
#include <iostream>
#include <vector>
#include <sstream>
#include <fstream>

using namespace std;

string rotateRt(string input);
vector<string> permute(int num);

int main(){
    cout << "n: ";
    int n;
    cin >> n;
    vector<string> permutations(permute(n));
    ofstream myfile;
    myfile.open("rosalind_perm.txt");
    myfile << permutations.size() << endl;
    for(int i = 0; i < permutations.size(); i++){
        myfile << permutations.at(i) << endl;
    }
    myfile.close();
}

/* Rotate input string right by 1 character */
string rotateRt(string input){
    int len = input.length();
    string a, b;
    if(len == 0){
        return "";
    }
    else if(len == 1){
        return input;
    }
    else{
        a = input.substr(0, len - 2);
        b = input.substr(len - 1) + " ";
        return b + a;
    }
}

/* To compute permutations we compute the permutations of num - 1,
append num to the front of each permutation, and find every cyclic
rotation. */
vector<string> permute(int num){
    vector<string> permutations;
    ostringstream convert;
    if(num == 1){
        permutations.push_back("1");
    }
    else{
        vector<string> prevPermutations(permute(num - 1));
        int size = prevPermutations.size();
        string prefix, suffix, word;
        convert << num;
        prefix = convert.str();
        convert.str("");
        for(int i = 0; i < size; i++){
            suffix = prevPermutations.at(i);
            word = prefix +  " " + suffix;
            int length = word.length();
            for(int j = 0; j < length / 2 + 1; j++){
                permutations.push_back(word);
                word = rotateRt(word);
            }
        }
    }
    return permutations;
}

In addition to Jamal's good points:

1: Consider sorting your #includes alphabetically. It's not a big issue, but I think it looks more tidy, and it's easier to see if something is already included or not.

2: Consider typedef vector<string> StringCollection (or some other meaningful name). This makes it easier to change the underlying data structure later, and makes the code look cleaner.

3: If you are using C++11, I'd write the for loop like this:

for (auto const& number : permutations) {
    myfile << number << endl;
}

Since you are learning, I see no reason not to get into C++11 right away.

4: Tutorials and the like often use mySomething as identifiers. my is a bad identifier prefix. Instead, write what role it has or what it represents. outputFile is much better than myFile.

5: Initialize variables right away. The std::ofstream constructor takes a filename, so there is no need to first initialize and then call open().

6: Instantiate variables as late as possible. In your rotateRt() (which should be called rotateRight(), by the way), you don't have to create your strings unless the last else triggers. Change it to this:

else {
    string a = input.substr(0, len - 2);
    string b = input.substr(len - 1) + " ";
    return b + a;
}

The same applies to for example ostringstream convert.

When you instantiate a variable as late as possible, you avoid constructing it unless you really have to. "As late as possible" implies as close as possible to the relevant context, which makes the code easier to read.

7: Consider rewriting permute() to reduce nesting.

8: Your code would normally be better suited as a class. You don't have to think too much about this yet, since you just started with C++. It's a good exercise to refactor this to use classes later.

9: In the case of int len = input.length();, you should use std::size_t instead of int.

10: Prefer C++-style comments: // (Effective C++ explains why, so I won't repeat it here.)

11: Use more whitespace. Especially more vertical whitespace. Insert an empty line in logical places to make your code much more readable.

12: You don't modify or copy the string you pass to rotateRt, so it should be reference-to-const instead: rotateRt(string const& input). (If you prefer, you can write const string&, it's the same thing.)

13: Consider rewriting permute() so that it's not recursive. Recursive functions that create a couple of objects can quickly chew up a lot of memory, and are often not very efficient. In short, your permute() function can be improved a great deal.

14: Consider refactoring the file name out into a constant:

const string filename = "rosalind_perm.txt";

This way it's easier to find if you have to change it, and by using the constant you are guaranteed you will only have to change it in one location.


Checking dates against a .properties file I am writing a script that will check the set values against a .properties file and I am just wondering if there is any nicer way to write what I have here.

#!/bin/bash
# @summary Check Values
# --------------------------------------------------------------------------------------------------
function _RunChecks
{
SuccessfulDiffRun="true"
timestamp() { date +"%F"; }
TodaysDate=$(timestamp)
#Read savedState.properties file
while read LINE; do eval $LINE; done < savedState.properties
#Compare the Values
echo "HotPatch : "
if [ "$TodaysDate" = "$WD_MANAGEGOLD_DATETIMESTAMP" ]; then
    echo Dates Are A Match : TodaysDate:$TodaysDate = HotpatchRunDate:$WD_MANAGEGOLD_DATETIMESTAMP
    if [ "$SuccessfulDiffRun" = "$WD_MANAGEGOLD_SUCCESS" ]; then
        echo SuccessfulDiffRun : $WD_MANAGEGOLD_SUCCESS
        echo Hotpatch Run Was Successful
    else
        echo SuccessfulDiffRun : $WD_MANAGEGOLD_SUCCESS
        echo Exiting
        exit 0
    fi
else
    echo Dates Not A Match: TodaysDate:$TodaysDate = HotpatchRunDate:$WD_MANAGEGOLD_DATETIMESTAMP
    exit 0
fi
echo ""
echo "RC Prod  : "
if [ "$TodaysDate" = "$WD_MANAGERC_DATETIMESTAMP" ]; then
    echo Dates Are A Match : TodaysDate:$TodaysDate = RCRunDate:$WD_MANAGERC_DATETIMESTAMP
    if [ "$SuccessfulDiffRun" = "$WD_MANAGERC_SUCCESS" ]; then
        echo SuccessfulDiffRun : $WD_MANAGERC_SUCCESS
        echo RC Run Was Successful
    else
        echo SuccessfulDiffRun : $WD_MANAGERC_SUCCESS
        echo Exiting
        exit 0
    fi
else
    echo Dates Not A Match: TodaysDate:$TodaysDate = HotpatchRunDate:$WD_MANAGEGOLD_DATETIMESTAMP
    exit 0
fi
}
# --------------------------------------------------------------------------------------------------
# @summary Start
_RunChecks

Security concerns

Judging by the eval, it seems the properties file contains assignment statements that bash understands. Or so you think. Using eval this way is most often a serious security risk. What if somebody fobs a way to tamper with the file and append a line like this:

bash /tmp/evilscript.sh

Not only the evil script will get executed by whoever is the effective user (hopefully not root), you will not even realize it happened.

Even if you're not particularly concerned by security issues (dangerous mindset though), it's just not worth the risk. One way to make this safer is to match the live against the expected format using the [[...]] built-in, or similar.

Performance

It seems there are just two lines to parse. At this scale, I doubt performance is a serious concern, or that alternative approaches would make a noticeable difference.

Simplify

I find the way you compare the previous and current values of the two variables overly tedious. I would take a much simpler, lazier approach. For example, keep a copy of the previous version of the file at a place not editable by its current users, and replace the tedious checks for each variable with a simple diff.


Hex-string representation of a byte array I have a legacy method toHex(byte[]) used in my application since the beginning of time to store a hexadecimal representation of a byte[] into the database. The byte[] buffer is a SHA256 checksum of a file, which is computed consistently.

Since this method is embodied as private method in a class, in order to compare the SHA256 of two files I could either copy the implementation to my other class or use a library-provided implementation. In fact I found org.apache.cxf.common.util.StringUtils.toHexString

This is CXF code:

/*     */   public static String toHexString(byte[] bytes) {
/* 215 */     StringBuilder hexString = new StringBuilder();
/* 216 */     for (int i = 0; i < bytes.length; ++i) {
/* 217 */       hexString.append(Integer.toHexString(0xFF & bytes[i]));
/*     */     }
/* 219 */     return hexString.toString();
/*     */   }

This is my code:

private static String toHex(byte[] checksumArray)
{
    StringBuilder result = new StringBuilder();
    for (byte checksumByte : checksumArray)
    {
        String hex = Integer.toString((checksumByte & 0xff) + 0x100, 16).substring(1);
        result.append(hex);
    }
    return result.toString();
}

I can see the two implementations are different, but I am not understanding in what they differ.

This is example data returned by the two implementations:

CXF : 99dd29352b873ccb7f17e43984c0c984288a397a308cd63b47537968eee991
Me  : 99dd29352b873ccb7f17e43984c00c9804288a397a308cd63b47537968eee991
Diff: 99dd29352b873ccb7f17e43984c0^c98^4288a397a308cd63b47537968eee991

As you can see "my" hex string is longer because of a couple of zeroes.
If I use the same implementation of toHexString to compare the SHA256 stored in database with the actual SHA256 of a file, I get consistent result. But if I try to use an authoritative implementation (assuming people at Apache Foundation write good code) I get a different result.

Is it a bug in my code? Unfortunately I don't know anything about that implementation, I just inherited the code.
Can someone explain me the algorithm behind my implementation compared with CXF's implementation?

In the meantime, can somebody recommend an "official" reference implementation of the conversion from byte[] to hex string?

Your code is good. The CXF code has a bug, and is missing leading-zero values when the byte value is less than 16.

Your code can be improved, though. It's doing a lot more work than it needs to do get the characters right.

Converting the value to a "longer" string and then taking the substring is a long-winded way of getting the padding right.

Consider a lookup-table for the task, and then use that as a mechanism for fast string building using a char array.

I have had a lot of success with this method:

private static final char[] hexchars = "0123456789abcdef".toCharArray();

private static String toHexRL(byte[] checksumArray) {
    int len = checksumArray.length * 2;
    char[] hchars = new char[len];
    int pos = 0;
    for (byte b : checksumArray) {
        hchars[pos++] = hexchars[(b>>4) & 0xf];
        hchars[pos++] = hexchars[b & 0xf];
    }
    return new String(hchars);
}

Note, I have put your method, my method, and the CXF method in to a test case here on Ideone: https://ideone.com/o66K1P

Note: To my knowledge, there's no authoritative (Java core library) for converting an array of byte to a hex string.


Removal of jQuery from script to auto-apply coupons from a link We are using a small extension that auto-applies coupons from a link.

The extension had pop-up JS based on jQuery:

It was our highest traffic page, in order to increase page speed we were removing unnecessary libraries and jQuery was one of them. So we needed to turn this code into pure JS.

The dev commented:

  As far as I can see, this script performs a very simple task in a very
  complicated way.

  First: there is no need to use fadeIn\fadeOut methods in this case.
  These methods freeze browser in loading moment. We need to use CSS
  transitions and just add opacity.

  Second: there is no need to change top and left params during resize.
  We can use CSS and make the browser do it for us.

He did small changes to CSS of the extension, and final pure JS equivalent is:

My response to the comments asking why both load and DOMContentLoaded are used:

  Yes, there is a reason to use DOMContentLoaded.

  Since until the DOM is built, we can not add events to the elements,
  they simply do not exist.

  For a sample, if I tried to get in global scope
  document.getElementsByClassName("modal-overlay__close"); this
  instruction would return []

  Also, we can not show the popup until all popup styles and popup
  picture are loaded.

  That's why, I use window load event.

As it stands, in most ways the code is now much better than the original. Your senior dev is absolutely correct in wanting to use CSS for the modal size. Despite this, there are a few suggestions I would make.

Instead of using window.onload use window.addEventListener("load", function(){});. If window.onload is used, then the function activating the modal could be overwritten by accident in the future.

There is no point in including Method in closeMethod. I'm sure that Method was added when the dev realized that just naming the function close hid the window.close function. closeModal would be a more descriptive name that also avoids this problem.

Is there any reason that both load and DOMContentLoaded events are listened for? These events should fire incredibly close to each other, and the code can likely be combined into a single initialization method to reduce complexity.

The ID of the popup-wrapper is overdefined. More than one method has it hardcoded in. If it were to be changed, it would be fairly easy to forget to change one or more occurrences. To fix this, it might be best to wrap everything in an immediately invoked function expression (IIFE) and pass the ID in. Using an IIFE also makes it possible to trivially avoid any troubles with global scope.

Instead of appending to className, use the classList API as this also makes removing the modal-overlay_visible class trivial. It also might make it possible to remove the modal-overlay_hidden class.

The DOMready function is exposed to the global scope. This isn't a huge issue on small pages, but should still be avoided to increase maintainability.

Lastly, consistency is a good thing. A style guide can make it easier to read code by ensuring everyone sticks to using double quotes (or single quotes) and uses the same CSS class patterns (I expected js-modal-close to be modal-overlay_close)

I could provide several different ways of writing this module. Which way is best heavily depends on your situation and is a judgement I can't make.


Comparing random number of bits I have a function that compares bits from a binary representation string and a chunk of memory and returns true if they are equal.

This is the function (which works as expected): I'll explain with comments what some external methods are supposed to do, accept that they work as expected for the sake of this question.

size_t bytes = (check_n_bits + 7) / 8;
size_t bytes_with_skip = (check_n_bits + skip_b_bits + 7) / 8;

//Check if `this` contains the number of bytes we need
if(this->canMoveForward(bytes_with_skip) == false) return false;

//Read the required number if bits. Ignore the skip_b_bits argument.
Bits *data = this->readBits(check_n_bits, skip_b_bits);

unsigned char *bin_string = Utils::removeSpaces(string);
size_t len = strlen((const char *) bin_string);
if(Utils::isValidBinString(bin_string) || len < check_n_bits) return false;

unsigned int last_bits = check_n_bits <= 8 ? check_n_bits : (bytes * 8) - check_n_bits;
last_bits = last_bits == 0 ? 8 : last_bits;

char tmp_bin_repr[9], tmp_bin_repr_2[9];

for(size_t i = 0; i < bytes ; i++) {
    int skip_bits_if_last = i + 1 == bytes ? (8 - last_bits) : 0;
    int bits_to_compare = i + 1 == bytes ? last_bits : 8;

    uint8_t c = data->read_uint8();
    sprintf((char *) &tmp_bin_repr, BYTETOBINARYPATTERN, BYTETOBINARY(c));
    for(int j = 0; j < bits_to_compare; j++) {
        tmp_bin_repr_2[j] = *(&(bin_string[i * 8]) + j);
    }

    if(memcmp((const char *) tmp_bin_repr + skip_bits_if_last, (const char *) tmp_bin_repr_2, bits_to_compare) != 0) {
        match = false;
        break;
    }
}

Helper macro:

#define BYTETOBINARYPATTERN "%d%d%d%d%d%d%d%d"
#define BYTETOBINARY(byte) \
    (byte & 0x80 ? 1 : 0), \
    (byte & 0x40 ? 1 : 0), \
    (byte & 0x20 ? 1 : 0), \
    (byte & 0x10 ? 1 : 0), \
    (byte & 0x08 ? 1 : 0), \
    (byte & 0x04 ? 1 : 0), \
    (byte & 0x02 ? 1 : 0), \
    (byte & 0x01 ? 1 : 0)

Let's call this function:

compareBinary(const char *string, size_t check_n_bits, size_t skip_b_bits)

How is this function used? The function is a method of an object that contains some data. When this method is called, it compares as many bits from a binary string representation of data as we pass it with the data contained in the object.

Example:

mObj->compareBinary("01010101 11110100 00", 18, 0);

This will compare 18 bits in total with the data that is hold in the object. First 8 chars of the passed string will be taken and the first byte of the data hold by the object will be converted to a string (using the helper macro). Then both will be compared. Second 8 chars, the same. Then we'd get the last 00 chars, take the third byte, convert it to a 8 chars binary representation, but only the first two chars will be compared.

This lets me compare a random number of bits with a simple binary representation of the data.

While the function works as expected and it's passing multiple unit tests, I believe it can be refactored and optimized. Can you give me any tips how to do it?

Bug

Your computation of last_bits is wrong:

unsigned int last_bits = check_n_bits <= 8 ? check_n_bits : (bytes * 8) - check_n_bits;
last_bits = last_bits == 0 ? 8 : last_bits;

Suppose check_n_bits were 18 as in your example.  This would make bytes 3, and so last_bits would be set to 24 - 18 = 6 instead of 2.  Later, you would check 6 bits in the 3rd byte instead of 2 bits.  This must mean that your unit tests aren't covering enough cases.

Awkward array usage

There are a couple of lines that are really awkward:

sprintf((char *) &tmp_bin_repr, BYTETOBINARYPATTERN, BYTETOBINARY(c));

Here you are casting the address of a character array into a character pointer.  Any time you use a cast, you should think about whether you really need that cast or if you are simply doing something wrong.  In this case, you are doing it wrong.  You can just pass the array like this:

sprintf(tmp_bin_repr, BYTETOBINARYPATTERN, BYTETOBINARY(c));

The second awkward line is this:

tmp_bin_repr_2[j] = *(&(bin_string[i * 8]) + j);

which can be simplified to this:

  tmp_bin_repr_2[j] = bin_string[i*8 + j];

No need for temp copy

Actually I don't think you really even need tmp_bin_repr_2 because all you are doing is making a copy of part of bin_string.  You could just compare with that part of bin_string directly if you modified your call to memcmp().  You can also remove the unnecessary casts as well:

if (memcmp(&tmp_bin_repr[skip_bits_if_last], &bin_string[i*8], bits_to_compare) != 0) {


Checking the neighbor of the current cell This is just checking the parameter you enter is a neighbour of the current Cell (which is obtained using getRow() and getColumn()).

Is there an easier way of doing this? How could I make this more compact and efficient?

public boolean isNeighbourOf(Cell c){

 boolean check = false;

 //check for not neighbours
 if(c.getRow() == getRow() && c.getColumn() == getColumn()){
     check = false;
 }

 else if(c.getRow() == getRow() - 1 && c.getColumn() == getColumn() - 1){
     check = false;
 }

 else if(c.getRow() == getRow() + 1 && c.getColumn() == getColumn() - 1){
     check = false;
 }

 else  if(c.getRow() == getRow() - 1 && c.getColumn() == getColumn() + 1){
     check = false;
 }

 else if(c.getRow() == getRow() + 1 && c.getColumn() == getColumn() + 1){
     check = false;
 }

 //check for neighbours
  else if(c.getRow() == getRow() - 1 && c.getColumn() == getColumn()){
     check = true;
 }
  else if(c.getRow() == getRow() + 1 && c.getColumn() == getColumn()){
         check = true;
  }
  else if(c.getRow() == getRow() && c.getColumn() == getColumn() + 1){
         check = true;
  }
  else if(c.getRow() == getRow() && c.getColumn() == getColumn() - 1){
         check = true;
  }
  return check;
}

return Math.abs(c.getRow   () - getRow   ()) +
       Math.abs(c.getColumn() - getColumn()) == 1;

You want to be in the same row and one column apart, or in the same column and one row apart. Computing the absolute value of the difference in row and column numbers tells you how far apart the cells are irrespective of orientation. For horizontal and vertical neighbors these must add up to 1; that is, one of them must be 1 and the other must be 0.


Generate the Norwegian equivalent to social security numbers - follow-up First of, here is my previous post.

That should explain a fair bit. The reason I'm posting one more time, is I have changed a lot. Using more classes, let them do all the work, and the overall structure.

#include <iostream>

const int DATE_LENGTH = 6;

class Date {
    private:
        char *ans;
    protected:
        int date[DATE_LENGTH];
        int year;
    public:
        Date() {
            askForDate();
        }

        ~Date() {
            if(ans) { delete ans; }
        }

        void askForDate() {
            using namespace std;
            ans = new char[DATE_LENGTH];
            cout << "Date of Birth (DDMMYY): ";         cin.getline(ans, DATE_LENGTH + 1);
            while (!validDate(ans)) {
                cin.getline(ans, DATE_LENGTH + 1);
            }

            for (int i = 0; i < DATE_LENGTH; i++) { date[i] = int(ans[i] - '0'); }
            year = 1900 + date[4] * 10 + date[5];
        }

        bool validDate(const char *d) {
            return strlen(d) == 6;
        }
};

class IndividSiffer : public Date {
    protected:
        int i1, i2, i3;
    public:
        IndividSiffer(int ii1, int ii2, int ii3) : i1(ii1), i2(ii2), i3(ii3) {}

        void individSiffer() {
            static int individ = -1;
            if(year >= 1900 && year <= 1999 ){
                individ++;
                i1 = i2 = i3 = 0;
            }
            i3 = individ % 10;
            if(individ < 100){
                i1 = 0;
                i2 = individ / 10 % 10;
            }else if(individ < 10){
                i1 = i2 = 0;
            }else{
                i1 = individ / 100 % 10;
                i2 = individ / 10 % 10;
            }
        }
};

class KontrollSiffer : public IndividSiffer {
    protected:
        int k1, k2;
    public:
        KontrollSiffer(int kk1 = 0, int kk2 = 0, int i1 = 0, int i2 = 0, int i3 = 0) : IndividSiffer(i1, i2, i3), k1(kk1), k2(kk2) {
            for (int index = 0; index < 500; index++) {
                individSiffer();
                generate();
            }
        }

        void generate() {
            using namespace std;
            k1 = 11 - ((3*date[0] + 7*date[1] + 6*date[2] + 1*date[3] + 8*date[4] + 9*date[5] + 4*i1 + 5*i2 + 2*i3) % 11);
            if(k1 == 11){ k1 = 0; }
            k2 = 11 - ((5*date[0] + 4*date[1] + 3*date[2] + 2*date[3] + 7*date[4] + 6*date[5] + 5*i1 + 4*i2 + 3*i3 + 2*k1) % 11);
            if(k2 == 11){ k2 = 0; }
            if((i3 != 0 && i3 != 2 && i3 !=4 && i3 !=6 && i3 !=8) && (k1 != 10 && k2 !=10)){
                for (int i = 0; i < DATE_LENGTH; i++) { cout << date[i]; }
                cout << i1 << i2 << i3 << k1 << k2 << endl;
            }
        }
};

int main(int argc, char *argv[]) {
    KontrollSiffer k;
    return 0;
}

I'd really like some input on how I structured it, or if things should be done differently.

Here are some suggestions which may help you improve your code.

Don't abuse using namespace std

Putting using namespace std within your program is generally a bad habit that you'd do well to avoid.

Use the appropriate #includes

The code uses the strlen() function a number of places but does not #include <cstring> which is where that library function is defined.

Eliminate unused variables

This code declares main with arguments argc and argv in the usual way, but neither argc nor argv are actually used.  This could be addressed by either declaring int main() or by modifying the code to actually use the variables.  There is a third way, which is to declare it as int main(int, char *[]) but this is not recommended because it's a bit confusing to read.

Separate I/O from object creation

The Date class should not do I/O directly to std::cout and std::cin.  Better design would be to create a custom inserter and custom extractor for the class to allow but not to force I/O operations for the creation of an object.  For example, your Date class might have the following two definitions:

std::ostream &operator<<(std::operator &out, const Date &d);
std::istream &operator>>(std::operator &in, Date &d);

The same is true of the KontrollSiffer class.

Perform better error checking for user input

As it is currently written, one can type in "Norway" as the Date and the code proceeds to proces that input as though it were a valid date, producing invalid results.  Better would be to include more checking beyond just whether the string is 6 characters long.

Prefer std::string to raw char *

The Date class uses a raw char * but would probably be better served by using std::string in this context.

Use aggregation rather than inheritance for composition

The inheritance mechanism is generally suited for "is-a" relationships.  For example, if one had a Birthday class, it might reasonably be derived from a Date base class, since a Birthday is a Date.  However, an IndividSiffer is not really a Date, but is rather constructed from one, so a better design might either have a Date as a private instance variable with the IndividSiffer class or to simply accept a const Date & variable as an argument to an IndividSiffer constructor.

Don't keep variables you don't need

The ans variable is only actually used within the askForDate() routine, so there's no reason to keep it as part of the Date class.  Better would be to declare it as a local variable within the askForDate() function.

Prefer to avoid using new and delete directly

The ans variable within the Date::askForDate() function is small enough to easily fit on the stack, so rather than using new to allocate it, better would be to declare it locally within the askForDate function as:

char ans[DATE_LENGTH];

That way, it is automatically created with the correct length and then discarded when the function is complete.

Think carefully about static variables

The code for IndividSiffer::individSiffer() uses a static variable individ which is a problem.  When you create two IndividSiffer instances in the same program, only the first one will have the variable initialized.  That is because a static variable only gets initialized once before the program is run.  This very greatly limits the use of these classes.

Consider using a standard container

The effect of the KontrollSiffer class is to create and print 500 individual numbers.  Rather than simply print and discard them, it might be more sensible for general use to create a collection of them, perhaps in a std::vector<IndividSiffer> for example.

Omit return 0

If your program completes successfully, the return 0 at the end of main() will be generated automatically, so it's not needed in C++ programs.


Parsing a string to an object I have this string containing this block of code:

[General]
StartWithLastProfile=0
Profilist.notifications=true
Profilist.dev=true
Profilist.dev-builds=80k
Profilist.launch_on_create=true

[Profile0]
Name=Dev Profilist
IsRelative=1
Path=Profiles/j0a1zjle.Unnamed Profile 1
Profilist.tie=0

[Profile55]
Name=Main
IsRelative=1
Path=Profiles/qekfxcdm.Unnamed Profile 1

Currently I use regex to read this into an object like this:

var str = '[General]\nStartWithLastProfile=0\nProfilist.notifications=true\nProfilist.dev=true\nProfilist.dev-builds=80k\nProfilist.launch_on_create=true\n\n[Profile0]\nName=Dev Profilist\nIsRelative=1\nPath=Profiles/j0a1zjle.Unnamed Profile 1\nProfilist.tie=0\n\n[Profile55]\nName=Main\nIsRelative=1\nPath=Profiles/qekfxcdm.Unnamed Profile 1';
console.time('parse');
var linePatt = /^(?:\[(.*)\]|(.*?)=(.*))$/gm;
var match;
var objParsed = {};
var lastKey;
while(match = linePatt.exec(str)) {
  console.info(match);
  // Array [ "[General]", "General", undefined, undefined ]
  // Array [ "StartWithLastProfile=0", undefined, "StartWithLastProfile", "0" ]
  if (match[2] == undefined) {
    lastKey = match[1];
    objParsed[lastKey] = {};
  } else {
    objParsed[lastKey][match[2]] = match[3];
  }
}
console.timeEnd('parse');
console.log('objParsed:', JSON.stringify(objParsed));

This gives me an object like this:

{
    "General": {
        "StartWithLastProfile": "0",
        "Profilist.notifications": "true",
        "Profilist.dev": "true",
        "Profilist.dev-builds": "80k",
        "Profilist.launch_on_create": "true"
    },
    "Profile0": {
        "Name": "Dev Profilist",
        "IsRelative": "1",
        "Path": "Profiles/j0a1zjle.Unnamed Profile 1",
        "Profilist.tie": "0"
    },
    "Profile55": {
        "Name": "Main",
        "IsRelative": "1",
        "Path": "Profiles/qekfxcdm.Unnamed Profile 1"
    }
}

The console.time and console.timeEnd tell me this takes on average from 4-7ms. My application is for a much bigger file, though, and goes from Profile0 to ProfileN the blocks repeat. Is there any faster way you would do this?

I believe a simple split on newline and a hard-coded parse would be better than the regex solution.... especially if you can trust your input data will be valid. In essence, the grouping and iterative looping in the regex is probably the slow part.

Of course, the console.info for each loop is also very slow.... and probably is something you added afterwards? If it is in your actual code, that's almost certainly the problem.

Let me explain the hard-parse with a code example:

var objParsed = {};
var lastKey;
var lines = str.split('\n');
for(var i = 0; i < lines.length; i++) {
    var clause = lines[i].trim();
    if (clause.length == 0) {
        continue;
    }
    if (clause[0] == '[') {
        lastKey = {};
        objParsed[clause.substring(1, clause.length - 1)] = lastKey;
    } else {
        var pos = clause.indexOf("=");
        lastKey[clause.substring(0, pos)] = clause.substring(pos + 1);
    }
}

See the comparative results in this jsfiddle


Calling a method in parallel which returns the future I have an async method which calls my Task class while my Task class does all the work:

    private ExecutorService executor = Executors.newFixedThreadPool(10);

    @Override
    public Future<DataResponse> executeAsync(DataKey key) {
        Future<DataResponse> future = null;

        try {
            Task task = new Task(key, restTemplate);
            future = executor.submit(task);
        } catch (Exception ex) {
            // logging exception here
        }

        return future;
    }

Here is my Task class which does all the work:

public class Task implements Callable<DataResponse> {

    private DataKey key;
    private RestTemplate restTemplate;

    public Task(DataKey key, RestTemplate restTemplate) {
        this.key = key;
        this.restTemplate = restTemplate;
    }

    @Override
    public DataResponse call() throws Exception {
        // some code here
    }
}

Now I need to call executeAsync method in parallel and then make a List<DataResponse> object and return it:

@Override
public List<DataResponse> executeSync(DataKey key) {
    List<DataResponse> responseList = new ArrayList<DataResponse>();

    // make a List of DataKey using single key passed to this method.
    List<DataKey> keys = new ArrayList<DataKey>();

    // here keys max size will be three
    List<Future> futures = new ArrayList<>(keys.size());
    for(DataKey key : keys) {
        Future<DataResponse> future = executeAsync(key);
        futures.add(future);
    }

    for (Future<DataResponse> future : futures) {
        try {
            responseList.add(future.get(300, TimeUnit.MILLISECONDS));
        } catch (Exception e) {
            // do something
        }
    }
    return responseList;
}

My intention is to call executeAsync method in parallel and return back responseList object. I mean I want to execute call method of my Task class in parallel for each of the key object so if I have three key object in keys list, then I want to execute call method for each key in parallel and then make a List<DataResponse> object and return it.

Since I don't want to wait 300 Milliseconds for each key future and then call next key future and then next call third key future. In this way it will wait 300 + 300 + 300 for each of the key. I want to execute call method in parallel in same 300 milliseconds for all three keys.

Does my above code looks right, and are there any better ways of doing this problem? I am still on Java 7.

An executor service will be able to do a invokeAll with a timeout and return a set of futures holding all the values or return when the timeout expires.

This means that your executeAsync should take a set of keys and build all the tasks from those at once and then submit the tasks as a batch:

@Override
public List<Future<DataResponse>> executeAsync(Collection<DataKey> keys,  long timeout, TimeUnit unit) {
    List<Task> tasks = new ArrayList<>(keys.size());
    for(DataKey key : keys){
        tasks.add(new Task(key, restTemplate));
    }

    return executor.invokeAll(tasks, timeout, unit);
}

@Override
public List<DataResponse> executeSync(DataKey key) {
    List<DataResponse> responseList = new ArrayList<DataResponse>();

    // make a List of DataKey using single key passed to this method.
    List<DataKey> keys = new ArrayList<DataKey>();

    List<Future> futures = executeAsync(keys, 300, TimeUnit.MILLISECONDS);

    for (Future<DataResponse> future : futures) {
        try {
            if(future.isDone())
                responseList.add(future.get());
            //otherwise future didn't complete in the 300 milliseconds
        } catch (Exception e) {
            // do something
        }
    }
    return responseList;
}


Prime factorization in Haskell I am a Haskell beginner. Here is my function to find prime factors of a number

primes = 2:takePrimes [3, 5 ..]
    where takePrimes (x:xs) = let smallPrimes = untilRoot x primes
                              in  if 0 `notElem` (map (mod x) smallPrimes)
                                  then x:takePrimes xs
                                  else takePrimes xs

untilRoot n = takeWhile (\x -> x*x < n)

firstPrimeDivisor :: Integer -> Integer
firstPrimeDivisor x = head [p | p <- primes, x `mod` p == 0]

primeFactor 1 = []
primeFactor x = let p = firstPrimeDivisor x
                in p:primeFactor (x `div` p)

How does it look?

It looks fine, but there are a few issues with it:

When we check if a number is a prime, we need to test all possible divisors up its root inclusively. If you print the primes list, you'll see that 9 is there, which is obviously a bug. untilRoot n = takeWhile (\x -> x*x < n) should be untilRoot n = takeWhile (\x -> x*x <= n). However, this bug does not affect the correctness of your factorization algorithm.
It does not scale well. In the firstPrimeDivisor function, all primess up to x are checked in the worst case. That's why it works slowly even for moderately large prime numbers(for instance, 10^9 + 7).

Here a more efficient solution which checks only O(sqrt(n)) divisors in the worst case:

factorize :: Integer -> Integer -> [Integer]
factorize _ 1 = []
factorize d n
    | d * d > n = [n]
    | n `mod` d == 0 = d : factorize d (n `div` d)
    | otherwise = factorize (d + 1) n

primeFactors :: Integer -> [Integer]
primeFactors = factorize 2

It is also much more concise. The idea behind it is to prove two statements first and then write a very simple code based on them:

The smallest divisor of any natural number greater than two is a prime. Let's assume that it is not the case and d = p * q (p, q > 1), where d is the smallest divisor of n. But then p < d and p is a divisor of n. Thus, d is not the smallest one. This statement also shows why your original solution is correct, even though the primes list is contains some composite numbers.
Any composite number has a divisor that does not exceed its square root. The proof by contradiction is very straightforward so I will omit it.


Is there anything to improve in this MergeSort code in Java? I have written an implementation of the merge sort algorithm in Java.

Here is the MergeSort class that I created.

public class MergeSort{

    public static void sort(Comparable[] a){
        Comparable[] b = new Comparable[a.length];
        sort(a, b, 0, a.length-1);
    }

    private static void sort(Comparable[] a, Comparable[] b, int lo, int hi){
        if(lo>=hi) return;
        int mid = lo+(hi-lo)/2;
        sort(a, b, lo, mid);
        sort(a, b, mid+1, hi);
        merge(a, b, lo, hi, mid);
    }

    private static void merge(Comparable[] a, Comparable[] b, int lo, int hi, int mid){
        int i=lo;
        int j=mid+1;

        for(int k=lo;k<=hi;k++){
            if(i<=mid && j<=hi){
                if(a[i].compareTo(a[j])>0) {b[k]=a[j++];continue;}
                else if(a[i].compareTo(a[j])<0) {b[k]=a[i++]; continue;}
                else {
                    b[k]=a[i++];
                    j++;
                    continue;
                }
            }
            if(j>hi && i<=mid) {b[k]=a[i++];continue;}
            if(i>mid && j<=hi) {b[k]=a[j++];continue;}
        }

        for(int n=lo;n<=hi;n++){
            a[n]=b[n];
        }
    }
}

The code passed some simple tests. I wonder is there anything wrong or missing out? Anywhere to improve?

Generics

In Java, it is not safe to create an array of a generic type. This is what you are doing...

Comparable is really Comparable<T>, and you are creating an array of Comparable[].

Because the generic types get removed when you compile the code (see Type Erasure), there is no safe way for Java to ensure that you have the same types of data in your array.

For example, you could have specified your input as:

Integer[] data = new Integer[10];

Then, even though it is an array of Integer, the java compiler only knows it is an array of Comparable, so it can allow you to say data[0] = "broken";, but that will fail, at runtime, because you cannot put a String in to an array of Integer. because Java can only tell at runtime whether the data is valid input to a Generically typed array, it warns you whenever you use one.

This leads to ugly warnings. The right solution is to use an appropriately typed collection List<Integer> data will be type-checkable at compile time, and java can 'do things right'.

An alternate solution is to specify the full type of the Comparable as part of the method definition (a Generic Method... ):

public static <T extends Comparable<?>> void sort(T[] a){
    ....
}

... and then follow that same generic method in to your inner methods

It is complicated. There is some reference material here:

Cannot create arrays of Parameterized Types
Generic Methods

Style

You are not using white-space to help make things readable. Consider this line of code:

    int mid = lo+(hi-lo)/2;

This should be:

    int mid = lo + (hi - lo) / 2;

See the java Code-Style guidelines for this:

  All binary operators except . should be separated from their operands by spaces. Blank spaces should never separate unary operators such as unary minus, increment ("++"), and decrement ("--") from their operands

Additionally, you have a number of complex 1-liners that should be split on to multiple lines:

       if(i>mid && j<=hi) {b[k]=a[j++];continue;}

That should be:

        if(i > mid && j <= hi) {
            b[k]=a[j++];
            continue;
        }

Algorithm

MidPoint

You have also copied your code from some reference texts, or otherwise done some mid-point research:

int mid = lo+(hi-lo)/2

This mechanism for getting the mid point of the array is the right pattern to use. Anyone who does it this way is either copying the code, has run in to the bug, or has been well-educated. This is a good thing, but, in case you did not know why you were doing it this way, now you know.

Bug

I believe you have a bug in the code when you have two values the same, one in each part of the merge....

... part of the reason this is hard to see is because your code is often doing more than one thing on each line:

            if(a[i].compareTo(a[j])>0) {b[k]=a[j++];continue;}
            else if(a[i].compareTo(a[j])<0) {b[k]=a[i++]; continue;}
            else {
                b[k]=a[i++];
                j++;
                continue;
            }

If I space out that code block, there are a number of things in there that are apparent:

            if(a[i].compareTo(a[j]) > 0) {
                b[k]=a[j++];
                continue;
            } else if(a[i].compareTo(a[j]) < 0) {
                b[k]=a[i++];
                continue;
            } else {
                b[k]=a[i++];
                j++;
                continue;
            }

First up, for some Comparable classes the compareTo method can be expensive. and, you will be doing it twice for many checks. You should do it once, and save away the result:

int comparison = a[i].compareTo(a[j])
if (comparison > 0) {
    ...
} else if (comparison < 0) {
    ...
} else {
    ...
}

OK, back to the bug.... in the final else block, you have:

            } else {
                b[k]=a[i++];
                j++;
                continue;
            }

This block increments both the i and the j pointers, but it only adds the i value to the merged set, which means the equal value at j is 'lost'.

There are two ways to solve this, but I prefer treating the equals values as if the one is larger than the other.... if you convert the one comparison check to be >= 0 instead of just > 0, you will successfully merge results.... then you can get rid of the whole last block entirely....

it also means you only need one compareTo check:

            if(a[i].compareTo(a[j]) >= 0) {
                b[k]=a[j++];
                continue;
            } else {
                b[k]=a[i++];
                continue;
            }

ReWrite

I am not one of those people who thinks 'continue' is a bad thing. I like it for the job it can do, but your use of the continue is excessive.... Every path through the loop is a 'continue... this is something easily solved with a little planning. Consider this rewrite:

private static void merge(Comparable[] a, Comparable[] b, int lo, int hi,
        int mid) {
    int i = lo;
    int j = mid + 1;

    for (int k = lo; k <= hi; k++) {
        if (i <= mid && j <= hi) {
            if (a[i].compareTo(a[j]) >= 0) {
                b[k] = a[j++];
            } else {
                b[k] = a[i++];
            }
        } else if (j > hi && i <= mid) {
            b[k] = a[i++];
        } else if (i > mid && j <= hi) {
            b[k] = a[j++];
        }
    }

    for (int n = lo; n <= hi; n++) {
        a[n] = b[n];
    }
}


Generating scatter plot from a CSV file Assume you have the following data in the form of a csv-file. The content looks something like this:

,Action,Comedy,Horror
1,650,819,
,76,63,
2,,462,19
,,18,96
3,652,457,18
,75,36,89

which can be interpreted as a table of the form:

           Action       Comedy       Horror
1           650          819
             76           63
2                        462           19
                          18           96
3           652          457           18
             75           36           89

The goal was to write a function that takes a lst with genre names as elements in form of a str and returns a scatter plot of the data, where the data that should appear on the scatter plot is in the second row of every index (76, 63 , and , 18, 96 and 75, 36, 89). The function should be able to distinguish between two-dimensional and three-dimensional scatter plots depending on the input.

from pandas import DataFrame
from csv import reader
import matplotlib.pyplot as plt
from mpl_toolkits.mplot3d import Axes3D

def genre_scatter(lst):
    """
    Creates an scatter plot using the data from genre_scores.csv.
    :param lst: a list with names of the genres considered
    :return: saves a pdf-file to the folder Fig with the name gen_1_ge_2.pdf
    """
    # First we need to determine the right columns of genre_scores.
    first_row = [row for row in reader(open('genre_scores.csv', 'r'))][0]
    index = [first_row.index(x) for x in lst]

    # Get the relevant data in the form of a DataFrame.
    # Please note that the first row of data for every index is not necessary for this task.
    data = DataFrame.from_csv('genre_scores.csv')
    gen_scores = [data.dropna().iloc[1::2, ind - 1].transpose() for ind in index]

    # rewrite the values in an flattened array for plotting
    coordinates = [gen.as_matrix().flatten() for gen in gen_scores]

    # Plot the results
    fig = plt.figure()
    if len(coordinates) == 2:
        plt.scatter(*coordinates)
        plt.text(70, 110, "pearson={}".format(round(pearson_coeff(coordinates[0], coordinates[1]), 3)))
        plt.xlabel(lst[0])
        plt.ylabel(lst[1])
        plt.savefig("Fig/{}_{}.pdf".format(*lst))
    else:
        ax = fig.add_subplot(111, projection='3d')
        ax.scatter(*coordinates)
        ax.update({'xlabel': lst[0], 'ylabel': lst[1], 'zlabel': lst[2]})
        plt.savefig("Fig/{}_{}_{}.pdf".format(*lst))
    plt.show()
    plt.close("all")

if __name__ == "__main__":
    genre_scatter(['Action', 'Horror', 'Comedy'])

The code works and I'm happy with the output but there are a few things that bug me and I'm not sure if I used them right.

I'm not incredibly familiar with list comprehension (I think that is what you call expressions of the form [x for x in list], please correct me if I'm wrong) and haven't used them very often, so I'm not quite sure if this here was the right approach for the problem. My biggest concern is the first use of this kind of expression, where I basically need the first row of the CSV file but create a list with all the rows only to use the first. Is there a smarter way to do this?
Is there a better way to label the axes? Ideally some function where I just could pass the *lst argument?

Please forget the pearson_coeff() part in the code, it's not really relevant for this.

This really isn't bad, in terms of base Python. The only thing that stands out to me is this:

first_row = [row for row in reader(open('genre_scores.csv', 'r'))][0]

Firstly, you aren't closing the file. Always close the file after you're done.

'r' is implicit, so you don't need to write it in the arguments to open.

Also, you're building up an entire list in memory from the CSV file, and then throwing it all away only to use the first row. Instead, you should use something like:

with open('genre_scores.csv') as f:
    csv_reader = reader(f)
    first_row = next(csv_reader)

You also ask:

  I'd like to implement something that makes sure that lst isn't longer than three elements (since four dimensional plots aren't really a thing). The only way I know to do this is assert len(lst) <=3, which gets the job done but it would be nice if it also could raise a useful error message.

Fairly straightforward, and I'll also assume that the minimum is 2:

if not (2 <= len(lst) <= 3):
    raise ValueError(f'Invalid lst length of {len(lst)}')


Python High Score Table I need to be able to store results of a game in a file with names and high scores where I will eventually want to review the file and sort it alphabetically, etc.  I'm unsure if I have the correct way of storing the results to be able to do this.

class_number = input('Please enter your class number: ')

from datetime import datetime
now = datetime.now()
time = ('%s/%s/%s %s:%s' % (now.day, now.month, now.year, now.hour, now.minute))

userscore = (user_name, ' ', str(total_score),' ', time, '  ')

if(int(class_number) == 1):
    with open('class1.txt', 'a') as myFile:
        myFile.writelines(userscore)
        myFile.close()

elif(int(class_number) == 2):
    with open('class2.txt', 'a') as myFile:
        myFile.writelines(userscore)
        myFile.close()

elif(int(class_number) == 3):
    with open('class3.txt', 'a') as myFile:
        myFile.writelines(userscore)
        myFile.close()

Your date format is unconventional, because the minutes are not zero-padded.  You could use %02d instead of %s, but an even better solution would be to use now.strftime('%d/%m/%Y %H:%M') or '{:%d/%m/%Y %H:%M}'.format(now).

PEP 8 says that you should put import statements at the top, for maintainability.

Calling open() in the context of a with block, as you have done, is good practice, as it means it will be automatically closed when exiting the block.  Therefore, you should eliminate the myFile.close() calls.

The three output blocks should be combined into one.

from datetime import datetime

now = datetime.now().strftime('%d/%m/%Y %H:%M')
class_number = input('Please enter your class number: ')

if class_number in ['1', '2', '3']:
    with open('class{}.txt'.format(class_number), 'a') as f:
        f.writelines((user_name, ' ', str(total_score), ' ', now, '  '))

Contrary to its name, writelines() does not actually add line termination characters to the output.  If you intend to append multiple records to a file, it would be better to put each record on a separate line.  In that case, consider using a csv.writer with delimiter=' ' to output tabular data.

import csv

…

if class_number in ['1', '2', '3']:
    with open('class{}.txt'.format(class_number), 'a') as f:
        writer = csv.writer(f, delimiter=' ')
        csv.writerow((user_name, total_score, now))


Fictional Hotel Website I've made a fictional hotel website that shows you hotel details when you click on a button.

jsFiddle

var hotel = {Name: "Park Hotel", Price: "£120.00", Rooms: 50}
var hotel2 = {name: "West End", price: "£240.00", rooms: 150}

el_p = document.getElementById("para1");
el_picture = document.getElementById("picture");
el_picture2 = document.getElementById("picture2");

function showPrice() {
    el_picture2.style.display = "none";
    el_picture.style.display = "block";
    el_p.innerHTML = " ";
    for(details in hotel) {
        el_p.innerHTML += (details += ": " + hotel[details] + "
");
    }
}

function showPrice2() {
    el_picture.style.display = "none";
    el_picture2.style.display = "block";
    el_p.innerHTML = " ";
    for(details in hotel2) {
        el_p.innerHTML += ( details += ": " + hotel2[details] + "
");
    }
}

Is there an easier way to code this? I'm just learning so I just did what I could.

What if you have 167 hotels? Would you copy/paste the code for all those hotels? I don't think you would.
Start with making the script more general, so you have only to feed the data into the function.

var hotel=new Array;
hotel[1] = {Name: "Park Hotel", Price: "£120.00", Rooms: 50, pict: "pict1"}
hotel[2] = {Name: "West End", Price: "£240.00", Rooms: 150, pict: "pict2}

(make sure you use the same keys all the time")

Now you can feed that into a function:

function showHotel(hotelnumber){
   var hoteldata=hotel[hotelnumber];
   document.getElementById['name'].innerHTML=hoteldata['Name'];
   document.getElementById['price'].innerHTML=hoteldata['Price'];
   // etc etc etc
   }

Inside the function you have all the elements you need to display the hotel information. Replace the content of all the elements (give your divs id's!) like name, price, rooms and the picture.

If you want to display a hotel, call the function with

showHotel(11);


Dynamic Programming: Fibonacci-like recurrence relation This is a problem from my Introduction to Algorithms textbook.

  Consider the inﬁnite sequence of numbers An|n>0 = (1,1,3,4,8,11,...) deﬁned recursively as follows.

$$
A_n = \left\{\begin{aligned}
&1 && n = 1,2\\
&B_{n-1}+A_{n-2} && n > 2
\end{aligned}
\right. \\
B_n = \left\{\begin{aligned}
&2 && n = 1,2\\
&A_{n-1}+B_{n-2} && n > 2
\end{aligned}
\right.
$$

  Write a recursive method to compute the nth value of A, for any given integer n.
  Write a “bottom-up” method to compute the nth value of A, for any given integer n.

I wrote a solution for each but found myself using auxiliary methods in both cases. A recent criticism comes to mind and I wonder if this is unnecessarily complex. Could it be simpler?

Of course any feedback on the efficacy of my approach is welcomed.

My recursive algorithm, stratified into two methods:

private static int computeAuxRecursive(int n) {
  if (n <= 0) {
    throw new IllegalArgumentException("n mustbe an integer larger than 0");
  }
  return n <= 2 ? 2 : computeNthRecursive(n - 1) + computeAuxRecursive(n - 2);
}

private static int computeNthRecursive(int n) {
 if (n <= 0) {
   throw new IllegalArgumentException("n must be an integer larger than 0");
 }
  return n <= 2 ? 1 : computeAuxRecursive(n - 1) + computeNthRecursive(n - 2);
}

Convenient testing method:

private static String retrieveSequenceRecursive(int length) {
    StringBuilder result = new StringBuilder();
    for (int i = 1; i <= length; i++) {
        result.append(", ").append(computeNthRecursive(i));
    }
    return result.substring(2);
}

For my bottom-up algorithm I elected to go with a map:

private static Map<Integer, int[]> memoizedMap = new HashMap<Integer, int[]>();

Pre-filled, according to specification, like so:

memoizedMap.put(1, new int[]{1, 2});
memoizedMap.put(2, new int[]{1, 2});

The idea is to use the Nth position requested as a key, and the array of length 2 to hold both the A and B value, using a flag to determine which is being requested.

Actual two methods used for Bottom up algorithm:

private static int computeNthBottomUp(int n) {
    if (n <= 0) {
        throw new IllegalArgumentException("n must be an integer larger than 0");
    }
    if (memoizedMap.containsKey(n)) {
        return memoizedMap.get(n)[0];
    }

    memoizedMap.put(n, new int[]{ bottomUpHelper(n, false), bottomUpHelper(n, true) });
    return memoizedMap.get(n)[0];
}
// flag boolean to check if we want the B value or not
private static int bottomUpHelper(int n, boolean bFlag) {
    if (memoizedMap.containsKey(n)) {
        return bFlag ? memoizedMap.get(n)[1] : memoizedMap.get(n)[0];
    }

    memoizedMap.put(n,
        new int[]{
            bottomUpHelper(n - 2, false) + bottomUpHelper(n - 1, true),
            bottomUpHelper(n - 2, true) + bottomUpHelper(n - 1, false)
        }
    );
    return bFlag ? memoizedMap.get(n)[1] : memoizedMap.get(n)[0];
}

Another convenience method:

private static String retrieveSequenceBottomUp(int begin, int end) {
    StringBuilder result = new StringBuilder();
    for (int i = begin; i <= end; i++) {
        result.append(", ").append(computeNthBottomUp(i));
    }
    return result.substring(2);
}

Sample tests:

@Test
public void testTenthInSequence() {
    assertEquals(76, computeNthRecursive(10));
    assertEquals(76, computeNthBottomUp(10));
}

@Test
public void testfirstTenRecursive() {
    assertEquals("1, 1, 3, 4, 8, 11, 21, 29, 55, 76", retrieveSequenceRecursive(10));
    assertEquals("1, 1, 3, 4, 8, 11, 21, 29, 55, 76", retrieveSequenceBottomUp(1, 10));
}

Recursive Solution

Code-wise, I see no issues. The one thing I'd point out is that it might be clearer to name your functions A and B instead of Nth and Aux, since the problem has to do with finding A and it isn't readily apparent what Aux means. It's a little unclear, but not a dealbreaker.

Bottom-up method

I don't really understand why your memoized map contains an array. If it's really a "pair" type such that memoizedMap[n] is \$(A_n, B_n)\$, then I'd introduce a new type for that. Arrays can mean anything, so once you make the decision to go with an array, the code is very difficult to follow. You're introducing some boolean flags, which are probably unnecessary too.

That said, I'm not sure that simply memoizing the recursive solution is what the problem intended. The intent seems to instead suggest that you compute the arrays iteratively. That is just the same way you could compute the Fibonacci sequence iteratively (in fact, the two sequences are very closely related, as \$A_1, B_2, A_3, B_4, ...\$ is the Fibonacci sequence): just keep the last few elements and update them as you go:

public static int getAn(int n) {
    if (n <= 2) return 1;

    int a0 = 1;
    int a1 = 1;
    int b0 = 2;
    int b1 = 2;

    for (int i = 0; i < n-2; ++i) {
        int nexta = b1 + a0;
        int nextb = a1 + b0;

        // updates
        a0 = a1;
        a1 = nexta;
        b0 = b1;
        b1 = nextb;
    }

    return a1;
}


How to make string.Format template more readable? Have a look to the following example. Clearly it's very hard to understand the meaning of each parameter passed to the string.Format to be replaced to the numeric sequence of {0}, {1}, ...

I want to improve the software quality: Maintainability\Code readability
http://en.wikipedia.org/wiki/Software_quality

The problem is also bound to the difficulty of associating numbers to the corrisponding values in the template. i.e. at a first glance what is the meaning of {8}?

               @"<?xml version='1.0'?>
                <RemoteXMLData>
                    <Username agent_diff_type=""{10}"" usertype=""{5}"" ticket_language=""{6}"" min_slip_stake=""{7}"" max_stake=""{8}"" location=""{9}"">{0}</Username>
                    <Password>{1}</Password>
                    <Reseller>{2}</Reseller>
                    <Function>AutoLogin</Function>
                    <AppKey>{3}</AppKey>
                    <Sub>{4}</Sub>
                </RemoteXMLData>";

What should I change?

string autoLoginRequest = String.Format(
AutoLoginRequestTemplate,
username,
password,
Config.Val1,
Config.Val2,
requestSub,
Language,
minValue,
maxValue,
agentDiffType);

Like @jesse said you can have a custom method, maybe pass a user object to that method.
Something like this:

class User
{
    public int AutoLoginRequestTemplate { get; set; }
    public string username { get; set; }
    public string password { get; set; }
    public string Val1 { get; set; }
    public string Val2 { get; set; }
    public string requestSub { get; set; }
    public string Language { get; set; }
    public int minValue { get; set; }
    public int maxValue { get; set; }
    public int agentDiffType { get; set; }

    /// <summary>
    /// Formats the user info.
    /// </summary>
    /// <param name="user">The user object.</param>
    /// <returns>A formated string</returns>
    public string FormatUserInfo(User user)
    {

        return String.Format("{0}{1}{2}{3}{4}{5}{6}{7}{8}{9}",
                                user.AutoLoginRequestTemplate,
                                user.username,
                                user.password,
                                user.Val1,
                                user.Val2,
                                user.requestSub,
                                user.Language,
                                user.minValue,
                                user.maxValue,
                                user.agentDiffType);
    }
}

Your question changed, now I see that you are trying to create an xml string.

For the sake of readabilty, I wouldnt use string.Format(), instead I would just do simple concatenation:

"<Password>"+passowrd+"</Password>"

Now I am not sure about performance, but here we are talking about readability (you probably can spend your time being productive at something else rather than trying to decide which type of concatenation is more efficient/readable)

Now in terms of best practice, I wouldn't create xml manually (just like I wouldnt try to create a JSON or CSV parser) instead I would use .NET framework to do this, which would certainly make it more readble, something like this:

        XElement xml = new XElement("RemoteXMLData",
            new XElement("Username",
                new XAttribute("agent_diff_type", agentDiffType),
                new XAttribute("agentDiffType", agentDiffType),
                new XAttribute("usertype", userType),
                new XAttribute("ticket_language", Language),
                new XAttribute("min_slip_stake", minValue),
                new XAttribute("max_stake", maxValue),
                new XAttribute("location", location),
                username),
            new XElement("Password",
                password),
            new XElement("Reseller",
                reseller),
            new XElement("Function",
                function),
            new XElement("AppKey",
                appKey),
            new XElement("Sub",
                sub)
        );


Check if a string has all unique characters This is a question from the book "Cracking the Coding Interview".

  Implement an algorithm to determine if a string has all unique
  characters What if you can not use additional data structures?

I can use std::map but in interview they check logic. Suggest me some improvement for optimization.

#include <iostream>
#include <string>

class Strings
{
   static const int SIZE = 26;
   char alphabetL[SIZE] = { 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J',
                            'K', 'L', 'M', 'N', 'O', 'P', 'Q', 'R', 'S', 'T',
                            'U', 'V', 'W', 'X', 'Y', 'Z'};

   char alphabetS[SIZE] = { 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j',
                            'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r', 's', 't',
                            'u', 'v', 'w', 'x', 'y', 'z'};

  public:
    Strings()  = default;
    ~Strings() = default;

    bool isAllUnique(std::string&);

  private:
    std::string toLower(std::string&);
};

std::string Strings::toLower(std::string& str)
{
    for (int i = 0; i < str.length(); i++)
    {
        for (int j = 0; j < SIZE; j++)
        {
            if (str[i] == alphabetL[j])
            {
                str[i] = alphabetS[j];
            }
        }
    }
    return str;
}

bool Strings::isAllUnique(std::string& str)
{
    int count[SIZE];
    for (int k = 0; k < SIZE; k++)
    {
        count[k] = 0;
    }
    str = toLower(str);
    for (int i = 0; i < str.length(); i++)
    {
        for (int j = 0; j < SIZE; j++)
        {
            if (str[i] == alphabetS[j])
            {
                count[j] = count[j] + 1;
                if (count[j] > 1)
                {
                    return false;
                }
                break;
            }
        }
    }
    return true;
}

int main()
{
    Strings obj;
    std::string str;
    std::cout << "Enter String\n";
    std::getline(std::cin, str);
    bool res = obj.isAllUnique(str);
    if (res == true)
    {
        std::cout << "There are unique characters in string\n";
    }
    else
    {
        std::cout << "There are no unique characters in string\n";
    }
}

People are very eager to present their own solutions, so let me attempt an actual review of your code:

Your names could be clearer. Strings is not very descriptive, and neither are alphabetS/L. SIZE doesn't tell anyone what kind of size it is or what it relates to.
Anything not nested under private or public in a class is automatically private. So you might as well move those stray members into the private section to make things clearer.
If you're using C++11 or higher then you can use constexpr.
Why are you returning a value from your toLower function after you pass in a mutable reference? Might as well drop the return and use the reference.
Your toLower function does a lot of work. Assuming you don't want to use the STL for this you can still roll your own function. If you only want to check characters from A-z then you can simply get the lowercase by adding the difference to any number representation smaller than 97 (a). Look at the ASCII chart for reference.
You can simply initialize count with int count[SIZE]{};. Alternatively you could use memset.
Try to avoid performing more than one task in a function. E.g. factor out the toLower functionality of isAllUnique. It could then be const as it has no need to mutate class state.
For the actual algorithm I prefer Incomputable's approach over the one suggested by Ben Steffan which can be used without overly relying on the STL.
Prefer prefix over postfix for the increment operator.
Don't compare to true. Instead simply do if (a_bool_var), or if (!a_bool_var) for the negation.
This could probably be done with just free functions without needing a class object.


Is brute force the accepted best practice for handling Excel COM 'busy' exceptions? Following on from a question about handling-com-exceptions-busy-codes I would like to know if the following model is the accepted best practice of handling Excel COM busy messages when accessing the Excel COM object model:

private void AskExcelToDoSomething()
{
    bool retry = true;
    do
    {
        try
        {
            // Call Excel OM
            retry = false;
        }
        catch (COMException e)
        {}
    } while (retry);
}

Assuming that IMessageFilter has been implemented and catches the errors for which it is intended and retries, is the above model the 'accepted' way of handling these busy exceptions? If so, is it not vulnerable to hanging if for some reason it keeps being rejected? Is there no better way of doing this that guarantees success?

You're missing the Thread.Sleep(500); call from the linked post's solution, which is much, much better than what you've posted here:

private void TryUntilSuccess(Action action)
{
    bool success = false;
    while (!success)
    {
        try
        {
            action();
            success = true;
        }

        catch (System.Runtime.InteropServices.COMException e)
        {
            if ((e.ErrorCode & 0xFFFF) == 0xC472)
            {   // Excel is busy
                Thread.Sleep(500); // Wait, and...
                success = false;  // ...try again
            }
            else
            {   // Re-throw!
                throw e;
            }
        }
    }
}

(above code from sepp2k's answer here)

  is it not vulnerable to hanging if for some reason it keeps being rejected?

Given that this code is meant to address a COM exception thrown when Excel is busy, it remaining in the loop would mean, well, that Excel is still busy - and possibly hung... in which case, the user will probably lose their patience and kill the application anyway.

The difference between sepp2k's code and the code you're having here, is that you're swallowing all COM exceptions - which is bad. I really don't see a reason not to use the code provided in the post you're following-up on, which filters the error code (if ((e.ErrorCode & 0xFFFF) == 0xC472)) and re-throws all other exceptions, leaving you with a way to exit the loop if you get a COM exception that's saying anything other than Excel is busy.


PHP exercise to read or write database values I just came up with the code below but I'm wondering if there is room for improvement? I.e. better readability and or performance, better structure, less lines of code?

The script should parse a simple HTTP GET or POST request. The request can contain the following parameters:

action (required, possible values are "read" and "write");
mykey (required)
id (only required when action equals "read")

Depending on a read or write request the script should print an array containing:

status;
id (only after writing / inserting a value into database);
value (in case of a read request);

Sample requests:

api.php?action=write&mykey=12345&value=im%20fine
api.php?action=read&mykey=12345&id=13

The script should also log the content of the request to file. (append)

Code:

<?php
  include("opendb.php");
  $logging = true;
  $log = serialize($_REQUEST) . PHP_EOL;

  $reply = array("id" => null, "status" => null);

  if(isset($_REQUEST["action"]) && isset($_REQUEST["mykey"])) {
    $action = strtolower($_REQUEST["action"]);
    $mykey = $mysqli->real_escape_string($_REQUEST["mykey"]);

    if($action == "read") {
      if(isset($_REQUEST["id"]) && is_numeric($_REQUEST["id"])) {
        $id = $_REQUEST["id"];
        // read message from database
        $sql = "SELECT * FROM data WHERE mykey = '$mykey' AND id = $id";

        if ($result = $mysqli->query($sql)) {
            $row = $result->fetch_assoc();
            $reply["id"] = $id;
            $reply["value"] = $row["value"];
            $reply["status"] = "success";
        } else {
            $log .= $mysqli->error . PHP_EOL;
            $reply["status"] = "error";
        }
      } else {
          $log .= "id missing" . PHP_EOL;
          $reply["status"] = "error (incorrect id)";
      }
    } elseif($action == "write") {
      // write message to database
      $value = $mysqli->real_escape_string($_REQUEST["value"]);

      $sql = "INSERT INTO `data` (`value`, `mykey`) VALUES ('$value', '$mykey')";

      if ($result = $mysqli->query($sql)) {
          $reply["id"] = $mysqli->insert_id;
          $reply["status"] = "success";
      } else {
          $log .= $mysqli->error . PHP_EOL;
          $reply["status"] = "error";
      }
    }
  } else {
      $log .= "invalid querystring" . PHP_EOL;
      $reply["status"] = "error (incorrect action, mykey)";
  }

  print_r($reply);

  if($logging) {
    file_put_contents('log/read_write_exercise.log', date('d-m-Y') . " " . print_r($log, TRUE), FILE_APPEND | LOCK_EX) ;
  }
?>

Instead of using a big stack of if/elseif, since you're just checking the value of a single variable you should be using a switch/case instead.
What if action and mykey are both set, but action is neither read nor write - Your user will get a blank response.. yet another reason to use a switch (with a default option).
Don't use real_escape_string, use prepared statements instead.
Don't SELECT * ... if you only need the value column, just select value ...
I would suggest creating a function to handle errors.
I would suggest holding your errors in an array and joining them rather than concatting to a string - this will be more efficient and manageable.
There is no reason to use LOCK_EX in conjunction with FILE_APPEND - If you lock the file and the script is called simultaneously one instance will be blocked or denied while the other is writing. See here for more info.
Is this not supposed to be an API? If it is you might consider outputting JSON instead of print_ring the results. I would suggest putting this in an output function as well.

include("opendb.php");
$logging = true;
$log = array(date('d-m-Y'));
$log[] = serialize($_REQUEST)

$reply = array("id" => null, "status" => 'success');

if(!isset($_REQUEST["mykey"])) error("No key provided.");
$action = empty($_REQUEST["action"]) ? "" : strtolower($_REQUEST["action"])

switch($action){

    case "read":
        if(!isset($_REQUEST["id"])) error('No ID provided.');
        $stmt = $mysqli->prepare("SELECT value FROM data WHERE mykey = ? AND id = ?");
        $stmt->bind_param('si', $_REQUEST["mykey"], $_REQUEST["id"]);
        $stmt->execute();
        $stmt->bind_result($value);
        if(!$stmt->fetch()) error("No results found.");
        $reply["id"] = $_REQUEST["id"];
        $reply["value"] = $value;
        output();

    case "write":
        if(!isset($_REQUEST["value"])) error('No value provided.');
        $stmt = $mysqli->prepare(INSERT INTO `data` (`value`, `mykey`) VALUES (?, ?));
        $stmt->bind_param('ss', $_REQUEST["value"], $_REQUEST["mykey"]);
        if(!$stmt->execute()) error($mysqli->error);
        $reply["id"] = $mysqli->insert_id;
        output();

    default: error('Invalid action.');
}

function error($msg){
    $GLOBALS['log'][] = $msg;
    $GLOBALS['reply']['status'] = 'error';
    $GLOBALS['reply']['error'] = $msg;
    output();
}

function output(){
    if($GLOBALS['logging']){
        file_put_contents('log/read_write_exercise.log',  implode(PHP_EOL, $GLOBALS['log']), FILE_APPEND) ;
    }
    header('Content-Type: application/json');
    echo json_encode($GLOBALS['reply'], JSON_PRETTY_PRINT);
    exit;
}


Locale-independent float print function The objective was to write a locale-independent function that prints floating-point numbers (i.e. one that always uses . as a decimal point regardless of the locale) that was going to be used in a library that was meant to produce JSON. I was aiming for the following:

Implementation simplicity
Ability to be used on different sorts of streams (writing into a file, a string, etc)
Reasonable corrrectness

Here's the main code:
#include <inttypes.h>
#include <math.h>
#include <stdbool.h>
#include <stdio.h>

typedef int (*printfn_t)(void *, const char *, ...);

#define print(...) if (printfn(stream, __VA_ARGS__) < 0) {\
    return false;\
}

static const int PRECISION = 15;

static int count_digits(double n) {
  return floor(log10(n) + 1);
}

bool print_number(
    printfn_t printfn,
    void *stream,
    double number
) {
    if (number == 0) {
        print("0");

        return true;
    }

    if (number < 0) {
        print("-");

        number = -number;
    }

    uint64_t integral = 0, decimal = 0;
    int exponent = 0;

    if (number > 9007199254740991) {
        exponent = count_digits(number) - 1;

        number = number / pow(10, exponent);
    } else if (number < 1 / pow(10, PRECISION)) {
        exponent = count_digits(number) - 1;

        number = number * pow(10, -exponent);
    }

    integral = number;
    decimal = round(fmod(number, 1) * pow(10, PRECISION));

    print("%" PRId64, integral);

    if (decimal != 0) {
        print(".");

        int decimal_length = count_digits(decimal);

        for (int i = 0; i < PRECISION - decimal_length; i++) {
            print("0");
        }

        print("%" PRId64, decimal);
    }

    if (exponent != 0) {
        print("e%i", exponent);
    }

    return true;
}

And here's how you use it:
int main() {
    double input = 0;

    scanf("%lf", &input);

    print_number(
        (printfn_t) fprintf,
        stdout,
        input
    );
}

Implementation simplicity
As code is incorrect in too many cases, simplicity assessment is moot.
Ability to be used on different sorts of streams (writing into a file, a string, etc)
Code assumes it can pass a void * like a FILE *.  Reasonable, but not specified by C.
Writing to a string deserves a size parameter.
Rather than attempt to use fprintf(), sprintf(), consider providing interface functions.
Reasonable corrrectness
corrrectness -->  correctness
OP is brave.  Quality conversion of a string to a double is hard.
To assess, I set up a test harness.
void test(double x) {
  // Reference output
  printf("R%.*e %20a\n", DBL_DECIMAL_DIG - 1, x, x);
  // OP's output
  printf("<");
  print_number((printfn_t) fprintf, stdout, x);
  puts(">\n");
}

int main(void) {
  const double x[] = {1.0, 1.0 / 7, DBL_MIN, DBL_TRUE_MIN, DBL_MAX, INFINITY,
      NAN, -DBL_MAX, -0.0, 1.0 + DBL_EPSILON, 1.0 - DBL_EPSILON};
  size_t n = sizeof x / sizeof *x;
  for (size_t i = 0; i < n; i++) {
    test(x[i]);
  }
}

Outright discrepancies included
R4.9406564584124654e-324            0x1p-1074
<0.-9223372036854775808e-324>                          ???

Rinf                  inf
<-9223372036854775808.-9223372036854775808e2147483647> ???

Rnan                  nan
<-9223372036854775808.-9223372036854775808>            ???

R-0.0000000000000000e+00              -0x0p+0
<0>                                                    Missing sign

R9.9999999999999978e-01 0x1.ffffffffffffep-1
<0.1000000000000000>                                   Off by 10x!

Non-finite
For NAN and infinity, consider a !isfinite(x) test and then handle those special cases.
Sign
For -0.0 correctness,
//if (number < 0) {
if (signbit(number)) {
    print("-");
    ...

Precision
double deserves up to DBL_DECIMAL_DIG (e.g. 17) places of output to properly distinguish all double from other double.  OP's precisions output wobbles with inconsistent significant precision output.
I adjusted OP's code to int PRECISION = DBL_DECIMAL_DIG;, then used select values like 1.0 - DBL_EPSILON (and many others) are not quite correct.
R9.9999999999999978e-01 0x1.ffffffffffffep-1
<0.99999999999999984>

As a reference, to print the exact value of a finite double. See Function to print a double - exactly
Numeric correctness.
Each *, / and some pow(10, ) inject 0.5 [ULP] or more error into the math and there is little code compensating for these accumulating errors.  Without internal extended math, I see OP's approach as unable to achieve a high quality result.
Code assumes double precision < 64 bits.  Reasonable, but not specified by C.

Recommendation
Test with the usual suspects as in the test harness, with values near powers of 10 and with various random values.


string.find versus this function I got bored and decided to write my own function to verify if a string contains another string.

I was wondering if there's any major difference between these two function except the one I wrote being more complicated.

bool Contains(string str, string find){
    for (int i = 0; i < str.length() - (find.length()-1); i++){
        if (find.length() > 0){
            int CharactersFound = 0;
            for (int x = 0; x < find.length(); x++){
                if (str[i + x] == find[x]){
                    CharactersFound++;
                }
                else {
                    CharactersFound = 0;
                }
            }
            if (CharactersFound == find.length()) return true;
        } else {
            return true;
        }
    }
    return false;
}
bool StrContains(string str, string find){
    if (str.find(find) != string::npos) return true;
    return false;
}

There's likely a difference in your implementation and the library's. The algorithm you used is an inefficient variant of the inefficient brute force algorithm (yes, doubly inefficient). Better algorithms exist for finding a substring, for example the Knuth-Morris-Pratt algorithm. I suspect the library uses something more clever than brute force.

It's strange and inefficient to have the find.length() > 0 inside the loop. The value of find.length() doesn't change inside the loop, so it's better to check it once, before the loop begins.

Counting the characters found is inefficient if you don't break on a mismatch.
Instead of counting the characters found, it would be better to use a boolean flag. When you find a character that doesn't match, you can set the flag and break out of the loop. After the loop, if the flag changed, you know the string wasn't found.

That is:

for (int i = 0; i < str.length() - (find.length()-1); i++) {
    bool mismatch = false;
    for (int x = 0; x < find.length(); x++) {
        if (str[i + x] != find[x]){
            mismatch = true;
            break;
        }
    }
    if (!mismatch) {
        return true;
    }
}
return false;

Instead of using a flag, what's even better is to extract the inner loop to a function, as Jerry-Coffin did.

You can use boolean expressions directly as return values. So instead of this:

if (str.find(find) != string::npos) return true;
return false;

You can write:

return str.find(find) != string::npos;

There are multiple accepted naming conventions in C++,
but I don't think there is when where local variables follow PascalCase. It would be better to change to camelCase.


List of all possible monetary totals from a set of cash This code prints out a list of all the possible sums that can be made from a set of cash.

For example, say you have 2 fives and 2 tens in your wallet. With these, you can make exact change for $0, $5, $10, $15, $20, $25, and $30.

import itertools
cash = {1: 1, 5: 3, 10: 2} # Keys are denominations (e.g. $1 bill)
                        # Values are the numbers of bills (e.g. three $1 bills)
totals = set() # Set of possible totals
cash_list = [i for i in sorted(cash) for j in range(cash[i])] # List of all bills
for r in range(len(cash_list)+1):
    for subset in itertools.combinations(cash_list, r): # All the bill combinations
        totals.add(sum(subset)) # Record the total of the bills
print(sorted(list(totals))) # Print all possible totals

Output:

[0, 1, 5, 6, 10, 11, 15, 16, 20, 21, 25, 26, 30, 31, 35, 36]

1. Review

The data structure cash is a map from denomination to the count of the bills in that denomination. This is a representation of a multiset, and it's a good idea here to use the built-in collections.Counter:

from collections import Counter
cash = Counter({1: 1, 5: 3, 10: 2})

Now you can replace this line:

cash_list = [i for i in sorted(cash) for j in range(cash[i])]

with a call to the Counter.elements method:

cash_list = list(cash.elements())

The code is not organized into functions. This makes it hard to test and hard to measure the performance. It would be better to write something like this:

from collections import Counter
from itertools import combinations

def change(cash):
    """Given a map from denomination to the count of bills in that
    denomination, return the set of values that can be made using a
    subset of the bills.

    """
    totals = set() # Set of possible totals
    cash_list = list(Counter(cash).elements()) # List of all bills
    for r in range(len(cash_list)+1):
        for subset in combinations(cash_list, r):
            totals.add(sum(subset))
    return totals

Now we can easily test it:

>>> change({1:1, 2:1, 4:1, 8:1}) == set(range(16))
True

and measure its performance:

>>> from timeit import timeit
>>> timeit(lambda:change({1:20}), number=1)
0.6443959611933678

2. Performance

The algorithm loops over all subsets of bills. But this doesn't take advantage of the fact that bills of the same denomination are indistinguishable. For example, we know that

change({1:25}) == set(range(26))

since if you have twenty-five $1 bills, you can make change for all dollar amounts from $0 to $25. But this calculation, that should be trivial, takes a very long time:

>>> timeit(lambda:change({1:25}), number=1)
22.855976961087435

And good luck waiting for, say, change({1:100}) to finish running. So we can improve the performance in these cases by taking advantage of the fact that if have \$n\$ bills of denomination \$d\$, the possible values are \$0, d, 2d, \ldots, nd\$, that is, range(0, (n + 1) * d, d). Using itertools.product we can write it like this:

from collections import Counter
from itertools import product

def change2(cash):
    """Given a map from denomination to the count of bills in that
    denomination, return the set of values that can be made using a
    subset of the bills.

    """
    ranges = [range(0, (n + 1) * d, d) for d, n in Counter(cash).items()]
    return set(map(sum, product(*ranges)))

This is slightly slower than the original code in cases where all the denominations are different:

>>> timeit(lambda:change({2**i:1 for i in range(20)}), number=1)
0.9978475249372423
>>> timeit(lambda:change2({2**i:1 for i in range(20)}), number=1)
1.0107885741163045

but it is massively faster in cases where there are many bills of a denomination:

>>> timeit(lambda:change2({1:25}), number=1)
0.000085500068962574