Type to represent multiple possible values I created this type for use in a "Wizard"-style user interaction. In each step, the user chooses an option called a Modifier and each of those options contains subselections that must be made.

These Modifiers are stored with two parallel structures, one of the fixed values and a second of the selectable ones using this type. When the uses chooses a suboption, the chosen value is merged into the fixed values and when completed, the values are used to modify the wizard model.

/**
 *  A portion of a modifier that consists of a set of possible values and a number of them
 *  that will be selected. After selection, the selected values will be added to the set values
 *  of the modifier.
 */
export class SelectableModifier {

    constructor(numSelectionsNeeded:number, options:Array<any>, selectionTime:string) {
        this._numSelectionsNeeded = numSelectionsNeeded;
        this._options = options;
        this._selectionTime = selectionTime;
    }

    /**
     * The number of options that need to be selected.
     */
    private _numSelectionsNeeded:number;
    /**
     * The available options
     */
    private _options:Array<any>;

    private _selectionTime:string;

    /**
     * Choose from this selection, decomposing it into the selected options.
     *
     * @param chosenIndices
     */
    public makeSelection(chosenIndices:Array<Number>):Array<any> {
        if (chosenIndices.length != this._numSelectionsNeeded) {
            throw "The selection requires that " + this._numSelectionsNeeded + " selections be made but " + chosenIndices.length + " were instead."
        }
        return this._options.filter((element, index)=> {
            return chosenIndices.indexOf(index) !== -1;
        });
    }

    get numSelectionsNeeded():number {
        return this._numSelectionsNeeded;
    }

    get options():Array<any> {
        return this._options;
    }

    get selectionTime():string {
        return this._selectionTime;
    }
}

How about this?

Use constructor(private _options: any[]) instead of field declaration and assignment.
As a defensive programming measure use = [] to initialize the array with an empty value in null/undefined was padded in.
Instead of numerous searches through chosenIndices (via indexOf) use a simple .map(...) function instead.

export class SelectableModifier {

  constructor(private _numSelectionsNeeded: number, private _options: any[] = [], private _selectionTime: string) { }

  get numSelectionsNeeded(): number {
    return this._numSelectionsNeeded;
  }

  get options(): any[] {
    return this._options;
  }

  get selectionTime(): string {
    return this._selectionTime;
  }

  public makeSelection(chosenIndices: number[]): any[] {
    if (chosenIndices.length != this.numSelectionsNeeded) {
      throw new Error("The selection requires that " + this._numSelectionsNeeded + " selections be made but " + chosenIndices.length + " were instead.");
    }

    return chosenIndices.map(indexOfChosen => this.options[indexOfChosen]);
  }
}

UPD1

If null/undefined choices should be treated as errors, you can check for their presence and throw.

public makeSelection(chosenIndices: number[]): any[] {
    if (chosenIndices.length != this.numSelectionsNeeded) {
        throw new Error(`The selection requires that ${this._numSelectionsNeeded} selections be made but ${chosenIndices.length} were instead.`);
    }

    const result = chosenIndices.map(indexOfChosen => this.options[indexOfChosen]);
    if (result.some(item => item == null))
        throw new Error('Selection can not be null/undefined.');
    return result;
}


Building a timelapse with ffmpy As a 'trying to learn Python' project, I am using ffmpy to stitch together a timelapse from a series of still images. I'd like the script to output a couple of formats for web use.

This is what I have:

#!/usr/bin/env python3

import datetime
import ffmpy
import os

now = datetime.datetime.now()
ydr = now.strftime('%Y')
mdr = now.strftime('%m')
ddr = now.strftime('%d')

ipath = str(os.path.dirname(os.path.abspath(__file__))) + '/images/' + ydr + '/' + mdr + '/*/*.jpg'
opath1 = str(os.path.dirname(os.path.abspath(__file__))) + '/videos/' + ydr + mdr + '.mp4'
opath2 = str(os.path.dirname(os.path.abspath(__file__))) + '/videos/' + ydr + mdr + '.webm'

ff = ffmpy.FFmpeg(
    inputs={ipath: '-loglevel info -pattern_type glob -framerate 18 '},
    outputs={opath1: '-c:v libx264 -vf "scale=1280:-1,hqdn3d=luma_spatial=1" -pix_fmt yuv420p'}
)
ff.run()

ff = ffmpy.FFmpeg(
    inputs={ipath: '-loglevel info -pattern_type glob -framerate 18 '},
    outputs={opath2: '-c:v libvpx -vf "scale=1280:-1,hqdn3d=luma_spatial=1" -b:v 1M -c:a libvorbis'}
)
ff.run()

It works, but it's kinda ugly and I'm pretty sure there's a more efficient and 'Pythonic' way of doing this. Any pointers?

Use path.join() instead of manually concatenating file paths

This will make sure that it will work on different OS's, windows uses \ backslashes for instance
No need to convert with strftime

A datetime has years, months and days as properties, if you want them in str format you could:

map(str, iterable) to convert them into strings

Code

import datetime
import os.path

now = datetime.datetime.now()
y, m = map(str, (now.year, now.month))

location = os.path.dirname(os.path.abspath(__file__))
ipath = os.path.join(location, 'images', y, m + '.jpeg')
video_path_mp4 = os.path.join(location, 'videos', y, m + '.mp4')
video_path_webm = os.path.join(location, 'videos', y, m + '.webm')


API request, conversion from 12-hour to 24-hour format and conversion from UTC to MET time The script calls the sunrise-sunset API to get one of the possible values, then converts it to 24-Hour format and MET time (as the times from the API are 12-hour and UTC). At the end, it joins the list to a string and prints it. I know this isn't commented very well, but are there any significant flaws in this code? I neither have any experience with Python nor with programming in general, this is just for a personal project and Python seemed to be the best way to do this, so I'm open for any advice and help.

import json
import urllib.request as request

def apiGetList(key):
    """ Function twilightApiGetList(key):
        Calls the sunrise-sunset api to return the value of a given key
        Possible keys: sunrise, sunset, solar_noon, day_length, civil_twilight_begin, civil_twilight_end,
        nautical_twilight_begin, nautical_twilight_end, astronomical_twilight_begin, astronomical_twilight_end
        All times returned are a list in 12-hour format, UTC time
    """
    with request.urlopen('https://api.sunrise-sunset.org/json?lat=44.244622&lng=-7.768863&date=today') as response:
        if response.getcode() == 200:
            source = response.read()
            data = json.loads(source)
            out = list(f"{data['results'][key]}")
            if len(out) == 10:
                out.insert(0, '0')
            else:
                pass
            return(out)
        else:
            print("Not successful")
            return

def convert12To24(lst):
    """ Function convert12To24(lst):
        Converts from 12-hour format to 24-hour format, input and output types are lists
    """
    if lst[9] == "A" and "".join(lst[0:2]) == "12":
        del lst[8:12]
        lst[0:2] = '00'
    elif lst[9] == "A":
        del lst[8:12]
    elif lst[9] == "P" and "".join(lst[0:2]) == "12":
        del lst[8:12]
    else:
        del lst[8:12]
        lst[0:2] = str(int("".join(lst[0:2])) + 12)
    return(lst)

def convertUtcToMet(time):
    """ Function convertUtcToMet(time):
        Converts from UTC time to MET time by adding two hours. Input and output time is in 24-hour format, type is a list
    """
    if int("".join(time[0:2])) < 8:
        time[0:2] = str(int("".join(time[0:2])) + 2)
        time.insert(0, '0')
    elif "".join(time[0:2]) == '22':
        time[0:2] = '00'
    elif "".join(time[0:2]) == '23':
        time[0:2] = '01'
    else:
        time[0:2] = str(int("".join(time[0:2])) + 2)
    return(time)

if __name__ == "__main__":
    ctb = apiGetList("civil_twilight_begin")
    cte = apiGetList("civil_twilight_end")

    print("".join(convertUtcToMet(convert12To24(ctb))))
    print("".join(convertUtcToMet(convert12To24(cte))))

Welcome to CodeReview! This is a great first question.

Do not do your own time math

...nor your own formatting when possible. Tell Python to do it for you. Some example code:

from dataclasses import dataclass
from datetime import timedelta, datetime, timezone
import requests, pytz

@dataclass(frozen=True)
class DayInfo:
    day_length: timedelta
    sunrise: datetime
    sunset: datetime
    solar_noon: datetime
    civil_twilight_begin: datetime
    civil_twilight_end: datetime
    nautical_twilight_begin: datetime
    nautical_twilight_end: datetime
    astronomical_twilight_begin: datetime
    astronomical_twilight_end: datetime

    @classmethod
    def get(cls, lat: float, long: float, tz: timezone) -> 'DayInfo':
        """ Function twilightApiGetList(key):
            Calls the sunrise-sunset api to return the value of a given key
            All times returned are in the provided timezone.
        """
        with requests.get(
            'https://api.sunrise-sunset.org/json',
            params={
                'lat': lat,
                'lng': long,
                'date': 'today',  # technically this can be omitted
                'formatted': 0,   # we want ISO8601 ("machine-readable")
            },
        ) as response:
            response.raise_for_status()
            data = response.json()

        if data['status'] != 'OK':
            raise requests.HTTPError(data['status'], response=response)
        results = data['results']

        return cls(
            day_length=timedelta(seconds=int(results['day_length'])),
            **{
                k: datetime.fromisoformat(results[k]).astimezone(tz)
                for k in (
                    'sunrise',
                    'sunset',
                    'solar_noon',
                    'civil_twilight_begin',
                    'civil_twilight_end',
                    'nautical_twilight_begin',
                    'nautical_twilight_end',
                    'astronomical_twilight_begin',
                    'astronomical_twilight_end',
                )
            },
        )

if __name__ == "__main__":
    day_info = DayInfo.get(44.244622, -7.768863, pytz.timezone('MET'))
    print(f'{day_info.civil_twilight_begin:%H:%m}')
    print(f'{day_info.civil_twilight_end:%H:%m}')

Notes:

You didn't parameterize the right things. Parameters should be coordinates and timezone.
Ask the API for machine-readable (ISO) dates.
Might as well parse everything from the API.
Do not use string manipulation to get your desired format. What is shown is string interpolation with standard Python time fields.
Use requests - it has a lot of niceties that make this easier.
Only issue one call to the API, returning an object that has both of the keys you care about. If you're never, ever going to use the other keys, you can delete them from this class, but they're inexpensive to generate - compared to a second HTTP call, which is much more expensive.


Using doParallel to cycle through *.rds files Overview

