Find the minimum gap (difference) between adjacent elements in a linked list Write a method called minGap that returns the    minimum gap between
  adjacent values in a list of integers.  The gap between    two
  adjacent values in a list is defined as the second value minus the
  first    value.  For example, suppose a variable called list stores
  these values:

  [1, 3, 6, 7, 12]

  The first gap is 2 (3 - 1), the second gap is 3 (6 - 3), the third gap is 1
  (7 - 6), and the fourth gap is 5 (12 - 7).  Thus, the call:

  list.minGap()

  should return 1 because that is the smallest gap.  Notice that the minimum
  gap could be a negative number.  For example, if list stores the following:

  [3, 5, 11, 4, 8]

  The gaps are 2 (5 - 3), 6 (11 - 5), -7 (4 - 11), and 4 (8 - 4).  Of these
  values, -7 is the smallest, so it would be returned.

  Your method should return 0 if the list has fewer than 2 elements.  You are
  writing a method for the LinkedIntList class:

    public class ListNode {
        public int data;       // data stored in this node
        public ListNode next;  // link to next node in the list

        <constructors>
    }

    public class LinkedIntList {
        private ListNode front;

        <methods>
    }

  Your method should not modify the list contents and is required to
  run in    O(n) time where n is the length of the list.  You may not
  call any other    methods of the LinkedIntList class and you may not
  construct any structured    objects to solve this problem.

I'm unsure if I'm doing the right thing for checking minimums by setting the int minGap to Integer.MAX_VALUE (let me know if this is bad). I want to get better at these problems, so any tips that can make my code more readable or easier to write would be awesome! I didn't know a lot about using multiple references to linked lists before I solved this problem, so any advice to solving these problems in the future would be much appreciated.

public int minGap() {
    if (front == null || front.next == null) {
        return 0;
    }
    int minGap = Integer.MAX_VALUE;
    ListNode prev = front;
    ListNode p = front.next;
    while (p != null) {
        int checkGap = p.data - prev.data;
        if (checkGap < minGap) {
            minGap = checkGap;
        }
        prev = p;
        p = p.next;
    }
    return minGap;
}

You don't need to store the whole previous node, just the value. Also, you can replace checking whether the gap is smaller with min, although it's mainly a matter of taste. And p is a rather opaque variable name

public int minGap() {
    if (front == null || front.next == null) {
        return 0;
    }
    int minGap = Integer.MAX_VALUE;
    int lastValue = front.data;
    ListNode currentNode = front.next;
    while (currentNode != null) {
       minGap = min(minGap, currentNode.data-lastValue);
       lastValue = currentNode.value;
       currentNode = currentNode.next;
    }
    return minGap;
}


Divide list into batches Three questions: is there a more performant way, is there a more suscinct, strike that, a way of expressing this where if you read just the body it is immediately apperent what the alorithm does, and should I be returning and IEnumerable of an IEnumerable, I mean what would be the point over and IEnumerable of IList?

public static IEnumerable<IEnumerable<T>> IntoBatches<T>(this IEnumerable<T> list, int size)
{
    if (size < 1)
    {
        yield return list;
    }
    else
    {
        var count = 0;
        var batch = new List<T>();
        foreach (var item in list)
        {
            batch.Add(item);
            if (size == ++count)
            {
                yield return batch;
                batch.Clear();
            }
        }
        if (batch.Count > 0) yield return batch;
    }
}

Bug
You have 2 big bugs in your method. The first is that you never ever set the count variable to 0 and the second that you are yielding the List<T>.
If I call your method with a List<int> containing 10000 ints and do a ToList() on the result I get 2 Lists both containing 9997 ints.
Although this is easy to fix like so
public static IEnumerable<IEnumerable<T>> IntoBatches<T>(this IEnumerable<T> list, int size)
{
    if (size < 1)
    {
        yield return list;
    }
    else
    {
        var count = 0;
        foreach (var item in list)
        {
            var batch = new List<T>();
            batch.Add(item);
            if (size == ++count)
            {
                yield return batch;
                batch = new List<T>();
                count = 0;
            }

        }
        if (batch.Count > 0) yield return batch;
    }
}

this solution takes for a List<int> having 10000 items with size:
  3: 0.506 ms
 13: 0.505 ms
113: 0.505 ms

whereas an array based solution like this (taken from here)
public static IEnumerable<IEnumerable<T>> Chunkify<T>(this IEnumerable<T> source, int size)
{

    using (var iter = source.GetEnumerator())
    {
        while (iter.MoveNext())
        {
            var chunk = new T[size];
            chunk[0] = iter.Current;
            for (int i = 1; i < size && iter.MoveNext(); i++)
            {
                chunk[i] = iter.Current;
            }
            yield return chunk;
        }
    }
}

takes
  3: 0.270 ms
 13: 0.270 ms
113: 0.270 ms

Edit
That Chunkify() method unfortunately has a bug, which is for a passed in IEnumerable<T> with a size which isn't dividable by the passed in chunk size will produce to many items.
E.g passed in a int[] with values 1,2,3,4 and an size argument of 3 will produce 1,2,3,4,0,0.
Fixed version
public static IEnumerable<IEnumerable<T>> Chunkify<T>(this IEnumerable<T> source, int size)
{
    int count = 0;
    using (var iter = source.GetEnumerator())
    {
        while (iter.MoveNext())
        {
            var chunk = new T[size];
            count = 1;
            chunk[0] = iter.Current;
            for (int i = 1; i < size && iter.MoveNext(); i++)
            {
                chunk[i] = iter.Current;
                count++;
            }
            if (count < size)
            {
                Array.Resize(ref chunk, count);
            }
            yield return chunk;
        }
    }
}


Avoid global variable for callback function called by function with no arguments My website has multiple pages that use the Google Maps api. Each page uses a different part of the api (e.g. autocomplete, displaying markers etc). However, every page needs to call the initMap() function for the api to be initialized. The logic that calls initMap() is controlled by google and it cannot take parameters.
maps.js
function initMap() {
  const mapOptions = {
    center: {lat: 0, lng: 0},
    zoom: 7
  };
  window.map = new google.maps.Map(document.getElementById("map"), mapOptions);

  if (typeof callback !== 'undefined') {
    callback(); // defined above html link to maps.js
  }
}

Instead of writing out a different initMap() function for every page, I opted to have it in one .js file which is linked to every page. After initializing the map, the function executes a callback function which is defined in each page's corresponding .js file.
index.js
function autocomplete() {
    var input = document.getElementById('address');
    new google.maps.places.Autocomplete(input);
}

As initMap() cannot take parameters, I had to define the callback function's name using a cross-script global variable before maps.js is linked. This variable is different on every page depending what function needs to be run in initMap() after the map is initialized.
<script type="text/javascript" src="js/index.js"></script>
<script> var callback = autocomplete </script> <!-- global variable, defines the callback function (in index.js) to initMap()(in maps.js) -->
<script type="text/javascript" src="js/maps.js"></script>

Is there a cleaner way to accomplish this?

You can avoid the global by creating a closure around the function to be called by the maps api code. To do this define another function that returns a function. You can pass in the callback here.
The below code is runnable in single file but you will need to split it across pages in your real code.
function createInitMap(callback){
    return () => {
        const mapOptions = {
            center: {lat: 0, lng: 0},
            zoom: 7
        };
        window.map = new google.maps.Map(document.getElementById("map"), mapOptions);

        if (typeof callback !== 'undefined') {
            callback();
        }
    }
}

/*
    In first page
*/
//<script type="text/javascript" src="js/index.js"></script>
let initMap = createInitMap(() => console.log('first callback'));
//<script type="text/javascript" src="js/maps.js"></script>

initMap(); // simulate google maps calling your function

/*
    In second page
*/
//<script type="text/javascript" src="js/index.js"></script>
initMap = createInitMap(() => console.log('second callback'));
//<script type="text/javascript" src="js/maps.js"></script>

initMap();  // simulate google maps calling your function

Ensure the return value of the function is assigned to the global name initMap (It might need to be window.initMap).


React container component to fetch paginated data for a stateless table component TableContainer.js does the common task of fetching data and passing it to a lower component, Table.js, which is a stateless functional component.

currentPage is stored in redux, I did this just to practice redux.

Question 1

Is this all reasonable?

Question 2

I noticed that the component will re-render when it receives a new currentPage and then re-renders again, once the data is loaded. The first re-render is not necessary as nothing changes before data is actually loaded. Should I just live with this or should I implement shouldComponentUpdate() to check if data has changed. Or is that potentially even more costly than the re-render itself?

Question 3

Is using componentDidUpdate() to check if currentPage has changed and then re-load the data a good way of controlling the load process?

Question 4

Is building the URL this way acceptable?

const pageParam = currentPage ? "?_page=" + currentPage : "";
fetch('https://jsonplaceholder.typicode.com/posts/' + pageParam)

TableContainer.js

import React from 'react';
import PropTypes from 'prop-types';
import Table from "../components/Table";
import Pagination from "../components/Pagination";
import {connect} from "react-redux";
import {changePage} from "../js/actions";

const PAGE_COUNT = 10;

const mapStateToProps = state => {
    return { currentPage: state.currentPage }
};

const mapDispatchToProps = dispatch => {
  return {
    changePage: page => dispatch(changePage(page))
  };
};

class ConnectedTableContainer extends React.Component {
    state = {
        data: [],
        loaded: false,
    };

    handlePageChange = page => {
        if (page < 1 || page > PAGE_COUNT) return;
        this.props.changePage(page);
    };

    loadData = () => {
        this.setState({ loaded: false });
        const { currentPage } = this.props;
        console.log("load data: " + currentPage);
        const pageParam = currentPage ? "?_page=" + currentPage : "";
        fetch('https://jsonplaceholder.typicode.com/posts/' + pageParam)
            .then(response => {
                if (response.status !== 200) {
                    console.log("Unexpected response: " + response.status);
                    return;
                }
                return response.json();
            })
            .then(data => this.setState({
                data: data,
                loaded: true,
            }))
    };

    componentDidMount() {
        this.loadData(this.props.currentPage);

    }

    componentDidUpdate(prevProps) {
       if (prevProps.currentPage != this.props.currentPage) {
            this.loadData();
        }
    }

    render() {
        const { loaded } = this.state;
        const { currentPage } = this.props;
        console.log("render page: " + currentPage);
        return (

                    <Pagination onPageChange={ this.handlePageChange } pageCount={ PAGE_COUNT } currentPage={ currentPage }/>

        )
    }
}

ConnectedTableContainer.propTypes = {
    changePage: PropTypes.func.isRequired,
    currentPage: PropTypes.number.isRequired,
};

ConnectedTableContainer.defaultProps = {
    currentPage: 1,
};

const TableContainer = connect(mapStateToProps, mapDispatchToProps)(ConnectedTableContainer);

export default TableContainer;

Question 1

  Is this all reasonable?

Absolutely!

  Question 2

  I noticed that the component will re-render when it receives a new
  currentPage and then re-renders again, once the data is loaded. The
  first re-render is not necessary as nothing changes before data is
  actually loaded. Should I just live with this or should I implement
  shouldComponentUpdate() to check if data has changed. Or is that
  potentially even more costly than the re-render itself?

This is normal, and I wouldn't worry about it. In fact, not having to worry about these types of things is a major goal of React.

