Running over a log file to extract log times I need to run over a log file with plenty of entries (25+GB) in Python to extract some log times.

The snippet below works.  On my test file, I'm ending up with roughly 9:30min (Python) and 4:30min (PyPy) on 28 million records (small log).

import datetime
import time
import json
import re

format_path= re.compile(
    r"^\S+\s-\s"
    r"(?P<user>\S*)\s"
    r"\[(?P<time>.*?)\]"
)

threshold = time.strptime('00:01:00,000'.split(',')[0],'%H:%M:%S')
tick = datetime.timedelta(hours=threshold.tm_hour,minutes=threshold.tm_min,seconds=threshold.tm_sec).total_seconds()
zero_time = datetime.timedelta(hours=0,minutes=0,seconds=0)
zero_tick = zero_time.total_seconds()
format_date = '%d/%b/%Y:%H:%M:%S'

obj = {}
test = open('very/big/log','r')

for line in test:
  try:
      chunk = line.split('+', 1)[0].split('-', 1)[1].split(' ')
      user = chunk[1]
      if (user[0] == "C"):
        this_time = datetime.datetime.strptime(chunk[2].split('[')[1], format_date)
        try:
            machine = obj[user]
        except KeyError, e:
            machine = obj.setdefault(user,{"init":this_time,"last":this_time,"downtime":0})
        last = machine["last"]
        diff = (this_time-last).total_seconds()
        if (diff > tick):
          machine["downtime"] += diff-tick
        machine["last"] = this_time

  except Exception:
    pass

While I'm ok with the time the script runs, I'm wondering if there are any obvious pitfalls regarding performance, which I'm stepping in (still in my first weeks of Python).

Can I do anything to make this run faster?

EDIT:

Sample log entry:

['2001:470:1f14:169:15f3:824f:8a61:7b59 - SOFTINST [14/Nov/2012:09:32:31 +0100] "POST /setComputerPartition HTTP/1.1" 200 4 "-" "-" 102356']

EDIT 2:

One thing I just did, was to check if log entries have the same log time and if so, I'm using the this_time from the previous iteration vs calling this:

this_time = datetime.datetime.strptime(chunk[2].split('[')[1], format_date)

I checked some of logs, there are plenty of entries with the exact same time, so on the last run on pypy I'm down to 2.43min.

You can use regular expressions with re module to speed up parsing dramatically. For example the following code extracts user and time information by using regular expression (note that user[0] == "C" condition also checked by the regexp):

import re
import datetime

match_record = re.compile(r"^[^ ]+ - (C[^ ]*) \[([^ ]+)").match
strptime = datetime.datetime.strptime

f = open("very/big/log", "rb")

for line in f:
    match = match_record(line)
    if match is not None:
        user, str_time = match.groups()
        time = strptime(str_time, "%d/%b/%Y:%H:%M:%S")
        print user, repr(time)

I think you can make it even faster by reading bigger chunks of data and using re.finditer method.
Don't use dictionaries to store information in the obj dictionary. Tuples is the way to go here and if you really need to use names for fields you can use collections.namedtuple. In any case tuples will be faster than dictionaries (and lists) and will take less memory.
Probably if user in obj will be little bit faster than try/except KeyError here. You can check execution times for small code blocks with timeit module.
Check the Python performance tips. For example Avoiding dots... chapter can be relevant for your code.

UPDATE

The following version of the code read bigger chunks of data and using re.finditer (Not sure if it faster or not. Also note the changes in the regexp):

import re
import datetime

iter_records = re.compile(r"^[^ ]+ - (C[^ ]*) \[([^ ]+) .*$", re.M).finditer
strptime = datetime.datetime.strptime

f = open("very/big/log", "rb")

chunk = ""
while True:
    # Probably buffer size can be even bigger
    new = f.read(1048576)
    chunk += new
    end = 0
    for match in iter_records(chunk):
        user, str_time = match.groups()
        time = strptime(str_time, "%d/%b/%Y:%H:%M:%S")
        print user, repr(time)
        end = match.end()
    if end > 0:
        chunk = chunk[end:]
    elif not new:
        break


Mark positions attacked by a queen on a chess board Problem - Given a chess board and the position of only one queen piece in it, mark all the position where the queen can move with X.

Kindly review this Scala code and suggest improvements.

import scala.collection.mutable._

object ChessQueenAttack {
  val board = Array.fill(8)(Array.fill(8)('0'))
  def printBoard = {
    println
    println
    board foreach { row => println(row.mkString(" "))}
  }

  def main(args: Array[String]): Unit = {
    printBoard
    val qR = args(0).toInt
    val qC = args(1).toInt
    board(qR)(qC) = 'Q'
    printBoard

    var (r,c) = (qR, qC)

    // horizontal
    for ( i <- 0 until 8 ) {
      if (i != qC) board(qR)(i) = 'X'
      if (i != qR) board(i)(qC) = 'X'
    }

    // top left
    r = qR -1
    c = qC -1
    while (r >= 0 && c >= 0) {
      board(r)(c) ='X'
      r -= 1
      c -= 1
    }
    // top right
    r = qR - 1
    c = qC + 1
    while (r >= 0 && c  < 8) {
      board(r)(c) = 'X'
      r -= 1
      c += 1
    }
    // bottom right
    r = qR + 1
    c = qC + 1
    while (r < 8 && c < 8 ) {
      board(r)(c) = 'X'
      r += 1
      c += 1
    }
    // bottom left
    r = qR + 1
    c = qC - 1
    while (r<8 && c>=0) {
      board(r)(c) = 'X'
      r += 1
      c -= 1
    }
    printBoard
  }
}

Output -

scalac ChessQueenAttack.scala
scala ChessQueenAttack 3 3

0 0 0 0 0 0 0 0
0 0 0 0 0 0 0 0
0 0 0 0 0 0 0 0
0 0 0 0 0 0 0 0
0 0 0 0 0 0 0 0
0 0 0 0 0 0 0 0
0 0 0 0 0 0 0 0
0 0 0 0 0 0 0 0

0 0 0 0 0 0 0 0
0 0 0 0 0 0 0 0
0 0 0 0 0 0 0 0
0 0 0 Q 0 0 0 0
0 0 0 0 0 0 0 0
0 0 0 0 0 0 0 0
0 0 0 0 0 0 0 0
0 0 0 0 0 0 0 0

X 0 0 X 0 0 X 0
0 X 0 X 0 X 0 0
0 0 X X X 0 0 0
X X X Q X X X X
0 0 X X X 0 0 0
0 X 0 X 0 X 0 0
X 0 0 X 0 0 X 0
0 0 0 X 0 0 0 X

Your solution is very procedural, and more like how a C program would be written.  I don't consider it to be idiomatic Scala.  You should decompose the problem into functions, such as isQueen and isAttacked in my solution below.

There should be very little code left over in main.  As a shortcut, then, you can just plop the main code directly into the ChessQueenAttack by making it extend App.

object ChessQueenAttack extends App {
  def isQueen(r: Int, c: Int): Boolean = { (r, c) == queenPos }

  def isAttacked(r: Int, c: Int): Boolean = {
    (r == queenPos._1) ||                       // Same row as queen
    (c == queenPos._2) ||                       // Same column as queen
    (r + c == queenPos._1 + queenPos._2) ||     // On / diagonal
    (r - c == queenPos._1 - queenPos._2)        // On \ diagonal
  }

  def board: String = {
    (0 until 8).map { r =>
      (0 until 8).map { c =>
        if (isQueen(r, c))    "Q" else
        if (isAttacked(r, c)) "X" else
                              "0"
      }.mkString(" ")
    }.mkString("\n")
  }

  val queenPos = (args(0).toInt, args(1).toInt)
  println(board)
}


FizzBuzz in Python using a class I recently read the book clean code by Robert C. Martin and I'm trying to apply his style of writing methods to the FizzBuzz programming example.

So far I have two "public" methods called: start and parseNumber, that respectively start the FizzBuzz program or parse a single number according to the FizzBuzz principle.

class FizzBuzz(object):

    def start(self, end_number):
        return ",".join(self._parse_numbers(end_number))

    def _parse_numbers(self, end_number):
        number_list = []
        for number in range(1, end_number+1):
            number_list.append(self.parse_number(number))
        return number_list

    def parse_number(self, number):
        if self._is_number_divisible_by_five_and_three(number):
            return "FizzBuzz"
        elif self._is_number_divisible_by_three(number):
            return "Fizz"
        elif self._is_number_divisible_by_five(number):
            return "Buzz"
        else:
            return str(number)

    def _is_number_divisible_by_five_and_three(self, number):
        return number % 15 == 0

    def _is_number_divisible_by_three(self, number):
        return number % 3 == 0

    def _is_number_divisible_by_five(self, number):
        return number % 5 == 0

I'm happy with the last 3 methods that are clearly doing one thing. But the _parse_numbers and parse_number methods can be improved, I just don't know how to do that.

First of all, you don't need to make a class for this. There's no real reason for FizzBuzz to be an object when all you have is a collection of functions anyway. Likewise, you have a lot of functions that don't really need to be functions. All your test functions could just be plain if statements and that'd make them more readable to Python. I see you've discovered the _ naming convention in Python, where names beginning with that are to be considered private. However you don't need to use these that often. Nothing's ever going to be truly private in Python without a lot of work, so instead allow people to access functions if they want to. Only mark functions private if there's actual problems with them attempting to use the function outside of a specific process.

parse_number is good, though I'd replace the tests with plain tests instead of your functions.

def parse_number(number):
    if number % 15 == 0:
        return "FizzBuzz"
    elif number % 3 == 0:
        return "Fizz"
    elif number % 5 == 0:
        return "Buzz"
    else:
        return str(number)

You could also add a docstring and maybe a comment, to indicate what the function does and returns as well as noting how number % 15 == 0 is the same as (number % 5 == 0) and (number % 3 == 0).

def parse_number(number):
    """Returns a string Fizzbuzz representation of a number"""

    # This is the same as (number % 5 == 0) and (number % 3 == 0)
    if number % 15 == 0:

Start seems like a not very worthwhile function and not a great name if you're no longer making a class. But if you want to keep it a name like fizzbuzz_string would be better.

Now, your _parse_numbers isn't a great name because it's too similar to an existing function. There's also little reason to mark it as private. Instead this could be your main method, or at least call it fizzbuzz. You could also build it easier and faster with something called a list comprehension. A list comprehension is basically a for loop like expression that will be evaluated to create a list. Yours is very simple, you just need to run parse_number on each number in your range, so this is how you could write it:

def parse_number(number):
    """Returns a string Fizzbuzz representation of a number"""

    # This is the same as (number % 5 == 0) and (number % 3 == 0)
    if number % 15 == 0:
        return "FizzBuzz"
    elif number % 3 == 0:
        return "Fizz"
    elif number % 5 == 0:
        return "Buzz"
    else:
        return str(number)

def fizzbuzz(end_number):
    """Return a list of Fizzbuzz parsed numbers up to end_number."""

    return [parse_number(number) for number in range(1, end_number+1)]

def fizzbuzz_string(end_number):
    return ",".join(fizzbuzz(end_number))