I have 165,000 *.rds files that are around 200,000 obs x 13 variables each.  These files are unique to grid files (2.5 square mile data) so I need to keep them in their unique grid numbers.  Currently, I'm loading each individual file, performing some function on the data, and resaving them to another directory; however, this took up to a week to process and I'm trying to improve the code to run in parallel.  I recently started 10 individual RStudio sessions and divided the grids into 10 separate sessions to run in parallel.  It worked fine and only took a day to run through.  This is not very efficient though and I would like to try to get it running in parallel using only one rsession.

The following code is a simple example I made that closely resembles the basic function of the loop.  This is not the exact code as it would not be useful to post all the files and all the code.

Question

Is this the proper way to run parallel when loading individual files,  applying a function to the data, and re-save to a directory?  It seems to be working, but I don't know if it is the correct way to do this.

RDS Files:

df1

df2

df3

Code

# Multicore
library(doParallel)
cl <- makeCluster(4)
registerDoParallel(cl)

# Function
check <- function(x){
  df <- readRDS(paste0("./", x))
  df$var <- df$var*2
  saveRDS(df, paste0("./temp/", x))
}

# Directory where *.rds files are
files <- list.files("./")

# for loop
for (i in unique(files)){
  check(i)
}

# foreach loop
foreach(i = unique(files)) %dopar% check(i)

Parallelization is not necessarily implemented nicely in R. However, it is far more ideal to use R's batch process than opening 10x Rstudio sessions as you saw (less of a resource drain per task).

Cores, cores, where art thou?

The first thing I would do is find out how many cores you have access to. Within this script 4 seem to be allocated. It is very important that you have at most or less than the total number of cores on your system for the parallel jobs to be appropriately scheduled.

# Find out how many cores exist
parallel::detectCores()

Speed

Speed-wise, I find the foreach iterator to consume more time than I really care for. The reason is because it has a lot of unnecessary overhead and combine operations that tend to dynamically grow objects. So, I normally just interface directly with R's parallel package's parLapply function.

Code

I realize that this is just a condensed example, however, without seeing everything, we really cannot give super specific feedback.

With that being said, you will probably end up with an error running this code since i is not defined within check(). Only x is in the function's scope.

Try to return some value to the foreach loop, e.g. an i value from check or a "pass-i", "fail-i" to figure out if a process fails.

Another bit to note is that the script is currently set to output to the input directory. Try to create dedicate directories for input and output. The reason for this is, if you were to re-run your script, the processed files would be included within the next run. (Unless the pattern specified by list.files is more specific to file nomenclature that you have.)

Also, since it seems like you are using spatial data. Is this a raster image that is contained within .rds? If so, could you get away with using a stack instead of the whole raster into memory?

Aside

If you are interested in learning more about how to parallelize with R, I would recommend looking over this slide deck (Disclaimer: I wrote it)

Edit

Per the submitter's comment that using lapply is not possible. Please note that a for loop in R is the same in this case as using an lapply. There are some benefits (speed being one) that make lapply typically better than using a for loop. Furthermore, the foreach here is really a cast over the parXapply statements.

library(parallel)
cl <- makeCluster(detectCores())

# Function
check <- function(x){
  df <- readRDS(paste0("./", x))
  df$var <- df$var*2
  saveRDS(df, paste0("./temp/", x))
}

# Directory where *.rds files are
files <- list.files("./")

# Obtain unique files
i = unique(files)

# lapply statement
lapply(i, FUN=check)

# parlapply
parLapply(cl, X=i, fun=check)

Edit 2

This edit is meant to show how to export functions or variables per submitter's issue. Since the submitter has not made available the functions or variables needed within the parallelization this is a generic example.

library(parallel)
cl <- makeCluster(detectCores())

# Load packages on cluster's R sessions
clusterEvalQ(cl, library(pkgname))

# Export functions or variables to cluster's R session
func <- function(a){
out <- a*a
return(out)
}

v = 1:10

clusterExport(cl,c("func","v"))

stopCluster(cl)


Idiomatic way to construct a SQL INSERT query in Python I have a bit of code to insert some values in a table. It seems a bit clunky - is there a neater way?

words is a list of dictionaries, and the entries contain other fields which I don't want to insert.

# add the list of words to the words table
time_stamp = datetime.datetime.now().isoformat()
sql = "INSERT INTO words (word, user_id, game_id, score, time_stamp) VALUES "
vals = []
for word in words:
    vals.append("('%s', %s, %s, %s, '%s')" %
                (word['str'], user_id, game_id, word['score'], time_stamp))
cur.execute(sql + ",".join(vals))

There is a neater, more Pythonic way of doing this using the str.format function. However, it is not the way you should do it.

Currently, the way you are building your query, you are vulnerable to SQL injection. This is the point @Mat's Mug was getting at in his comment. What if a malicious user decides to give the following as a user ID:

user_id = "1,1,99,'today');DROP TABLES;INSERT INTO words (word, user_id, game_id, score, time_stamp) VALUES ('Goodbye', 1"

If this happens, a disaster beyond your imagination will occur*!

To prevent this, the cursor from the MySQLdb module (which I assume you are using) allows you to pass in a tuple or dict of the parameters to the execute function:

time_stamp = datetime.datetime.now().isoformat()
sql = "INSERT INTO words (word, user_id, game_id, score, time_stamp) VALUES {}"

# Create the query
sql = sql.format(','.join(["('%s', %s, %s, %s, '%s')" for _ in words]))

# Create the parameter tuple.
params = ()
for word in words:
    params += (word['str'], user_id, game_id, word['score'], time_stamp,)

# Sending a tuple is easiest in this situation
cur.execute(sql, params)

If you want to look at another database solution for Python, take a look at SQLAlchemy. Its a pretty powerful framework that makes working with databases much simpler.

*=Sorry for the Phantom of the Opera quote. I'm feeling oddly dramatic right now.

Update

To answer your question about removing the for-loop, the best you can hope for is to use a list comprehension. In many situations, list comprehensions are typically the Pythonic way of iterating over a collection. However, in your situation, this probably is not the case.

The comprehension for your current code would look like this:

','.join([str(tuple([word['str'], user_id, game_id, word['score'], time_stamp]))
          for word in words])

This is really only effective creating the end to your query. Unfortunately, in addition to being hard to read, it is still vulnerable to SQL injection.

In your case, I think a for-loop would be the cleanest way to implement what you want.


Ensuring valid origin and destination countries Having the following code in PHP 5.2, where:

$row: Is an array that contains some things, but the ones that are interesting for the post are country_from and country_to, which are country names.
$pBCountries: Array containing strings with country names. Here's the content of the array: http://pastebin.com/f241JeqV

The function is very easy. It checks if the countries (country_from and country_to) are included on the array $pBCountries. If the countries are not found, it appends the "not found country name" to the array $notFoundArray, which is returned at the end of the function.

function countryChecker($row, $pBCountries)
{
    $notFoundArray = array();
    $c_fromFound = false;
    $c_toFound = false;
    foreach ($pBCountries as $country) {
        if (in_array($row["country_from"], $country)) {
            $c_fromFound = true;
        } elseif (in_array($row["country_to"], $country)) {
            $c_toFound = true;
        }
    }
    if ($c_fromFound == false) {
        array_push($notFoundArray, $row["country_from"]);
    }
    if ($c_toFound == false) {
        array_push($notFoundArray, $row["country_to"]);
    }
    return $notFoundArray;
}

The function works perfectly, but I'm pretty sure I can make it shorter and prettier.

PS: I can't update the PHP version as it's not my own machine.

Representing a country (OOP)

So it seems that your array of countries ($pBCountries) has the following structure:

[
    [
        "id" => "0",
        "name" => "Netherlands"
        "country_code" => "NL"
    ],
    [
        "id" => "1",
        "name" => "United Kingdom"
        "country_code" => "UK"
    ],
]

Now that we have that figured out, lets take a look at how we can improve this. Generally we do not like to work with arrays like this, as it is extremely unclear how they are structured to outside users (as you have seen from the comments to your question).

What we do like working with are objects. Objects are a much better way of representing stuff than arrays are. We can create a simple object that represents a country like so:

class Country {
    private $id;
    private $name;
    private $code;

    /**
     * @param string|int $id
     * @param string $name
     * @param string $code
     */
    public function __construct($id, $name, $code) {
        $this->id = (string) $id;
        $this->name = (string) $name;
        $this->code = (string) $code;
    }

    /**
     * @return string
     */
    public function getId() {
        return $this->id;
    }

    /**
     * @return string
     */
    public function getName() {
        return $this->name;
    }

    /**
     * @return string
     */
    public function getCode() {
        return $this->code;
    }
}

Now we have a simple, immutable country object. The private fields cannot be changed from the outside.

Creating the Country objects

Now we still need a function which creates a bunch of these Country objects from your array. For this we are going to create a createMultipleFromArray() function on the Country object:

class Country {
    /* private fields */

    /**
     * @param array countries
     * @return Country[]
     */
    public static function createMultipleFromArray(array $countries) {
        $countries = array();

        foreach ($countries as $country) {
            array_push($countries, self::createFromArray($country));
        }

        return $countries;
    }

    /**
     * @param array $country
     * @return Country
     */
    public static function createFromArray(array $country) {
        return new Country($country["id"], $country["name"], $country["country_code"]);
    }

    /* constructor and getters */
}

I have omitted the private fields, constructor and getters from this snippet and replaced them with a comment, so that you can understand where I would place the methods but without me having to copy them over to this snippet as well.

You have probably noticed that I have annotated most functions with some @return and @param annotations. These annotations are extremely useful as they tell you what type is expected for certain arguments and what type will be returned. If you are using an IDE like PhpStorm this is even more helpful, as PhpStorm will know which methods are available on a Country object and can auto-complete them for you. This is a lot less error prone than fiddling with array indices yourself.

Now we can create an array of Country objects by simple doing:

// p.s. $pBCountries is a pretty bad name for an array of countries
$countries = Country::createMultipleFromArray($pBCountries);

Array Intersection

Now that we have cleaned up that mess of an array, lets get back to the actual function. First of all, your checkCountry() function would be better off taking two explicit $country_from and $country_to parameters, instead of relying on an array to be passed which may or may not contain these indices. This makes the function more clean.

The functionality of your function can actually be extracted to a much more general concept called array intersection. Array intersection is a function that takes two arrays and returns a new array containing only the elements that are in both arrays.

To achieve this, we first need a function which can determine if two Country objects are the same. We do this by creating an equals() method on the Country object:

class Country {
    /* private fields and constructor */