It's important to realize that re-renders in React only update the DOM when there are changes and only update the relevant DOM nodes. DOM operations are very time consuming, because DOM objects are incredibly heavy objects, and updates have knock-on effects. This is one of the main reasons React doesn't use DOM objects internally. Instead, it uses a virtual DOM. Through a process called reconciliation, React can update the real DOM from the virtual DOM incredibly fast.

In the state where the page changes, but you don't have data yet, React will call render(), very quickly realize there are no changes to the element tree, then stop - this probably happens in roughly a millisecond.

On that note, you do need to be careful when you use Redux - specifically react-redux / connect(). Try to keep mappings of state to props as minimal as possible – don't map an entire object from the Redux tree to a prop for a component if you only need one property of that object. The reason this is important, is because:

Your reducers generate new objects every time they run (if you're using them correctly)
Redux and React use JavaScript's default comparison when deciding if it should update a component
Every object is distinct in JavaScript, even if their structure is identical

So a reducer takes one object, then generates a new object, and even if the properties of these objects are exactly the same, React/Redux will treat it like a change and update components. This is OK for one component, but if you make this a habit your application, you'll be missing out on one of React's biggest features: performance.

// Do this:

const GoodComponent = (props) => `page number: ${props.page}`;

connect(state => {
  page: pagination.page
})(GoodComponent)

// ... not this:

const BadComponent = (props) => `page number: ${props.pagination.page}`;

connect(state => {
  pagination: pagination
})(BadComponent)

  Question 3

  Is using componentDidUpdate() to check if currentPage has changed and
  then re-load the data a good way of controlling the load process?

Absolutely - this is probably why componentDidUpdate() was created.

  Question 4

  Is building the URL this way acceptable?

  const pageParam = currentPage ? "?_page=" + currentPage : "";
  fetch('https://jsonplaceholder.typicode.com/posts/' + pageParam)

It's probably OK for now, although you could use template literals. I generally avoid constructing the query string manually, except for the simplest cases. This is because the second you have multiple query params and some of them are optional, the construction gets tricky – what adds the question mark? how to handle trailing ampersands? trailing question mark?. Instead, use the URL API's searchParams property. Or, find a library that gives you more conveniences, like being able to provide query params as a dictionary/object in fetch()'s second argument.


Getting data from database through Ajax post request Is the method used below to get data from a database efficient and optimal? The data is in a MySQL database, and in my server I have a PHP file with the following code to return some information:
if($_POST["method"] == "requestBusinessFood") {
    requestBusinessFood();
}

function requestBusinessFood() {
    $categoryID = $_POST["category"];

    $host = 'localhost';
    $user = 'root';
    $pass = '';
    $db = 'fooddatabase';

    $conn = mysqli_connect($host, $user, $pass, $db);
        $sql = "SELECT * FROM `foodtablebusiness` WHERE category = " . $categoryID;

        $result = mysqli_query($conn, $sql);
        $rows = array();
        while($r = mysqli_fetch_assoc($result)) {
         $rows[] = $r;
        }

        echo json_encode($rows);
}

On the webpage, I have a js file to retrieve the information in the following way:
  function createAJAXRequestToPopulateList(category) {
    return $.ajax({ url: '../server.php',
                    data: {method: 'requestBusinessFood',
                           category: category},
                    type: 'post'
    });
  }

function addActivityItem(){
    var selector = document.getElementById("categorySelector");
    ajaxRequest = createAJAXRequestToPopulateList(selector.options[selector.selectedIndex].value);
    ajaxRequest.done(populateList);
}

function populateList(responseData) {
    console.log(responseData);
}

PHP

SQL injection vulnerabilities

This code is wide-open to SQL injection attacks. User input should be sanitized as supplied to the query using parameters (e.g. with mysqli_prepare() and bind_param()).

Indentation

The indentation is somewhat consistent but then in some places it increases without a block-level change - e.g. in the PHP code most lines are indented with four spaces, which is very common, and then when $sql is declared it increases to eight spaces.

$conn = mysqli_connect($host, $user, $pass, $db);
      $sql = "SELECT * FROM `foodtablebusiness` WHERE category = " . $categoryID;

Fetching results

Instead of using a while loop with mysqli_fetch_assoc() just to push into an array, use mysqli_fetch_all() to get an array with one call.

Selecting fields

Additionally, the SQL query selects all fields - i.e. with SELECT *. Instead of selecting *, specify the field names needed in order to exclude any fields not needed in the front end.

Javascript

Variable scope

In this line:

ajaxRequest = createAJAXRequestToPopulateList(selector.options[selector.selectedIndex].value);

it makes a global variable ajaxRequest because there is no var, let or const keyword before it. Use const to limit the scope to the function addActivityItem.

Utilizing jQuery

It appears that jQuery is used, given the usage of $.ajax() (though if that is wrong then the following won't work). Presuming that is the case, then the other code can be simplified using jQuery utilities.

For example:

var selector = document.getElementById("categorySelector");
ajaxRequest = createAJAXRequestToPopulateList(selector.options[selector.selectedIndex].value);

can be simplified to the following using the .val() method:

const selector = $('#categorySelector');
const ajaxRequest = createAJAXRequestToPopulateList(selector.val());


Merging vertical lines that coincide I have a list of lines and I need to merge those who are similar (for example my machine vision script will return 3 small vertical line instead of one long vertical line).

I came up with this.
I'm not pleased with it, what do you think?

import math

class Point():
    def __init__(self, x, y):
        """
        Represent a point in a plan
            - x : x (on the width axis, 0 is at the left)
            - y : y (on the height axis, 0 is on the top)
        """
        self.x = int(x)
        self.y = int(y)

    def __str__(self):
        return '(Point : x=%s, y=%s)' %(self.x, self.y)

class Line():
    def __init__(self, a, b):
        """
        Represent a plan
            - a : viewanalyser.Point, start of the line
            - b : viewanalyser.Point, end of the line
        """
        self.a = a
        self.b = b

    def __str__(self):
        return 'Line with points : a=%s, b=%s' %(self.a, self.b)

def find_vLinesToMerge(vLines, tolerance):

    """
    Find vertical lines that can be merged (meaning they have the same x,
    there is a tolerance, for ecample if tolerance = 20 then 2 vertical lines
    spaced for less than 20 pixel on the x absciss are considered to be mergeable
        -vLines : list(Lines) : MUST BE VERTICAL LINES ONLY
        -tolerance : (int) : tolerance for detecting similar vertical lines
    """

    toMerge = []
    alreadyFlaggedForMerge = set()
    for i in range(len(vLines)):
        if i in alreadyFlaggedForMerge:
            continue
        toMergeInThisRun = set()
        for j, line in enumerate(vLines):
            if math.fabs(vLines[i].a.x - line.a.x)<tolerance:
                alreadyFlaggedForMerge.add(i)
                alreadyFlaggedForMerge.add(j)
                toMergeInThisRun.add(i)
                toMergeInThisRun.add(j)
        toMerge.append(toMergeInThisRun)

    return toMerge

vertical_lines = []
vertical_lines.append(Line(Point(1, 7), Point(1, 13)))#---------------------a / index[0]
vertical_lines.append(Line(Point(1, 14), Point(1, 27)))#--------------------a / index[1]
vertical_lines.append(Line(Point(1, 28), Point(1, 35)))#--------------------a / index[2]
vertical_lines.append(Line(Point(50, 1), Point(50, 5)))#--------------------b / index[3]
vertical_lines.append(Line(Point(80, 7), Point(80, 13)))#-------------------c / index[4]
vertical_lines.append(Line(Point(1, 36), Point(1, 55)))#--------------------a / index[5]
vertical_lines.append(Line(Point(50, 6), Point(50, 13)))#-------------------b / index[6]
vertical_lines.append(Line(Point(100, 7), Point(100, 13)))#-----------------d / index[7]
vertical_lines.append(Line(Point(100, 14), Point(100, 200)))#---------------d / index[8]
vertical_lines.append(Line(Point(100, 201), Point(100, 500)))#--------------d / index[9]
vertical_lines.append(Line(Point(80, 14), Point(80, 200)))#-----------------c / index[10]
vertical_lines.append(Line(Point(1000, 7), Point(1000, 13)))#---------------e / index[11]

print find_vLinesToMerge(vertical_lines, 20)

Output :

[set([0, 1, 2, 5]), set([3, 6]), set([10, 4]), set([8, 9, 7]), set([11])]

This codes looks pretty good. I have refactored your code to better use some of the nice things that Python provides, and to help illustrate the following points. Refactored code is below.  Changes made include:

1. pep8:

I changed the code to more closely conform to pep8. This is important when sharing code as the consistent style makes it much easier for other programmers to read your code. There are various tools available to assist in making the code pep8 compliant. I use PyCharm which will show pep8 violations right in the editor.

2. Converted Point and Line into namedtuples:

Converting these objects to named tuples makes them immutable.  This allows them to be keys in dicts and too be added to sets.  This has many benefits.

3. Iterate on list of lines, instead of index:

Since the lines are now immutable, they can be added directly to a set, and the index of the line in the list is no longer needed for this.

4. Iterate inner loop only on unconsidered pairs:

Start the inner loop from the current line only, which removes about half of the comparisons.

5. Turn the test data into a structure that can test the results:

In the test data structure, each line is a key to  dict which references the expected output group for that line segment.  This works because the line is now immutable.

6. Test the results using an assert, instead of print:

This allows the programmer to simply look for pass/fail status and not need to examine print output to verify the code is working.

7. (An update from Comments) Pull line_v1 out of inner loop:

Seeded to_merge_in_this_run = {v_line1} with  single element set literal instead instead of an empty set.  This allowed the removal of already_flagged_for_merge.add(v_line1) as unneeded.

Code:

import math
from collections import namedtuple

Point = namedtuple('Point', 'x y')
Line = namedtuple('Line', 'a b')

def find_v_lines_to_merge(v_lines, tolerance):

    """
    Find vertical lines that can be merged (meaning they have the same x,
    there is a tolerance, for example if tolerance = 20 then 2 vertical
    lines spaced for less than 20 pixel on the x axis are considered to
    be merge-able.
        -vLines : list(Lines) : MUST BE VERTICAL LINES ONLY
        -tolerance : (int) : tolerance for detecting similar vertical lines
    """

    to_merge = []
    already_flagged_for_merge = set()
    for i, v_line1 in enumerate(v_lines):
        if v_line1 not in already_flagged_for_merge:
            to_merge_in_this_run = {v_line1}
            for v_line2 in v_lines[i+1:]:
                if math.fabs(v_line1.a.x - v_line2.a.x) < tolerance:
                    already_flagged_for_merge.add(v_line2)
                    to_merge_in_this_run.add(v_line2)
            to_merge.append(to_merge_in_this_run)

    return to_merge

Test Harness:

test_data = dict((
    (Line(Point(1, 7), Point(1, 13)), 'a'),
    (Line(Point(1, 14), Point(1, 27)), 'a'),
    (Line(Point(1, 28), Point(1, 35)), 'a'),
    (Line(Point(50, 1), Point(50, 5)), 'b'),
    (Line(Point(80, 7), Point(80, 13)), 'c'),
    (Line(Point(1, 36), Point(1, 55)), 'a'),
    (Line(Point(50, 6), Point(50, 13)), 'b'),
    (Line(Point(100, 7), Point(100, 13)), 'd'),
    (Line(Point(100, 14), Point(100, 200)), 'd'),
    (Line(Point(100, 201), Point(100, 500)), 'd'),
    (Line(Point(80, 14), Point(80, 200)), 'c'),
    (Line(Point(1000, 7), Point(1000, 13)), 'e'),
))

# get the test data
test_data_list = test_data.keys()

# shuffle it to a random order
from random import shuffle
shuffle(test_data_list)

# merge the lines
merged_lines = find_v_lines_to_merge(test_data_list, 20)

# verify that the merged lines all match as expected
for merged_line in merged_lines:

    # get the set of line names for each segment in this merge
    line_names = set([test_data[segment] for segment in merged_line])

    # assert there is one and only one name
    assert len(line_names) == 1


Check for Scrabble double letter bonus This function checks if the cell of a Scrabble board is a double letter bonus. It has a 23 cyclomatic complexity, which is higher than the threshold of 20. I don't know how to do it another way, I think that this is the only way to do it. Here is my function :

checkDoubleLetterCell(row: number, column: number): boolean
{
if((row === middle- 1 || row === middle + 1)
&& (column === middle - 1 || column === middle + 1)
|| (row === 0 || row == SCRABBLE_SIZE - 1 || row === middle)
&& (column === middle + Math.round(middle/2) || column === middle - Math.round(middle/2))
|| (column === 0 || column === SCRABBLE_SIZE - 1 || column === middle)
&& (row === middle + Math.round(middle/2) || row === middle - Math.round(middle/2))
|| (column === middle + 1 || column === middle - 1)
&& (row === middle + Math.round(middle/2) + 1 || row === middle - Math.round(middle/2) - 1)
|| (row === middle + 1 || row === middle - 1)
&& (column === middle + Math.round(middle/2) + 1 || column === middle - Math.round(middle/2) - 1))
{
    return true;
}
return false;
}

So, I am going to answer you javascript question using python code because I think the concepts are identical and I was too lazy to make sure my javascript syntax was right.

A couple of ways to simplify your expression include:

Use a lookup table:

I find this implementation more intuitive and much easier to debug.

double_letter_cells = [
    #ABCDEFGHIJKLMNO
    '...1.......1...',  # 1
    '...............',  # 2
    '......1.1......',  # 3
    '1......1......1',  # 4
    '...............',  # 5
    '...............',  # 6
    '..1...1.1...1..',  # 7
    '...1.......1...',  # 8
    '..1...1.1...1..',  # 9
    '...............',  # 10
    '...............',  # 11
    '1......1......1',  # 12
    '......1.1......',  # 12
    '...............',  # 14
    '...1.......1...',  # 15
]

def checkDoubleLetterCell(row, col):
    return double_letter_cells[row][col] == '1'

Build some intermediate products:

If you really need the stacked logic, you can produce some intermediate products which are then much easier to verify and understand.

SCRABBLE_SIZE = 15
middle = int(SCRABBLE_SIZE / 2)

def checkDoubleLetterCell(row, col):
    one_off_middle_row = (row == middle - 1) or (row == middle + 1)
    one_off_middle_col = (col == middle - 1) or (col == middle + 1)
    outer_middle_row = (
        row == 0 or row == SCRABBLE_SIZE - 1 or row == middle)
    outer_middle_col = (
        col == 0 or col == SCRABBLE_SIZE - 1 or col == middle)

    middle_middle_row = row == middle + int(middle / 2) + 1 or \
                        row == middle - int(middle / 2) - 1
    middle_middle_col = col == middle + int(middle / 2) + 1 or \
                        col == middle - int(middle / 2) - 1

    middle_middle_plus_row = row == middle + int(middle / 2) + 2 or \
                             row == middle - int(middle / 2) - 2
    middle_middle_plus_col = col == middle + int(middle / 2) + 2 or \
                             col == middle - int(middle / 2) - 2

    return (
        one_off_middle_row and one_off_middle_col or
        outer_middle_row and middle_middle_col or
        outer_middle_col and middle_middle_row or
        one_off_middle_col and middle_middle_plus_row or
        one_off_middle_row and middle_middle_plus_col
    )

Test Code:

And for completeness, here is the code I used to test the above.

for i in range(SCRABBLE_SIZE):
    for j in range(SCRABBLE_SIZE):
        assert (double_letter_cells[i][j] == '1') \
               == checkDoubleLetterCell(i, j)


Code to Parse dollars and cents? I've written code to parse dollars and cents entered by the user.  The value returned is the total number of cents.

For example:

f('$1.50') = 150

f('1.5') = 150

f('0') = 0

f('1000') = 1000

f('$12,500.00') = 12500000

functions.GetTotalCents = function (dollarsAndCentsString) {
    // Cast the value passed in to a string in case a number was passed in.
    dollarsAndCentsString = dollarsAndCentsString.toString();
    // First, discard the '$' glyph, if it was passed in.
    if (dollarsAndCentsString.split('$').length == 2)
        dollarsAndCentsString = dollarsAndCentsString.split('$')[1];
    // If the user delimmited the groups of digits with commas, remove them.
    dollarsAndCentsString = dollarsAndCentsString.replace(/,/g, '');
    // Next, divide the resulting string in to dollars and cents.
    var hasDecimal = (dollarsAndCentsString.split('.')).length == 2;
    var dollarsString, centsString;
    dollarsString = dollarsAndCentsString.split('.')[0];
    var centsString = hasDecimal ? dollarsAndCentsString.split('.')[1] : '0';
    var dollars = parseInt(dollarsString, 10);
    var cents;
    if (centsString.length == 1)
        cents = parseInt(centsString, 10) * 10;
    else cents = parseInt(centsString, 10);
    if (cents > 99 || isNaN(cents) || isNaN(dollars) || !isFinite(dollars) || !isFinite(cents))
        return 0;
    var totalCents = dollars * 100 + cents;
    return totalCents;
};

Anyone care to critique my code?

Wouldn't it be simpler to:

strip out any '$' and ',' characters;
convert to a real number;
multiply by 100;
take the integer part.

You can probably do that in one line (my Javascript is a bit rusty):

return Math.round(100 * parseFloat(dollarsAndCentsString.replace(/[$,]/g, '')));

Having said that, I think your f('1000') example is probably wrong: my intuition says I should interpret '1000' as dollars, not cents.


Implement a stack class in Python Problem : Implement a stack class in Python. It should support 3 APIs:

  stack.top(): prints element at top of stack

  stack.pop(): takes out an element from top of stack,

  stack.push(): adds a new element at top of stack

Does this code covers all the edge cases of implementing a stack?

class Stack():

    def __init__(self):
        self.item = []

    def size(self):
        return len(self.item)

    def top(self):
        if len(self.item) >= 1:
            print self.item[len(self.item) -1]
        else :
            print "Empty list"

    def pop(self):
        if len(self.item) >= 1:
            self.item.pop()
        else:
            raise IndexError

    def push(self,item):
        self.item.append(item)
        print self.item

new_stack = Stack()
new_stack.push(19)
new_stack.push(20)
new_stack.push(119)
new_stack.push(202)
new_stack.push(195)
new_stack.push(205)
new_stack.push(149)
new_stack.push(230)

print new_stack.size()
new_stack.top()
new_stack.pop()
new_stack.top()
new_stack.pop()
new_stack.pop()
new_stack.pop()
new_stack.pop()
new_stack.pop()
new_stack.pop()
new_stack.pop()
new_stack.top()

Thanks for sharing your code,

instead of using size, you should implement __len__, this way you can use len(my_stack).

def __len__(self):
    return len(self.item)

I think a method that prints an element from a collection is a bit unusual. What I would expect is a method called peek that returns the top element but doesn't remove it. By just printing it you're limiting its use to console only applications, and the only way to retrieve the element is to remove it!

You should consider adding an error message to go with your IndexError, and maybe reconsider the exception type, an IndexError sounds like you're accessing an index, but you're not maybe a custom StackEmpty exception would be more suitable.

I think self.item should be self.items (plural) as it will be representing zero or more items.

Bonus Stuff

you could implement the __iter__ method. This would allow the caller to do something like

for element in my_stack:
    do_stuff()

You code is also missing docstrings, you could optionally add those to document your code.

You could implement the __str__ method to provide a nice string representation so you can simply say

print my_stack

Hopefully you found this helpful, keep it up!


Support for sharing stories on Facebook Is there a way to write this code block cleaner and more readable?

 var shareNavigate = function () {
     scope.sharingActions.shareOnFacebook().done(function () {
             app.router.navigateToStories();
         });
 },
     sharePublishNavigate = function () {
         scope.sharingActions.shareOnFacebook().done(function () {
                 scope.model.publish().done(function () {
                         app.router.navigateToStories();
                     });
             });
     },
     loginSharePublishNavigate = function () {
         facebookActions.login().done(function () {
                 sharePublishNavigate();
             });
     };
 if (!noViews && hasEditPermissions) {
     shareNavigate();
 } else if (noViews) {
     if (hasEditPermissions) {
         sharePublishNavigate();
     } else if (!isLoggeedIn) {
         loginSharePublishNavigate();
     } else {
         shareNavigate();
     }
 }

First I define functions and then in each case I call the relevant function. I am using jQuery deferreds and since I need operation to done, I have many nested methods. Is there any way to beautify this code?

I'd prefer to rewrite this

 if (!noViews && hasEditPermissions) {
     shareNavigate();
 } else if (noViews) {
     if (hasEditPermissions) {
         sharePublishNavigate();
     } else if (!isLoggeedIn) {
         loginSharePublishNavigate();
     } else {
         shareNavigate();
     }
 }

To this:

 if (hasEditPermissions) {
     if (noViews) {
         sharePublishNavigate();
     } else {
         shareNavigate();
     }
 } else {
     if(loggeedIn) {
         shareNavigate();
     } else {
         loginSharePublishNavigate();
     }
 }

The !noViews is redundant and it is usually confusing if you start an if-else expression with a negation. Besides that, try avoiding else if in favor of nested expressions. You should also try to make the boolean expressions functionally independent - if you have checked noViews somewhere, don't check it again.

I am not sure if the truth table for the two blocks are identical. I'll leave that up to you to verify.


Retrieving all the email addresses on a web page I recently discovered the file_get_contents function and wanted to put it to some use, alongside preg_match_all, off the information in this Stack Overflow thread.

<?php
    //use a site with lots of email address on.
    $string = file_get_contents("https://stackoverflow.com/questions/3901070/in-php-how-do-i-extract-multiple-e-mail-addresses-from-a-block-of-text-and-put");

    //initialise an empty array.
    $matches = array();

    //regular expression that matches most email addresses, courtesy of @Eric-Karl.
    $pattern = '/[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,4}/';

    //perform global regular expression match, ie search the entire web page for a particular thing, and store it in the previously initialised array.
    preg_match_all($pattern, $string, $matches);

    //output array of values; remove duplicate email addresses, but maintain incremental key count.
    var_dump(array_values(array_unique($matches[0])));

    //store above in array for upcoming bit.
    $neaterArray = array_values(array_unique($matches[0]));

    //count the amount of variables in the array.
    $count = count($neaterArray);

    //implode array values to string.
    $emailsAsString = implode(", ", $neaterArray);

    //present the string, prepended with the count.
    echo "<h3>$count email addresses in total:</h3> $emailsAsString";

?>

I trust my comments are enough to justify the choices I made. I used the stackoverflow thread I mentioned as a test case, because it had many test/fake emails, but all in an appropriate format for the regex.

Here's the output for this case:

array (size=19)
  0 => string 'apple-touch-icon@2.png' (length=22)
  1 => string 'example@slu.edu' (length=15)
  2 => string 'a+b@google.com.sg' (length=17)
  3 => string 'test1+2@gmail.com' (length=17)
  4 => string 'test-2@yahoo.co.jp' (length=18)
  5 => string 'test@test.com' (length=13)
  6 => string 'test@test.co.uk' (length=15)
  7 => string 'test@google.com.sg' (length=18)
  8 => string 'email@domain.info' (length=17)
  9 => string 'email@domain.inf' (length=16)
  10 => string 'first.lastname@domain.be' (length=24)
  11 => string 'lastname@domain.be' (length=18)
  12 => string 'HIDDENFORLOGICALREASONS@cameranh.rs.gov.br' (length=42)
  13 => string 'HIDDENFORLOGICALREASONS@cameranh.rs.go' (length=38)
  14 => string 'myemail@office21.company.com' (length=28)
  15 => string 'mymail@yahoo.com' (length=16)
  16 => string 'my-e.mail@yahoo.com' (length=19)
  17 => string 'joe@mysite.com' (length=14)
  18 => string 'name@example.com.sv' (length=19)
19 email addresses in total:

apple-touch-icon@2.png, example@slu.edu, a+b@google.com.sg, test1+2@gmail.com, test-2@yahoo.co.jp, test@test.com, test@test.co.uk, test@google.com.sg, email@domain.info, email@domain.inf, first.lastname@domain.be, lastname@domain.be, HIDDENFORLOGICALREASONS@cameranh.rs.gov.br, HIDDENFORLOGICALREASONS@cameranh.rs.go, myemail@office21.company.com, mymail@yahoo.com, my-e.mail@yahoo.com, joe@mysite.com, name@example.com.sv

I realise the string at the end isn't particularly necessary, I just wanted to make use of implode.

Some questions:

Is this a good way to go about solving the problem? Are there parts that could be optimised with more suitable functions?
Is initialising the array imperative for this program? I tried it without the original declaration (in the preg_match_all line) and it still functioned.
Is there a part/are there parts of the code that comes across as poorly written?

First, your code isn't exactly substantial, so it's hard to write a good review.

Up first, file_get_contents() works, but curl is faster.

It's a little longer, but try this:

function file_get_contents_curl($url) {
    $ch = curl_init();

    curl_setopt($ch, CURLOPT_AUTOREFERER, TRUE);
    curl_setopt($ch, CURLOPT_HEADER, 0);
    curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1);
    curl_setopt($ch, CURLOPT_URL, $url);
    curl_setopt($ch, CURLOPT_FOLLOWLOCATION, TRUE);

    $data = curl_exec($ch);
    curl_close($ch);

    return $data;
}

Moving on to your regex:

/[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,4}/

Instead of A-Za-z, you can end with a /i, and make your search case insensitive:

/[a-z0-9._%+-]+@[a-z0-9.-]+\.[a-z]{2,4}/i

Depending on how complex you'll allow emails to be, you can add (or remove), some valid characters.

Next, you perform a array_values(array_unique($matches[0])) twice.

var_dump(array_values(array_unique($matches[0])));

$neaterArray = array_values(array_unique($matches[0]));

Into:

$neaterArray = (array_values(array_unique($matches[0])));
var_dump($neaterArray);

Your variable naming follows a camelCase standard all-throughout, so that's alright.

Your comments are generally good, but //store above in array for upcoming bit is incorrect as I pointed out above.

  Is this a good way to go about solving the problem? Are there parts that could be optimised with more suitable functions?

Swapping file_get_contents() with curl will improve the speed of page download.

  Is initialising the array imperative for this program? I tried it without the original declaration (in the preg_match_all line) and it still functioned.

You don't need to initialise the variable first, you really only would if it where in a for loop, in this case you can just output to a new variable.

  Is there a part/are there parts of the code that comes across as poorly written?

Not really, variable naming could be expressed better by replacing $string with $input and $neaterArray with $duplicateRemovedArray, but other than that, the code is good.


Make LinkedList in C++ more clean I have been working on a linked list for some while now and started out with this.

Is my code clean? Or is there some way to make it better and more readable?

The reason for this question is for me to be a better programmer. I would love to get one step further for making more clean code.

my class:

using namespace std;

class Node
{

public:
    int nX;
    Node *next;
    //constructor
    Node(){ head = NULL; }          //Initialising head to NULL
    void addValueLeft(int nVal);
    void addValueRight(int nVal);
    void deleteValueLeft();
    void deleteValueRight();
    void printList();
private:
    Node *head;   //a pointer to the first Node
};

my main:

int _tmain(int argc, _TCHAR* argv[])
{
    using namespace std;
    //creating a new object
    Node list;

    list.addValueLeft(5);
    list.addValueLeft(6);
    list.addVaueRight(8);
    list.deleteValueRight();
    list.printList();

    return 0;
}

my functions:

    void Node::addValueLeft(int nVal)
    {
        Node *newNode = new Node;
        if(!head)
        {
            newNode->nX = nVal;
            newNode->next = NULL;
            head = newNode;
        }
        else
        {
            newNode->nX = nVal;
            newNode->next = head;
            head = newNode;
        }
    }

    void Node::printList()
    {
        if(!head)       //list is empty - nothing to print
        {
            //cout << "nothing to print - list empty " << endl;
        }
        else
        {
            Node *tempNode = new Node;
            tempNode = head;
            while(tempNode)
            {
                cout << tempNode-> nX << endl;
                tempNode = tempNode->next;
            }
        }
    }

    void Node::addValueRight(int nVal)
    {
        Node *tempNode1 = new Node;

        if(!head)                   //List is empty
        {
            tempNode1 -> nX   = nVal;
            tempNode1 -> next = head;
            head = tempNode1;
        }
        else
        {
            tempNode1 = head;
            while(tempNode1 -> next != NULL)        //Go to last Node
            {
                tempNode1 = tempNode1->next;

            }
            Node *tempNode2 = new Node;         //create temp node
            tempNode2 -> nX = nVal;
            tempNode2 -> next = NULL;
            tempNode1 -> next = tempNode2;   //tempNode1 will be last Node
    }
    }

    void Node::deleteValueLeft()
    {
        if(!head)       //list is empty - Nothing to Delete
        {
            //cout << " deleteValueLeft:  Nothing to delete" << endl;
        }
        else
        {
            head = head->next;
        }
    }

    void Node::deleteValueRight()
    {
        if(head == NULL)    //Nothing to delete
        {
            //cout << "Nothing to delete " << endl;
        }
        else
        {
            Node *tempNode1 = new Node;
            Node *oldNode = new Node;
            tempNode1 = head;
            if(tempNode1->next == NULL) //only one element
            {
                head = NULL;
            }
            else
            {
                while(tempNode1->next!=NULL)        //Go to last Node
                {
                    oldNode = tempNode1;
                    tempNode1 = tempNode1->next;

                }
                oldNode->next = NULL;
            }
        }

    }

Here's some feedback (not an exhaustive list).

using namespace std;

Please stop doing this. when you add using namespace directives in header files, any file including that header will have potential namespace clashes (which is the problem namespaces are trying to prevent).

class Node
{
public:
    int nX; // (1)
    Node *next; // (2)
    Node(){ head = NULL; }  // (3)

Notes:

The only ways lines (1) and (2) are acceptable, are:

if instances of your class are valid regardless what you place in nX and next.

Client code example:

Node n;
n.nX = 1001; // is this valid?
n.next = n; // is this valid?

if Node class is moved from the global namespace to the private section of another class. That way, the outer class will protect the node's data.

Otherwise, nX and next should be private data, with getters to read the values and setters to validate and set the values.

Line (3) is a bad implementation for a constructor, because it leaves a new instance in a partially-initialized state (it should initialize nX, next and head, explicitly and in this specific order).

Better implementation:

explicit Node(int value = 0): nX(value), next(nullptr), head(nullptr) {}

Note regarding class API design:

You are implementing both a node (for an element of the list) and a list of nodes. You should have two classes instead of one, and the head pointer should be a member of the list, not a node:

class Node {
    int nX; Node* next;
public:
    // ... rest of node interface here
};

class List {
    Node *head;
public:
    List() {}
    const std::size_t size() const;
    void add_value_end(int value);
    // ... rest of interface here
};

Eventually, you should consider using node as an internal structure of the list:

class List {
    class Node { /* same as above */ };
    Node *head;
public:
    // same as above
};


Sudoku Checker in Java public class SudokuChecker{

static int[][] sMatrix={

                    {5,3,4,6,7,8,9,1,2},
                    {6,7,2,1,9,5,3,4,8},
                    {1,9,8,3,4,2,5,6,7},
                    {8,5,9,7,6,1,4,2,3},
                    {4,2,6,8,5,3,7,9,1},
                    {7,1,3,9,2,4,8,5,6},
                    {9,6,1,5,3,7,2,8,4},
                    {2,8,7,4,1,9,6,3,5},
                    {3,4,5,2,8,6,1,7,9}
                };
static int rSum=0;

static int cSum=0;

static int[] rSumArray=new int[9];

static int[] cSumArray=new int[9];

static int[] boxSumArray=new int[9];

static boolean checkArrayStatus(int[] rSumArray,int[] cSumArray,int[] boxSumArray)
{
    int i=0;

    boolean sudukoStatus=true;

    while(i<9){
        if(rSumArray[i]!=45&&cSumArray[i]!=45&&rSumArray[i]!=45)
        {
            sudukoStatus=false;
            break;
        }
        i++;
    }
    return sudukoStatus;
}

    public static void main(String[] args) {
        for(int i=0 ; i<sMatrix.length ; i++){
            for(int j=0 ; j<sMatrix.length ; j++){
                rSum+=sMatrix[i][j];
                cSum+=sMatrix[j][i];
                }
            rSumArray[i]=rSum;
            cSumArray[i]=cSum;
            rSum=0;
            cSum=0;
        }

        for(int i=0 ; i< sMatrix.length ; i++){
            for(int j=0 ; j<sMatrix.length ; j++){
                if(i<=2&&j<=2)
                {
                    boxSumArray[0]+=sMatrix[i][j];
                }
                if(i<=2&&(j>=3&&j<=5))
                {
                    boxSumArray[1]+=sMatrix[i][j];
                }
                if(i<=2&&(j>=6&&j<=8))
                {
                    boxSumArray[2]+=sMatrix[i][j];
                }
                if((i>=3&&i<=5)&&(j<=2))
                {
                    boxSumArray[3]+=sMatrix[i][j];
                }
                if((i>=3&&i<=5)&&(j>=3&&j<=5))
                {
                    boxSumArray[4]+=sMatrix[i][j];
                }
                if((i>=3&&i<=5)&&(j>=6&&j<=8))
                {
                    boxSumArray[5]+=sMatrix[i][j];

                }
                if((i>=6)&&(j<=2))
                {
                    boxSumArray[6]+=sMatrix[i][j];
                }
                if((i>=6)&&(j>=3&&j<=5))
                {
                    boxSumArray[7]+=sMatrix[i][j];
                }
                if((i>=6)&&(j>=6))
                {
                    boxSumArray[8]+=sMatrix[i][j];
                }
            }
        }

        if(checkArrayStatus(rSumArray,cSumArray,boxSumArray))
        {
            System.out.println("The matrix is sudoku compliant");
        }
        else
        {
            System.out.println("The matrix is not sudoku compliant");
        }
    }
}

This a program which verifies if a 2D matrix is a Sudoku or not. The sum of every row, column and 3x3 matrices have to be 45.  Please review the code and provide feedback on best practices and code optimization.

You have waaaay too many sums you are checking. KISS-Principle --> Keep it Simple {and} Stupid:

private boolean checkSudokuStatus(int[][] grid) {
    for (int i = 0; i < 9; i++) {

        int[] row = new int[9];
        int[] square = new int[9];
        int[] column = grid[i].clone();

        for (int j = 0; j < 9; j ++) {
            row[j] = grid[j][i];
            square[j] = grid[(i / 3) * 3 + j / 3][i * 3 % 9 + j % 3];
        }
        if (!(validate(column) && validate(row) && validate(square)))
            return false;
    }
    return true;
}

private boolean validate(int[] check) {
    int i = 0;
    Arrays.sort(check);
    for (int number : check) {
        if (number != ++i)
            return false;
    }
    return true;
}

Short Explanation:

A Sudoku has three elements to validate. Each has to contain the single digit numbers from 1 to 9 exactly once.
These three elements are: each single row, each single column, the 9 3x3 sqares.
These elements are exactly mapped by the three variables you see in the for-loop.
Each of these elements is given to a validate function to verify, that it contains the numbers only once. Originally this was a golfed solution, so the square calculation is heavily algorithmic.
The validate function is quite simple. It initializes a counter to 0, and then walks the sorted "validated element" through, while always incrementing by 1. this gives the numbers from 1 to 9. In fact it is just a shortened for-loop, written a little different it's:

this one

for(int i = 1; i <= 9; i++){
    if(check[i-1] != i){
       return false;
    }
}
return true;

Longer Explanation

How I got to the algorithmic generation for the squares:

[(i / 3) * 3 + j / 3]      [i * 3 % 9 + j % 3]

(i / 3) * 3 + j / 3

This part is relatively simple. It gives the correct "column" by help of implicit integer conversion: The first part of the equation is a little difficult:

  \$ (i / 3) \$
  \$ {0,1,2} \mapsto 0 \$
  \$ {3,4,5} \mapsto 1 \$
  \$ {6,7,8} \mapsto 2 \$

take it * 3 and you got the sqare you want to get in to. then if we now iterate row-wise, we need to jump to the next column every 3 Elements. That's done with j / 3.

Now for the difficult one:

 i * 3 % 9 + j % 3

  \$ (i * 3 \% 9) \$
  \$ {0,3,6} \mapsto 0 \$
  \$ {1,4,7} \mapsto 3 \$
  \$ {2,5,8} \mapsto 6 \$

This one is for "jumping" the sqares vertically. Then we just have to iterate 0, 1, 2. This is accomplished by running the % 3 operation.


Unefficent data wrangling in R so I've been working with R quite a while now, but due to very stressful deadlines in my last university module I had, I wrote code that I'm not very proud of. I would like to be more efficient in data wrangling, e.g. reading files, rearranging and cropping dfs. Here's a short excerpt of my code
 # Reading files
 org_bm <- read.delim("DE_gebesee_metrx_org_physiology-daily.txt")
 t_bm <- read.delim("DE_gebesee_metrx_t_physiology-daily.txt")
 co2_bm <- read.delim("DE_gebesee_metrx_co2_physiology-daily.txt")

# Format Dates
org_bm$datetime <- as_datetime(org_bm$datetime) + years(98)
t_bm$datetime <- as_datetime(t_bm$datetime) + years(98)
co2_bm$datetime <- as_datetime(co2_bm$datetime) + years(98)

# Select cols
org_ghg <- select(org_ghg, c(datetime,
                         aN_n2o_emis.kgNha.1.,
                         aC_ch4_emis.kgCha.1.,
                         aC_co2_emis_hetero.kgCha.1.,
                         aC_co2_emis_auto.kgCha.1.))
t_ghg <- select(t_ghg, c(datetime,
                     aN_n2o_emis.kgNha.1.,
                     aC_ch4_emis.kgCha.1.,
                     aC_co2_emis_hetero.kgCha.1.,
                     aC_co2_emis_auto.kgCha.1.))
co2_ghg <- select(co2_ghg, c(datetime,
                         aN_n2o_emis.kgNha.1.,
                         aC_ch4_emis.kgCha.1.,
                         aC_co2_emis_hetero.kgCha.1.,
                         aC_co2_emis_auto.kgCha.1.))

As you can see I mastered c/p and "search and replace" XD
But seriously, I'm wondering how I could "automate" these tasks. Maybe loading everything into a list and than write helper functions to apply certain actions (i.e. date formatting, selecting cols) to every list entry ?
I've always struggled to find good practice tasks, b/c during class I usually don't have the time to read up on nicer ways to make things work. So if anybody has some suggestions on where to find exercises on data analysis tasks, I would very much appreciate that.
Looking forward to read your replies !
Leon

You can do more conversion as you read the data. The magic is done by the colClasses parameter to read.delim(). That parameter allows you to (1) convert the DateTime whilst reading and (2) drop unwanted columns.
Now I don't know what your data looks like, but here's a rough outline of how to read a datetime (untested). Note that you can do the + years(98) within the data-parsing class as well.
library(methods)
setClass("myDateTime")
setAs("character",
      "myDateTime",
      function(from) as.DateTime(from, format="%Y-%m-%d %H:%M:%S", tz=Sys.timezone())) + years(98)

d = read.delim(..., colClasses = c(..., "myDateTime", ...))

For the columns you want to drop, code them as "NULL" (note the quotes).
d = read.delim(..., colClasses = c("numeric", "myDateTime", "NULL", ...))

This not only shortens your code, but by explicitly telling read.delim() the datatype of each column the read is 2x faster. And of course there aren't two copies of the data in memory after the parsing.
You might also want to look at the skip= and col.names=c() parameters as they allow you to decide the names of the variables, rather than having those decided by the input file. If you decide to use colCLasses=c() you'll almost certainly want to skip the column headings rather than program a class which reads strings for row 1 and a different datatype for the remaining rows.
BTW, if you have to do complex manipulation of input data, then the tidyverse's readr and dplyr libraries have useful improvement over the shipped R libraries. Regardless of your use or not of Tidyverse, its book is worth reading if you are an R programmer: https://r4ds.had.co.nz/


Better way to query and pass the elements to conditional operator in Javascript I have the following function which has the required functionality. Is there any way that I can reduce the code but have the same functionality?

addClickEventListeners = () => {
  let multipleChoiceQuestionButtons = document.querySelectorAll('[name="multiple_choice"]');
  let trueFalseQuestionButtons = document.querySelectorAll('[name="true_false"]');
  let multipleChoiceMultiAnswersQuestionButtons = document.querySelectorAll('[name="multi_answers"]');
  if (multipleChoiceQuestionButtons.length > 0) {
    multipleChoiceQuestionButtons.forEach((button) => {
      button.addEventListener('click', someFunction, false);
    });
  } else if (trueFalseQuestionButtons.length > 0) {
    trueFalseQuestionButtons.forEach((button) => {
      button.addEventListener('click', someFunction, false);
    });
  } else if (multipleChoiceMultiAnswersQuestionButtons.length > 0) {
    multipleChoiceMultiAnswersQuestionButtons.forEach((button) => {
      button.addEventListener('click', someFunction, false);
    });
  }
};

Interesting question;

You can make this code slightly less efficient (however, the user will never notice), and so we can just attach listeners to everything.

const addClickEventListeners = () => {

  const addListener = (button) => button.addEventListener('click', someFunction, false);

  document.querySelectorAll('[name="multiple_choice"]').forEach(addListener);
  document.querySelectorAll('[name="true_false"]').forEach(addListener);
  document.querySelectorAll('[name="multi_answers"]').forEach(addListener);

};

This should behave the same way as far as I can tell.

You could also create your buttons that require someFunction with a css class dedicated for that selection or even better with a data attribute for that purpose. Then you just query on that and attach the listener.


Recursive Memoized Fibonacci in C++ I was reading an article on Wikipedia about memoization, so I wanted to try and apply it. I am fairly new to this subject, so I thought I'd start small. Does the program use memoization correctly? Is there anything that I can improve?

#include <iostream>

int fibonacci(int num, int* memoize);

int main()
{
    int num;

    std::cout << "i: ";
    std:: cin >> num;

    // i will assume that num > 2, just to keep it short and simple
    int* memoize = new int[num + 1];
    memoize[0] = 0;
    memoize[1] = 1;

    // -1 indicates that the value is not within the array
    for (int i = 2; i <= num; i++)
        memoize[i] = -1;

    std::cout << num << "th fibonacci number: " << fibonacci(num, memoize) << std::endl;

    delete [] memoize;
    return 0;
}

int fibonacci(int num, int* memoize)
{
    // if value has not been calculated yet
    if (memoize[num] == -1)
    {
        // calculate it and store it in the array
        memoize[num] = fibonacci(num - 1, memoize) + fibonacci(num - 2, memoize);
    }

    return memoize[num];
}

I read that the recursive solution to fibonacci is not preferred, but I am still trying to figure out the iterative solution.

Not much to say: your code works as advertised, code is readable, formatting is consistent.

Things that could be better though: don't use raw arrays with new unless you absolutely have to. Prefer std::vector, it has a nice interface and manages its own memory. It also has a constructor that lets you fill it with a given value.

std::vector<int> cache(num+1, -1);
cache[0] = 0;
cache[1] = 1;

Then pass that as a std::vector<int>& to your Fibonacci function. No more open-coded loop, no more new[]/delete[], so less chances of bugs and leaks.

Another technique that would avoid that vector at all would be to have the Fibonacci function do its own memoizing, without external state. That's easier on the user, and you can do a more general technique with a static std::map in the function being memoized to keep the state. (This is not thread-safe as is, but a shared vector or raw array isn't either.)

Here's an example:

#include <iostream>
#include <map>

int fibonacci(int num)
{
  static std::map<int, int> cache{{0, 0}, {1, 1}};

  auto found = cache.find(num);
  if (found != std::end(cache)) {
    // For debugging purposes, to check that the cache is doing something
    std::cout << "Found in cache: " << num << " -> " << found->second << '\n';
    return found->second;
  }

  int result = fibonacci(num - 1) + fibonacci(num - 2);
  cache[num] = result;
  return result;
}

And the corresponding main: I've moved the call to fibonacci out of the printout statement, don't hide important function calls in there.

int main()
{
  int num;
  std::cout << "i: ";
  // TODO: input validation
  std::cin >> num;

  int res = fibonacci(num);
  std::cout << num << "th fibonacci number: " << res << std::endl;
}

Of course this is a bit overkill for the situation at hand. There are more general techniques, you'll find some for example here: Writing Universal memoization function in C++11.

The iterative approach is much more efficient and pretty simple. In pseudo code:

fib(n):
  prev = 0
  curr = 1
  i = 2
  while i <= n
    next = prev + curr
    prev = curr
    curr = next
    i++
  return curr


Array find min value with user-defined comparison compare function My user defined comparison compare function is defined with an array:

$boardkey_to_values=Array(19=>2601248,23=>2601248,39=>2603445,
    43=>2256319,47=>2632006,59=>2603445,63=>2232152,67=>2260713,71=>2632006,...)

so 23>19 EDIT:23=19, 39>19, 39<23,...

I have an array of values $board_id, and I want to get the array of all the keys for the minimum values.

Here's my solution, but I don't think it is really intuitive.

$min_key=Array(0);
foreach ($board_id as $key => $value) {
if ($boardkey_to_values[$board_id[$min_key[0]]]>$boardkey_to_values[$value])
    $min_key=Array($key);
if ($boardkey_to_values[$board_id[$min_key[0]]]==$boardkey_to_values[$value] && $min_key[0]!=$key)
    $min_key[]=$key;
}

Here is an example to show how this should behave:

say $board_id = [19,23,39,47,71]

it will look at the values corresponding to this numbers [2601248,2601248,2603445,2632006,2632006]

it looks for the smallest value: 2601248

so the result ($min_key should be [19,23])

This should be O(n)* where n is count($board_id).

$mapped = array_map(function ($id) use ($boardkey_to_values) {
    return array($boardkey_to_values[$id], $id);
}, $board_id);
asort($mapped);
list($min_value, $id) = reset($mapped);
$min_ids = array($id);
while (list($value, $id) = next($mapped)) {
    if ($value == $min_value) {
        $min_ids[] = $id;
    }
    else {
        break;
    }
}

* ignoring the call to asort which is probably O(n log n).


Python name generator This is a basic name generator I just wrote and want some feedback.
It basically chooses between a vowel or a consonant and appends it to a string. Note that it never puts two vowels or two consonants directly together, as it checks for that.

import random

vowels = ['a', 'e', 'i', 'o', 'u']
consonants = ['b', 'c', 'd', 'f', 'g', 'h', 'j', 'k', 'l', 'm', 'n', 'p', 'q', 'r', 's', 't', 'v', 'x', 'y', 'z']

def generate_name(length):
    if length <= 0:
        return False

    full_syl = ''
    for i in range(length):
        decision = random.choice(('consonant', 'vowel'))

        if full_syl[-1:].lower() in vowels:
            decision = 'consonant'
        if full_syl[-1:].lower() in consonants:
            decision = 'vowel'

        if decision == 'consonant':
            syl_choice = random.choice(consonants)
        else:
            syl_choice = random.choice(vowels)

        if full_syl == '':
            full_syl += syl_choice.upper()
        else:
            full_syl += syl_choice

    return full_syl

for i in range(100):
    length = random.randint(3, 10)
    print(generate_name(length))

Why does generate_name(0) return False? I would expect an empty string '', or maybe None.

Rather than check if full_syl == '': (not a great name, by the way; why not just name?) during the loop, I would build the whole name in lowercase then  use str.title: return full_syl.title(). This removes both .upper and .lower from the forloop, simplifying the code.

Why pick every letter randomly? In this part:

decision = random.choice(('consonant', 'vowel'))

if full_syl[-1:].lower() in vowels:
    decision = 'consonant'
if full_syl[-1:].lower() in consonants:
    decision = 'vowel'

the first value of decision is ignored for every letter but the first. If you want strict alternation, you only need to randomly pick the type of the first letter. You could even use itertools.cycle to switch back and forth between vowels and consonants for each pick.

Rather than keep adding letters to a string, I would probably build a list or iterator of letters then str.join them. Otherwise you create many unnecessary string objects for the partial names, which are immediately discarded.


First C++ calculator This is my first C++ program. It would be nice if you can tell me anything I could add or do differently.
I am new to C++ programming and I hope to learn more.
#include <iostream>
using namespace std;

int main()
{
    float a, b, result;
    char op;

    cout << "Enter an operator." << endl;

    cin >> op;

    cout << "Enter first value." << endl;

    cin >> a;

    cout << "Enter second value." << endl;

    cin >> b;

    cout << "----------------------" << endl;

    switch (op)
    {
    case '+':
        result = a + b;
        cout << result;
        break;

    case '-':
        result = a - b;
        cout << result;
        break;

    case '*':
        result = a * b;
        cout << result;
        break;

    case '/':
        result = a / b;
        cout << result;
        break;

    default:
        cout << "Invalid operation. Program terminated." << endl;
        return -1;
    }
}

there is a lot of thing you can do to further add depth:

move the printing out of the switch
display an help with current operators
implements others operator (pow(), sin(), square(), root()...), you'll then have to check the number of arguments
use a std::map<string, *functionPointer> to avoid the switch
implement a parsing function so you can just enter something like "3+2*8", this would require care as you have to check operator precedence, using prefix or postfix representation could help and allow ignoring ")(".


Optimize Conway's Game of Life I have coded up an implementation of Conway's Game of Life and I have a performance bottleneck in it which I wish to be optimized. The main logic is in the Universe class. I have omitted all code which is not applicable here for brewity:

public class Universe {

    private static final int FLIP_INDEX = 0;
    private static final int FLOP_INDEX = 1;
    private final boolean[][][] universeDoubleBuffer;
    private final int height;
    private final int width;
    private int flipFlopIndex = FLIP_INDEX;

    public Universe(boolean[][] universeState) {
        height = universeState.length;
        width = universeState[0].length;
        universeDoubleBuffer = new boolean[2][height][width];
        for (int y = 0; y < height; y++) {
            for (int x = 0; x < width; x++) {
                universeDoubleBuffer[FLIP_INDEX][y][x] = universeState[y][x];
            }
        }
    }

    public boolean[][] recalculateUniverseState() {
    int newFlipFlopIndex = (flipFlopIndex == FLIP_INDEX ? FLOP_INDEX : FLIP_INDEX);
    for (int y = 0; y < height; y++) {
        for (int x = 0; x < width; x++) {
            int liveNeighbors = countLiveNeighbors(x, y);
            boolean isLiving = universeDoubleBuffer[flipFlopIndex][y][x];
            if (!isLiving && liveNeighbors == 3) {
                universeDoubleBuffer[newFlipFlopIndex][y][x] = true;
            } else if (isLiving && (liveNeighbors == 2 || liveNeighbors == 3)) {
                universeDoubleBuffer[newFlipFlopIndex][y][x] = true;
            } else {
                universeDoubleBuffer[newFlipFlopIndex][y][x] = false;
            }
        }
    }
    stampPatterns();
    logger.info("Old:" + Arrays.deepToString(universeDoubleBuffer[flipFlopIndex]));
    logger.info("New:" + Arrays.deepToString(universeDoubleBuffer[newFlipFlopIndex]));
    flipFlopIndex = newFlipFlopIndex;
    return universeDoubleBuffer[flipFlopIndex];
}

    private int countLiveNeighbors(int x, int y) {
        int result = 0;
        for (CellNeighbor neighbor : CellNeighbor.values()) {
            try {
                boolean isLiving = universeDoubleBuffer[flipFlopIndex][y + neighbor.getYOffset()][x + neighbor.getXOffset()];
                if (isLiving) {
                    result++;
                }
            } catch (IndexOutOfBoundsException e) {
                logger.info("Cell's neighbor was off the grid.", e);
            }
        }
        return result;
    }
}

Here is the code for CellNeighbor:

public enum CellNeighbor {

    TOP_LEFT(-1, 1), TOP(0, 1), TOP_RIGHT(1, 1), RIGHT(1, 0), BOTTOM_RIGHT(1, -1), BOTTOM(0, -1), BOTTOM_LEFT(-1, -1), LEFT(-1, 0);

    private int xOffset;
    private int yOffset;

    private CellNeighbor(int xOffset, int yOffset) {
        this.xOffset = xOffset;
        this.yOffset = yOffset;
    }

    public int getXOffset() {
        return xOffset;
    }

    public int getYOffset() {
        return yOffset;
    }
}

My problem is that if I create a world big enough (I tested with 5000 * 5000 for example) life really slows down and the time spent in recalculateUniverseState() reaches almost a second (with an 5000 * 5000 universe the average was 766ms).

I have tried without a double buffer (swapping new boolean[][] arrays) and without the try/catch block but it yielded no significant performance improvements.

My question is that how can I optimize the above code to be faster?

First of all, this should be obvious, but when you have performance issues in code that involves a tight inner loop, you want to simplify that loop as much as you can.  If you can save one cycle in the loop by spending a dozen or a hundred cycles somewhere else, do it, because every cycle in the loop gets multiplied by 5000².

Also, what you really want to minimize in the inner loop are memory accesses.  Accessing local variables is fast, since they're typically cached and the JVM knows that their values cannot change unless the code in the method itself changes them.  In comparison, pulling a random element from a large array is relatively slow, since it requires a full RAM access, as is, typically, accessing a member variable or calling an uncached method.

I've done something like this before,* so, rather than listing every improvement I might make to your code, let me start by sketching how I might rewrite your core update loop:

// assume these arrays are (height + 2) by (width + 2)
boolean[][] oldBuffer = universeDoubleBuffer[flipFlopIndex],
            newBuffer = universeDoubleBuffer[newFlipFlopIndex];

for (int y = 1; y <= height; y++) {
    int environment
        = (oldBuffer[y-1][0] ? 32 : 0) + (oldBuffer[y-1][1] ?  4 : 0)
        + (oldBuffer[y  ][0] ? 16 : 0) + (oldBuffer[y  ][1] ?  2 : 0)
        + (oldBuffer[y+1][0] ?  8 : 0) + (oldBuffer[y+1][1] ?  1 : 0);

    for (int x = 1; x <= width; x++) {
        environment = ((environment % 64) * 8)
            + (oldBuffer[y-1][x+1] ? 4 : 0)
            + (oldBuffer[y  ][x+1] ? 2 : 0)
            + (oldBuffer[y+1][x+1] ? 1 : 0);

        newBuffer[y][x] = lookupTable[ environment ];
    }
}

A few things to note about this code:

There is no countLiveNeightbors() method; in fact, the code does not explicitly count live neighbors at all.  Instead, the pattern of the nine cells including and surrounding the cell at (x, y) is maintained in the variable environment, which is used as an index to a 512-element lookup table.

For example, if the surroundings of the current cell look like this (# = live cell, _ = dead cell):

# # _
_ # #
# _ _

then the value of the environment variable will be:

(1 * 256) + (1 * 32) + (0 * 4) +
(0 * 128) + (1 * 16) + (1 * 2) +
(1 *  64) + (0 *  8) + (0 * 1) = 370

Of course, you'll have to set up this lookup table before using it, but that is something you can do outside the core loop (e.g. in your constructor), so it's not performance-critical.

(In fact, while I haven't benchmarked this, I'd at least consider making lookupTable a local variable and rebuilding it every time the update method is called, as the extra cost of rebuilding the table might be balanced out by extra optimization opportunities available to the compiler / JVM if it knows that no other code can modify the table.  You may want to test it both ways and see which one is faster.)

One additional advantage of such a lookup table based implementation is that, by changing the lookup table, it can simulate any two-state cellular automaton using the Moore neighborhood, not just Conway's Game of Life specifically.
By reusing the parts of the environment pattern that are shared between adjacent cells, the code only needs to do three reads from the oldBuffer array per cell, as opposed to nine in your version.  Uncached array access is expensive, so this is likely to provide a significant speedup.  (Also, like your code, mine also makes sure to access the buffers as close to sequentially as possible, iterating first by rows and then by columns.  This is also important for CPU cache locality.)
The code above doesn't update the cells at the edges of the array, which means it doesn't have to worry about (literal) edge cases such as array indices being out of bounds.  (Hopefully, the compiler / JVM may notice this too, and may omit some of its internal array bounds checks.)

If you want your grid to be surrounded by dead cells, you can just initially mark these border cells as dead and leave them untouched in your update method.  Alternatively, if, say, you want the grid to wrap around, you can update the edge cells in a separate loop (which can be less efficient, since it only runs for a tiny fraction of all the cells).

Actually, there are a few ways in which the code I suggested above could be optimized further.  For example, an obvious optimization is to get rid of the two-dimensional array accesses in the inner loop, since they require two array lookups each.

There are (at least) two ways you could do this:

a) In the outer loop, save the previous, current, and next rows of oldBuffer in local variables, like this:

boolean[] prevRow = oldBuffer[y-1],
          currRow = oldBuffer[y],
          nextRow = oldBuffer[y+1];

b) Make the buffers themselves one-dimensional arrays, and adjust the indexing so that, instead of buffer[y][x], you use buffer[ y * (width+2) + x ].  You can precalculate the offset y * (width+2), and possibly also (y ± 1) * (width+2), in the outer loop to save a bit of arithmetic, or you could rely on the compiler / JVM to do this for you.  Again, you could try it both ways and see if there's any difference.