Now you can easily just call print fizzbuzz_string(number) and get the full list without needing an object or the other extraneous functions.


Random quote generator from API I have built a random machine generator website. It fetches the quotes from forismatic and has a button to get a new quote. When it gets a new quote, the website color changes. It also features a button to tweet the quote and a button to copy to clipboard (using clipboard.js).

//Needs JQuery
function getNewQuote(){
  //Changes the text of elements with class .quote-text
  //and .quote-author to have the corresponding
  //values on the json
  var leftQuoteIcon = "<i class='fa fa-quote-left'></i> ";
  //callback function that actually updates the DOM
  function updateQuote(json){
    $(".quote-text").fadeOut(1000, function() {
      $(this).html(leftQuoteIcon+json.quoteText);
    }).fadeIn(1000);

    var author = json.quoteAuthor;
    if (author === ""){//check if author is empty
      author= "Unknown";
    }
    $(".quote-author").fadeOut(1000, function() {
      $(this).text("-- "+author);
    }).fadeIn(1000);
    colorChange();
  }

  //makes json request
  $.getJSON("https://crossorigin.me/http://api.forismatic.com/api/1.0/",
            {_: new Date().getTime(), //to prevent caching http://stackoverflow.com/a/31948654/1952996
            method: "getQuote",
            format: "json",
            lang : "en"},
            updateQuote);
}

//Open new window with tweet ready with current quote
function tweet(){
  var author = $(".quote-author").text();
  var quote = $(".quote-text").text();
  window.open("https://twitter.com/intent/tweet?text="+quote+" "+author+"&hashtags=FamousQuotes");
}

function colorChange(){
  var colors = ["#E74C3C", "#9B59B6", "#3498DB", "#1ABC9C", "#27AE60",
                "#F1C40F", "#D35400", "#34495E", "#797D7F"];
  var newColor = colors[Math.floor(Math.random()*(colors.length))];
  //Change color of body, buttons and text
  $("body").css("background-color", newColor);
  $(".button-colored").css({"background-color": newColor,
                           "border-color": newColor});
  $(".quote").css("color", newColor);

}

$(document).ready(function(){
  getNewQuote();//Get first quote for page load

  $("#newquote").on("click", getNewQuote);//Associate getNewQuote with button

  $("#twitter-share").on("click", tweet);

  //Copy to clipboard
  new Clipboard("#copy-clipboard");

  $("#copy-clipboard").attr("data-clipboard-target", ".quote");
});

From spending some time looking at your code

Purely style, but I think grouping statements belonging together and commenting on them sometimes makes your code easier to read

$(document).ready(function(){
  //Take care of the initial quote
  getNewQuote();

  //Take care of click handlers
  $("#newquote").on("click", getNewQuote);//Associate getNewQuote with button
  $("#twitter-share").on("click", tweet);

  //Take care of clipboard
  new Clipboard("#copy-clipboard");
  $("#copy-clipboard").attr("data-clipboard-target", ".quote");
});

I would move the magic constants 1000 to a properly named constant
A logical OR could make this look much cleaner: ( Returns expr1 if it can be converted to true; otherwise, returns expr2. ) "" and undefined cannot be converted to true.

var author = json.quoteAuthor || "Unknown";
$(".quote-author").fadeOut(1000, function() {
  $(this).text("-- "+author);
}).fadeIn(1000);

Inside getNewQuote() I would have placed the JSON call on top of the code
You can have a list of css classes inside a jQuery call:

$("body").css("background-color", newColor);
$(".button-colored").css({"background-color": newColor,
                          "border-color": newColor});

could be

$("body, .button-colored").css("background-color", newColor);
$(".button-colored").css("border-color", newColor});

Though I do wonder how beautiful changing every single element to 1 color is, I would probably have gone with matching background, foreground, and text colors.

Other than that, this code is readable and maintainable.


Prime engine: generation, primality, factorization I've just written my first C# class, so I'm looking for some feedback before I go and write more code. The goal of the code is to provide a Singleton-like class to deal with common operations on the set of prime numbers that I will need to complete projecteuler.net problems.

I'm interested in feedback for all aspects of the implementation (short of the actual prime generation algorithms, the 6*i +/- 1 method was used solely for its simplicity).

I'm also interested in feedback about how the interfaces are split up. I split these up into slices of functionality in an attempt to allow future implementation changes. I am planning to use DI (e.g. SimpleInjector) to bind the singleton instance to each of the interfaces, but I'm seriously starting to doubt this pattern.

I'm less interested in feedback about the selections of methods, I just chose a relatively minimal subset of methods that I know that I will need at some point - more methods will probably be added as needed.

Interfaces

public interface IPrimeGenerator
{
    IEnumerable<long> PrimesUntilValue(long value);
    IEnumerable<long> PrimesUntilCount(int count);
}

public interface IPrimeChecker
{
    bool IsPrime(long value);
}

public interface IPrimeFactorizer
{
    IEnumerable<long> PrimeFactors(long value);
    IEnumerable<long> UniquePrimeFactors(long value);
}

Implementation

public class PrimeEngine: IPrimeGenerator, IPrimeChecker, IPrimeFactorizer
{
    private readonly ICollection<long> _primeCollection;
    private long _indexFactor;
    private long _maxChecked;

    public PrimeEngine()
    {
        _primeCollection = new Collection<long> {2, 3};
        _indexFactor = 1;
        _maxChecked = 3;
    }

    private void CheckNextPossiblePrimeDoublet()
    {
        var low = 6 * _indexFactor - 1;
        var high = low + 2;

        if (IsPrime(low))
        {
            _primeCollection.Add(low);
        }

        if (IsPrime(high))
        {
            _primeCollection.Add(high);
        }

        _indexFactor += 1;
        _maxChecked = high;
    }

    private IEnumerable<long> GetPossibleSmallerPrimeFactors(long value)
    {
        FillPrimesUntilRoot(value);
        return PrimesUntilRoot(value);
    }

    public bool IsPrime(long value)
    {
        var primePool = GetPossibleSmallerPrimeFactors(value);
        return primePool.All(prime => value % prime != 0);
    }

    private void FillPrimesUntilValue(long value)
    {
        while (_maxChecked < value)
        {
            CheckNextPossiblePrimeDoublet();
        }
    }

    private void FillPrimesUntilCount(int count)
    {
        while (_primeCollection.Count < count)
        {
            CheckNextPossiblePrimeDoublet();
        }
    }

    private static long FloorOfRoot(long value)
    {
        return (long) Math.Floor(Math.Sqrt(value));
    }

    private void FillPrimesUntilRoot(long value)
    {
        FillPrimesUntilValue(FloorOfRoot(value));
    }

    public IEnumerable<long> PrimesUntilValue(long value)
    {
        FillPrimesUntilValue(value);
        return _primeCollection.TakeWhile(prime => prime <= value);
    }

    public IEnumerable<long> PrimesUntilCount(int count)
    {
        FillPrimesUntilCount(count);
        return _primeCollection.Take(count);
    }

    public IEnumerable<long> PrimesUntilRoot(long value)
    {
        return PrimesUntilValue(FloorOfRoot(value));
    }

    public IEnumerable<long> PrimeFactors(long value)
    {
        FillPrimesUntilRoot(value);
        foreach (var prime in PrimesUntilRoot(value))
        {
            if (prime > value) break;

            while (value % prime == 0)
            {
                yield return prime;
                value /= prime;
            }
        }

        if (value != 1)
        {
            yield return value;
        }
    }

    public IEnumerable<long> UniquePrimeFactors(long value)
    {
        return PrimeFactors(value).Distinct();
    }
}

This is excellent code for a beginner. It's neat, the methods are short, etc. However, there are a couple things I'm concerned with.

1) Your engine has a lot of state. It looks like this is mostly related to the generation of the next prime sequences, and any found primes are cached, so it shouldn't be too bad. However, you have to test that calling PrimesUntilValue followed by UniquePrimeFactors, for example, doesn't generate "duplicate" values or skip any.

2) Given that you are using an approximation for primes, your code is structured neatly. However, do note that actually generating the sequence might get a bit messier than this and don't expect to be able to swap it out neatly. Given that your class's state is solely used for generating the next prime and that all previously found values are cached, it shouldn't be an issue.

3) This code could lead to massive memory use in an application. Since you are already caching the values and all, you should consider making it a static class and making the cached collection a thread-safe hashmap. Then you have one instance of the prime engine and can do things like PrimeEngine.PrimesUntilRoot, etc. The hashmap would prevent duplicate primes from being entered into the collection.


Printing every 10th result in an alternating ± series Problem:

In the following expression 1-2+3-4+5-6 ... -98+99-100, print result every 10th (10, 20, 30, ...) calculation.

My code:

result  = 0
operand = 1
counter = 0
for i in range(1, 101):
    result = result + (i * operand)
    counter = counter + 1
    operand = operand *  -1 #Convert between positive and negative number
    if counter >= 10:
        counter = 0
        print(result)

My problem:

I think this code has not good readability. For example, I used * -1 for changing positive and negative number. But it requires math skill(multiplying a negative number) to understand code.

And I don't sure variable name 'counter' and 'operand' is appropriate for this context.

Is there any suggestion?

The first thing you can do to make the sign change obvious is to use operand = -operand, this way you avoid the need for the multiplication operator. Also changing the operand name to sign can help.

You also don't need counter as it will be equal to i before the test. Just change the test to use the modulo (%) operator instead of reseting counter.

And using augmented assignment for result can lead to a more readable line too:

result = 0
sign = 1
for i in range(1, 101):
    result += sign * i
    sign = -sign
    if i % 10 == 0:
        print(result)

Now, this probably need to be made more reusable, this means building a function out of this code and avoiding mixing computation and I/Os:

def alternating_sum(start=1, stop=100, step=10):
    result = 0
    sign = 1
    for i in range(start, stop+1):
        result += sign * i
        sign = -sign
        if i % step == 0:
            yield result

Usage being:

for result in alternating_sum(1, 100, 10):
    print(result)

Also, depending on the problem at hand, you can leverage the fact that every two numbers add-up to -1. So every 10 numbers add-up to -5. If you don't need much genericity, you can simplify the code down to:

def alternating_every_ten(end=100):
    reports = end // 10
    for i in range(1, reports + 1):
        print('After', 10 * i, 'sums, result is', -5 * i)


Simple Flask API backend to maintain a word list I made a little toy project last semester. I would like to get some insights about my code, particularly the part of handling the database. It is a CRUD kind of traditional API.

def fword(lon,lat,radius,ids):
    try:
        result = {}
        c, conn = connection()
        c.execute("SELECT * FROM wordlocation WHERE ((longitude-%s)*(longitude-%s)+(latitude-%s)*(latitude-%s))<%s",(lon,lon,lat,lat,radius))
        words = []
        for row in c.fetchall():
            t = {}
            if row[0] in ids:
                continue
            t["name"] = row[1]
            t["definition"] = row[4]
            t["id"] = row[0]
            t["lat"] = row[2]
            t["long"] = row[3]
            words.append(t)
        result["words"] = words
        j = json.dumps(result)
        c.close()
        conn.close()
        return j
    except Exception as e:
        logging.info(str(e))