    /**
     * @param Country $other_country
     * @return bool
     */
    public function equals(Country $other_country) {
        return $this->id == $other_country->getId() &&
            $this->name == $other_country->getName() &&
            $this->code == $other_country->getCode();
    }

    /* Getter functions */
}

Now we can create a separate class that can perform an intersection of two arrays of Country objects:

class CountryFilter {
    /**
     * @param Country[] $countries_to_filter
     * @param Country[] $countries
     */
    public static function intersect(array $countries_to_filter, array $countries) {
        $intersected_countries = array();

        foreach ($countries as $country) {
            if (self::contains($countries_to_filter, $country)) {
                array_push($intersected_countries, $country);
            }
        }

        return $intersected_countries;
    }

    /*
     * @param Country[] $country_array
     * @param Country $country
     */
    public static function contains(array $country_array, Country $country) {
        foreach ($country_array as $country_from_array) {
            if ($country->equals($country_from_array)) {
                return true;
            }
        }

        return false;
    }
}

The contains() method I think doesn't really belong on the CountryFilter class, but actually on a class like CountryList, which is a class that holds an array of countries and provides the contains() functionality. But I figured that would probably take it too far, so I kept it at the CountryFilter class.

Assuming that you have changed $row['country_from'] and $row['country_to'] to be actual Country objects and $pBCountries to be an array of Country objects, we can now perform this function call to achieve the same results as the countryChecker() method:

$intersection = CountryFilter::intersect($pBCountries, [$row['country_from'], $row['country_to']]);

This refactor resulted in much more general classes and functionality with a lot of reusability. The intersect() method can be used for any two arrays of countries and is therefore very versatile. Each of these small pieces of code are very readable and easy to understand, which will in turn make other code that utilizes these two classes more easy to understand as well.

Thus code readability and reusability have greatly been improved.


Get a nested property of a complex object at any level of depth Given a complex object (with nested objects at any depth) I need to be able to quickly retrieve a value for a given property name.

I have designed the following code, I would like to know:

How to tune my code for maximum performance considering passing a big object?
Am I missing some case in data.a.x?
Any other improvements?

var data = {
  a: {
    a1: 'hello',
    a2: true,
    a3: false,
    a4: {},
    a5: [],
    a6: ['red', 'blue', true],
    a7: null,
    a8: undefined,
    a9: NaN,
    a10: function() {},
    a11: Math,
    a12: JSON,
    a13: /a-z/,
    a14: new ReferenceError,
    a15: new Date,
    a16: new Number(7),
    a17: new String("abc"),
    a18: new Boolean(true)
  },
  b: {
    b1: {
      b1a: true,
      b1b: 100
    },
    b2: {
      b2a: true,
      b2b: false,
      b2c: 10.10
    }
  }
}

var app = {
  findProperty: function(data, name) {
    var toType = function(obj) {
      return ({}).toString.call(obj).match(/\s([a-zA-Z]+)/)[1].toLowerCase()
    };
    var data = data,
      process = true,
      result = {
        isFound: false,
        value: null
      };
    var recursion = function(data, name) {
      var type = toType(data);
      if (type === 'object') {
        Object.keys(data).forEach(function(property) {
          if (process) {
            if (name in data) {
              process = false;
              result.isFound = true;
              result.value = data[name];
            } else if (typeof data[property] === 'object') {
              recursion(data[property], name);
            }
          }
        });
      }
    };
    recursion(data, name);
    return result;
  }
}

console.clear();
// found
var a1 = app.findProperty(data, 'a1');
console.log('a1', a1.isFound, a1.value);

var a2 = app.findProperty(data, 'a2');
console.log('a2', a2.isFound, a2.value);

var a3 = app.findProperty(data, 'a3');
console.log('a3', a3.isFound, a3.value);

var a4 = app.findProperty(data, 'a4');
console.log('a4', a4.isFound, a4.value);

var a5 = app.findProperty(data, 'a5');
console.log('a5', a5.isFound, a5.value);

var a6 = app.findProperty(data, 'a6');
console.log('a6', a6.isFound, a6.value);

var a7 = app.findProperty(data, 'a7');
console.log('a7', a7.isFound, a7.value);

var a8 = app.findProperty(data, 'a8');
console.log('a8', a8.isFound, a8.value);

var a9 = app.findProperty(data, 'a9');
console.log('a9', a9.isFound, a9.value);

var a10 = app.findProperty(data, 'a10');
console.log('a10', a10.isFound, a10.value);

var a11 = app.findProperty(data, 'a11');
console.log('a11', a11.isFound, a11.value);

var a12 = app.findProperty(data, 'a12');
console.log('a12', a12.isFound, a12.value);

var a13 = app.findProperty(data, 'a13');
console.log('a13', a13.isFound, a13.value);

var a14 = app.findProperty(data, 'a14');
console.log('a14', a14.isFound, a14.value);

var a15 = app.findProperty(data, 'a15');
console.log('a15', a15.isFound, a15.value);

var a16 = app.findProperty(data, 'a16');
console.log('a16', a16.isFound, a16.value);

var a17 = app.findProperty(data, 'a17');
console.log('a17', a17.isFound, a17.value);

var a18 = app.findProperty(data, 'a18');
console.log('a18', a18.isFound, a18.value);

var b = app.findProperty(data, 'b');
console.log('b', b.isFound, b.value);

var b1 = app.findProperty(data, 'b1');
console.log('b1', b1.isFound, b1.value);

var b2 = app.findProperty(data, 'b2');
console.log('b2', b2.isFound, b2.value);

var b1a = app.findProperty(data, 'b1a');
console.log('b1a', b1a.isFound, b1a.value);

var b1b = app.findProperty(data, 'b1b');
console.log('b1b', b1b.isFound, b1b.value);

var b2a = app.findProperty(data, 'b2a');
console.log('b2a', b2a.isFound, b2a.value);

var b2b = app.findProperty(data, 'b2b');
console.log('b2b', b2b.isFound, b2b.value);

// not found
var xxx = app.findProperty(data, 'xxx');
console.log('xxx', xxx.isFound, xxx.value);

var yyy = app.findProperty(data, 'yyy');
console.log('yyy', yyy.isFound, yyy.value);

toTry function reference

You can take advantage of the JSON.stringify's replacer argument.
This way it becomes much more simple:

var app =
  findProperty: function(data, name) {
    var result = {
      isFound: false,
      value: null
    };
    JSON.stringify(data, function(k, v) {
      if (k == name) {
        result.isFound = true;
        result.value = v;
        return undefined; // stop stringification
      } else {
        return v; // continue with same value
      }
    });
    return result;
  }
}

Compared to your original function, there are two points to consider:

one little caveat is that a Date object is returned stringified, e.g. for a15 in your dataexample:

has an original value of new Date.
is returned as Date {Thu Jan 14 2016 18:34:10 GMT+0100} by your function.
is returned as 2016-01-14T17:34:10.220Z by the mine.

your function excludes Array objects, while the mine processes them, e.g. calling app.findProperty(data, '0') will return red (from a6: ['red', 'blue', true] in your dataexample).

My version seems to be faster (approximatively x2) using your data example, but it's not certain how it'll be in any case.

The snippet below tries to illustrate it, simultaneously controlling the results between the two versions:

var data = {
  a: {
    a1: 'hello',
    a2: true,
    a3: false,
    a4: {},
    a5: [],
    a6: ['red', 'blue', true],
    a61: {a61a: 'red', a61b: 'blue'},
    a7: null,
    a8: undefined,
    a9: NaN,
    a10: function() {},
    a11: Math,
    a12: JSON,
    a13: /a-z/,
    a14: new ReferenceError,
    a15: new Date,
    a16: new Number(7),
    a17: new String("abc"),
    a18: new Boolean(true)
  },
  b: {
    b1: {
      b1a: true,
      b1b: 100
    },
    b2: {
      b2a: true,
      b2b: false,
      b2c: 10.10
    }
  }
}

var app = {
  findProperty: function(data, name) {
    var toType = function(obj) {
      return ({}).toString.call(obj).match(/\s([a-zA-Z]+)/)[1].toLowerCase()
    };
    var data = data,
      process = true,
      result = {
        isFound: false,
        value: null
      };
    var recursion = function(data, name) {
      var type = toType(data);
      if (type === 'object') {
        Object.keys(data).forEach(function(property) {
          if (process) {
            if (name in data) {
              process = false;
              result.isFound = true;
              result.value = data[name];
            } else if (typeof data[property] === 'object') {
              recursion(data[property], name);
            }
          }
        });
      }
    };
    recursion(data, name);
    return result;
  },

  findProp: function(data, name) {
    var result = {
      isFound: false,
      value: null
    };
    JSON.stringify(data, function(k, v) {
      if (k == name) {
        result.isFound = true;
        result.value = v;
        return undefined; // stop stringification
      } else {
        return v; // continue with same value
      }
    });
    return result;
  }
}

var names = [
  'a', 'a1', 'a2', 'a3', 'a4', 'a5', 'a6', 'a61', 'a61a', 'a7', 'a8', 'a9',
  'a10', 'a11', 'a12', 'a13', 'a14', 'a15', 'a16', 'a17', 'a18',
  'b', 'b1', 'b2', 'b1a', 'b1b', 'b2a', 'b2b', 'b2c', 'xxx', 'yyy'
];
var results = {};

function exec(func) {
  results[func] = [];
  begTime = Date.now();
  for (var i = 0, n = names.length; i < n; i++) {
    var name = names[i],
        result = app[func](data, name);
    results[func][i] = result.value;
    console.log(name, result.isFound, result.value);
  }
  console.log(
    '--- using ', func + ':', (Date.now() - begTime) + 'ms');
}

console.clear();
exec('findProperty');
exec('findProp');
for (var i = 0, n = results.findProperty.length; i < n; i++) {
  if (results.findProperty[i] !== results.findProp[i]) {
    console.log(
      '*** Error in ' + i + ':',
      results.findProperty[i], '<-->', results.findProp[i]
    );
  }
}

Apart from the already mentioned Date difference, you'll see an "error" on the NaN value: this is normal since NaN is equal to nothing, even itself.


Bi Directional Dictionary I wrote a class that extends the dict class to provide indexing like functionality, so I can find which key(s) map to a given value in the dict. It's pretty bare bones and there are a lot of things I haven't fleshed out like docstrings (will add these later when I have a final version), how to handle unhashable elements (it currently just fails, but this might be changed in the future), etc.

The extremely bare bones nature of this is because this is something I whipped up in a few minutes for immediate use.

from collections import defaultdict  # The class currently doesn't exist in a separate module, and there are many other imports used by surrounding code.

# Do something