Also, for cellular automata like the Game of Life, where only a small fraction of the cells typically change in each time step, there are much faster algorithms available than even the generic table lookup method described above.

As a first step, ChrisW's suggestion of caching the live neighbor count is likely to be faster whenever the environment of each cell changes relatively infrequently.
An even greater speedup may be obtained by storing a list of active cells, i.e. cells that changed or had at least one of their neighbors change state during the previous update step, and only iterating through those cells on the next update.  (Since the total number of active cells is bounded above by the size of the grid, you can use a simple array as a circular buffer to efficiently store this list.)

Effectively combining active cell lists with double buffering can be somewhat tricky.  An alternative solution is to use a single buffer, but divide your update method into two phases:

In the first phase, you iterate through the list, calculate the new state for each active cell, and store it in the list.
In the second phase, you iterate through the list again and update the grid buffer to match the new states calculated in the first phase.

(That said, using both activity lists and double buffering does have one advantage: it allows you to treat cells that oscillate with a period of two, which are quite common in the Game of Life, as inactive.  This does require you to maintain separate active cell lists for each buffer.)
Finally, if you want a really fast algorithm for simulating Conway's Game of Life, look up Hashlife.  It's literally orders of magnitude faster than any "naïve" simulation algorithm, especially for sparse and highly repetitive patterns like many constructed ones.