def pword(lat,lng,word,definition):
    try:
        c, conn = connection()
        c.execute("INSERT INTO wordlocation (word,longitude,latitude,definition,lang) VALUES(%s,%s,%s,%s,%s)",
        (thwart(word),lat,lng,thwart(definition),'english'))
        conn.commit()
        c.close()
        conn.close()
        return "success!"
    except Exception as e:
        logging.info(str(e))

def cword(word, username):
    try:
        c, conn = connection()
        blank_word = " " + word
        c.execute("UPDATE userstat SET completed=CONCAT(IFNULL(completed,''), %s) , score=score+1 WHERE username=%s;",(blank_word, username))
        conn.commit()
        c.close()
        conn.close()
        return "successful collect"
    except Exception as e:
        logging.info(str(e))

def stat(username):
    try:
        c, conn = connection()
        c.execute("SELECT * FROM userstat WHERE username=%s", (username,))
        dict = {}
        u = c.fetchone()
        dict['username'] = username
        dict['words'] = str(u[1]).split()
        dict['score'] = u[2]
        return json.dumps(dict)
    except Exception as e:
        logging.info(str(e))

The backend logic:

@app.route("/getWord", methods=['GET'])
def getWord():
    if 'long' in request.args and 'lat' in request.args and 'radius' in request.args and 'ids' in request.args:
        try:
            longitude = float(request.args["long"])
            latitude = float(request.args["lat"])
            radius = float(request.args["radius"])
            ids = request.args.getlist("ids")
        except:
            return "Bad Request Parameters"
        try:
            allwords_json = fword(longitude,latitude,radius,ids)
            return allwords_json
        except Exception as e:
            logging.info(str(e))
    else:
        return "bad request"

@app.route("/postWord",methods=["GET","POST"])
def postWord():
    if 'lat' in request.args and 'long' in request.args and 'word' in request.args and 'definition' in request.args:
        try:
            lat = float(request.args['lat'])
            lng = float(request.args['long'])
            word = str(request.args['word'])
            definition = str(request.args['definition'])
            return pword(lat,lng,word,definition)
        except Exception as e:
            logging.info(str(e))
    else:
        return "badPost"

There are many other endpoints that is in a similar fashion. One issue I am aware of is the variable naming seems not readable.

Obligatory reference to the style guide. You have incorrect whitespace, for example, and (worse) it's inconsistent:

@app.route("/getWord", methods=['GET'])
                    # ^ yes
@app.route("/postWord",methods=["GET","POST"])
                     # ^ preferably not, but *definitely* be consistent

if 'long' in request.args and 'lat' in request.args and 'radius' in request.args and 'ids' in request.args:

Would be neater as:

if all(arg in request.args for arg in ['long', 'lat', 'radius', 'ids']):

This also allows you to extract a REQUIRED_ARGS constant if you wish.

Your exception handling is too broad. For example:

try:
    lat = float(request.args['lat'])
    lng = float(request.args['long'])
    word = str(request.args['word'])
    definition = str(request.args['definition'])
    return pword(lat,lng,word,definition)
except Exception as e:
    logging.info(str(e))

You should have as little as possible inside the try block, and avoid bare except.

What could actually go wrong inside the try block? E.g. for the first two lines, given that you've already checked for the keys, only a ValueError would get thrown. And as you have similarly broad error handling in pword, you'd only get an error out of that line if the logger itself failed.

Also note that there's a logging.exception that actually takes an error object and deals with the traceback correctly.

You note correctly that the naming is poor. It is also, again, inconsistent:

def fword(lon,lat,...
def pword(lat,lng,...

These parameters have the same meaning, so should have the same name (and ideally the same order). Given that you'd always expect the pair, you could even use a (latitude, longitude) tuple as e.g. a coords argument. Also:

def pword(...):
def postWord():

Finally you have a bunch of functions that all create a cursor and connection, and two of them are almost identical but for the query string and arguments. Look into whether you could reduce duplication by abstracting those similarities out.


Interweaving an array's values I've written a function to sort an array that interweaves an arrays values in the way of if you had two separate arrays [1,2,3,4,5] on one side and [6,7,8,9,0] on the other and it becomes [1, 6, 2, 7, 3, 8, 4, 9, 5, 0].

The code works, but I'm wondering how it can be cleaned up and be more performant without the need for three additional arrays and to see if it can be done on the cards array itself.

let data = [1,2,3,4,5,6,7,8,9,0];

const interweave = data => {
  const _data = [];
  const l = data.slice(0, data.length / 2);
  const r = data.slice(data.length / 2, data.length);
  if (l.length === r.length) {
    for (let i = 0; i < l.length; i++) {
      _data.push(l[i], r[i]);
    }
  }
  return _data;
}

interweave(data);

One approach would be to utilize .splice() instead of .slice() and .shift().

If the input array is expected to be mutated const copy = [...arr] can be removed and the array methods can be called on input arr : data

let data = [1,2,3,4,5,6,7,8,9,0];

const interweave = arr => {
  const copy = [...arr];  // copy input
  const r = copy.splice(Math.ceil(copy.length / 2)); // splice half of input
  for (let i = 1; r.length; i += 2) // set `i` to `1` increment by `2`
    copy.splice(i, 0, r.shift()); // use `.splice()` and `.shift()`
  return copy; // return `copy`
}

console.log(interweave(data));


Data transformation involving dictionaries and lists My initial problem is to transform a List of Mappings whose values are Lists of values. The transformed list must contain the Cartesian product of all lists (those that are values in the dictionaries) that belong to separate dictionaries (in other words, the values of the lists present in the same directories are "coupled").

Basically, if you disregard the keys of the dictionaries, this would be solved simply with itertools.product.

Input:

[
   {
      ('B3G', 'B1'): [1.0, 2.0],
      ('B1G', 'B1'): [11.0, 12.0]
   },
   {
      ('B2G', 'B1'): [1.5, 2.5, 3.5]
   }
]

Output:

[
  {('B3G', 'B1'): 1.0, ('B1G', 'B1'): 11.0, ('B2G', 'B1'): 1.5},
  {('B3G', 'B1'): 1.0, ('B1G', 'B1'): 11.0, ('B2G', 'B1'): 2.5},
  {('B3G', 'B1'): 1.0, ('B1G', 'B1'): 11.0, ('B2G', 'B1'): 3.5},
  {('B3G', 'B1'): 2.0, ('B1G', 'B1'): 12.0, ('B2G', 'B1'): 1.5},
  {('B3G', 'B1'): 2.0, ('B1G', 'B1'): 12.0, ('B2G', 'B1'): 2.5},
  {('B3G', 'B1'): 2.0, ('B1G', 'B1'): 12.0, ('B2G', 'B1'): 3.5}
]

To make the matter more confusing, the keys of each dictionaries are Tuples of strings.

Here is a possible implementation, using a class to isolate the whole mess.

@dataclass
class ParametricMapping:
"""Abstraction for multi-dimensional parametric mappings."""

mappings: List[Mapping[Tuple[str], Sequence[float]]] = field(default_factory=lambda: [{}])

@property
def combinations(self) -> List[Mapping[Tuple[str], float]]:
    """Cartesian product adapted to work with dictionaries, roughly similar to `itertools.product`."""

    labels = [label for arg in self.mappings for label in tuple(arg.keys())]
    pools = [list(map(tuple, zip(*arg.values()))) for arg in self.mappings]

    def cartesian_product(*args):
        """Cartesian product similar to `itertools.product`"""
        result = [[]]
        for pool in args:
            result = [x + [y] for x in result for y in pool]
        return result

    results = []
    for term in cartesian_product(*pools):
        results.append([pp for p in term for pp in p])

    tmp = []
    for r in results:
        tmp.append({k: v for k, v in zip(labels, r)})

    if len(tmp) == 0:
        return [{}]
    else:
        return tmp

Question: How can I improve this to make it cleaner (priority #1) and faster (#2)?

TL;DR: Scroll to the end for the provided code with suggested improvements

Suggestions

1. Standalone cartesian_product helper function outside scope of combinations

A preferable solution would be to simply use itertools.product as most Python-savvy readers will be familiar with it - and it is well-documented for those that aren’t. However, since you explicitly mention it...

If you still don't want to use itertools.product:

While it does improve clarity to construct a hierarchy of scopes that expose things only when necessary, in this case, I believe that making cartesian_product a nested function inside combinations serves only to confuse the purpose of combinations. It is better to define cartesian_product above, making the code below cleaner and easier to understand. Now someone reading your code will first see cartesian_product’s definition and understand its relatively simple purpose. Then, inside ParametricMapping.combinations, readers are already familiar with cartesian_product, and their train of thought isn’t derailed by trying to understand a nested function.

2. flatten helper function

This helper function should be used thusly:

for term in cartesian_product(*pools):
    results.append(flatten(term))

This may seem silly as flattening is a simple operation, but in this example there are a few fairly tricky list/dict comprehensions nearby. Therefore, it may help to replace that piece with a simple flatten call to clean up some confusion and to emphasize that this is  a straight-forward operation - A fact that could be lost on some readers of the code in its current state. My point here is that stacking lots of loops and comprehensions on top of each other (especially without documentation/comments) can quickly get messy and confusing.

3. Consolidate some code

If you took both of the above suggestions, a nice opportunity to consolidate some code has presented itself. After the above changes, the section that was originally

results = []
for term in cartesian_product(*pools):
    results.append([pp for p in term for pp in p])

should now be

results = []
for term in itertools.product(*pools):
    results.append(flatten(term))

This block can be expressed clearly and concisely with the following list comprehension:

results = [flatten(term) for term in itertools.product(*pools)]

Note that because of the separation of functionality into helper functions, the purpose of this list comprehension is abundantly clear. It creates results, which is a list containing the flattened outputs of product.

4. Check for empty mappings at top of combinations

Instead of the if/else at the end of combinations that checks for len(tmp) == 0, make the first two lines of combinations the following:

if self.mappings == [{}]:
    return [{}]

The else clause at the bottom of combinations is no longer necessary, and you can simply return tmp. This is cleaner because it handles the case where mappings is empty immediately, which means those cases bypass all the code that was being executed before when len(tmp) was evaluated at the end of combinations. This also allows readers to make the assumption that mappings is non-empty when they get into the actual work being done by combinations, which is just one less thing to worry about. Alternatively, a more concise option would be to replace

if len(tmp) == 0:
    return [{}]
else:
    return tmp

with

return tmp or [{}]

This works because if mappings is empty, the value of tmp will be an empty list, which will evaluate to False, returning the value following the or. This more concise version may come at the cost of decreased readability.

5. Define labels and pools using helper methods or non-init fields

Remove the first two code lines from combinations, and move them to either helper methods, or additional fields that are processed post-initialization. I recommend doing this to further clarify the work actually being done by combinations and to clean up the code overall. These both have the added benefit of exposing labels and pools as additional attributes of the dataclass that can be accessed outside of combinations.

See the ParametricMapping1 class defined in the section below for an implementation using helper methods, and see the alternative ParametricMapping2 class defined below it for an implementation using non-init fields.

TL;DR

In the end, if all suggestions are followed, the code should include the below declaration of flatten, along with one of the two following blocks (ParametricMapping1, or ParametricMapping2):

def flatten(l):
    return [item for sublist in l for item in sublist]

With either ...

@dataclass
class ParametricMapping1:
    mappings: List[Mapping[Tuple[str], Sequence[float]]] = field(default_factory=lambda: [{}])

    def _labels(self) -> List[Tuple[str]]:
        return flatten(self.mappings)

    def _pools(self) -> List[List[Sequence[float]]]:
        return [list(map(tuple, zip(*arg.values()))) for arg in self.mappings]

    @property
    def combinations(self) -> List[Mapping[Tuple[str], float]]:
        if self.mappings == [{}]:
            return [{}]

        pool_values = [flatten(term) for term in itertools.product(*self._pools())]
        return [dict(zip(self._labels(), v)) for v in pool_values]

Or ...

@dataclass
class ParametricMapping2:
    mappings: List[Mapping[Tuple[str], Sequence[float]]] = field(default_factory=lambda: [{}])
    labels: List[Tuple[str]] = field(init=False, repr=False)
    pools: List[List[Sequence[float]]] = field(init=False, repr=False)

    def __post_init__(self):
        self.labels = flatten(self.mappings)
        self.pools = [list(map(tuple, zip(*arg.values()))) for arg in self.mappings]

    @property
    def combinations(self) -> List[Mapping[Tuple[str], float]]:
        pool_values = [flatten(term) for term in itertools.product(*self.pools)]
        return [dict(zip(self.labels, v)) for v in pool_values] or [{}]

Edit (2019-01-09 - 1530):

The definitions of _labels and self.labels in the above two code blocks, respectively, have been simplified per @MathiasEttinger's excellent suggestion. See revision history for their original definitions.


Pulsing Alpha Channel The following script makes an image pulse in Unity by fluctuating its alpha channel.  However, I feel like there is a lot of duplicate code in the update method.  How can I clean up that code to have it not repeat itself with +-, true false, etc.?

using UnityEngine;
using UnityEngine.UI;

[RequireComponent(typeof(Image))]
public class Pulse : MonoBehaviour
{
    private Image image;

    [Tooltip("How long in seconds it takes to complete a pulse")]
    [SerializeField]
    [Range(0.0f, 60.0f)]
    private float time = 2.0f;

    [Tooltip("Minimum alpha to pulse to")]
    [SerializeField]
    [Range(0.0f, 1.0f)]
    private float minAlpha = 0.0f;

    [Tooltip("Maximum alpha to pulse to")]
    [SerializeField]
    [Range(0.0f, 1.0f)]
    private float maxAlpha = 1.0f;

    private bool alphaIncreasing = false;

    private void Awake()
    {
        image = GetComponent<Image>();
    }

    void Update()
    {
        var color = image.color;
        float alphaChange = Time.deltaTime / time / 2.0f;
        if (alphaIncreasing)
        {
            color.a += alphaChange;
            if (color.a >= maxAlpha)
            {
                color.a = maxAlpha;
                alphaIncreasing = false;
            }
        }
        else
        {
            color.a -= alphaChange;
            if (color.a <= minAlpha)
            {
                color.a = minAlpha;
                alphaIncreasing = true;
            }
        }
        image.color = color;
    }
}

Everything can be controlled by a changing limit.

The (2 * limit - 1) results in either 1 for limit 1 or -1 for limit 0 controlling whether you are increasing or decreasing.

color.a * sign >= limit if limit = 1 and color.a > 1 or limit = 0 and color.a < 0

and maxAlpha - limit is either 1 if limit is 0 or 0 if limit is 1

Remove

private float minAlpha = 0.0f;

private bool alphaIncreasing = false;

Add

private float limit = 0.0f;

New Update code.

void Update()
{
    var color = image.color;
    float alphaChange = Time.deltaTime / time / 2.0f;
    var sign = 2 * limit - 1;
    color.a += sign * alphaChange;
    if (color.a * sign >= limit)
    {
        color.a = limit;
        limit = maxAlpha - limit;
    }
    image.color = color;
}


8-Bit ALU in Verilog I'm an EE student who's taken a a couple digital logic/design courses, but they were focused on schematic representation, so I'm teaching myself Verilog to implement what I've learned.

For a basic Hello World project, I've implemented a basic 8-bit ALU with addition/subtraction with carry/borrow, NOT, AND, OR, XOR, and carry/zero/sign flags.

I've implemented it on an FPGA and everything works, but I want to make sure I'm not falling for any beginner traps which can lead to dramatic/undesired results in the final implementation of the circuit.

module ALU(A, B, Cin, Op, Out, Cout, Zero, Sign);
    input[7:0] A, B;
    input Cin;
    //011-A+B, 111-A-B, 000-NOT A, 001-A AND B, 010-A OR B, 100-A XOR B
    input[2:0] Op;
    output reg[7:0] Out;
    output Cout;
    output reg Zero;
    output reg Sign;

    wire FinalCin;          //Final value of Cin fed to ALU after XOR
    wire[7:0] FinalB;       //Final value of B fed to ALU after XOR
    wire[7:0] AddResult;
    wire[7:0] LogicResult;

    Adder DEV_ADD(A, FinalB, FinalCin, AddResult, Cout);
    LogicUnit DEV_LOG(A, B, Op, LogicResult);

    assign FinalCin = Cin ^ Op[2];  //Cin XOR Subtract Line
    assign FinalB = B ^ {8{Op[2]}}; //XOR B with Subtract Line

    always @ (Op or AddResult or LogicResult) begin
        if(Op == 3'b011 || Op == 3'b111)
            Out <= AddResult;
        else if(Op == 3'b000 || Op == 3'b001 || Op == 3'b010 || Op == 3'b100)
            Out <= LogicResult;
        else
            Out <= 8'b00000000;
    end

    always @ (Out) begin
        if(Out == 8'b00000000)
            Zero <= 1'b1;
        else
            Zero <= 1'b0;

        Sign <= Out[7];
    end
endmodule

Adder Module:

module Adder(A, B, Cin, S, Cout);
    input[7:0] A, B;
    input Cin;
    output wire[7:0] S;
    output wire Cout;

    assign {Cout, S} = A + B + Cin;

endmodule

Logic Unit:

module LogicUnit(A, B, Op, Out);
    input[7:0] A, B;
    input[2:0] Op;  //000-NOT A, 001-A AND B, 010-A OR B, 100-A XOR B
    output reg[7:0] Out;

    always @ (Op or A or B) begin
        case(Op)
            3'b000: Out <= ~A;
            3'b001: Out <= A & B;
            3'b010: Out <= A | B;
            3'b100: Out <= A ^ B;
        endcase
    end

endmodule

Design Issue:

From a design perspective I see one issue in LogicUnit. Out is an inferred latch because it is not assigned in all conditions of Op. The risks associated with these latches and how to use them property (when needed) have already been discussed on other Stack Exchange sites; such as here on Electrical Engineering Stack Exchange and here on Stack Overflow. Synthesis considers all possibilities. Depending on your setup it may or may not recognize Op>4 as out of bounds. It is safer to give it an explicit assignment and allow the synthesizer to choose how to optimize.

In your situation a latch should not be inferred. The easiest way to resolve this is to ass a default statement:

always @* begin
  case(Op)
    3'b000: Out = ~A;
    3'b001: Out = A & B;
    3'b010: Out = A | B;
    3'b100: Out = A ^ B;
    default: Out = A ^ B; // <<-- or any expression to a known value
  endcase
end

You may have noticed I also changes the <= to =. This has to do with coding style recommendations and suggestions that I will cover next.

Coding style:

Blocking vs Non-Blocking Assignments:

The only major coding style concerning I see is that you are using non-blocking assignments (<=) for all assignments in your always blocks. Non-blocking assignments should be used for flops (edge-sensitive) or latches (level-sensitive). Blocking assignments (=) should be used for combinational logic; which is what your logic is.

Blocking and non-blocking update the LHS at different regions in the scheduler. Blocking is immediate evaluation and assignment. Non-blocking is immediate evaluation with postponed assignment. Non-blocking was introduced to resolve a race condition when evaluating and assigning synchronous logic. Using non-blocking inappropriately, such as using it everywhere, can introduce another form of race conditions.

It is recommended to keep blocking and non-blocking assignments in separate always blocks. There are technically legal ways to safely mix them, but most avoid it all together

FYI, by using blocking assignment you could merge the always blocks that assign Out and Zero.

Sensitivity list:

What you have is legal and 100% complaint with the original Verilog IEEE standard (IEEE1364-1995). However automatic sensitivity (@* or @(*)) was added in IEEE1364-2001 and is the preferred and recommended way of declaring conbinational and latching logic with Verilog. @* use the RHS varabiles and nets used in the always-block to determine what signals it needs to be sensitive to. It does not look for signals used inside the body of tasks or functions (signals in the port list are fine) used with in the always block. This is a limitation, but it is also not recommended to have synthesizable tasks/functions reference non-port signals.

@* does more that reduce typing. By allowing verilog to manage the sencitity list for you, it reduces the risk of forgetting a signal (behavior vs synthesis mismatch), having unnecessary signals (possible simulation overhead), and typos (compiling errors or will use the wrong signal).

In summary use @* (or the synonymous @(*)).

Module Instances:

You are using connect by order which is okay for small modules with simple connectivity and whose port list isn't under constant editing. If you use connect by name, the port connections are explicit and it will not matter if the port order is re-arranged. It also helps any human readers of your code understand how the ports are intended to connect.

Adder DEV_ADD( .A(A), .B(FinalB), .Cin(FinalCin), .S(AddResult), .Cout(Cout) );

Module header:

Your module header is is complaint IEEE1364-2001 non-ANSI style (History: in IEEE1364-1995, declaring something to be an output and a reg had to be done in separate statements). I have heard of one case where some tools do not support non-ANSI header with one-line output reg. I have know idea how prevalent this issue is as most new code is written with the ANSI header style. Legacy code and some generated code is written as non-ANSI that is comparable with IEEE1364-1995. ANSI header style is currently the most popular because it is the simplest and cleanest way of managing ports. It becomes more useful as the port list grows and when it is edited often.

You don't need to change your code if you do want to. Be aware if you ever run into an issue with another simulator, synthesizes, or other tool that takes in Verilog. A compiling issue could be because some developers skipped support for one-line output reg for non-ANSI headers. It is technically a bug for the tool but it is unlikely to get much attention. The developers are more focused in adding/fixing/improving higher demand features. Plus there are two work arounds: ANSI and 1995's non-ANSI.

Here is an example of you ALU headed in the three flavors.

ANSI style (popular): IEEE1364-2001 ==> IEEE1800-2012(SystemVerilog)

module ALU( // PORT ORDER, DIRECTION, AND TYPE
  input [7:0] A, B,
  input Cin,
  //011-A+B, 111-A-B, 000-NOT A, 001-A AND B, 010-A OR B, 100-A XOR B
  input [2:0] Op,
  output reg [7:0] Out,
  output Cout,
  output reg Zero,
  output reg Sign );

non-ANSI style (1995 style): IEEE1364-1995 ==> IEEE1800-2012(SystemVerilog)

module ALU(A, B, Cin, Op, Out, Cout, Zero, Sign); // PORT ORDER
  // PORT DIRECTION
  input [7:0] A, B;
  input Cin;
  //011-A+B, 111-A-B, 000-NOT A, 001-A AND B, 010-A OR B, 100-A XOR B
  input [2:0] Op;
  output [7:0] Out;
  output Cout;
  output Zero;
  output Sign;
  // OUTPUT TYPE
  reg [7:0] Out;
  reg Zero;
  reg Sign;

non-ANSI style (2001 style): IEEE1364-2001 ==> IEEE1800-2012(SystemVerilog)

module ALU(A, B, Cin, Op, Out, Cout, Zero, Sign); // PORT ORDER
  // PORT DIRECTION AND TYPE
  input [7:0] A, B;
  input Cin;
  //011-A+B, 111-A-B, 000-NOT A, 001-A AND B, 010-A OR B, 100-A XOR B
  input [2:0] Op;
  output reg [7:0] Out;
  output Cout;
  output reg Zero;
  output reg Sign;


Syncs an array to a database table Hey guys have a bit of code below I'm looking to optimize, the code is working, just feel it could be shorter as I had to ignore DRY principles sadly - couldn't think of a better way.

Objective: Minimize the if ($isMulti) { } else {} statement

PHPDoc:

/**
 * Syncs an array key => value pair to database column => value
 *
 * Will ignore keys where a column does not exist for which will allow you to enter a dirty array
 * as long as it has the key => value pairs that you wish to update.
 *
 * Cannot override
 *
 * @param array $data
 *
 * @returns void
 */

Code:

final protected function syncToDatabase(array $data)
{
    $isMulti = FALSE;

    $model      = $this->model;
    $identifier = $this->tableIdColumn;

    $sh = (count($data) > 0) ? $data : FALSE;

    if (!$sh) {
        return;
    }

    // determine if the array has multiple result sets
    foreach ($sh as $x) {
        if (is_array($x)) {
            $isMulti = TRUE;
            break;
        }
    }
    /////////////////////

    if ($isMulti) {
        foreach ($sh as $item) {
            $row = $model::get()
                ->filter(
                    array(
                        "{$identifier}" => $item[ $identifier ]
                    )
                )
                ->first();

            if (!is_null($row)) {
                // update existing row
                $row->update($item);
                $row->write();
            }
            else {
                // row not found insert
                $new = $model::create();
                foreach ($item as $res => $val) {
                    $new->$res = $val;
                }
                $new->write();
            }
        }
    }
    else {
        $row = $model::get()
            ->filter(
                array(
                    "{$identifier}" => $sh[ $identifier ]
                )
            )
            ->first();

        if (!is_null($row)) {
            // update existing row
            $row->update($sh);
            $row->write();
        }
        else {
            // row not found insert
            $new = $model::create();
            foreach ($sh as $res => $val) {
                $new->$res = $val;
            }
            $new->write();
        }
    }
}

Example Invocations:

$array1 = array(
    "Column1" => "Foo",
    "Column2" => "Bar",
    "Column3" => "Foobar",
);

$array2 = array(
    array(
        "Column1" => "Foo",
        "Column2" => "Bar",
        "Column3" => "Foobar",
    ),
    array(
        "Column1" => "Some",
        "Column2" => "Random",
        "Column3" => "Values",
    )
);

syncToDatabase($array1);
syncToDatabase($array2);

A function does one thing, and one thing only.

final protected function syncToDatabase(array $data)
{
    $isMulti = FALSE;

    // ...

    // determine if the array has multiple result sets
    foreach ($sh as $x) {
        if (is_array($x)) {
            $isMulti = TRUE;
            break;
        }
    }

    if ($isMulti) {
        // ...
    } else {
        // ...
    }
}

Please don't do this. You are trying to provide two different functions under a single interface. That's error prone, and not exactly necessary either.

The calling site should already know whether it's invoking this function with a single result set, or an array of such.

Use this knowledge in your API design. In your case, this means stripping your syncToDatabase of all the special handling regarding multi updates.

final protected function syncToDatabase(array $item) {
    $model      = $this->model;
    $identifier = $this->tableIdColumn;

    $row = $model::get()
        ->filter(
            array(
                "{$identifier}" => $item[ $identifier ]
            )
        )
        ->first();

    if (is_null($row)) {
        // row not found insert
        $new = $model::create();
        foreach ($item as $res => $val) {
            $new->$res = $val;
        }
        $new->write();
    }
    else {
        // update existing row
        $row->update($item);
        $row->write();
    }
}

If you know that you need multi row updates commonly, provide that by a dedicated function, only for that task:

final protected function syncToDatabaseAll(array $items) {
    foreach ($items as $item) {
        $this->writeRow($item);
    }
}

Reinventing the wheel

foreach ($item as $res => $val) {
    $new->$res = $val;
}

Why? Only 3 lines further down, you used the correct function to update a row from an array.

$row->update($item);

Even worse - what you just implemented yourself can have unitended side effects if the row object has additional properties with special meaning, which coincidentally collide with (not actually ignored) keys in the passed data.

So your logic can be written more compact and less error prone once again:

$row = $model::get()->[...]->first();

if (is_null($row)) {
    $row = $model::create();
}

$row->update($item);
$row->write();

syncToDatabase is a pretty long name, you would commonly just call the whole process persist.

Your PHPDoc comment is a lie.

  Will ignore keys where a column does not exist for which will allow you to enter a dirty array as long as it has the key => value pairs that you wish to update.

This however is not what your function actually does.

For starters, you implicitly require a valid key/value pair for the primary key.

Not provided? $item[ $identifier ] will throw a warning and then default to null, as you attempt to read from a key which didn't exist before.

Neither did your comment describe in any way the "multi update" functionality. Especially the heuristic you are using to determine whether the passed array contains one or more data sets.

If only one of the (not actually ignored) additional fields is an array, the heuristic triggers and leads to a wrong handling of the argument.

Error handling?

$row->update($item);
$row->write();

Depending on where your ORM does validation, either of these two lines can yield runtime errors.

Especially in the case of the multi row persist, you need to think about how to handle the error case. If one row fails to insert/update, what to do with the rest?

Try to persist as much as possible? Inform the invoking site? And how does the ORM handle validation / SQL errors in the first place? Error codes? Exceptions?

Understanding what the database does

You've got a nasty bug / conceptual error in your code.

You code does work "fine" if the $identifier field is either entirely missing from $item, or if it contains a valid value.

Well, actually it does not work fine in the first case. Because what actually happens, is that accessing $item[ $identifier ] sets this field to null, so you are querying the database for a row with NULL as the primary key. You then continue to insert a new row with explicit null as the primary key, which the database then automatically replaces by the next sequence number. This side effect is a bug, and should be fixed.

But what happens if $item[ $identifier ] was set before, not to null, and not to an existing primary key either? You create a new row, but you did not use the sequence number generator intended by the database for that. Even being set to an empty string will most likely break this already.

Actually, when does it occur that the row, which was supposed to be updated, can't be found?

Either someone just guessed a random primary key (which should not happen), or the row was deleted in the meantime.

In the latter case - is it actually the expected behavior to re-add the formerly deleted row? Or throw an error? Un-assign the (known to be invalid) primary key and let the database assign a new one?

Either way, you certainly don't want a primary key in your table which wasn't generated by the sequence number generator. For various reasons.


Extension of JavaScript "typeof" I've written this "little" typeof extension for my JS. What do you think about it?

Its aim is to provide a single reliable typeof in JavaScript, consistent between native vars and objects. I've tested it both in FF 4+, IE 7+, Chrome 12, Safari 5. Also, if you have a different browser (also the version), can you test it?

typeOf = function(e) {
  if (typeof e === "undefined") {
    return "undefined";
  } else if (typeof e === "object") {
    if (e === null) {
      // null è un oggetto
      return "null";
    } else if (e instanceof String) {
      return "string";
    } else if (e instanceof Number) {
      return "number";
    } else if (e instanceof Boolean) {
      return "boolean";
    } else if (e instanceof Date) {
      return "date";
    } else if (e instanceof RegExp) {
      return "regexp";
    } else if (e instanceof Error) {
      return "error";
    } else if (e.isArray
                || e instanceof Array) {
      return "array";
    } else if (e instanceof Window) {
      return "window";
    } else if (e.nodeType) {
      switch (e.nodeType) {
        case 1:
          return "element";
        case 2:
          return "attribute";
        case 3:
          return "text";
        case 9:
          return "document";
      }
    } else if (e.call) {
      return "function";
    }
  } else if (typeof e === "function") {
    if (e.call) {
      return "function";
    } else if (e instanceof RegExp) {
      // V8
      return "regexp";
    }
  }

  return typeof e;
};

I don't think much of it, to be honest.  It's not as reliable as you might think, because once you start passing objects between contexts (e.g. different windows) it will fail miserably.  Add to that the fact that

Object.prototype.toString.call(<any value>);

could do most of your work for you and your extension starts to look a little redundant and unnecessary.

Here's a detailed article on this problem: Cross-context isArray and Internet Explorer


String split and tables I have a table structured like this:

UnitScanDB = {
    ["profiles"] = {
        ["Goblin"] = {
            ["Bars"] = {
                {
                    {
                        ["track"] = "Cloak of Shadows#Anti-Magic Shell#Hand of Freedom#Spell Reflection",
                    }, -- [1]
                    {
                        ["track"] = "..."
                    }, -- [2]
                    {
                        ["track"] = "..."
                    }, -- [3]
                }
            }
        }
    }
}

And I would like to convert it to this format:

UnitScanDB = {
    ["profiles"] = {
        ["Goblin"] = {
            ["Bars"] = {
                {
                    {
                        {
                            ["track"] = "Cloak of Shadows",
                        }, -- [1]
                        {
                            ["track"] = "Anti-Magic Shell",
                        }, -- [2]
                        {
                            ["track"] = "Hand of Freedom",
                        }, -- [3]
                        {
                            ["track"] = "Spell Reflection",
                        }, -- [4]
                    }, -- [1]
                    {
                     --stuff
                    }, -- [2]
                    {
                     --stuff
                    }, -- [3]
                }
            }
        }
    }
}

To do this automatically, I created this function:

function UnitScan:updater()
    local list = db:GetProfiles()
    for p, name in ipairs(list) do
        local icon = db.profiles[list[p]]["Bars"]
        for x, value in ipairs(icon) do
            for y, value in ipairs(icon[x]) do
                if (icon[x][y].track) then
                    for aura in string.gmatch(icon[x][y].track,"[^#]+") do
                        t = {}
                        t.track = aura
                        tinsert(icon[x][y], t)
                    end
                end
            end
        end
    end
end

Where

db = UnitScanDB
db:GetProfiles() = function returns all the keys in "profiles"

Is there a more elegant way to do this without using all those cycles, perhaps using the metatables?

IMO, there is a cleaner way to do this without even involving metatables. You have a bunch of tables that are nested together that contains some data on each level and you're looking to access and possibility modify a particular branch.

This is not that different from say a HTML DOM tree. This means you can use any standard tree traversal algorithm to find what you're looking for. For example, a table_gsub function(can't think of a better name atm), it takes a table, a string-key you're looking for, and a function to call when a match is found:

function table_gsub(t, selector, f)
    local visit = {}
    for k, v in pairs(t) do
        if type(v) == 'table' then
            table.insert(visit, v)
        end
    end
    if t[selector] then f(t) end
    for _, each in ipairs(visit) do
        table_gsub(each, selector, f)
    end
end

And a convert function to your desire format:

local function convert_format(t)
  assert(type(t.track) == 'string')
  for each in t.track:gmatch "%w[^#]+" do
    table.insert(t, {track = each})
  end
  t.track = nil
end

A simple test to make sure it does the right thing:

table_gsub(profiles, "track", convert_format)
dump(profiles)

It should output something like:

profiles = {
  Goblin = {
    Bars = {
      {
        {
          {
            track = "Cloak of Shadows"
          },
          {
            track = "Anti-Magic Shell"
          },
          {
            track = "Hand of Freedom"
          },
          {
            track = "Spell Reflection"
          }
        },
        {
          track = "..."
        },
        {
          track = "..."
        }
      }
    }
  }
}

You could of course take this idea further with multiple key-string selectors to match against giving you something like jquery-style lua table selection.


Framework to define and require JavaScript modules I'm working on a webapp that I use gulp to build. It uses useref to gather and concatenate/uglify the assets used in the HTML files.

In this case, as things are getting concatenated, it doesn't make much sense to use a framework like RequireJS to separate out my modules, and I think browserify is overkill as I don't need access to any node equivalents on the client side.

I have seen that some apps use a global App object that they attach things to. I wanted something slightly more robust so that I could "require" and "define" and not have the modules exposed on the App object, and be able to name them like in a require() arguments list instead of using App.component every time I wanted to reference one.

Here is my attempt for simple code that emulates a full on require/define methodology. This allows me to put the <script> tags that get concatenated in any order I want to in the HTML, and name the modules as they get passed in to definitions of other modules that require them.

'use strict';

(function(window) {

    var components = {},
        waiting = {};

    window.App = {};

    App.define = function(name, reqs, func) {
        if (typeof reqs === "function") {
            func = reqs;
            reqs = [];
        }
        App.require(reqs, func, name);
    };

    App.require = function(reqs, func, name) {
        var loaded = 0;

        if (!reqs.length) load(func, reqs, name);

        function callback() {
            if (++loaded === reqs.length) {
                load(func, reqs, name);
            }
        }

        for (var i = 0; i < reqs.length; i++) {
            if (components.hasOwnProperty(reqs[i])) {
                callback();
            } else {
                waitFor(reqs[i], callback);
            }
        }
    };

    function waitFor(name, callback) {
        if (waiting.hasOwnProperty(name)) {
            waiting[name].push(callback);
        } else {
            waiting[name] = [callback];
        }
    }

    function load(func, reqs, name) {
        var result = func.apply(window, orderedReqs(reqs));
        if (name) {
            components[name] = result;

            if (waiting.hasOwnProperty(name)) {
                for (var i = 0; i < waiting[name].length; i++) {
                    waiting[name][i](name);
                }
            }

            delete waiting[name];
        }
    }

    function orderedReqs(reqs) {
        var args = [];

        for (var i = 0; i < reqs.length; i++) {
            args.push(components[reqs[i]]);
        }

        return args;
    }

})(window);

Then I can do this in my module javascript files:

App.define('a', function() {
    console.log('a loaded');
});

App.define('b', ['a'], function(a) {
    // use a to do something if I had made it an object
    console.log('b loaded');
});

This custom solution I think is simple enough for my needs but also lends itself to some nice code with only one global and the module definitions hidden away inside of it.

What do other people do when they want something vanilla? Does this seem like a good solution? Feel free to critique my coding style, as any criticism is good criticism.

First off, you could start by naming the variables meaningfully. Your define looks like this:

App.define = function(name, reqs, func) {
    if (typeof reqs === "function") {
        func = reqs;
        reqs = [];
    }
    App.require(reqs, func, name);
};

If I were another dev (which I am), I'd wonder what reqs mean. Is it requests? Requirements? Oh wait, dependencies! That's the word. How about func? Should it be a callback? Hmm... probably moduleDefinition would be better, right? Naming them properly would allow others to review your code easily, and maybe allow you to remember your code, just in case you went on vacation for 2 weeks.

Now I know you're trying to emulate AMD, which is fine (I personally prefer it over Browserify). However, one recurring complaint from devs is the issue of the "array of dependencies". What if you had more than a handful of dependencies for the module?

App.define('MyModule',['oh','my','vegetables','thats','very','long'], function('its','very','unweildly','dont','you','say'){
  // End of ceremonious module definition.
});

I personally would recommend the "wrapped CommonJS" approach, where you pass in require into your module definition, and just have it run there. I'm assuming all your modules are loaded, nothing async and your solution is just to have them load in any order.

App.define('MyModule', function(require){
  var Foo = require('foo');
  var Bar = require('bar');
  var John = require('john');
  // and so on
});

Though technically longer in the long run, but it's more manageable and readable than an array and another list of variable names. Another advantage, unlike the real AMD (which does static analysis to fine CJS-style requires), is that your "modules" are already loaded in memory. Calling require this way, synchronously, is possible as require will merely return a reference to the loaded module.


Euler Problem 101 - Optimum polynomial Here is my solution in Haskell for the Euler problem #101. It relies on building a specific base of polynomes given the values we want to fit (memories of my linear algebra courses from a long time ago). The algorithm is quite fast but as a new Haskell programmer, I'd like your opinion on what could be done to improve this code, be it either Haskell conventions, style or optimization.

module Polynome.Optimal where

-- given a list, returns a polynome of degree length xs which is zero on all element of the list
getPolynome :: [Int] -> Int -> Int
getPolynome = foldr f (const 1) where
    f x acc n = (n-x) * acc n

getLists :: [Int] -> [[Int]]
getLists xs = zipWith (const . ($ xs)) (map remove [0..]) xs

remove :: Int -> [Int] -> [Int]
remove n = go 0
    where go _ [] = []
          go i (y:ys) | i == n = ys
                       | otherwise = y: go (i+1) ys

getBase :: [Int] -> [Int -> Int]
getBase xs = map getPolynome $ getLists xs

scalePolynome :: Int -> Int -> (Int -> Int) -> Int -> Int
scalePolynome x y p t = (p t * y) `div` p x

getPolyFit :: [(Int,Int)] -> Int -> Int
getPolyFit vs = foldr f (const 0) ls where
    (xs, _) = unzip vs
    bs = getBase xs
    ls = zipWith (\ (x,y) b -> scalePolynome x y b) vs bs
    f b acc t = b t + acc t

exValues :: [(Int,Int)]
exValues = [(i, u i) | i <- [1..], let u t = 1- t + t^2 - t^3 + t^4 - t^5 + t^6 - t^7 + t^8 - t^9 +t^10]

getPartialFit :: Int -> [(Int,Int)] -> Int -> Int
getPartialFit n = getPolyFit . take n

getFit :: [(Int,Int)] -> (Int -> Int) -> Int
getFit vs p = snd (head (dropWhile predicate $ zip vs ps)) where
    ps = map (\(x, _) -> p x) vs
    predicate ((x, y), p) = p == y

-- result 10 exValues returns the solution to problem 101
result :: Int -> [(Int,Int)] -> Int
result n values = sum fits where
    bops = map (($ values) . getPartialFit) [1..n]
    fits = map (getFit values) bops

I know it's hard to name things, but try to come up with more descriptive names for your functions. As a general rule, I would avoid using the prefix get... for pure functions. It's just noise and doesn't help the reader understand what the function is. In Haskell we try to describe what things are and less how things are done.

Some names which I think are somewhat better...

zeroPoly :: [Int] -> (Int -> Int)

e.g. zeroPoly [1,2,3] is a polynomial which is zero on 1, 2 and 3.

remove1 :: [a] -> [[a]]

e.g. remove1 as is all the ways of removing one element from the list as. Also, using the polymorphic signature alerts the reader that the function isn't specific to Ints.

I would also introduce a type alias:

type Poly = Int -> Int

so you can write the signature of scalePolynome (and other functions) as:

scalePolynome :: Int -> Int -> Poly -> Poly

which helps to describe better what it is doing. Anywhere you can use the alias  Poly instead of Int -> Int will help the reader.

In fact, I would even write the signature this way:

scalePolynome :: (Int,Int) -> Poly -> Poly

since you use (x,y) tuples elsewhere for points on the function.

You should be careful about using div here. Are you sure that with your basis polynomials you'll always get a integral result? To be safe you might have to perform the calculations with Rationals.

Instead of getBase I would probably call the function basisPolynomials - again, just avoid the prefix get....

Finally, note that your getList and remove functions can be written in terms of the inits and tails functions from Data.List:

import Data.List

remove1 :: [a] -> [[a]]
remove1 as = zipWith (++) (inits as) (tail $ tails as)


Analyzing records of London Underground outages You have a file that describes historic data of the London Underground showing dates when stations have been unavailable. The file contains these fields, separated by a pipe character:

  Date: Format of this is mm-dd-yyyy.
  Line: Each line has a unique name. For example, “Northern”.
  Station: Each station has a unique name. For example, “Angel”.
  Notes: Describes the outage

  Here is an example of one entry in the file, which shows that the Northern Line was not operating at Bank on 23 August 2019 due to a function at London Bridge.

08-23-2019|Northern|Bank |No service on Bank branch due to birthday booking at London Bridge station.

  Write a script that solves the following problems:

  Display each station that was affected by an outage on 10 July 2019.
  Between 10 July 2017 and 14 November 2017, which lines had outages and what was the number of stations affected?

I was able to complete both problems but the review of the work said it was done poorly.

# I created a test input file in the same folder as a proxy input.

import datetime  # import built in datetime module for problem 2
with open('input.txt') as file:  # read the input file into a list of lines
    file_input = file.read().splitlines()

# split the file input on the pipe
LonUndOutages = []
for var in file_input:
    LonUndOutages.append(var.split("|"))

# creating empty lists for the answers for Problem 2
Prob1Output = ""
LonUndStations = []
LonUndLines = []

# solving problem 1 and prep for the answers for Problem 2
print("Problem 1:")
for line in LonUndOutages:
    if line[0] == "07-10-2019":
        Prob1Output = Prob1Output + line[2] + "was affected by an outage on July 10, 2019\n"
    line[0] = datetime.datetime.strptime(line[0], '%m-%d-%Y')  # convert the date in line[0] to a datetime object
    if datetime.datetime(2017,7,10) < line[0] < datetime.datetime(2017,11,14):
        # the above line checks if line[0] is in the time range july 10 2017 to nov 14, 2017
        if line[1] not in LonUndLines:  # ensures LonUndLines doesnt have duplicates
            LonUndLines.append(line[1])  # adds each line that had an outage to a list
        if line[2] not in LonUndStations:  # ensures LonUndStations doesnt have duplicates
            LonUndStations.append(line[2])  # adds each station that had an outage to a list

if not Prob1Output:
    print("There were no stations affected by an outage on July 10, 2019")
else:
    print(Prob1Output)
print()
print("Problem 2:")
print("The following lines had outages:")
print(*LonUndLines, sep=', ')
print("During the period between July 10, 2017 and November 15, 2017 there were " + str(len(LonUndStations))
      + " station(s) affected")

That's a fun problem.

Don't use more memory than you have to

with open('input.txt') as file:  # read the input file into a list of lines
    file_input = file.read().splitlines()
# ...
for var in file_input:
    LonUndOutages.append(var.split("|"))

shouldn't be necessary. Rather than calling file.read(), iterate on the file object itself. It will yield lines that you can split on directly.

Naming conventions

LonUndOutages

becomes

lon_und_outages

and similarly for your other variables.

Unpack your line

Assign variables to the results of your split, like so:

date, line, station, notes = outage

Note that there's a nomenclature collision, because your spec describes one column as line, so call each file line a record or an outage.

Separate output from logic

        Prob1Output = Prob1Output + line[2] + "was affected by an outage on July 10, 2019\n"

You're storing output early for two purposes: to eventually show that output, and to know whether there were stations affected. This dual-purpose code is generally not advisable. Store the confirmed outages in a separate list, or yield them from a generator function, to be consumed and formatted by an output function that has no logic at all.

General

Run a linter that will tell you PEP8 hints, or use an IDE that will do the same. Make functions to house your logic.

Example

from collections import namedtuple
from datetime import date, datetime
from typing import Iterable, Set

Record = namedtuple('Record', ('date', 'line', 'station', 'notes'))

def read_file(fn: str) -> Iterable[Record]:
    with open(fn) as file:
        for record in file:
            date_str, *others = record.split('|')
            record_date = datetime.strptime(date_str, '%m-%d-%Y').date()
            yield Record(record_date, *others)

def stations_on_date(records: Iterable[Record], record_date: date) -> Set[str]:
    return set(r.station for r in records if r.date == record_date)

def lines_and_stations_between_dates(
    records: Iterable[Record],
    start: date,
    end: date,
) -> (Set[str], Set[str]):
    matching = [r for r in records if start <= r.date <= end]
    lines = set(r.line for r in matching)
    stations = set(r.station for r in matching)
    return lines, stations

def print_iter(to_print: Iterable[str]):
    msg = ', '.join(to_print)
    print(msg or 'none')

def main():
    records = tuple(read_file('input.txt'))

    date1 = date(2019, 7, 10)
    print(f'Stations affected by an outage on {date1}:')
    stations1 = stations_on_date(records, date1)
    print_iter(stations1)

    start2 = date(2017, 7, 10)
    end2 = date(2017, 11, 14)
    range_desc = f'between {start2} and {end2}'
    lines2, stations2 = lines_and_stations_between_dates(records, start2, end2)

    print(f'Lines with outages {range_desc}:')
    print_iter(lines2)

    print(f'Number of stations affected {range_desc}:')
    print(len(stations2))

if __name__ == '__main__':
    main()

Note:

Output and logic are separated
Use a namedtuple for better structure
Hold onto a date, not a datetime
Use set to avoid duplicates in the output


Alternative to setInterval and setTimeout JavaScript's setTimeout() and setInterval() are evil and not precise:

Both functions have a delay of a varying quantity of milliseconds.
Both functions are very resource-intensive because they execute several times every second.

A new alternative is window.requestAnimationFrame(), which is less resource-intensive, disabled on page blur, and does not slow down other stuff. This makes it the perfect substitute for a modern setTimeout() and setInterval().

Description

These functions use requestAnimationframe() to check if the time has passed
based on the elapsed Time calculated from Date.now().  The time passed is more precise than the native functions and theoretically less resource-intensive.  Another advantage (or disadvantage) is that the functions are not executed on page blur.

Good for: animations, visual effects

Bad for: timers, clock

RafTimeout

window.rtimeOut=function(callback,delay){
 var dateNow=Date.now,
     requestAnimation=window.requestAnimationFrame,
     start=dateNow(),
     stop,
     timeoutFunc=function(){
      dateNow()-start<delay?stop||requestAnimation(timeoutFunc):callback()
     };
 requestAnimation(timeoutFunc);
 return{
  clear:function(){stop=1}
 }
}

RafInterval

window.rInterval=function(callback,delay){
 var dateNow=Date.now,
     requestAnimation=window.requestAnimationFrame,
     start=dateNow(),
     stop,
     intervalFunc=function(){
      dateNow()-start<delay||(start+=delay,callback());
      stop||requestAnimation(intervalFunc)
     }
 requestAnimation(intervalFunc);
 return{
  clear:function(){stop=1}
 }
}

Usage

var interval1,timeout1;
window.onload=function(){
 interval1=window.rInterval(function(){console.log('interval1')},2000);
 timeout1=window.rtimeOut(function(){console.log('timeout1')},5000);
}

/* to clear
interval1.clear();
timeout1.clear();
*/

Demo

jsFiddle

Questions

Normally I don't write functions inside functions, but in this case it's probably a good solution. What about memory leaks if I create hundreds of these time-based functions?
Is there a better solution to clear those functions?
For heavy animations and multiple intervals and timeouts, I was thinking to activate a single requestAnimationFrame() loop which would check for intervals and timeouts inside a previously stored array... (but I think there should be no difference if there is just one requestAnimationframe or multiple). So how do the browsers handle those multiple requestAnimationframes()?

Note:

If the code above does not work here is the original code:

window.rInterval=function(a,b){var c=Date.now,d=window.requestAnimationFrame,e=c(),f,g=function(){c()-e<b||(e+=b,a());f||d(g)};d(g);return{clear:function(){f=1}}}//callback,delay
window.rtimeOut=function(a,b){var c=Date.now,d=window.requestAnimationFrame,e=c(),f,g=function(){c()-e<b?f||d(g):a()};d(g);return{clear:function(){f=1}}}

Update 4/23/2014:
Okay then, off to your updated code. Nothing much I can do here now. Here are some generic code optimization tips:

Name stuff verbosely. It's one issue I find with others' code. They tend to name it like t for time, or l for length. Unless it's something implied, like i,j and k for counters (and we do see loops), please name them verbosely.

Don't ever worry about long code. That's what minifiers are for. Name them verbosely, structure them properly. Don't even bother using the "comma-separated vars". Minifiers do that.

Ternaries tend to be messy and unreadable. If possible, use if-else. Again, minifiers also convert them to ternaries to be shorter.

As for possible bottlenecks, splice is an overhead because it resizes the array, shifting the contents around. It's much avoided like unshift and shift for the same reason. I don't know if keeping a "hole" in the array is a performance benefit either, so let's leave it as is.

A general performance tip is just to avoid creating objects inside the loops. If it can't be avoided, just make sure you free them up when done (free references).

So far, that's about it. I'll leave the fine-tuning to you.
Previous answer:

Normally i don't write functions inside functions, but in this case it's prolly a good solution. What about memory leaks if i create hundreds of this timebased functions?

That wouldn't be an issue. Besides, how many functions do you think the FB homepage, or any site for that matter, creates? What you should be wary about, though, is when your timers create functions on every iteration. That's what you should look out for. Along the lines of this:
function tick(){
  requestAnimationFrame(tick);

  // Oh...
  var aFunctionMadeOnEachTick = function(){...};
  aFunctionMadeOnEachTick();
}

Although some JS engines are smart enough to figure this out and pull out the function, I have seen performance decreases when doing this in some of my projects.

Is there a better solution to clear those functions?

I was thinking about something along the lines of a common function that can be referenced by each API function to generate a runner. This way, we have one function sitting internally waiting to be called, instead of one created each call.
;(function (ns) {

  // An array of references/ids/booleans/whatever that can be used to clear off running timers
  var timers = [];

  // The common function
  // Haven't figured out how the cb and delay gets called on the next tick, but
  // this should be a good template to think about.
  function runningFn(cb,delay) {
    if(iShouldStillRun) requestAnimationFrame(runningFn);
    if(Date.now() > something) execute();
  }

  // Your API functions
  ns.setTimeOut    = function (callback, delay) {
    runningFn(callback,delay);
  }
  ns.setInterval   = function (callback, delay) {...}
  ns.clearInterval = function (callback, delay) {...}
  ns.clearTimeout  = function (callback, delay) {...}

}(this.Timers = this.Timers || {}));

For heavy animations and multiple intervals,timeouts i was thinking to activate a single requestAnimationFrame loop which check for intervals and timeouts inside a previously stored array...

The last time I checked, looping through an array with 700+ items caused stuttering in the UI. Also, by the nature of timers, they can actually be delayed by these operations. Again, JS is single-threaded, and timers also live in that thread. If JS is busy churning, it can delay async tasks and that includes timers.
rAF at a glance
One thing about rAF is that it's built for animation. It strives to give you a 60fps. However, when it can't, it isn't any different from setInterval and setTimeout. Also, it depends on browser implementation. Some browsers throttle timers according to different situations, like unfocused windows/tabs.
Let's start cherry-picking
Now here are some interesting things about your implementation:

You have an internal function in each instance of a timer. Looks like the root of your worries.
But you also return an object for each instance for the clear. Why not store some sort of reference to running timers, and create another global function that clears them?
Like you found out, 60fps is 16ms per draw. However, timers are as fast as 4ms.


Repository pattern best practices using EF 6 I have been Googling for a few days and am trying to find the best practices when it comes to using the Repository pattern.  But what I found that there are no standards and everyone claims that their approach is the best approach.  However, I have come up with this design based on what I have read online.

It it good practice to pass DataContext?  Please feel free to correct me and explain.

I'm using MVC 5 with EF 6.

 public interface IRepository<T> where TEntity  : class
 {
        IEnumerable<TEntity> GetUsers();
        IQueryable<TEntity> SearchFor(Expression<Func<TEntity, bool>> predicate);
        TEntity GetById(int id);
        void Save(TEntity model);
        void Delete(int id);
 }

public class HostRepository : IRepository<HostViewModel>
{
    private EntityDataModelContext db = new EntityDataModelContext();

    //
    public IEnumerable<HostViewModel> GetUsers()
    {
        var userList = from u in db.Hosts select u;
        var users = new List<HostViewModel>();

        if (userList.Any())
        {
            foreach (var user in userList)
            {
                users.Add(new HostViewModel() { Id = user.Id,  FirstName = user.FirstName });
            }

        }
        return users;
    }
    //implement rest here......
 }

//I use an IoC framework to inject the dependencies

public class HostController : Controller
{
    public IRepository<HostViewModel> hostRepository { get; set; }

    public HostController(IRepository<HostViewModel> hostRepo)
    {
        hostRepository = hostRepo;
    }

    public ActionResult Index()
    {
        var users = hostRepository.GetUsers();
        return View(users);
    }
}

The Interface

The idea of a generic repository interface is that it should be applicable to all entity types. So GetUsers shouldn't be there, because it's only relevant for a particular type of entity. If you want members specific to an entity type, you should have a second interface which extends the generic one:

IUserRepository : IRepository<User>
{
    IEnumerable<User> GetUsers();
}

One of the debates you may well have seen is whether it's good practice to have IQueryable members, or to take Expression parameters.

The pro is that it's much more flexible. Rather than having to have, for example, FilterByStartDate(DateTime date), FilterByEndDate(DateTime date), FilterByUserGroup(int groupId), etc., I can just have FilterBy<T>(Predicate<T> condition). Likewise, by exposing IQueryable, you can call additional LINQ methods and it will all get translated into SQL, which can reduce the amount of work your database has to do, and the amount of data you have to transfer.

The con is that what you have is a leaky abstraction. It looks like I should be able to feed in any reasonable predicate, but in fact, many of them will throw exceptions at runtime because EF doesn't know how to translate them into SQL. So now the details of how exactly EF- which is supposed to be an abstraction over your database- can turn LINQ queries into SQL leak out of your repository, and every consumer of the repository has to worry about them.

On balance, my suggestion would be: at first, do NOT expose IQueryable or allow arbitrary Expressions to be passed in. Add specific methods for specific queries. If you ultimately find this isn't adequate, you can refactor by adding methods in.
Save is a confusing method. Does it mean insert or update? Generally with Entity Framework, you want an Add(TEntity entity) method and a Save() method. Add should be what inserts a record, and Save should commit any updates you've made to existing items. So, as a toy example, I could do:

var user = userRepository.GetById(userId);
DoThing(user);
user.ThingLastDoneTime = DateTime.Now;
userRepository.Save();

Note that I don't have to pass the user to Save, because it's actually just calling the DbContext's Save() method.

HostViewModel

This is where I start to get a bit more confused. Why is something called ViewModel being treated as an entity, or stored in a database?

Without knowing more it's hard to comment on what's going on here exactly, but EF is supposed to let you handle Entities- these are essentially domain objects. So a User is a good candidate for an entity. A Host might be, depending on what "host" actually means in this context. But a ViewModel? That seems like extreme mixing of concerns between your UI and your core domain.

Or, possibly what's going on is that your actual entity is a Host, and the repository is not only doing its usual repository business, but also in charge of mapping from an entity to a view model. This is also a problematic mixing of concerns. You may, for example, have a view model which does not have sufficient information to create a new entity from. So how would you handle the Add method for that type? Transformation between entities and view models is a UI concern and should be kept away from the repository pattern, which is a data access/persistence pattern.

HostRepository

As already mentioned, it's very weird that GetUsers should be returning a collection of HostViewModels.

The bulk of that method can be simplified with LINQ, too:

return db.Hosts.Select(user => new HostViewModel() { Id = user.Id,  FirstName = user.FirstName });

The Controller

I assume your comment about IoC containers is for us and not really in the code. If it's in the code, it shouldn't be!
hostRepository should also be private. Other classes don't need to get it, and shouldn't be allowed to set it except through the constructor.
You've shortened hostRepository to hostRepo in the constructor. Generally you shouldn't shorten variable names, especially if- as in this case- they're going to be exposed publically to consumers. To deal with class-level vs method level variables, all of the following are popular:

For public class members, use PascalCase (first letter capital) rather than camelCase
For private class fields and properties use _camelCase, with a leading underscore.
As an alternative to the previous, for private class fields and properties, use camelCase. Then inside methods, prefix them with this.


Script to load the ISS location and data I wrote a script to fetch the location for the ISS using two simple APIs.

This is my first time writing a python script. Any advice or modifications would be highly appreciated. How is the code, and where are the areas I can improve upon?

This code is also on GitHub.

import urllib.request # to make requests to the api
import json # to parse the json response
import reverse_geocoder as rg # to get address from location

url = "http://api.open-notify.org/astros.json"
response = urllib.request.urlopen(url) # request the api, returns a JSON object
json_result = json.loads(response.read()) # read the JSON object

# Separate different values based on keys.
people = json_result['people'] # people currently in space
number_in_space = json_result['number'] # total people in space
print("People in space : ", number_in_space)
print("----NAMES----")

for p in people:
    print(p['name'])

# ISS data
iss_url = "http://api.open-notify.org/iss-now.json"
iss_response = urllib.request.urlopen(iss_url)
iss_json_result = json.loads(iss_response.read())
# Store the positions
latitude = iss_json_result['iss_position']['latitude'] # store the latitude
longitude = iss_json_result['iss_position']['longitude']
print("Latitude: ", latitude," -- ", "Longitude: ", longitude)

address = rg.search((latitude, longitude)) # Get the address from location tuple returns a list
address_name = address[0]['name']
address_admin1 = address[0]['admin1']
address_admin2 = address[0]['admin2']
address_cc = address[0]['cc']
print("----Address----")
print(address_name, ", ", address_admin1, address_admin2, address_cc)

Not bad for a first Python script!

In general, you can greatly benefit from giving parts of your code names, by encapsulating them in functions. This makes it also re-usable and let's you add a docstring giving more detail on what this function does.

If you add a main function which you call under a if __name__ == "__main__": guard you can also import these functions from another script without all of your code running.

Instead of urllib.request, you can use the requests module, it even has a method to directly return a JSON object.

While it is nice that the astronauts response contains the number of astronauts in space, you could get the same information from calling len(people), which is \$\mathcal{O}(1)\$ for Python lists.

Familiarize yourself with f-strings (Python 3.6+). They are a nice and compact way to write formatted strings, since they can contain arbitrary Python expressions (I used it for the address here).

import requests
import reverse_geocoder

def get_json(url):
    """Retrieve JSON object from url.

    Raises an exception if connection fails.
    """
    response = requests.get(url)
    response.raise_for_status()  # make sure an exception is raised if something goes wrong
    return response.json()

def get_astronauts():
    """Returns a list of all people currently in space"""
    url = "http://api.open-notify.org/astros.json"
    return get_json(url)['people']

def get_iss_location():
    """Returns the current latitude and longitude of the ISS"""
    url = "http://api.open-notify.org/iss-now.json"
    position = get_json(url)['iss_position']
    return position['latitude'], position['longitude']

def get_address(position):
    """Do a reverse lookup getting the closest address to a given position"""
    return reverse_geocoder.search(position)[0]

def main():
    astronauts_in_space = get_astronauts()
    print("People in space : ", len(astronauts_in_space))
    print("----NAMES----")
    for astronaut in astronauts_in_space:
        print(astronaut['name'])

    iss_position = get_iss_location()
    print("Latitude: {}, Longitude: {}".format(*position))

    address = get_address(iss_position)
    print("----Address----")
    print(f"{address['name']}, {address['admin1']} {address['admin2']} {address['cc']}")

if __name__ == "__main__":
    main()


Initializing BufferedReader and DataOutputStream in infinite loop Is it a good practice to initialize the BufferedReader and DataOutputStream in infinite loop?

        ServerSocket welcomeSocket = new ServerSocket(8080);
        Socket connectionSocket = welcomeSocket.accept();

        while (true) {
            BufferedReader inFromClient = new BufferedReader(new InputStreamReader(connectionSocket.getInputStream())); //close it
            DataOutputStream outToClient = new DataOutputStream(connectionSocket.getOutputStream()); //close it
            //... Do read/write stuff
        }

Connection establishes successfully and communication happens normally. Just want to know that whether it is a good practice or not? It is a Java code.

Thanks.

At the very least use a try-with-resources:

while (true) {
    try(BufferedReader inFromClient = new BufferedReader(new InputStreamReader(connectionSocket.getInputStream())),
            DataOutputStream outToClient = new DataOutputStream(connectionSocket.getOutputStream())){
        //... Do read/write stuff
    }
}

Then when the loop body is done (by normal control flow or by exception) the streams will be closed normally.

However if you want a lot of clients connecting at a time and you want them all to be able to interact at the same time you will need to delve into the async IO in the java.nio.channels package.


Which is better code for implementing quicksort? This is how I used to implement quicksort:

def quickSort(alist):
   quickSortHelper(alist,0,len(alist)-1)

def quickSortHelper(alist,first,last):
   if first<last:

       splitpoint = partition(alist,first,last)

       quickSortHelper(alist,first,splitpoint-1)
       quickSortHelper(alist,splitpoint+1,last)

def partition(alist,first,last):
   pivotvalue = alist[first]

   leftmark = first+1
   rightmark = last

   done = False
   while not done:

       while leftmark <= rightmark and alist[leftmark] <= pivotvalue:
           leftmark = leftmark + 1

       while alist[rightmark] >= pivotvalue and rightmark >= leftmark:
           rightmark = rightmark -1

       if rightmark < leftmark:
           done = True
       else:
           temp = alist[leftmark]
           alist[leftmark] = alist[rightmark]
           alist[rightmark] = temp

   temp = alist[first]
   alist[first] = alist[rightmark]
   alist[rightmark] = temp

   return rightmark

alist = [54,26,93,17,77,31,44,55,20]
quickSort(alist)
print(alist)

But recently I have come across this way of implementing quicksort:

def quickSort(L):
    if L == []: return []
    return     quickSort([x for x in L[1:] if x< L[0]]) + L[0:1] + quickSort([x for x in L[1:] if x>=L[0]])

a = [54,26,93,17,77,31,44,55,20]

a = quickSort(a)
print(a)

Which implementation is better in terms of memory usage and time complexity?

Both are very similar - list of 100000 random int elements (from 0 to 100000) was sorted in (average time taken from 10 tries):

#1st implementation
Average time:
Random: 0.4825087092408925

#2nd implementation
Average time:
Random: 0.48071902671725464

That being said, both implementations aren't good. A lot of people forget to include shuffling at the beginning of quicksort and it has really long call stack. When I added some data sets:

data_sets = {'Sorted': [x for x in range(how_many)],
             'Reverse': list(reversed([x for x in range(how_many)])),
             'Random': [random.randint(0, how_many) for x in range(how_many)],
             'A lot of equal keys': [x if x % 2 else 0 for x in range(how_many)],
             'Same keys': [1] * how_many}

I managed to sort data which was 900 elements in length (more than that gave me recursion error) and performance took a big hit in case of sorted / reversed / lot of equal keys data:

Average time:
Same keys: 0.061130084848424526
Sorted: 0.060439534806328445
A lot of equal keys: 0.032078340092241295
Reverse: 0.06125047935425505
Random: 0.0030011541824106593

EDIT:
As per suggestion in comment, I ran quicksort (with shuffling) and compared it with list.sort(). quicksort was first, then I ran list.sort(), waited for a bit and re-run with reversed order - times were very close in both cases.

100000 items, 10 tries. Tests were ran on my laptop, so hardware is different than in original post.

#QUICKSORT
Average time:
A lot of equal keys: 0.9257455763466982
Sorted: 1.605769951669822
Reverse: 1.5616443986206265
Random: 1.471783668415066
Same keys: 0.28260723572820756

#SORT
Average time:
A lot of equal keys: 0.025862182302070193
Sorted: 0.004639602319800995
Reverse: 0.004855047960047393
Random: 0.10296205183842413
Same keys: 0.0034818929132122674