class DoubleDict(dict):
    def __init__(self, *args, **kwargs):
        super().__init__(*args, **kwargs)
        self.values_dict = defaultdict(lambda: [])
        for key, value in self.items():
            self.values_dict[value] += [key]

    def __setitem__(self, key, value):
        var = self.get(key)
        if var in self.values_dict:
            self.values_dict[var].remove(key)
            if not self.values_dict[var]:
                del self.values_dict[var]
        super().__setitem__(key, value)
        self.values_dict[value] += [key]

    def index(self, value):
        return self.values_dict[value][-1]

    def indices(self, value):
        return self.values_dict[value]

# Do Something

My concerns are to have this be as performant as feasible, and sane and generally correct (there may be some unavoidable tradeoffs) approach. I would probably make this into its own module after incorporating feedback.

A couple of minor notes:

1) Add proper typing so that users of your code will be able to type-check their own code correctly.  I think this would look like:

from typing import Dict, List, TypeVar

_KT = TypeVar('_KT')
_VT = TypeVar('_VT')

class DoubleDict(Dict[_KT, _VT]):
    super().__init__(*args, **kwargs)
    self.values_dict: Dict[_VT, List[_KT]] = defaultdict(lambda: [])
    for key, value in self.items():
        self.values_dict[value] += [key]

... etc

2) I'd eliminate the index syntactic sugar, personally, because it makes it easier for the caller to forget that this isn't a one-to-one mapping in both directions.  Make them get the list and check to make sure that it's got the length they expect.

Alternatively, if you'd want it to be a one-to-one mapping in most cases, then enforce that in the setter and make it part of the class's contract.


Calculate the sum of interior angles of a polygon Of the languages that I use, I have the most experience with C++. I am looking to sharpen my skills with C, so I wrote this small calculator program.

What I have here is a sample of how I would probably write larger programs.

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

int getSumInteriorAngles(const unsigned int numSides)
{
    return 180 * (numSides - 2);
}

int main(int argc, char** argv)
{
    /* Keep string constants of the questions. */

    const char* howManySides =
    "How many sides does the shape have? ";

    const char* askAnotherShape =
    "Would you like to find interior angles for another polygon? (1 or 0) ";

    /* Table of polygon names from 3 to 12 sides */
    const char* names[] = {
        "triangle",
        "quadrilateral",
        "pentagon",
        "hexagon",
        "heptagon",
        "octagon",
        "nonagon",
        "decagon",
        "hendecagon",
        "dodecagon"
    };

    /* Input variable */
    int numSides = 0;

    /* Assume that the input is invalid. */
    int sidesValid = 0;

    /* Input variable */
    int askAgain = 0;

    do {
        do {
            sidesValid = 0;

            /* Ask for the number of sides. */
            printf(howManySides);

            /* Get the user input. */
            scanf("%i", &numSides);

            /* Handle invalid input. */
            if(numSides < 0) {
                printf("The number of sides cannot be negative...\n");
            }
            else if(numSides == 0) {
                printf("The number of sides cannot be zero...\n");
            }
            else if(numSides < 3) {
                printf("The shape must have at least 3 sides...\n");
            }
            else sidesValid = 1;
        }
        while(sidesValid == 0);

        /* If we have a name for this polygon, then get it from the table. */
        if(numSides <= 12) {
            char shapeName[128];
            /* Don't forget to copy the string terminator, so add 1 to the
               copy length. */

            memcpy(shapeName, names[numSides - 3],
                strlen(names[numSides - 3]) + 1);

            printf("The sum of the interior angles of a %s is %i\n", shapeName,
                getSumInteriorAngles(numSides));
        }

        /* If we don't have a special name for this polygon, then just refer
           to it non-specifically. */

        else if(numSides > 12) {
            printf("The sum of the interior angles of this polygon is %i\n",
                getSumInteriorAngles(numSides));
        }

        printf(askAnotherShape);
        scanf("%i", &askAgain);

    } while(askAgain);

    return 0;
}

The most serious problem I see here is failing to check the return value from scanf().  Never ignore that - it's the only way we have to determine whether conversions were successful.

A less serious issue is that we we're using a main() that takes command-line arguments, but never use them.  Prefer instead the no-argument version:

int main(void)

The copying of the shape's name isn't useful - we can just pass the pointer directly to printf():

        printf("The sum of the interior angles of a %s is %i\n",
               names[numSides - 3],
               getSumInteriorAngles(numSides));

We could even choose which to print by using a suitable pointer:

const char* names[] = {
    "a triangle",
    "a quadrilateral",
    "a pentagon",
    "a hexagon",
    "a heptagon",
    "an octagon",
    "a nonagon",
    "a decagon",
    "a hendecagon",
    "a dodecagon"
};

// ...

const char *name = "this polygon";
if (numSides <= 12) {
    name = names[numSides - 3];
}

printf("The sum of the interior angles of %s is %i\n",
       name, getSumInteriorAngles(numSides));

This also stops the code printing the ungrammatical "a octagon". :-)


Updating a field's change I'm writing simple jQuery script for updating one field on another field's change. It's working, but I'm trying to write good code and use best practices. Can you please tell me what my mistakes and 'bad' things in this code?

Maybe I should use getTotalUserScore() in updateTotalUserScore(), not in $('.js-question-score').change() function, or I shouldn't use 2 functions for so simple piece of logic, or my variables names are bad. Tell me anything you feel about this code. Also, I've realized that my total-score field sometimes updates with delay.

$('.js-question-score').change(function () {
    var totalScore = getTotalUserScore();
    updateTotalUserScore(totalScore);
});

function updateTotalUserScore(totalScore) {
    $('.js-total-user-score').text(totalScore)
}

function getTotalUserScore() {
    var totalScore = 0;
    $('.js-question-score').each(function() {
        totalScore += parseInt($(this).val());
    });
    return totalScore;
}

<form>
  <input class="js-question-score" name="first" min="0" max="20"  value="0" type="number"/>
  <input class="js-question-score" name="second" min="0" max="20"  value="0" type="number"/>
  <input class="js-question-score" name="third" min="0" max="20"  value="0" type="number"/>
</form>

  User total score:

    0

You can do it in only one event. Get sums of all value on each changement like this :

$(".js-question-score").change(function(){
  var res=0;
  $(".js-question-score").each(function()
  {
    res+=parseInt($(this).val());
  });
   $(".js-total-user-score").text(res);
});

<form>
  <input class="js-question-score" name="first" min="0" max="20"  value="0" type="number"/>
  <input class="js-question-score" name="second" min="0" max="20"  value="0" type="number"/>
  <input class="js-question-score" name="third" min="0" max="20"  value="0" type="number"/>
</form>

  User total score:

    0


Sum of all numbers whose remainder and quotient are the same This prints out the sum of the numbers that have same remainder and quotient according to the input number. It's an online judge problem, and it currently throws a time-out since it's taking too long. Yes, the number being divided could be any positive integer.

Are there any suggestions on speeding up this code? And are there any small tips or good practice habits that makes Python code faster and more memory efficient?

import sys
n = input()
result = 0
for num in xrange(1, sys.maxint):
    if num % n == num / n:
        result += num

    if num / n > n:
        break
print result

Since this is Code Review, here's a clean-up of your code:

from itertools import count

def sum_numbers_with_equal_quot_rem(divisor):
    result = 0

    for num in count(1):
        if num % divisor == num // divisor:
            result += num

        if num // divisor > divisor:
            break

    return result

sum_numbers_with_equal_quot_rem(40)
#>>> 31980

sys.maxint is the largest int, but not the largest integer, so your loop was broken (although it would have taken a while to find out). This fixes that and makes it Python 3 compatible.

Now, let's add some introspection:

if num % divisor == num // divisor:
    result += num
    print(num % divisor, num // divisor)

and for divisor = 40 we get

1 1
2 2
3 3
4 4
5 5
...
37 37
38 38
39 39

This makes sense as num is fully determined by its divisor and remainder and there must be at least one number with divisor = remainder = k for k in range(1, divisor). There isn't one for 0 or less and there isn't one for divisor or more. This suggest we use:

def sum_numbers_with_equal_quot_rem(divisor):
    result = 0

    for quot_rem in range(1, divisor):
        num = quot_rem * divisor + quot_rem
        result += num

    return result

where num = quot_rem * divisor + quot_rem comes from

help(divmod)
#>>> Help on built-in function divmod in module builtins:
#>>>
#>>> divmod(...)
#>>>     divmod(x, y) -> (div, mod)
#>>>
#>>>     Return the tuple ((x-x%y)/y, x%y).  Invariant: div*y + mod == x.
#>>>

Namely, the invariant div * y + mod == x.

This gives us an even better option, if we look carefully.

Compare these:

k * divisor + k
(k+1) * divisor + (k+1)

They differ by 1 * divisor + 1. So we are adding up:

$$
1 \, (\text{divisor} + 1) + 2 \, (\text{divisor} + 1) + \cdots + (\text{divisor} - 1)(\text{divisor} + 1)
$$

Letting \$\text{divisor} = d\$, this is just

$$
(d + 1) \sum_{k=1}^{d - 1} k = (d + 1) \left( \frac{1}{2} (d-1) \, d \right) = \frac{d^3 - d}{2}
$$

giving the much simpler

def sum_numbers_with_equal_quot_rem(divisor):
    return (divisor**3 - divisor) // 2

sum_numbers_with_equal_quot_rem(40)
#>>> 31980


Refactor multiple if-else conditions when condition is a minor change I'm new programmer and I'm working on Xamarin MVVM app and I have a pin view like

So, basically  I have numbers from 0-9 if you pick one number its visible then if you pick a second one first one changed to * and I store all numbers into string called PinCode
ViewModel Code:
  public string PinCode { get; set; } = string.Empty;

        private async Task<bool> SelectedButton(Button button)
        {
            //If button is a number then
            if (button.Text != null)
            {
                if (PinCode.Length < 4)
                {
                    PinCode = PinCode + button.Text;

                    //Assign every number field text depending of PinCode length
                    if (PinCode.Length == 1)
                    {
                        PinNumberOne = button.Text;
                    }
                    else if (PinCode.Length == 2)
                    {
                        PinNumberTwo = button.Text;
                        PinNumberOne = "*";
                    }
                    else if (PinCode.Length == 3)
                    {
                        PinNumberThree = button.Text;
                        PinNumberOne = "*";
                        PinNumberTwo = "*";
                    }
                    else if (PinCode.Length == 4)
                    {
                        PinNumberFour = button.Text;
                        PinNumberOne = "*";
                        PinNumberTwo = "*";
                        PinNumberThree = "*";
                    }
                }
            }
            // if it's backspace button then
            else
            {
                PinCode = PinCode.Remove(PinCode.Length - 1);

                if (PinCode.Length == 3)
                {
                    PinNumberFour = "_";
                }
                else if (PinCode.Length == 2)
                {
                    PinNumberFour = "_";
                    PinNumberThree = "_";
                }
                else if (PinCode.Length == 1)
                {
                    PinNumberFour = "_";
                    PinNumberThree = "_";
                    PinNumberTwo = "_";
                }
                else if (PinCode.Length == 0)
                {
                    PinNumberFour = "_";
                    PinNumberThree = "_";
                    PinNumberTwo = "_";
                    PinNumberOne = "_";
                }
            }
            return true;
        }

As you can see, I have a lot of repeated code and I know it's possible to improve this method much better, but I can not find the way. If you guys have an idea of a recursion or how can I refactor this code to do something much clear and clean I really appreciate it. Regards

This is a common problem. Sometimes it's tricky to generalise a process even when its iterative in nature.
By keeping track of both:

the location of the user's cursor (cursor_position), and
an iterable data structure storing which characters are shown (pinNumbers),

it is possible to implement this behaviour without so much repeated code. This gave the desired behaviour after some thorough testing (in my imagination).
public string PinCode { get; set; } = string.Empty;

private int cursor_position = -1;
private string[] pinNumbers = new string[] {"_", "_", "_", "_"};

    private async Task<bool> SelectedButton(Button button)
    {
        //If button is a number then
        if (button.Text != null)
        {
            if ( cursor_position < 3 )
            {
                PinCode = PinCode + button.Text;

                if ( cursor_position > 0 )
                {
                    pinNumbers[cursor_position] = "*";
                }

                cursor_position += 1;
                pinNumbers[cursor_position] = button.Text;
            }
        } else { // apparently backspace?
            if ( cursor_position >= 0 )
            {
                PinCode = PinCode.Remove(PinCode.Length - 1);
                pin_numbers[cursor_position] = "_";
                cursor_position -= 1;
            }
        }

        pinNumberOne   = pinNumbers[0]; // update actual shown values with array
        pinNumberTwo   = pinNumbers[1]; // you could probably take out the "pinNumberX" middleman
        pinNumberThree = pinNumbers[2]; // but I dont know what relation these variables have to
        pinNumberFour  = pinNumbers[3]; // the rest of your code

        return true;
    }

Depending on how you are assigning values to the actual rendered characters you may be able to remove the first four of the last five lines. You likely could just refer the rendered components directly to the values in the pinNumbers array.


Quick Sort on C++ vector I've seen other implementations around, but they seem pretty complicated. This seems to work for me, but is there anything I'm missing? Also, any tips on how I can improve the quality of the code is appreciated.

#include <iostream>
#include <vector>

template<typename Iter>
void quickSort(std::vector<typename Iter::value_type>& vec, Iter left,
    Iter right) {

    auto size = std::distance(left, right);
    if (size <= 1) {
        return;
    }
    auto pivot = std::next(right, -1);
    if (size == 2 && *pivot < *left) {
        std::iter_swap(left, pivot);
    }
    auto wall = left;
    auto curr = left;

    while (curr != right) {
        if (*curr < *pivot) {
            std::iter_swap(wall, curr);
            wall++;
        }
        curr++;
    }

    std::iter_swap(pivot, wall);
    quickSort(vec, left, wall);
    quickSort(vec, wall + 1, right);

}

int main() {
    std::vector<int> myVec = { 6, 5, 2, 3, 2, 4, 34, 2434, 251, 4, 12, 4, 5,
        634, 523, 5, 4, 353, 3, 5, 345, 7, 86786, 8, 7, 9, 1 };
    quickSort(myVec, myVec.begin(), myVec.end());
    return 0;
}

You don't need to pass the vector itself. left and right iterators provide all the necessary information.

That said, you pass a correct right: too often people pass it as .end() - 1 which indeed leads to unnecessary complications.
Partitioning is an important algorithm on its own right and deserves to be factored out.
The lines

if (size == 2 && *pivot < *left) {
    std::iter_swap(left, pivot);
}

serve no purpose. The code works fine without them.
The trickiest part is achieving best performance.

The poor choice of pivot may result in quadratic time complexity. In a professional implementation choosing pivot is the most complex part.
In general, C++ is very good in eliminating tail recursion, but in this particular case it may use some help. Specifically, you'd want to recurse into smaller partition, and iterate over the larger one.
Another optimization is a timely switch to insertion sort. Instead of descending all the way to size <= 1 it is beneficial to stop recursion earlier (say, when size <= 16). Once the recursions are completed, the range is almost sorted, and insertion sort runs in linear time.


how to efficiently draw line numbers in edittext ondraw without causing lag in scrolling I have an implementation of an edit text to show line numbers and also highlight the current line.
I think it needs more optimization because when I scroll through the edit text with about 500 lines of code, it lags, yet when I comment out the code, the scrolling is smooth.
I need help optimizing this onDraw method in which I do everything above.
The line numbers appear on the right with a custom separator e.g
  1|
...
 10|
..
100|
..

public class NumberedEditText extends AppCompatEditText {

  private Rect nRect;
  private Rect nRect_;
  private int nOffset;
  private Paint nPaint;
  private Paint nPaint_;
  private boolean nShow;
  private int nAlignment;
  private char nSeparator;
  private boolean nLineHlt;

  public NumberedEditText (Context context, AttributeSet attrs) {
    super (context, attrs);

    nRect = new Rect ();
    nRect_ = new Rect ();
    nPaint = new Paint ();
    nPaint_ = new Paint ();

    nAlignment = NUMBER_ALIGNMENT_LEFT_SEP_LEFT;
    nSeparator = '|';
    nLineHlt = true;
    nShow = true;
    nOffset = 2;

    nPaint_.setColor (Color.parseColor ("#222222"));

    nPaint.setColor (Color.LTGRAY);
    nPaint.setStyle (Paint.Style.FILL);
    nPaint.setTypeface (Typeface.MONOSPACE);
  }

  @Override
  public void onDraw (Canvas canvas) {
  // I use short variable names because i think
  // they are executed much faster.
    if (nShow) {
      nPaint.setTextSize (getTextSize () - nOffset);

      int bl = getBaseline ();
      int lc = getLineCount ();
      int lh = getLineHeight ();

      // i use an array here for efficient
      // random access to store the
      // positions where the line numbers
      // will be according to the number
      // of lines
      String lcs = Integer.toString (lc);
      int nl = lcs.length ();
      char[] ch = new char[nl + 1];
      int cl = ch.length;
      String s;

      // i fill the array with spaces
      // ready to be replaced with numbers
        for (int i = 0; i < nl; i++)
          ch [i] = 32;
       // set the last character to the separator
        ch [nl] = nSeparator;
        // now i loop thru all the numbers
        // replacing the characters in the array
        // (with spaces) with numbers
        // to form something like
        // '   1|'
        // '  10|'
        // ' 100|'
          for (int i = 1, j = cl - 2; i <= lc; i++, bl += lh) {
            if (i == 10 || i == 100 || 1 == 1000 || i == 10000 || i == 100000)
              j--;
            s = Integer.toString (i);
            if (j > -1)
              s.getChars (0, s.length (), ch, j);

            canvas.drawText (ch, 0, cl, nRect.left, bl, nPaint);
          }
          // now i set the padding and an
          // additional margin
      setPadding ((int) nPaint.measureText (lcs + "__"), getPaddingTop (), getPaddingRight (), getPaddingBottom ());
    }

    // this is to highlight the current line
    Layout l = getLayout ();
    if (nLineHlt && l != null) {
      int ln = l.getLineForOffset (getSelectionStart ());
      getLineBounds (ln, nRect_);
      canvas.drawRect (nRect_, nPaint_);
    }
    super.onDraw (canvas);
  }
}
```

// I use short variable names because i think
// they are executed
much faster.

You are wrong. Execution speed does not depend on the length of identifiers anyhow.

// i use an array here for efficient
// random access to store the
// positions where the line numbers
// will be according to the number
// of lines

As usual the use of (primitive) arrays leads to procedural solution involving al lot of looping. Unnecessary looping is the most common performance issue.

   // now i loop thru all the numbers
   // replacing the characters in the array
   // (with spaces) with numbers

Instead of looping you should calculate what you need.
Because of your poor naming I'm not analyzing in detail what your code really does, but it looks like in mainly calculates the indentation needed.
Given the number of the current line and the overall line count I would do it this way:
 // this could also be a member variable
 // so that it does not need to be
 // calculated each time the draw method runs
 int indentedNumberWith =
           String.valueOf(lineCount).length();

 String indentedLineNumber =
           String.format(
                         "% "+indentedNumberWith+"s",
              // note this ^ space
                         String.valueOf(lineNumber));

This should replace the loop with the if cascade...


Number of HTML classes for one element My project uses at least 100 helper classes like the below:

.pa5 {padding:5px} .pa10 {padding:10px} /* ... */
.pt5 {padding-top:5px} .pt10 {padding-top:10px} /* ... */
.pr5 {padding-right:5px} .pr10 {padding-right:10px} /* ... */
 /* ... */

It comes handy in some cases like
...
 but I think we overused it or, even worse, misused it.

For example, the below  tag contains 8 classes to define how it display (background, background hover, padding, border radius, diplay inline block, margin and float left). It looks like the way you use style="..." to inline style it:

In other case, even when we had the class profileInfo, we still use it with a lot of helper classes:

I think this is a worse practice and could cause significant performance overhead on page rendering. Since I haven't worked a lot with CSS, I could not explain it clearly to make us change the way of using it. What do you think? Does your project have an explicit rule to limit the number of CSS classes for one element?

The use of that many classes is a sign of incorrect CSS usage, but not because of performance.  I doubt eight or ten classes on an element will be a noticeable performance issue.

I've never heard of an explicit limit on the number of classes assigned to an element, but that may due to a common understanding that it is not correct to create a class for every visual attribute.

An important (and possibly the primary) purpose of CSS is to associate rendering characteristics with conceptual (semantic) markup.  Classes should be named for the concepts they represent, not their appearance.  Like this:

.tableofcontents {
    padding-left: 10px;
    padding-top: 5px;
}

If the goal is to avoid duplication (though I honestly don't think duplicating a few lines of declarations in various blocks is harmful), one can assign multiple conceptual classes to a block:

.menu,
.logo,
.heading,
.tableofcontents {
    padding-top: 5px;
}

.menu,
.tableofcontents {
    padding-left: 10px;
}


The multilingual duck So, we have this "Rubberduck Settings" window, with a number of "tabs" dedicated to each configurable feature. The "root" tab contains the application's "general settings" and, in the next release, we've found something to put there:

The display language! Credits to @SimonAndréForsberg and @Vogel612, for the Swedish and German translations respectively.. I did the French translation myself.

The first thing I tried after Simon's pull request was merged, was of course trying to switch the display language to Swedish... and that blew up with a CultureNotFoundException.

I had to handle the case when a culture wasn't supported/installed on the user's system, so this is how I went about the DisplayLanguageSetting class, which gets serialized into the application's xml configuration file:

namespace Rubberduck.Settings
{
    [XmlType(AnonymousType = true)]
    public class DisplayLanguageSetting
    {
        [XmlAttribute]
        public string Code { get; set; }

        public DisplayLanguageSetting()
        {
            // serialization constructor
        }

        public DisplayLanguageSetting(string code)
        {
            Code = code;

            CultureInfo culture;
            try
            {
                culture = CultureInfo.GetCultureInfo(code);
                _exists = true;
            }
            catch (CultureNotFoundException)
            {
                culture = RubberduckUI.Culture;
                _exists = false;
            }

            _name = RubberduckUI.ResourceManager.GetString("Language_" + Code.Substring(0, 2).ToUpper(), culture);
        }

        private readonly string _name;
        private readonly bool _exists;

        [XmlIgnore]
        public string Name { get { return _name; } }

        [XmlIgnore]
        public bool Exists { get { return _exists; } }

        public override bool Equals(object obj)
        {
            var other = (DisplayLanguageSetting) obj;
            return Code.Equals(other.Code);
        }

        public override int GetHashCode()
        {
            return Code.GetHashCode();
        }
    }
}

So the "general settings" tab is only loading the instances where the Exists getter is returning true. I'm particularly not fond of what I did in the LoadLanguageList method... would there be a better way?

namespace Rubberduck.UI.Settings
{
    public partial class GeneralSettingsControl : UserControl
    {
        private readonly IGeneralConfigService _configService;
        private readonly DisplayLanguageSetting _currentLanguage;

        public GeneralSettingsControl()
        {
            InitializeComponent();
            TitleLabel.Text = RubberduckUI.SettingsCaption_GeneralSettings;
            LanguageLabel.Text = RubberduckUI.Settings_LanguageLabel;

            LoadLanguageList();
        }

        public GeneralSettingsControl(DisplayLanguageSetting displayLanguage, IGeneralConfigService configService)
            : this()
        {
            _configService = configService;
            _currentLanguage = displayLanguage;
            LanguageList.SelectedItem = displayLanguage;
        }

        public DisplayLanguageSetting SelectedLanguage
        {
            get { return (DisplayLanguageSetting)LanguageList.SelectedItem; }
        }

        private void LoadLanguageList()
        {
            var settings = new[]
            {
                new DisplayLanguageSetting("en-US"),
                new DisplayLanguageSetting("fr-CA"),
                new DisplayLanguageSetting("sv-SV")
            };

            LanguageList.Items.AddRange(settings.Where(item => item.Exists).ToArray());
            LanguageList.DisplayMember = "Name";
        }
    }
}

I would group all of your properties together. Right now you have Code separated from the other properties by the ctors.
When overriding Equals, it's important to be sure that it can't throw an exception. The current implementation can throw an InvalidCastException.

    public override bool Equals(object obj)
    {
        var other = (DisplayLanguageSetting) obj;
        return Code.Equals(other.Code);
    }

A proper implementation would look something more like this.

public override bool Equals(object obj)
{
    var other = obj as DisplayLanguageSetting;

    if (other == null)
        return false;

    return Code.Equals(other.Code);
}

I don't much care for the control either.

 public partial class GeneralSettingsControl : UserControl

That's more or less all I need to see to know I don't care for it. How's that? Because that's all I need to know about the class to know that business logic is being put into the code behind instead of a presenter class. We've got a "smart" form on our hands here. Invert the control. This should be implementing an interface that exposes properties for the different view elements.

public interface IGeneralSettingsView
{
    DisplayLanguageSetting CurrentSetting {get; set;}
    IList<DisplayLanguageSetting> {get; set;}
}

Note that once you do this, the class no longer needs a Configuration Service at all. (The presenter will still need it though).


Function to perform checks on data I'm currently trying to do data quality checks in python primarily using Pandas.
The code below is fit for purpose. The idea is to do the checks in Python and export to an Excel file, for audit reasons.
I perform checks like the below in 4 different files, sometimes checking if entries in one are also in the other, etc.
df_symy is just another DataFrame that I imported inside the same class.

I'm wondering if there's a better way to do this task, for example:

create a function for each check
create a more meaningful way to check the data and present the results, etc

def run_sharepoint_check(self) -> pd.DataFrame:
        """Perform checks on data extracted from SharePoint.

        Check 1: Amount decided is 0 or NaN
        Check 2: Invalid BUS ID
        Check 3: Duplicated entries. It also tags the latest entry
        Check 4: SYMY Special not in SharePoint
        """
        # Use this function to check the data we have from SharePoint
        # and return DF with checks in the excel file
        # Another function will be used to process the data and produce
        # the final list
        df = self.sharepoint_data.copy()

        df_symy = self.symy_data
        df_symy = df_symy.loc[df_symy['COMMITMENT_TYPE'].str[0] == 'S']

        symy_cols = ['BUYER_NUMBER', 'Balloon ID',
                     'CLD_TOTAL_AMOUNT', 'POLICY_CURRENCY']

        df = df.merge(right=df_symy[symy_cols],
                      how='outer',
                      left_on=['Balloon ID', 'BUS ID'],
                      right_on=['Balloon ID', 'BUYER_NUMBER'])

        check_1 = df['Amount decided'] == 0 | df['Amount decided'].isna()
        df.loc[check_1, 'check_1'] = 'Amount decided is 0 or NaN'

        check_2 = df['BUS ID'].isna()
        df.loc[check_2, 'check_2'] = 'Invalid BUS ID'

        check_3 = df.duplicated(subset=['Balloon ID', 'BUS ID'], keep=False)
        df.loc[check_3, 'check_3'] = 'Duplicated entry'

        check_3_additional = ~df.duplicated(
            subset=['Balloon ID', 'BUS ID'], keep='first'
        )
        # Filter only the entries that are duplicated
        # Out of those, the first one is the latest
        df.loc[(check_3) & (check_3_additional),
               'check_3'] = 'Duplicated entry (Latest)'

        # Match Balloon+BUSID (SYMY) to SharePoint
        check_4 = (~df.duplicated(subset=['Balloon ID', 'BUS ID'],
                                  keep='first')) & (df['BUS ID'].isna())
        df.loc[check_4, 'check_4'] = 'SYMY SA not in SharePoint'

        check_cols = ['check_1', 'check_2', 'check_3', 'check_4']

        # .fillna('OK') just for visual purposes in Excel.
        df[check_cols] = df[check_cols].fillna('OK')

        # self.data_checks_dfs in the init method is an empty dictionary.
        self.data_checks_dfs['SharePoint_checks'] = df

        return df

So, how can I improve this?
Does anyone have this type of task that has been automated using Python?

It seems difficult to comment on the meaningfulness of your proposed solution. You are the one to decide on that since you know the context around your problem. With that being said, I'm afraid there is not much to go on here.
Nevertheless, one observation I make about your code is that the checks use .loc to locate rows that receive a "non-OK", and finally, after all checks, you use a fill to turn nans into "OK". To make your code more concise and likely faster, consider using np.where. So for instance, your check_1 would be turned into:
df["check_1"] = np.where(
   (df['Amount decided'] == 0) | (df['Amount decided'].isna()),
   "Amount decided is 0 or NaN", "OK")

Also, I would rename check_N into something that actually means a little more, like amount_check for check_1 and so on.


Reading two .dat files for calculating location distance I'm creating a program that reads data from two .dat files: one for food-bank location and the other for residence.  Right now, the program can calculate the distance between food-bank and residence and insert it into a distances vector.

My concerns are:

Is my logic right?
Can I determine the distance, in miles, between each location in residences and the closest food bank in foodbanks?  Note: the function will calculate in meters.

#include <iostream>
#include <math.h>
#include <fstream>
#include <vector>
#include<iterator>

using namespace std;

struct Foodbank
{
    double x;
    double y;

    //input stream overload
    friend std::istream &operator>>(std::istream &is, Foodbank &f) {
        return is>>f.x>>f.y;
    }
    //output stream overload
        friend std::ostream &operator<<(std::ostream &os, Foodbank const &f) {
            return os << f.x  << "\t"<< f.y<<"\t";

        }
};
struct Residence
{
    double x;
    double y;
    //input stream overload
    friend std::istream &operator>>(std::istream &is, Residence &r) {
        return is>>r.x>>r.y;
    }
    //output stream overload
    friend std::ostream &operator<<(std::ostream &os, Foodbank const &r) {
        return os << r.x  << "\t"<< r.y<<"\t";

    }
};

double distanceCalculate(double x1, double y1, double x2, double y2);
int main(int argc, const char * argv[])
{

    ifstream foodbankFile("foodbanks.dat");
    ifstream residenceFile("residences.dat");

    //vectors data
    vector<Foodbank> foodbankData;
    vector<Residence> residenceData;
    vector<double> distances;

    copy(std::istream_iterator<Foodbank>(foodbankFile),
         std::istream_iterator<Foodbank>(),
         std::back_inserter<std::vector<Foodbank>>(foodbankData));
      std::cout<<"read "<<foodbankData.size()<<" data: "<<std::endl;

    copy(std::istream_iterator<Residence>(residenceFile),
         std::istream_iterator<Residence>(),
         std::back_inserter<std::vector<Residence>>(residenceData));

    std::cout<<"read "<<residenceData.size()<<" data: "<<std::endl;
    for(auto r:residenceData)
    {
        for(auto f:foodbankData)
        {
            //std::cout<<f.x <<", "<<f.y <<", "<<std::endl;
            distances.push_back(distanceCalculate(f.x,f.y,r.x,r.y));
        }
        //std::cout<<r.x <<", "<<r.y <<", "<<std::endl;
    }

     std::cout<<"read "<<distances.size()<<" data: "<<std::endl;
    return 0;
}

// the distance in metres
double distanceCalculate(double x1, double y1, double x2, double y2)
{
    double x = x1 - x2;
    double y = y1 - y2;
    double dist;

    dist = pow(x,2)+pow(y,2);           //calculating distance by euclidean formula
    dist = sqrt(dist);                  //sqrt is function in math.h

    return dist;
}

Stop doing this:

using namespace std;

Yes every text book does this. But once you start writing programs more than 10 lines long it causes issues. Prefix items in the standard namespace with std:: (it was designed to be short). See detailed explanation here: Why is using namespace std considered bad practice

OK. What is the difference between Foodbank and Residence looks like they should simply be a Location class. If in later code you you add differences you can derive from the Location (or better use a location member (prefer composition over inheritance)).

Be consistent (and neat with your spacing).

return os << r.x  << "\t"<< r.y<<"\t";
//                     ^^^      ^^^      Hard to read

return os << r.x  << "\t" << r.y << "\t";

When initializing your arrays. No need to use std::copy. Just use the iterators in the vectors constructor.

vector<Foodbank> foodbankData;
copy(std::istream_iterator<Foodbank>(foodbankFile),
     std::istream_iterator<Foodbank>(),
     std::back_inserter<std::vector<Foodbank>>(foodbankData));

// Can be replaced by a single line:
vector<Foodbank> foodbankData(std::istream_iterator<Foodbank>(foodbankFile),
                              std::istream_iterator<Foodbank>());

Not sure why you are pushing the distance into a long single array. That does not help you work stuff out.

for(auto r:residenceData)
{
    for(auto f:foodbankData)
    {
        //std::cout<<f.x <<", "<<f.y <<", "<<std::endl;
        distances.push_back(distanceCalculate(f.x,f.y,r.x,r.y));
    }
}

I would suspect that what you want is a 2D array. The FoodBanks on the Y axis and the Residences on the X axis (as an example). Then given any Residences  you just need to scan the array vertically to find the closest FoodBank.

std::vector<std::vector<double>>   distance;
for(auto r:residenceData)
{
    distance.push_back(std::vector<double>());

    for(auto f:foodbankData)
    {
        //std::cout<<f.x <<", "<<f.y <<", "<<std::endl;
        distances.back().push_back(distanceCalculate(f.x,f.y,r.x,r.y));
    }
}

Wow thats a lot of parameters

double distanceCalculate(double x1, double y1, double x2, double y2)

If your FoodBank and Residence just inherited from a base of Location (or had a location member) then you can pass location as the start and end point and calculate the distance between them.


Create AsyncCursor wrapper Is this good OOP style? Do I need to split this class into two classes? If so, how do I do this? What methods should I need to add to the interfaces?

public sealed class DeferredResultCollection<TResult> : IEnumerable<TResult>, IDisposable
{
    private readonly IAsyncCursor<TResult> _asyncCursor;
    private readonly IList<TResult> _list;

    public DeferredResultCollection(IAsyncCursor<TResult> asyncCursor)
    {
        _asyncCursor = asyncCursor;
    }

    public DeferredResultCollection(IList<TResult> list)
    {
        _list = list;
    }

    public IEnumerator<TResult> GetEnumerator()
    {
        if (_list != null)
        {
            foreach (var item in _list)
            {
                yield return item;
            }
        }

        if (_asyncCursor != null)
        {
            for (; _asyncCursor.MoveNextAsync().Result;)
            {
                foreach (var result in _asyncCursor.Current)
                {
                    yield return result;
                }
            }
        }
    }

    IEnumerator IEnumerable.GetEnumerator()
    {
        return GetEnumerator();
    }

    public void Dispose()
    {
        if (_asyncCursor != null)
        {
            _asyncCursor.Dispose();
        }
    }
}

Is it good OOP style?

No. The class contains a IAsyncCursor<TResult> field and a IList<TResult> field,
but depending on how it's constructed,
only one of these will be actually used.
This is a pretty clear indication that we have two classes masquerading as one.

  Maybe I need to split this class into two classes?

You hit the nail on the head!

  How to do this? What methods should I need to take out to the interfaces?

The interface should contain GetEnumerator and that's it.
That's the common functionality.
Let's call the interface DeferredResultCollection<TResult>.

Next, add two classes that implement this interface,
one using a IAsyncCursor<TResult> and the other a IList<TResult>,
let's call these CursorBasedDeferredResultCollection<TResult> and ListBasedDeferredResultCollection<TResult>.

Optionally, you could add a factory,
let's call it DeferredResultCollections,
with two methods, such that both return objects of the interface type DeferredResultCollection<TResult>,
but one of them takes a IAsyncCursor<TResult> and the other a IList<TResult> as parameter,
and create and return objects with the appropriate implementations.
This has the benefit that users don't need to learn the names of the implementation classes.
In fact they don't even need to know them at all (they can be private).


Module for sending messages through PubNub I have created a Drupal 6 module that sends messages through PubNub when the user updates information about a Node. I have a separate app that, when launched, calls my Drupal site and receives a JSON array containing a lot of information about some nodes. I then update the info in my other app by PubNub->publish from Drupal every time the page is reloaded after the Node is updated. I have the following in my foo.module file:

hook_admin: //not really a hook

function moduleName_admin(){
  //Here we are pulling in the Pubnub.php file from the module directory
  // NOTE: This file requires PubnubAES.php even when you aren't using encryption so include it too
  module_load_include('php', 'moduleName', 'Pubnub');
  $pubnub = new Pubnub(
    "PUBKEY",
    "SUBKEY",
    "",
    false
  );
  $form = array();
  $query = "SELECT info FROM tables;"

  $results = db_query($query);

  while($r=db_fetch_object($results)){
    $form['moduleName_fieldDescription_'.$r->nid] = array(
    '#type' => 'radios',
    '#title' => "Select one for ".$r->info.":",
    '#options' => array(
      t('Foo'),
      t('Bar'),
      t('Baz')
    ),
    '#default_value' => variable_get('moduleName_fieldDescription_'.$r->nid, '0'),
  );

    $pubnub->publish(array(
      'channel' => 'channelName',
      'message' => array("id"=>$r->nid, "selected"=>variable_get('moduleName_fieldDescription_'.$r->nid, '0'))
    ));
  }
  return system_settings_form($form);
}

JSON page:

function moduleName_JSON(){
  $query = "SELECT info FROM table;"
  $results = db_query($query);

  $infoObject = array();
  for($i=0;$r = db_fetch_object($results); $i++){
    $infoObject[$i] = array(
      'info' => $r->info,
      'id' => $r->nid,
      'selected' => variable_get('moduleName_fieldDescription_'.$r->nid, '0'),
    );
  }
  //This next part will wrap the JSON in a function so jQuery can be called like
  //$.ajax( {
    //url:"example.com/moduleName/json",
    //dataType: 'jsonp',
    //callback: 'foo',
    //success:function(data){
      //checkListInfo = data;
      //styleSheetLoaded();
    //}
  //} );
  $callback = check_plain($_REQUEST['callback']);
  if (isset($callback) && $callback != '') {
    header("Content-type: text/javascript");
    echo $callback ."(";
    drupal_json($infoObject);
    echo ");";
  } else {
    drupal_json($infoObject);
  }
}

hook_menu:

function moduleName_menu(){
  $items = array();
  $items['moduleName/editPage'] = array(
    'title' => 'Module edit page',
    'description' => 'Edit this page to make changes to pubnub',
    'page callback' => 'drupal_get_form',
    'page arguments' => array('moduleName_admin'),
    'access arguments' => array('Edit moduleName Permissions'),
    'type' => MENU_NORMAL_ITEM,
  );
  $items['moduleName/json'] = array(
    'title' => 'JSON Array containing module info',
    'page callback' => 'moduleName_JSON',
    'access arguments' => array('Change moduleName values'),
    'tpye' => MENU_CALLBACK,
  );
  return $items;
}

This is slightly changed up code from my working Drupal 6 module that uses a live PubNub account. It works great for my purpose. I just want to know how to do it a little more to Drupal standards.

I know I probably have the publish time wrong. Right now it publishes when the page is reloaded instead of right before the page sends the info. I don't think this is right but I am very new to Drupal.

There is a Drupal Module that is in the process of being built that will allow PubNubs integration in Drupal 7/8 but I am on D6 and I am just including the files myself.

If you are receiving the data you expect, I think you are 99% there.

Alternatively, you could include the PubNub JS client into your Drupal app (vs. the PHP client), then you can publish asynchronously, or even completely after page load.

Link to the JS client and docs are available here, with a specific example of how to publish here


UpdateGame function for Tetris I am writing a Tetris game while learning Haskell and I'm trying to make the following code more concise and easier to read. How can I improve it?

Note that the function is not finished yet (There is no check for complete lines, for instance); I currently want to improve its readability.

updateGame :: IORef InputsData -> IORef GameState -> IO()
updateGame keysIORef gameIORef = do
    game <- readIORef gameIORef
    keys <- readIORef keysIORef
    newTime <- getCurrentTime
    elapsed <- getMilliseconds $ diffUTCTime newTime (time game)

    -- Updating current time
    modifyIORef gameIORef (\g -> g {time = newTime})
    game <- readIORef gameIORef

    -- Updating keys cooldown
    modifyIORef gameIORef (\g -> g {keyCooldowns = updateCooldowns (keyCooldowns game) elapsed})
    game <- readIORef gameIORef

    -- Processing side movements
    when (not $ isOnCooldown (keyCooldowns game) (SpecialKey KeyRight)) $ do
        if right keys
          then when (canTetrominoMove (currentTetromino game) (Helpers.R) (tank game)) $ do
            modifyIORef gameIORef (\g -> g {currentTetromino = (currentTetromino g) {x = x (currentTetromino g) + 1}})
            modifyIORef gameIORef (\g -> g {keyCooldowns = addCooldown (keyCooldowns game) (SpecialKey KeyRight) 100})
          else if left keys
            then when (canTetrominoMove (currentTetromino game) (Helpers.L) (tank game)) $ do
              modifyIORef gameIORef (\g -> g {currentTetromino = (currentTetromino g) {x = x (currentTetromino g) - 1}})
              modifyIORef gameIORef (\g -> g {keyCooldowns = addCooldown (keyCooldowns game) (SpecialKey KeyRight) 100})
            else return ()
    game <- readIORef gameIORef

    -- Processing tetromino rotations
    when (not $ isOnCooldown (keyCooldowns game) (Char 'z')) $ do
        if xKey keys
          then when (canTetrominoRotate (currentTetromino game) (Helpers.R) (tank game)) $ do
            modifyIORef gameIORef (\g -> g {currentTetromino = rotateTetromino (currentTetromino g) Helpers.R})
            modifyIORef gameIORef (\g -> g {keyCooldowns = addCooldown (keyCooldowns game) (Char 'z') 200})
          else if zKey keys
            then when (canTetrominoRotate (currentTetromino game) (Helpers.L) (tank game)) $ do
              modifyIORef gameIORef (\g -> g {currentTetromino = rotateTetromino (currentTetromino g) Helpers.L})
              modifyIORef gameIORef (\g -> g {keyCooldowns = addCooldown (keyCooldowns game) (Char 'z') 200})
            else return ()
    game <- readIORef gameIORef

    -- Updating current tetromino falling time
    modifyIORef gameIORef (\g -> g {tetrominoFallingTime = (tetrominoFallingTime g) - elapsed})
    game <- readIORef gameIORef

    -- Making current tetromino fall & restarting current tetromino falling time
    when (tetrominoFallingTime game < 0) $ do
        modifyIORef gameIORef (\g -> g {tetrominoFallingTime = (tetrominoFallingTime g) + getTetrominoFallingSpeed (difficultyLevel g)})
        if canTetrominoFall (currentTetromino game) (tank game)
            then do
                modifyIORef gameIORef (\g -> g {currentTetromino = (currentTetromino g) {y = y (currentTetromino g) - 1}})
            else do
                modifyIORef gameIORef (\g -> g {tank = addTetrominoToTank (currentTetromino game) (tank game)})
                let (currentTetromino', tetrominoBag', rndGen') = drawTetrominoFromBag (rndGen game) (tetrominoBag game)
                modifyIORef gameIORef (\g -> g {currentTetromino = currentTetromino', rndGen = rndGen', tetrominoBag = tetrominoBag'})

    addTimerCallback 1 $ updateGame keysIORef gameIORef
    postRedisplay Nothing

You should separate IO actions from pure code. In your case the principal task of updateGame is to create the new game state given input data, and for this no IO is necessary. The fact that the state is stored and modified in an IORef.

It's not just the matter of style and code separation. Calling modifyIORef multiple times in the function means that you can easily get race conditions, if there is another thread working with the game state. It would get or modify an inconsistent state.

So my suggestion would be to split it into two parts - one a pure function, and another that does all the necessary IO:

updateGame :: UTCTime -> InputsData -> GameState -> GameState
updateGame time input game = ...

updateGameIO :: IORef InputsData -> IORef GameState -> IO ()
updateGameIO modifyIORef gameIORef = step
  where
    step = do
      time <- getCurrentTime

      keys <- readIORef keysIORef
      modifyIORef gameIORef (updateGame time keys)

      addTimerCallback 1 step
      postRedisplay Nothing

This way, you ensure that the game state is updated atomically, and keep the main function pure.

Updating a field in a complex data type is a common task, and it can be greatly simplified using the lens library. Instead of

\g -> g {keyCooldowns = updateCooldowns (keyCooldowns game) elapsed}

you let lens generate so-called lenses for your data type, so for example keyCooldowns won't be of type GameState -> Something, but Lens' GameState Something, and then the function that modifies the field inside GameState would be

over keyCooldowns (flip updateCooldowns elapsed)

See A Little Lens Starter Tutorial


Getting the outliers of both dates and values This is working now, but it looks very inelegant. I'm sure there is a nicer way of doing this.

# Get the outliers of both the dates and values
maxDate   = []
minDate   = []
maxVal    = []
minVal    = []
for k, v of series
  for d, i in v
    series[k][i][1] = nearZero if d[1] is 0
  series[k] = v = _.filter  v,    (d) -> not _.isNull d[1]
  maxDate.push  _.max       v,    (d) -> d[0]
  minDate.push  _.min       v,    (d) -> d[0]
  maxVal .push  _.max       v,    (d) -> d[1]
  minVal .push  _.min       v,    (d) -> d[1]
maxDate   = _.max _.map maxDate,  (d) -> d[0]
minDate   = _.min _.map minDate,  (d) -> d[0]
maxVal    = _.max _.map maxVal,   (d) -> d[1]
minVal    = _.min _.map minVal,   (d) -> d[1]

Some notes:

I know many programmers like it, but this kind of perfectly aligned code looks pretty weird to me. Maybe my problem is that it's usually a sign that there is a repeated pattern that could be abstracted but has been prettified instead.
maxDate = []. Using imperative programming in JS/CS is very common, indeed, but I'd definitely take a functional approach when doing maths or logic (not that I know of any coding that does not deal with maths or logic ;-)).
(d) -> d[0]: Note that you can de-structure arrays: ([x, y]) -> x.
_.map maxDate, (d) -> d[0]. In my opinion list-comprehensions are more declarative than maps that use lambdas: (d[0] for d in maxDate).
Don't reuse variables: Those 4 accumulators hold two completely different structures in the course of the computation, that's not a good practice. Not even in imperative programming.

As I said, I'd write it in functional style, inmutable variables all the way through. It's a pity that CS has no real list-comprehensions and the results of nested loops must be flattened, but we'll have to live with it (_.flatten(xs, true) comes in handy as a one-level flattener). It would look something like this (you say you really need the modified series, so I'll use mash from my mixin):

nested_date_value_pairs =
  for key, pairs of series
    for [date, val] in pairs when val isnt null
      [date, if val is 0 then nearZero else val]
modified_series = _.mash(_.zip(_.keys(series), nested_date_value_pairs))
dates_values = _.zip(_.flatten(nested_date_value_pairs, true)...)
[[minDate, maxDate], [minVal, maxVal]] = ([_.min(xs), _.max(xs)] for xs in dates_values)


Instagram widget feed function I've adapted an Instagram widget from this example so that it now appends 8 images to their respective IDs using an Instagram username. I want my widget implementation to be better and more efficient.  I'm very new to this so I'm sure there are parts that are no longer needed or that could be greatly improved.

var accessToken = ''; // Access token
var instagram = function () {
  return {
    getUserIDByUsername: function (strUser) { // By username
      var getUserURL = 'https://api.instagram.com/v1/users/search?q=' + strUser + '&access_token=' + accessToken + ''
      $.ajax({
        type: "GET",
        dataType: "jsonp",
        cache: false,
        url: getUserURL,
        success: function (data) {
          var getUserID = data.data[0].id;
          instagram.getUserDetailsByUserID(getUserID);
        }
      });
    },
    getUserDetailsByUserID: function (userID) { // By userid
      $.ajax({
        type: "GET",
        dataType: "jsonp",
        cache: false,
        url: "https://api.instagram.com/v1/users/" + userID + "/?access_token=" + accessToken + "",
        success: function (data) {
          if (data.meta && data.meta.code == "200") {
            //console.log(data);
            instagram.loadImages(userID);
          } else if (data.meta && data.meta.code == "400" && data.meta.error_message) {
            console.log(data.meta.error_message);
          }
        }
      });
    },
    loadImages: function (userID) { // Get user photos
      var getImagesURL = "https://api.instagram.com/v1/users/" + userID + "/media/recent/?access_token=" + accessToken + ""
      $.ajax({
        type: "GET",
        dataType: "jsonp",
        cache: false,
        url: getImagesURL,
        success: function (data) {
          console.log(data);
          size = data.data[0].images.standard_resolution.url;
          $("#insta-1").append("");
          size = data.data[1].images.standard_resolution.url;
          $("#insta-2").append("");
          size = data.data[2].images.standard_resolution.url;
          $("#insta-3").append("");
          size = data.data[3].images.standard_resolution.url;
          $("#insta-4").append("");
          size = data.data[4].images.standard_resolution.url;
          $("#insta-5").append("");
          size = data.data[5].images.standard_resolution.url;
          $("#insta-6").append("");
          size = data.data[6].images.standard_resolution.url;
          $("#insta-7").append("");
          size = data.data[7].images.standard_resolution.url;
          $("#insta-8").append("");
        }
      });
    }
  }
}();
$(document).ready(function () {
  var strUser = "saturdaysnyc"; // Instagram username
  if (isNaN(strUser)) {
    instagram.getUserIDByUsername(strUser);
  } else {
    instagram.getUserDetailsByUserID(strUser);
  }
});

first thing one could spot is this duplications:

size = data.data[0].images.standard_resolution.url;
          $("#insta-1").append("");
          size = data.data[1].images.standard_resolution.url;
          $("#insta-2").append("");
          size = data.data[2].images.standard_resolution.url;
          $("#insta-3").append("");
          size = data.data[3].images.standard_resolution.url;
          $("#insta-4").append("");
          size = data.data[4].images.standard_resolution.url;
          $("#insta-5").append("");
          size = data.data[5].images.standard_resolution.url;
          $("#insta-6").append("");
          size = data.data[6].images.standard_resolution.url;
          $("#insta-7").append("");
          size = data.data[7].images.standard_resolution.url;
          $("#insta-8").append("");

One could change it to something more condensed like:

for(var i=0; i<9; i++) {
    size = data.data[i].images.standard_resolution.url;
    $("#insta-"+i).append("")
}

Also creating new nodes multiple time is ineficient since browser will re-paint it every time, so you could change it to (I assume that #insta-X elements are just your way to hook it in some place in html, if that's the only reason you may ditch them and create ie.

)

links = "";
for(var i=0; i<9; i++) {
    size = data.data[i].images.standard_resolution.url;
    links += "";
}
$(".insta-container").append(links);

Also it looks like with size you accidentally declare global variable (window.size) - not a good idea - you need to make var size to make it local:

loadImages: function (userID) { // Get user photos
      var size, //declare local var, then use it as before
          getImagesURL = "https://api.instagram.com/v1/users/" + userID + "/media/recent/?access_token=" + accessToken + ""
      ;

Also you may want to read about hoisting to know those var ... declarations should be at the top of your function.


Hibernate - entity design with multiple dependencies I am currently working on an API and came across the following design.

    // not mandatory
    @ManyToOne
    @JoinColumn(name = "person_id")
    private Person person;

Actually, a telephone number should belong to a company, person or a contact form. However, this is logically not possible with the current design. So there is a small validation at the service level to make sure that there is always at least one dependency.

Now I was wondering if there is a better way to solve this. Because theoretically something can always go wrong and dependencies can be lost and the data would still be valid according to today's status.

I thought about creating a separate phone number object for each entity that inherits from the phone number. But somehow I don't really like the solution, but I think it's better than the previous one. What I don't like so much is that a lot more objects are created which also contain inheritance.

Is the new approach ok or how do you solve such problems?

I don't see anything wrong with your approach.

Another option is to add a PhoneNumberType to the PhoneNumber table.

In any case you should ensure the table has appropriate constraints added so that a PhoneNumber must relate to a table.