*) Please don't take that code as an example of good coding style.  It is pretty fast, though.

Addendum: Here's a basic implementation of the activity list method, using a single buffer, and with the Conway's Game of Life rules hardcoded.

It uses a byte-packed cell state buffer, where the lowest bit of the byte indicates whether the cell is currently in the activity list, the second bit stores the actual state of the cell, and the following (four) bits store the number of surrounding live cells (to avoid having to recalculate it whenever the cell is examined):

// assume that buffer is a (height) by (width) byte array, and that
// xQueue and yQueue are (height * width) int arrays, of which the
// first (activeCells) elements contain the current active cell coords

// loop over active cells, and find those whose state will change
int changedCells = 0;
for (int i = 0; i < activeCells; i++) {
    int x = xQueue[i], y = yQueue[i];
    byte packedState = buffer[y][x];  // = 4*neighbors + 2*state + active
    boolean wasAlive = (oldState & 2 == 2);

    // quickly check if cell will live under the Game of Life rules
    // (10-11 = alive, 2 neighbors; 12-13 = dead, 3 neighbors; 14-15 = alive, 3 neighbors)
    boolean isAlive = (10 <= packedState && packedState <= 15);

    // add cell to queue if it died or was born
    // we know that changedCells <= i, so it's safe to reuse the same queue space
    if (wasAlive != isAlive) {
        xQueue[changedCells] = x;
        yQueue[changedCells] = y;
        changedCells++;
        // assume active bit is already 1, so no need to change
    }
    else {
        buffer[y][x] = packedState - (byte)1;  // unset active bit
    }
}

// carry out deferred state updates
activeCells = changedCells;
for (int i = 0; i < changedCells; i++) {
    int x = xQueue[i], y = yQueue[i];
    boolean wasAlive = (buffer[y][x] & 2 == 2);
    byte delta = (byte)(wasAlive ? -4 : +4);  // change in neighbor count * 4

    // update neighbor counts of adjacent cells, and mark them as active
    int yMin = (y > 0 ? y-1 : 0), yMax = (y < height-1 ? y+1 : height-1);
    int xMin = (x > 0 ? x-1 : 0), xMax = (x < width-1 ? x+1 : width-1);
    for (int ny = yMin; ny <= yMax; ny++) {
        for (int nx = xMin; nx <= xMax; nx++) {
            byte newState = buffer[ny][nx] + delta;
            // if this cell is not yet active, add it to the queue for next time step
            if (newState & 1 == 0) {
                xQueue[activeCells] = nx;
                yQueue[activeCells] = ny;
                activeCells++;
                newState += (byte)1;
            }
            buffer[ny][nx] = newState;
        }
    }
    // the middle cell only needs to adjusted by +/-2
    buffer[y][x] -= (byte)(delta / 2);
}

Before the first pass, all cells should be initialized to have the correct neighbor counts, have the active bit set and be added to the active cell queue.  I've omitted this part of the code, since it's not performance-critical.

Note that this method does not use any padding cells along the edges of the state array.  Alternatively, we could add the padding and permanently set the active bit for those cells, but not include them in the queue, allowing the min/max coords calculations to be simplified.  I suspect this would not make much difference, but there's no way to be sure without trying it.


Linux/Mac hashing SHA-1 To do password exchange on the SQL server I need to use SHA-1 to generate a hash (no plain password exchange).
This is done differently on Linux/Mac so I abstracted it slightly.
ThorCryptWrapper.h
#ifndef THORS_ANVIL_MYSQL_DETAILS_THOR_CRYPTO_WRAPPER_H
#define THORS_ANVIL_MYSQL_DETAILS_THOR_CRYPTO_WRAPPER_H

#ifdef  __APPLE__
#define COMMON_DIGEST_FOR_OPENSSL
#include <CommonCrypto/CommonDigest.h>
#define THOR_SHA1(data, len, dst)   CC_SHA1(data, len, dst)
#else
#include <openssl/sha.h>
#define THOR_SHA1(data, len, dst)   SHA1(data, len, dst)
#endif

namespace ThorsAnvil
{
    namespace MySQL
    {

typedef unsigned char           ThorSHADigestStore[SHA_DIGEST_LENGTH];

inline void thorSHA1(ThorSHADigestStore& dest, ThorSHADigestStore& src)
{
    THOR_SHA1(src, SHA_DIGEST_LENGTH, dest);
}

inline void thorSHA1(ThorSHADigestStore& dest, std::string const& src)
{
    THOR_SHA1(reinterpret_cast<const unsigned char*>(&src[0]), src.length(), dest);
}

    }
}

#endif

There's not a huge amount to review here.

It looks like the Apple code is intended to be a drop-in replacement for OpenSSL, so you could probably just rename to match, rather than creating a new name:

#ifdef  __APPLE__
#define COMMON_DIGEST_FOR_OPENSSL
#include <CommonCrypto/CommonDigest.h>
#define SHA1 CC_SHA1
#else
#include <openssl/sha.h>
#endif

Whichever approach you take, don't forget to #undef the macro when you've finished using it (definitely before the end of the header), to avoid polluting the macro namespace for others.

I would prefer src.data() to &src[0] as the idiomatic way to access a string's characters as an array of char.

And I think I prefer sizeof src to SHA_DIGEST_LENGTH in the first overload, so that the connection is clear.

Is there any reason that src can't be a reference to const in both versions?

inline void thorSHA1(ThorSHADigestStore& dest, ThorSHADigestStore const& src)
{
    THOR_SHA1(src, sizeof src, dest);
}

inline void thorSHA1(ThorSHADigestStore& dest, std::string const& src)
{
    auto const src_bytes = reinterpret_cast<const unsigned char*>(src.data());
    THOR_SHA1(src_bytes, src.length(), dest);
}

I withhold comment on the suitability of SHA-1 for this purpose, as that appears to be something you're stuck with.


Python Turtle based Pong game I'm new to Python but I've coded in other languages mainly for hardware. I made Pong in Python using turtle but it's a little glitchy. I was wondering if any of you guys could check it out and give advice. If you have any advice on how to improve collision detection let me know please.

import os
import math
import random
import time
import turtle

#set up screen
screen = turtle.Screen()
screen.bgcolor("green")
screen.title("Pong")

# set up border
border_pen = turtle.Turtle()
border_pen.speed(0)
border_pen.color("white")
border_pen.penup()
border_pen.setposition(-300,-300)
border_pen.pendown()
border_pen.pensize(3)
for side in range(4):
    border_pen.fd(600)
    border_pen.lt(90)
border_pen.hideturtle()

#set score to 0
score = 0

#set time to zero
time = 0
seconds = 0

#Draw score
score_pen = turtle.Turtle()
score_pen.speed(0)
score_pen.color("white")
score_pen.penup()
score_pen.setposition(-290, 310)
scorestring = "Score %s" %score
score_pen.write(scorestring, False, align="left", font=     ("Arial", 14, "normal"))
score_pen.hideturtle()

#Draw timer
time_pen = turtle.Turtle()
time_pen.speed(0)
time_pen.color("white")
time_pen.penup()
time_pen.setposition(260, 310)
timestring = "Time %s" %time
time_pen.write(timestring, False, align="left", font= ("Arial", 14, "normal"))
time_pen.hideturtle()

#create the player turtle
player = turtle.Turtle()
player.color("blue")
player.shape("square")
player.shapesize(0.5, 4)
player.penup()
player.speed(0)
player.setposition(-280,-250)#(x,y)
player.setheading(90)
playerspeed = 15

#create the AIplayer turtle
AIplayer = turtle.Turtle()
AIplayer.color("black")
AIplayer.shape("square")
AIplayer.shapesize(0.5, 4)
AIplayer.penup()
AIplayer.speed(0)
AIplayer.setposition(280,250)#(x,y)
AIplayer.setheading(90)
AIplayerspeed = 15

#create the pong
pong = turtle.Turtle()
pong.color("red")
pong.shape("circle")
pong.shapesize(0.5, 0.5)
pong.penup()
pong.speed(10)
pong.setposition(0,0)#(x,y)
pongspeed = 15
pong.goto(0, 265)
pong.dy = -5
pong.dx = 5

#Move player up and down
def move_up():
    y = player.ycor()
    y += playerspeed
    if y > 265:
        y = 260
    player.sety(y)

def move_down():
    y = player.ycor()
    y -= playerspeed
    if y < -265:
        y = -260
    player.sety(y)

#keyboard bindings
turtle.listen()
turtle.onkey(move_up, "Up")
turtle.onkey(move_down, "Down")
#turtle.onkey(fire_bullet, "space")

def isCollision(t1, t2):
    distance = math.sqrt(math.pow(t1.xcor()-    t2.xcor(),2)+math.pow(t1.ycor()-t2.ycor(),2))
    if distance < 20:
        return True
    else:
        return False

#main game loop
while True:

    #move pong ball
    pong.sety(pong.ycor() +pong.dy)
    pong.setx(pong.xcor() +pong.dx)

    #check for bounce and redirect it
    if pong.ycor() < -300:
        pong.dy *= -1
    if pong.ycor() > 300:
        pong.dy *= -1
    if pong.xcor() < -300:
        pong.dx *= -1
        print("Game Over")
        exit()
    if pong.xcor() > 300:
        pong.dx *= -1

    #move AI paddle (might speed up pong movement)
    y = pong.ycor()
    y += AIplayerspeed
    AIplayer.sety(y)
    if AIplayer.ycor() > 265:
        AIplayerspeed *= -1
    if AIplayer.ycor() < -250:
        AIplayerspeed *= -1

    #collision pong and player
    if isCollision(pong, player):
        pong.dy *= -1
        pong.dx *= -1
        #Update the score
        score += 10
        scorestring = "Score: %s" %score
        score_pen.clear()
        score_pen.write(scorestring, False, align="left", font=("Arial", 14, "normal"))

    #collision pong and AIplayer
    if isCollision(pong, AIplayer):
        pong.dy *= -1
        pong.dx *= -1

    #updates timer and increases ball speed
    if seconds > 29:
        pong.dy *= -2
        pong.dx *= -2

    if seconds > 59:
        pong.dy *= -3
        pong.dx *= -3

     #displays timer but makes game laggy
#    seconds += 0.1
#    time = seconds
#    timestring = "Time: %s" %time
#    time_pen.clear()
#    time_pen.write(timestring, False, align="Left", font=("Arial", 14, "normal"))

First - You have numeric constants scattered all over the code.

Try to change the board width to 500 and you will immediately understand. Numeric constants like board width shall appear exactly once when assigned to a constant which is used throughout the code lateron. This applies to board size, pong size and player size.

define

board_width = 600
board_height = 600

and use it to draw the border like

for side in range(2):
    border_pen.fd(board_width)
    border_pen.lt(90)
    border_pen.fd(board_height)
    border_pen.lt(90)

Second - you have calculated dependent numeric constants

If constants depend on others you shall not evaluate these. Try to change the bong size and you will have a hell of a work to find all depending values. You are hiding the dependencies and again make your code unmaintainable. All constants that have dependencies shall be initialized not by numeric values, but by expressions using the first class constants.

replace

score_pen.setposition(-290, 310)

with

score_pen.setposition(-board_width/2 + 10, board_height/2 + 10)

Third - structure your code

Whenever you feel you need a comment to explain what a block of code is doing you probably found a function (or a class). if you chose nice function names comments may be obsolete. Also you have less name collisions on local variables leading to better names.

# set up border
border_pen = turtle.Turtle()
#[...]
border_pen.hideturtle()

should be

def set_up_border():
    pen = turtle.Turtle()
    #[...]
    pen.hideturtle()

also

#Draw score
score_pen = turtle.Turtle()
#[...]
score_pen.hideturtle()

should be

def create_score():
    pen = turtle.Turtle()
    #[...]
    pen.hideturtle()
    return pen

score_pen = create_score()

Fourth - reuse code and do not repeat yourself

When you now look at your new functions create_score() and create_timer() you will notice they are nearly identical. They only differ in position and the string displayed. But they share font, color and so on. So we add some parameters and unify these

def create_text(pos, txt):
    pen = turtle.Turtle()
    pen.speed(0)
    pen.color("white")
    pen.penup()
    pen.setposition(pos)
    pen.write(txt, False, align="left", font= ("Arial", 14, "normal"))
    pen.hideturtle()
    return pen

which we use like

score_pen = create_text(score_position, "Score %s" % score)
time_pen = create_text(time_position, "Time %s" % time)

the same way we can unify player and AIplayer. If you want to change player size there is now a single line of code to change. Actually these players, text fields and the bong should be classes but for now we leave it like that.

All the points up to now are valid for any programming language and absolutely important. Maintainability, readability and testability count.

Some algorithmic issues

Bouncing

An object bouncing on another one may not penetrate it. so it is not sufficient to reverse the direction when a collision is detected, but you also have to exit the foreign object immediately. this is done by "mirroring" the entered object to the outside again. So instead of

#move pong ball
pong.sety(pong.ycor() +pong.dy)
#[...]

#check for bounce and redirect it
if pong.ycor() < -300:
    pong.dy *= -1
#[...]

we do not draw immediately but fix penetration before updating the screen

#move pong ball
y = pong.ycor() + pong.dy
#[...]

#check for bounce and redirect it
if y < -board_height/2:
    pong.dy *= -1
    y = -board_height - y
#[...]

pong.sety(y)
#[...]

BTW: Game over is not recognised if the AI player would miss the bong.

Tower running

Setting x and y coordinates ii separate calls where the pen is down results in the pong performing tower running instead of smooth angular movement.

AIplayerspeed

Your AI player does not move with its own speed but with bong speed as it directly takes bong y position. AIplayerspeed acts as an offset to bong y position. This should be fixed to

either rename and calculated this offset from bong and player size to be center aligned
or implemented as a max speed to approach bong y position (again centeraligned)

math.sqrt

square root is an expensive function which in game design is avoided wherever possible. In your case you coud simply square the max distance

distance = math.sqrt(math.pow(t1.xcor()-    t2.xcor(),2)+math.pow(t1.ycor()-t2.ycor(),2))
if distance < 20:
    #[...]

becomes

sq_distance = math.pow(t1.xcor() - t2.xcor(), 2) + math.pow(t1.ycor() - t2.ycor(), 2)
if distance < math.pow(20, 2):
    #[...]


Fixing double arithmetic errors I am trying to account for the errors in double arithmetic in a C# program I am writing, and came up with this first solution that seems to work well for most cases:

    double errorMargins = returnDouble * Double.Parse(Unit_Class_Library.Properties.Resources.AcceptedDeviationConstant);
    double roundedValue = 0;
    for (int i = 1; i <= returnDoubleAsString.Length; i++)
    {
        roundedValue = RoundToSignificantDigits(returnDouble, i);
        if (Math.Abs(roundedValue - returnDouble) < Double.Parse(Unit_Class_Library.Properties.Resources.AcceptedDeviationConstant))
        {
            returnDouble = roundedValue;
            i = returnDoubleAsString.Length;
        }

    }

    return returnDouble;
}

public static double RoundToSignificantDigits(double d, int digits)
{
    if (d == 0)
        return 0;

    bool isNegative = false;
    if (d < 0)
        isNegative = true;

    double scale = Math.Pow(10, Math.Floor(Math.Log10(Math.Abs(d))) + 1);
    double returnValue =  scale * Math.Round(d / scale, digits);

    if (isNegative)
        returnValue = -returnValue;

    return returnValue;
}

However, I found that when you convert a number to a string it rounds the number making the above function not work.

For example, when 720.72499999999991 is inputted (with a AcceptedDeviationConstant of 0.000000001), the double as string would be "720.725" which causes the above loop to exit prematurely and not do its job leaving the number as 720.72499999999991. So what I did was convert the double to a string and then back to a double (the code block below) and this seems to work how I wanted it too, returning 720.725 in the above example.

string returnDoubleAsString = "" + returnDouble;
returnDouble = double.Parse(returnDoubleAsString);

I was wondering if it is okay to just leave it as the second one but it seems "improper" to do it like this. If it is okay in practice, what kind of comment should I leave to make it clear to other programmers what is going on and is there a link I can put in the comment that explains why this works?

Updated:

Seems like C# has its own format specifier "R" Round trip

  The round-trip ("R") format specifier guarantees that a numeric value that is converted to a string will be parsed back into the same numeric value. This format is supported only for the Single, Double, and BigInteger types.

public static double RoundTrip(double d)
{
    return Double.Parse(d.ToString("R"));
}

Original:

The way to represent floating point numbers exactly on a stream in readable format is to print out there representation in hex format (rather than decimal):

Not sure how it is done in C# or Java but I bet if you dig the exact same method is used down deep in these languages.

In C++ and C the methods are the same.
The C++ version.

double x = 5.6789;

std::cout << x << "\n";   // Human readable dot format.
                          // Has loss of precision as number may not
                          // be exactly representable in decimal.

// This the equivalent of using the "%a" string formatter.
// fprintf(stdout, "%a\n", x);
//
std::cout.flags(std::ios_base::fixed | std::ios_base::scientific);
std::cout << x << "\n";   // Hex Format human readable (well nearly human readable)
                          // Prints each part of the float as a hex value
                          // That can be represented exactly.
                          // on a stream of char

Updated after reading: Printing double without losing precision

// In C++11 there is an additional manipulator
// that achieves the same affect.
std::cout << std::hexfloat << x << "\n";

The results:

5.6789                    // Easy to read (not exact)

0x1.6b7318fc50481p+2      // Difficult to read (but exact).

0x1.6b7318fc50481p+2      // Difficult to read (but exact).


Comma Code - Automate the Boring Stuff I'm currently working through an exercise of Chapter 4 from Automate the Boring Stuff, which reads as follows:

"Say you have a list value like this: spam = ['apples', 'bananas', 'tofu', 'cats']
Write a function that takes a list value as an argument and returns a string with all the items separated by a comma and a space, with and inserted before the last item. For example, passing the previous spam list to the function would return 'apples, bananas, tofu, and cats'. But your function should be able to work with any list value passed to it."

Since I'm an absolute beginner to Python (and programming in general), I wanted to get some advice on how to make this code cleaner. It works with any size list, but I've reviewed some other solutions on SO and it seems like there are a million ways to build this out. How can I make this more straightforward?
spam = ['apples', 'bananas', 'tofu', 'cats']

def commaCode(listVar):
    if len(listVar) == 0: # condition for an empty list
        print('There are no items in this list.')

    if len(listVar) == 1: # condition for a list with 1 item
        return (str(listVar[0])+'.')

    if len(listVar) == 2: # condition for a list with 2 items
        return (str(listVar[0]) + ' and ' + str(listVar[1]) + '.')

    if len(listVar) >=3: # conditions for lists with 3+ items
        listDisp = ''
        for i in range(len(listVar[:-1])):
            listDisp = listDisp + str(listVar[i]) + ', '
        listDisp = listDisp + 'and ' + str(listVar[-1])
        return (listDisp)

commaCode(spam)

You want to look at str.join
by the last case, len(listVar) is run 4 times, which is very repetitive.  Better to set it to a temporary variable, and check that each time.
Python has some binary convenience  operators for common things like appending.  Instead of listDisp = listDisp + foo use listDisp += foo
instead of calling str() on everything as you build the string, why not call it up front, so it can't be missed somewhere along the way.  Something at the top like stringList = [ str(i) for i in listVar ] will remove the need for all those str() calls sprinkled throughout the code.
Naming:  listVar is a variable.  It doesn't need to have 'var(iable)' in its name.  How about 'inputList'?  Names are important - they're messages to the future (you or whoever reads your code) about what you are/were trying to accomplish.  Similar issue with the name commaCode.
range(len(listVar[:-1])) is the same as range(len(listVar)-1), which is both easier to read, and can take advantage of point 2 and end up written as range(listVarLen-1) which is definitely clearer.

Overall though, for a beginner, not bad!