Function to generate an Int from a series of bits I wrote the following function in order to handle some data from a QR code:

func genInt(fromBitArray array: [Int]) -> Int {
    var n = 0
    for i in 0...array.count - 1 {
        n += array[i] << ((array.count - 1) - i)
    }
    return n
}

genInt(fromBitArray: [1, 1, 0]) would return 6.

Is there a better/more efficient way to do this?

Improving the code

Your code computes
$$
 a_0 2^{n-1} + a_1 2^{n-2} + \ldots a_{n-2} 2 + a_{n-1} \, ,
$$
that is the polynomial
$$
 P(x) = a_0 x^{n-1} + a_1 x^{n-2} + \ldots a_{n-2} x + a_{n-1} \,
$$
evaluated at \$ x=2 \$. A well-known, efficient method to evaluate polynomials is Horner's method:
$$
 P(x) = (\ldots ((a_0 x + a_1) x + a_2) x + \ldots ) x + a_0 \, .
$$

Applied to your task that gives

func genInt(fromBitArray array: [Int]) -> Int {
    var n = 0
    for i in 0...array.count - 1 {
        n = 2 * n + array[i]
    }
    return n
}

Instead of the bit shift (by a variable amount) we have now multiplications by the fixed factor \$ 2 \$. But this can be simplified further. Inside the loop we need now only the array element at the current index, but not the index itself. Therefore we can enumerate the array instead:

func genInt(fromBitArray array: [Int]) -> Int {
    var n = 0
    for digit in array {
        n = 2 * n + digit
    }
    return n
}

And for this kind of combining array (or more general: sequence) elements there is a dedicated method in the Swift standard library, reduce():

func genInt(fromBitArray array: [Int]) -> Int {
    return array.reduce(0) { (accum, digit) in
        return 2 * accum + digit
    }
}

Naming

The “official” Swift naming conventions are describe in the API Design Guidelines, e.g.

Name variables, parameters, and associated types according to their roles, rather than their type constraints.
Begin names of factory methods with “make”, e.g. x.makeIterator().
Avoid abbreviations.

With these rules in mind I would suggest something like

func makeInteger(fromBits array: [Int]) -> Int

Or if one sees this as a way to construct an integer then a custom initializer would be appropriate

extension Int {
    init(bits array: [Int]) {
        self = array.reduce(0) { (accum, digit) in
            return 2 * accum + digit
        }
    }
}

// Example:
let value = Int(bits: [1, 1, 0])

Miscellaneous

It is expected that the array contains only the values zero and one. With an assertion this can be verified during the test phase, without performance impact on the released code:

extension Int {
    init(bits array: [Int]) {
        self = array.reduce(0) { (accum, digit) in
            assert(digit == 0 || digit == 1, "invalid binary digit")
            return 2 * accum + digit
        }
    }
}

If you want to create other integer types from a bit array in the same manner then you can defined the initializer on the BinaryInteger protocol instead:

extension BinaryInteger {
    init(bits array: [Int]) {
        self = array.reduce(0) { (accum, digit) in
            assert(digit == 0 || digit == 1, "invalid binary digit")
            return 2 * accum + Self(digit)
        }
    }
}

// Example:
let value = Int16(bits: [1, 1, 0])


Improving Twitter JSON parser for big data set I have the following fully working code that

imports JSON files,
parses the tweets contained in JSONs,
records them in a table in a data frame.

Considering that per run I currently analyze 1,400 JSONs (about 1.5Gb), the code takes quite some time to run. Please suggest if there is a plausible way to optimize the code in order to increase its speed. Thanks!

import os
import json
import pandas as pd
import numpy as np
import matplotlib.pyplot as plt
import os

tweets = []

for dirs, subdirs, files in os.walk('/Users/mymac/Documents/Dir'):
    for file in files:
        if file.endswith('.json'):
            print(file)
            for line in open(file) :
                try:
                    tweet = json.loads(line)
                    tweets.append(tweet)
                except:
                    continue

tweet = tweets[0]

ids = [tweet['id_str'] for tweet in tweets if 'id_str' in tweet]
text = [tweet['text'] for tweet in tweets if 'text' in tweet]
lang = [tweet['lang'] for tweet in tweets if 'lang' in tweet]
geo = [tweet['geo'] for tweet in tweets if 'geo' in tweet]
place = [tweet['place'] for tweet in tweets if 'place' in tweet]

df=pd.DataFrame({'Ids':pd.Index(ids),
                 'Text':pd.Index(text),
                 'Lang':pd.Index(lang),
                 'Geo':pd.Index(geo),
                 'Place':pd.Index(place)})
df

Just a few quick considerations:

You have import os twice
You are not using matplotlib and numpy, so the imports can go
The line tweet = tweets[0] is useless
You're not closing the files you open, you should use the with keyword

Two optimizations:

I'd remove the print(file). This is probably single best optimization you can do
You're already looping once, why do you loop another five times?

What about something like this (not tested!):

from collections import defaultdict

elements_keys = ['ids', 'text', 'lang', 'geo', 'place']
elements = defaultdict(list)

for dirs, subdirs, files in os.walk('/Users/mymac/Documents/Dir'):
    for file in files:
        if file.endswith('.json'):
            with open(file, 'r') as input_file:
                for line in input_file:
                    try:
                        tweet = json.loads(line)
                        for key in elements_keys:
                            elements[key].append(tweet[key])
                    except:
                        continue

df=pd.DataFrame({'Ids': pd.Index(elements['id']),
                 'Text': pd.Index(elements['text']),
                 'Lang': pd.Index(elements['lang']),
                 'Geo': pd.Index(elements['geo']),
                 'Place': pd.Index(elements['place'])})
df


Is there a cleaner way of building a list of the previous, next months than this? At first I tried using a timedelta but it doesn't accept months as an argument, so my next working example is this:

from datetime import datetime
current_date = datetime.now()

months = []
for month_adjust in range(2, -3, -1): # Order by new to old.
    # If our months are out of bounds we need to change year.
    if current_date.month + month_adjust <= 0:
        year = current_date.year - 1
    elif current_date.month + month_adjust > 12:
        year = current_date.year + 1
    else:
        year = current_date.year
    # Keep the months in bounds.
    month = (current_date.month + month_adjust) % 12
    if month == 0: month = 12
    months.append(current_date.replace(year, month=month, day=1))

Is there a cleaner way?

Based on this answer, I would do something like :

from datetime import date
from dateutil.relativedelta import relativedelta
today = date.today()
months = [today + relativedelta(months = +i) for i in range(2,-3,-1)]

It's always better not to have to play with the dates manually as so many things can go wrong.

(I don't have access to a machine with relativedelta here so it's not tested but I'm pretty confident it should work)


Duplicate-preserving collection intersection Python sets are magical things. Mutable, deduplicating data stores with handy operator overloads for &, ^ and |.

Specifically among the others, sets overload the & operator to do mutual membership tests like so:

>>> set("aasd") & set("aabc")
{'a'}

This is endlessly handy, but the huge downside of sets is they do not allow duplicate entries because they're hash tables, so for instance a fuzzy-finder will be a little too fuzzy.

What to do? Why, spin my own, of course!

def membertester(a, b):
    """a slow (iterative) dupe-preserving sorting subset tester,
    for mutual membership tests (like set overloads &)"""
    c = []
    a, b = sorted(list(a)), sorted(list(b))
    (l, maxl, s, minl) = {
        True:  (a, len(a), b, len(b)),
        False: (b, len(b), a, len(a))
    }[ len(a) >= len(b) ]

    for i in range(maxl):
        try:
            if s.count(l[i]) >= l.count(l[i]):
                c.append(a[i])

        except IndexError:
            pass
    return c

How it works, because it might be a little cryptic:

Retrieve the longer and shorter of the two sorted lists, and their lengths, using a switch statement.
For each item in the longer range, take note of the item if the number of occurences of the same item in the short list is greater than or equal to the number of occurences of the item in the longer list.

Used as above:

>>> membertest("aasd", "aabc")
['a', 'a']

Success!

How can I improve it? Is there a faster or more efficient way, or perhaps a less flimsy algorithm?

Assertions that should pass:

assert membertest("a" * 6, '') == []
assert membertest("abdeg", "rrertabasdddhjkdeg") == ['a', 'b', 'd', 'e', 'g']
assert membertest("abddeg", "rrertabasdddhjkdeg") == ['a', 'b', 'd', 'd', 'e', 'g']

An inconsistency I don't understand, but which is definitely a bug:

>>> is_subset('10', '122')
['0']
>>> is_subset("ab", "acc")
['a']

I don't understand this in the slightest.

I wrote this function specifically for use with A-Za-z, so numbers in strings was an oversight I didn't consider.

The other bugs noted in @alexwchan's great answer are definitely not intentional.

A few quick comments:

As written, the code doesn’t always give the correct answer. Here are some cases I found that give what appear to be incorrect results:

>>> membertester('00000112', '000000011')
['2']
>>> print membertester('0001', '000/2')
['0', '0', '1']
>>> print membertester('002', '001/')
['0', '2']
>>> print membertester('10', '122')
['0']

If I’ve understood the intention of the program correctly, those are all wrong. If they’re correct, you need to do a better job of explaining the program.
The way you’ve written it is a bit fiddly – some comments interspersed with the program would make it much easier to read. That, and choosing better variable names – characters are cheap, don’t just use single letters.

For example, how about longer and shorter instead of l and s?
The construction for assigning (l, maxl, s, minl) is very weird, and it took me several reads to make sure I’d understood it correctly. This is too clever – I’d break it up into multiple statements. A few ways to do simplify it:

You don’t use the minl variable, so get rid of it. (If you meant to use it, is that a bug?)
The for loop can be replaced by for i, item in enumerate(longer), doing away with the need for the maxl variable.
You could assign the two remaining variables in a neater way:

shorter, longer = sorted([a, b], key=len)

or as an if statement if you wanted to be really explicit.

After a bit of tidying, this is what my version of the function looks like (note that it still has the same bugs as above):

def membertester(list_a, list_b):
    """a slow (iterative) dupe-preserving sorting subset tester,
    for mutual membership tests (like set overloads &)"""
    common = []
    shorter, longer = sorted([list_a, list_b], key=len)

    for idx, item in enumerate(longer):
        try:
            if shorter.count(item) >= longer.count(item):
                common.append(list_a[idx])
        except IndexError:
            pass
    return common

Sidebar: how did I find those examples?

I’m not psychic, and I’m not good at spotting obscure bugs. Instead, I used Hypothesis, which is a powerful randomised, property-based testing library. Here’s the property I used:

  Every mutual member of two lists must be a member of each list.

Sounds reasonable, right?

And here’s how I expressed that as a test:

from hypothesis import given, strategies as st

@given(st.text(), st.text())
def test_that_mutual_members_are_in_both_lists(list_a, list_b):
    mutual_members = membertester(list_a, list_b)
    for item in mutual_members:
        assert item in list_a
        assert item in list_b

Hypothesis tries hundreds of randomised examples, and checks whether this property holds – it found a bug on the second run. I highly recommend trying it.

Sidebar 2: what causes that bug?

Notice that in all of the examples, we have len(b) > len(a). So we have longer = b, shorter = a. As you go through the for loop, you’re iterating over the elements of b, but you’re using a to choose which element gets marked as common. You want to replace

c.append(a[i])

with

c.append(l[i])

With that in mind, you can make the code much cleaner:

def membertester(list_a, list_b):
    """a slow (iterative) dupe-preserving sorting subset tester,
    for mutual membership tests (like set overloads &)"""
    common = []
    shorter, longer = sorted([list_a, list_b], key=len)

    for item in longer:
        if shorter.count(item) >= longer.count(item):
            common.append(item)

    return common

I’ve been unable to find any bugs in this implementation.


Can XSS or anything else get through this? I coded a php chat that stores the text in a txt file and retrieves it, and I want to know if XSS can get past it or if someone can replace xhr.open("POST","uhh.php"); with something that contains the same php code but without htmlspecialchars or something like that.  What are the vulnerabilities? (More specifically, I don't want people putting html elements through the chat, and that also leads to security issues.)

This is uhh.php:

<?php
$usr=$_POST["usr"];
$msg=$_POST["msg"];
if($usr&&$msg){
    $txt="$usr: $msg";
    $txt=htmlspecialchars(join("\n",str_split($txt,33)));
    $split=explode("\n",file_get_contents("uhh.txt") . $txt);
    while(count($split)>10){
        $split=array_slice($split,1);
    }
    file_put_contents("uhh.txt",join("\n",$split) . "\n");
}
?>
watchu lookeen at???

And this is the code to the html page (JavaScript included):

<!DOCTYPE html>

<title>Chatting</title>

Bagelhead: lol
Bagelhead: yes t is
Abe: ok proxy off now
Bagelhead: gtg
Bagelhead: :P
Test: Test
Abe: &lt;p&gt;Test&lt;/p&gt;
Abe: Test
TEST: ab
TEST: abe

<input maxlength="9001" placeholder="Press / to chat" type="text" id="chatbar">

uhh.txt is just a blank text file.

Looks safe to me, but I would change the way you request the server. This way, you send many requests to the server and for more then few people using it, the server could be flooded by these requests. It would however require switching txt file for script file. Then you could use so called long poll.

You are probably wondering what long poll is and how it works. Long poll is technique where client opens connection to the server and leaves it open. When new message comes, server sends it and closes the connection. Trick is, client immediately opens new connection. Same happens when old connection times out.

Problem here is, when you use PHP on server side, it just executes some code and closes the connection. So, you have 2 possibilities: Best idea is to use server written in node.js, which is made for such things (and there are other advantages like that you don't have to store the message anywhere, you just recieve it and immediately send it to all clients and other...). If you want or have to use PHP, then you must use the sleep() function. Then, you have while loop, that each for example 200 miliseconds check for new message in database and either sends it or sleeps for another 200 miliseconds.

If you want to try, here is part of my client code which takes care of server communication:

//function, that returns right kind of AJAX object based on browser
//I don't have worst case scenario (AJAX not supported) here, because I usually check for required support at the beginning of the script
function createAjax() {
    if (window.XMLHttpRequest) {
        return new XMLHttpRequest();
    } else {
        return new ActiveXObject('Microsoft.XMLHTTP');
    }
}

/*
Class that takes care of receiving new messages from server
PARAMS:
    longPollServerURL - string - URL of server file, which acts as long poll server
    lastIdGetURL - string - URL of txt file, where last message id is saved
    newMessageCallback - function - callback to get called when new message is received
        it is called with 4 params:
            id of the message, time when the message was sent as unix timestamp,
            nickname of the person who sent it, message text itself
*/
function ChatLongPoll(longPollServerURL, lastIdGetURL, newMessageCallback) {
    var mainContext = this,
        httpRequest = createAjax();

    //function that starts long poll and sets what should happen when connection ends
    this.startLongPoll = function() {
        httpRequest.onreadystatechange = function() {
            if (httpRequest.readyState == 4) {
                var _decodedResponse = JSON.parse(httpRequest.responseText);
                if (_decodedResponse.status == 'new') {
                    for (var i = 0; i <_decodedResponse.messages.length; i++) {
                        newMessageCallback(_decodedResponse.messages[i].id, _decodedResponse.messages[i].time, _decodedResponse.messages[i].from, _decodedResponse.messages[i].messageText);
                        mainContext.lastMessageId = _decodedResponse.messages[i].id;
                    }
                }
                mainContext.openLongPoll();
            }
        };
        mainContext.openLongPoll();
    };

    //function that opens connection to the server
    this.openLongPoll = function() {
        httpRequest.open('GET', longPollServerURL + '?lastId=' + mainContext.lastMessageId, true);
        httpRequest.send(null);
    };

    //function that downloads ID of last message sent
    this.getLastMessageId = function() {
        var _id = false;
        httpRequest.onreadystatechange = function() {
            if (httpRequest.readyState == 4) {
                _id = httpRequest.responseText;
            }
        };
        httpRequest.open('GET', lastIdGetURL, false);
        httpRequest.send(null);
        return _id;
    };

    this.lastMessageId = this.getLastMessageId();
}

/*
Class that takes care of sending messages to server
PARAMS:
    newMessageServerURL - string - URL of file, which saves and validates the message
    callback - function - function to handle response from server (ok / wrongFormat / noName / ... status etc... (that depends on how you make your server))
*/
function ChatSendMessage(newMessageServerURL, callback) {
    var httpRequest = createAjax();

    httpRequest.onreadystatechange = function() {
        if (httpRequest.readyState == 4) {
            callback(JSON.parse(httpRequest.responseText));
        }
    };

    //function that sends the message to server and saves it to httpRequest.lastMessage
    this.sendMessage = function (from, messageText) {
        var L_postData = 'from=' + encodeURIComponent(from) + '&messageText=' + encodeURIComponent(messageText);

        httpRequest.lastMessage = messageText;

        httpRequest.open('POST', newMessageServerURL, true);
        httpRequest.setRequestHeader('Content-type', 'application/x-www-form-urlencoded');
        httpRequest.send(L_postData);
    };
}

/*
Short class that just simplifies access to those 2 classes before
PARAMS:
    newMessageServerURL - string - URL of file, which saves and validates the message
    longPollServerURL - string - URL of server file, which acts as long poll server
    lastIdGetURL - string - URL of txt file, where last message id is saved
    messageReceivedCallback - function - callback to get called when new message is received, it is called with 4 params: id of the message, time when the message was sent as unix timestamp, nickname of the person who sent it, message text itself
    sentMessageCallback - function - function to handle response from server (ok / wrongFormat / noName / ... status etc... (that depends on how you make your server))
*/
function ServerCommunicator(newMessageServerURL, longPollServerURL, lastIdGetURL, messageReceivedCallback, sentMessageCallback) {
    var longPoll = new ChatLongPoll(longPollServerURL, lastIdGetURL, messageReceivedCallback);
    var sendMessage = new ChatSendMessage(newMessageServerURL, sentMessageCallback);

    this.sendMessage = sendMessage.sendMessage;
    this.startLongPoll = longPoll.startLongPoll;
}


Web service getting value using LINQ from queue table in SQL database As a test of my C# skills, I have been asked to create a simple web service which will take a message from a queue table in a SQL database and send it to a web application when a button is pressed on the application.  I have never written a web service before so I am going in a little blind here so was after some advise on whether I had done this correct or not.

The stored procedure usp_dequeueTestProject gets a value from the top of the list in a table and then deletes the row in that table.  At the moment I do not have this being archived anywhere, would it be better practice to instead of delete this, to just mark it as sent instead?

[WebService(Namespace = "http://tempuri.org/")]
[WebServiceBinding(ConformsTo = WsiProfiles.BasicProfile1_1)]
[System.ComponentModel.ToolboxItem(false)]
// To allow this Web Service to be called from script, using ASP.NET AJAX, uncomment the following line.
// [System.Web.Script.Services.ScriptService]
public class Service1 : System.Web.Services.WebService
{
    [WebMethod]
    public string GetDataLINQ()
    {
        try
        {
            TestProjectLinqSQLDataContext dc = new TestProjectLinqSQLDataContext();
            var command = dc.usp_dequeueTestProject();
            string value = command.Select(c => c.Command).SingleOrDefault();
            return value;
        }
        catch (Exception ex)
        {
            return ex.ToString();
        }
    }
}

I've opted for using LINQ for starters, I am not sure if this is the best way to do it or not?  I am only passing through the one string as well... In reality I guess you would normally want to send more than one field, such as datetime sent, message type etc, but wasn't sure what data type to use for this? I have seen this done using a Strut, but wasn't sure if this was correct?  Any guidance would be greatly appreciated.

LINQ is a good choice. You're querying something, so it lends itself to the task.

That being said, the exception handling is atrocious. Making your return values null, some command's value from the database or some big honkin' exception text is abusing the string data type. I'd remove the try..catch altogether and let your client app handle the exception if it knows what to do. In the best case, your catch could log the error somewhere then re-throw.


Extract value from scala collect case I have the following code which reads data from a key/value data storage.
At some stage I want to go through everything in the storage and check expiry times and remove values - as done in the code below.

My problem here is that dataMapStorage.get(key) is called twice for keys which pass the filter and this can potentially be a heavy operation (I/O).

dataMapStorage.keys
  .filter { key =>
    val value = dataMapStorage.get(key)
    currentTime.getMillis - value.getMillis > timeout
  }
  .foreach { key =>
    val value = dataMapStorage.get(key) // Want to avid this double lookup
    // ... do some work with "value"
    dataMapStorage.remove(key)
  }

I presume the way around it is using collect (I do not want to map through the storage and extract all the values as this might be large), but am stuck here too with the 'get' method:

dataMapStorage.keys
  .collect {
    case key if {
      val value = dataMapStorage.get(key)
      currentTime.getMillis - value.getMillis > timeout
    } => (key, value) // How can I get the "value"???
  }
  .foreach { pair =>
    val value = pair._2
    // ... do some work with "value"
    dataMapStorage.remove(pair._1)
  }

How can I run this code getting the value from storage only once and not first creating a list of all the values?

Your filter isn't actually producing any benefit if the resulting collection isn't saved for further processing.

dataMapStorage.keys
  .foreach { key =>
    val value = dataMapStorage.get(key)
    if (currentTime.getMillis - value.getMillis > timeout) {
      // ... do some work with "value"
      dataMapStorage.remove(key)
    }
  }

Also, if dataMapStorage is just a key->value Map then couldn't you simplify it?

dataMapStorage.foreach { case (key, value) =>
    if (currentTime.getMillis - value.getMillis > timeout) {
      // ... do some work with "value"
      dataMapStorage.remove(key)
    }
  }

You could use collect() here but since the procedure results in a side-effect I think foreach() is more explanatory.

Of course the real Scala FP approach is to avoid mutable data structures.

val newDataMapStorage = oldDataMapStorage.filter { case (key, value) =>
    if (currentTime.getMillis - value.getMillis > timeout) {
      // ... do some work with "value"
      false
    } else true
  }


Roguelike Building Generator I decided to make a roguelike for fun. Here's the first part, a building generator (in the link at the bottom of the post, each square represents a building):

var mm=millis()
//var mm2=0

var rooms=[]
var S=63 //What's a better name for S?
S+=S%2 //forces S to be even

for(var i=0;i<S;i++){
    rooms[i]=[]
    for(var j=0;j<S;j++){
        rooms[i][j]=Infinity
    }
}

var pick=function(array){

    var queue=[]
    //var mm3=millis()
    for(var i=1;i<S-1;i++){
        for(var j=1;j<S-1;j++){
            if(rooms[i][j]==Infinity){
                var t = min(min(rooms[i-1][j],rooms[i][j-1]),
                            min(rooms[i+1][j],rooms[i][j+1]))
                if(t<Infinity){
                    queue.push([i,j,pow(S,2)])
                }
            }
        }
    }
    //mm2+=millis()-mm3
    return queue[~~(Math.random()*queue.length)]
}

var createRoom=function(pos){
    rooms[pos[0]][pos[1]]=pos[2]
}

var drawRooms=function(){
    pushMatrix()
    //scale(4.0)
    colorMode(HSB)
    background(255, 0, 255)
    noStroke()
    for(var i=0;i<S;i++){
        for(var j=0;j<S;j++){
            if(rooms[i][j]!==Infinity){
                fill(rooms[i][j]*10,255,255)
                rect(i*width/S,j*height/S,ceil(width/S),ceil(height/S))
            }
        }
    }
    popMatrix()
}

var redo=function(x,y,t){
    if(rooms[x][y]!==Infinity&&rooms[x][y]>t){
        rooms[x][y]=t
        t++
        redo(x+1,y,t)
        redo(x,y+1,t)
        redo(x-1,y,t)
        redo(x,y-1,t)
    }
}

createRoom([S/2,S/2,1])
for(var i=0;i<pow(S,2)/10;i++){
    createRoom(pick())
}
redo(S/2,S/2,0)

drawRooms()
fill(0)
var benchmark=millis()-mm
text('Took '+benchmark+' milliseconds to run.',10,10)

I would have done it in processing.py, but I had some trouble with matrices. If anyone knows how to use them in vanilla python, feel free to give me an explanation.

Lastly, here's the link I mentioned if you want to have a look at it:

(The "rainbow" part represents the distance from the center "room" (the reddest square) to the colored room.)

https://www.khanacademy.org/computer-programming/roguelike-room-generator/6424540231008256

General disclaimer: I have tried to find as many things in your code that you can fix, but I may not have found everything. Similarly, just because there may be a lot below doesn't mean the code is useless.

Before the review

I will be reviewing the code as posted on KhanAcademy, since I cannot get the code working here. I presume it is run through some kind of compiler. You are currently using an undefined function millis and use min and pow without the Math prefix, which makes those undefined in regular javascript as well.

Use a linter

You have completely disabled the linter in your code. I recommend against this, except for the cases where you have to include a dependency within your own code and cannot spare the time to rewrite it to the same code style you are using.

Linters force you to write consistent code. Their rules have been debated over and over, and their defaults are usually workable. If you do not like a certain rule, you can disable it, or change the rule. If you have a good reason to do something in a specific way and it trips a rule, you can disable that rule for that single line or single block.

KhanAcademy seems to use JSHint. You can find the documentation here. In your case you consistently don't use semicolons. I personally prefer to always use them, because they prevent typos from causing weird bugs, but you can tell JSHint to stop bugging you about it with // jshint asi: true. Most of the weird behaviour will be catched by other rules.

Readability

Naming your variables

The biggest issue with your code is the naming of your variables and functions. Variables and function names are the primary way of understanding what a piece of code does. Comments come second to elaborate on some expectations you have.

mm is used in the beginning. From the context I understand that this is the start time of your benchmark, so name it something like startTime.
Single letter variable names are usually a bad idea. S is used to define the dimensions of your grid. Lets just call it that: dimensions.
In your pick function, you use queue. A queue is a structure where you push things at the end and retrieve things from the beginning. Instead you pick a random element. A better name would be pool or collection or pickableRooms.
Variables with a number in them usually indicate that you should rename them. dir2 is no exception. Considering that it is in a createPath function, I assume that it is a direction. After reading the context, I think this is actually something like oppositeDirection.
pos2 is used just below that. Since it is going to point to the position in the direction given, lets call it adjecentPosition.
t in your redo function puzzles me. I think it is supposed to be the distance from the center, so lets call it distance.
The function name redo is unclear. You should replace it with what the function actually does, which seems to be to recalculate the distances for created rooms.

Unused variables

You have an unused variable in the argument list of your pick function. I am surprised the linter did not catch that. In any case, if you do not use an argument to a method in the method body, you should not include it in the signature.

If you would have used this argument array, you should have renamed it to what the variable actually contains, not its type. You could have used roomMatrix or distanceMatrix for example.

Odd bitwise NOT

You are using a double bitwise NOT operator in your code. After trying it out, I am guessing that this is some kind of operation to get the integer part of a float.

Since you never pass it negative numbers1, considering you are calculating an index, just use Math.floor for readability.

Indentation

Somewhere in your code you have the following lines:

var t = min(min(rooms[i-1][j],rooms[i][j-1]),
            min(rooms[i+1][j],rooms[i][j+1]))

I think there are two things wrong with those two lines:

You are manually aligning the two lines. Don't do this, because when you start using version control it will bite you. Lines that have not been changed should not be changed, because they cause merge conflicts and cause lines to show up in commits that have only whitespace changes.
If you put an opening brace at a certain indentation, you should put the closing brace at the same indentation. This goes for [, { and (.

The following code is better readable, and highlights that you believe that you can only use two arguments in the Math.min(..) function, which is not the case.

var t = min(
  min(rooms[i-1][j],rooms[i][j-1]),
  min(rooms[i+1][j],rooms[i][j+1])
)

So we can change it to the following code, which makes it clear that you want to actually get the minimum of all adjacent rooms.

var t = min(
  rooms[i-1][j],
  rooms[i][j-1],
  rooms[i+1][j],
  rooms[i][j+1]
)

Code structuring

Functions

You have put part of your code into functions. A notable exception is the initialisation code. Consider moving it to it's own function.

Function syntax

The syntax var a = function (arguments) is a bit odd. It nicely demonstrates that a function is actually a variable that you can call, but it does not seem very useful in this case. Consider just using function a (arguments).

I noticed that KhanAcademy finds this wrong syntax. I am clueless why this is.

Abstract away the underlying data structure

Your functions all interact with the underlying data structure in some way or another. You are currently also not checking for any errors. Once you start doing that, you will probably notice that you are duplicating code.

You probably want to hide the implementation of getting adjacent rooms to separate functions, such as north, and add error handling in there to handle cases where you are trying to get a room outside the boundaries.

Working of the program

Overlapping squares

Because of how you are calculating the width, you are adding up errors on the width of your squares, causing some of them to be off by one pixel.

Any easy way of dealing with that is by calculating an effective width and height, and use that everywhere:

var effectiveWidth = floor(width / dimensions)
var effectiveHeight = floor(height / dimensions)

and

for (var i = 0; i < dimensions; i++) {
  for (var j = 0; j < dimensions; j++) {
    if (rooms[i][j] !== Infinity) {
      fill(rooms[i][j] * 255 / dimensions, 255, 255)
      rect(i * effectiveWidth, j * effectiveHeight, effectiveWidth, effectiveHeight)
    }
  }
}

The dimensions constant

You have a constant that defines on what grid we are playing. Except that you 'auto-correct' it. Don't produce erroneous behaviour when wrong input is given, but instead just give an error.

var dimensions = 6

if (dimensions % 2 !== 0) {
  throw new Error('Dimensions must be even')
}

You are off by one... twice

When you set dimensions to 4, you can have a maximum of 2 by 2 rooms. When you set it to 6, you can have a maximum of 4 by 4 rooms. You'll notice that the center is therefore also never in the center.

What you actually want is to have the dimensions to be odd, so you can actually get the middle. Then you have to loop over all possible rooms. Then you just have to check if you are trying to find the value of a room outside the grid, and return Infinity in that case.

The result

//The "rainbow" part represents the distance from the center "room" (the reddest square) to the colored room.

//jshint asi: true

var startTime = millis()

var rooms = []
var paths = []
var dimensions = 5
var numberOfRooms = Math.floor(pow(dimensions, 2))
var padding = 20

var startX = Math.floor(dimensions / 2)
var startY = Math.floor(dimensions / 2)

var squareWidth = floor((width - padding * 2) / dimensions)
var squareHeight = floor((height - padding * 2) / dimensions)

if (dimensions % 2 !== 1) {
  throw new Error('dimensions should be odd')
}

var initialize = function() {
  for (var i = 0; i < dimensions; i++) {
    rooms[i] = []
    paths[i] = []
    for (var j = 0; j < dimensions; j++) {
      rooms[i][j] = Infinity
      paths[i][j] = [false, false, false, false] // up, right, down, left
    }
  }
}

var north = function(pos) {
  if (pos[1] === 0) {
    return Infinity
  }

  return rooms[pos[0]][pos[1] - 1]
}

var west = function(pos) {
  if (pos[0] === 0) {
    return Infinity
  }

  return rooms[pos[0] - 1][pos[1]]
}

var south = function(pos) {
  if (pos[1] === dimensions - 1) {
    return Infinity
  }

  return rooms[pos[0]][pos[1] + 1]
}

var east = function(pos) {
  if (pos[0] === dimensions - 1) {
    return Infinity
  }

  return rooms[pos[0] + 1][pos[1]]
}

var pickRandomRoom = function() {
  var pool = []

  for (var i = 0; i < dimensions; i++) {
    for (var j = 0; j < dimensions; j++) {
      if (rooms[i][j] === Infinity) {
        var pos = [i, j]

        var t = min(
          north(pos),
          west(pos),
          south(pos),
          east(pos)
        )

        if (t < Infinity) {
          pool.push([i, j, pow(dimensions, 2)])
        }
      }
    }
  }

  return pool[Math.floor(Math.random() * pool.length)]
}

var createRoom = function(pos) {
  rooms[pos[0]][pos[1]] = pos[2]
}

var createPath = function(pos, dir) {
  var oppositeDirection = (dir + 2) % 4
  var adjacentPosition = [pos[0], pos[1]]
  paths[pos[0]][pos[1]][dir] = true
  switch (dir) { //there's gotta be some better way to do this
    case 0:
      adjacentPosition[0]--
        break
    case 1:
      adjacentPosition[1]--
        break
    case 2:
      adjacentPosition[0]++
        break
    case 3:
      adjacentPosition[1]++
        break
  }
  paths[adjacentPosition[0]][adjacentPosition[1]][oppositeDirection] = true
}

var drawRooms = function() {
  pushMatrix()
  //scale(4.0)
  colorMode(HSB)
  background(255, 0, 255)

  //noStroke()
  for (var i = 0; i < dimensions; i++) {
    for (var j = 0; j < dimensions; j++) {
      if (rooms[i][j] !== Infinity) {
        fill(rooms[i][j] * 255 / dimensions, 255, 255)
        rect(
          padding + i * squareWidth,
          padding + j * squareHeight,
          squareWidth,
          squareHeight
        )
      }
    }
  }
  popMatrix()
}

var recalculateDistances = function(x, y, distance) {
  if (
    x >= 0 &&
    x < dimensions &&
    y >= 0 &&
    y < dimensions &&
    rooms[x][y] !== Infinity &&
    rooms[x][y] > distance
  ) {
    rooms[x][y] = distance
    distance += 1
    recalculateDistances(x, y + 1, distance)
    recalculateDistances(x + 1, y, distance)
    recalculateDistances(x, y - 1, distance)
    recalculateDistances(x - 1, y, distance)
  }
}

initialize()
createRoom([startX, startY, 1])
for (var i = 0; i < numberOfRooms - 1; i++) {
  createRoom(pickRandomRoom())
}
recalculateDistances(startX, startY, 0)

drawRooms()
fill(0)
var benchmark = millis() - startTime
text('Took ' + benchmark + ' milliseconds to run.', 10, 10)

Or as working on KhanAcademy

Footnotes

1 Math.floor returns the highest integer smaller than the argument passed to it, so Math.floor(-1.5) === -2. The bitwise not trick returns something different: ~~-1.5 === -1. Add a comment to these kind of tricks so your future self does not have to figure out what such an odd trick does.


Userscript hiding boxes on LinkedIn When looking at people you may know on LinkedIn (requires you to be logged in) there are sometimes some annoying boxes that are for people who are email contacts but not on LinkedIn.  I don't want to see these.  If I wanted to connect with them and invite them to LinkedIn, I would have.  So I wrote a userscript!  I wrote a few versions, because I wasn't sure about all of these things.  I've never written a userscript before, so let me know if any of this is weird, or not idiomatic, or something

This version uses jQuery!  That seems like overkill though...

// ==UserScript==
// @name Hide invites
// @namespace http://dannnno.github.io/
// @description Hides those "Invite to network boxes" on LinkedIn
// @include https://www.linkedin.com/people/pymk/hub/*
// @require https://ajax.googleapis.com/ajax/libs/jquery/2.1.4/jquery.min.js
// @grant GM_log
// ==/UserScript==

$(document).on('scroll', function (event) {
    $('li.card.guest').hide();
});

This version uses jQuery in a Chrome compatible way:

// ==UserScript==
// @name Hide invites - Chrome compatible
// @namespace http://dannnno.github.io/
// @description Hides those "Invite to network boxes" on LinkedIn in a Chrome compatible way
// @include https://www.linkedin.com/people/pymk/hub/*
// @require https://ajax.googleapis.com/ajax/libs/jquery/2.1.4/jquery.min.js
// @grant GM_log
// ==/UserScript==

var version = "2.1.4";

// Code adapted from http://stackoverflow.com/a/12751531/3076272
if (typeof jQuery !== "function") {
    add_jQuery(version);
}

function add_jQuery (jqVersion) {
    var jqVersion = jqVersion || version;
    var targ = document.getElementsByTagName('head')[0] || document.body || document.documentElement;
    var scriptNode = document.createElement('script');
    scriptNode.src = 'http://ajax.googleapis.com/ajax/libs/jquery/' + jqVersion + '/jquery.min.js';
    scriptNode.addEventListener("load", function () {
        var scriptNode = document.createElement("script");
        scriptNode.textContent = 'jQuery.noConflict(true);';
        targ.appendChild(scriptNode);
    }, false);
    targ.appendChild(scriptNode);
}

jQuery(document).on('scroll', function (event) {
    jQuery('li.card.guest').hide();
});

And this version does it without jQuery:

// ==UserScript==
// @name Hide invites - no jQuery!
// @namespace http://dannnno.github.io/
// @description Hides those "Invite to network boxes" on LinkedIn without jQuery
// @include https://www.linkedin.com/people/pymk/hub/*
// ==/UserScript==

// Throttling adapted from https://developer.mozilla.org/en-US/docs/Web/Events/scroll
(function() {
    var throttle = function(type, name, obj) {
        var obj = obj || window;
        var running = false;
        var func = function() {
            if (running) { return; }
            running = true;
            requestAnimationFrame(function() {
                obj.dispatchEvent(new CustomEvent(name));
                running = false;
            });
        };
        obj.addEventListener(type, func);
    };

    throttle("scroll", "optimizedScroll");
})();

// handle event
window.addEventListener("optimizedScroll", function() {
    var results = document.querySelectorAll('li.card.guest:not([style*="display:none"]):not([style*="display: none"])';
    for (var i = 0; i < results.length; ++i) {
        results[i].style.display = "none";
    }
});

Some bits of these were adapted from other sources (there are code comments to that effect).  Feel free to review those as much or as little as you'd like.

After I did a little playing around in a LinkedIn page's source code and I found out that LinkedIn already uses jQuery: that means you don't need to worry about checking if jQuery exists in the page because the page already has it.

Therefore, you can strip away all the jQuery injection nonsense, leaving your code looking like this;

var version = "2.1.4";

jQuery(document).on('scroll', function (event) {
    jQuery('li.card.guest').hide();
});

If you would like a review on how you did the on the vanilla JavaScript, leave a comment.


Compare hostnames and leases between CSV and DHCP info I have 3 nested loops and they do the following:

Get scopes from DHCP
Get hostnames from csv file
Compares hostnames and leases, matches are added to a hash table

There are 5 DHCP servers, 100's of scopes and thousands of leases.
What would be a good method to speed this up?
$DHServers = Get-DhcpServerInDC #get DHCP info
$hashtable = @{} #create hash table

foreach ($server in $DHServers){
$scopes = Get-DHCPServerv4Scope -ComputerName $server.dnsname #get all scopes in DHCP

    foreach ($_ in (Import-Csv C:\script\Asset_List.csv | Select-Object -ExpandProperty asset)){ #get hostnames from list

        foreach ($scope in $scopes){
            if($scope | Get-DhcpServerV4Lease -ComputerName $server.dnsname | Where-Object HostName -like "$_*" ){ #compares the hostname to find which lease it is in
                $scopename=$scope.name #matches give scope name
                $hashtable[$_] = $scopename #when a match is found, add keys and values to table
            }
        }
    }
}

Piping the DHCP calls through foreach-object instead and then comparing at the end, cut the time down from 3min to 1min. Everything I have read made it seem like foreach was faster. Having DHCP being processed down the pipe with foreach-object instead of several iterations with foreach makes foreach-objects more efficient.
$servers = Get-DHCPServerInDC
$hashtable = @{}

# scopes into the leases to go into the PsCustomObject to be matched later
$leases = $servers | ForEach-Object {
    $server = $_.dnsname
    Get-DHCPServerv4Scope -computerName $server  #get scopes on DHCP serves

    } | ForEach-Object {
    $scope = $_
    $_ | Get-DHCPServerV4Lease -ComputerName $server | #get leases in all the scopes

     ForEach-Object {
        [pscustomobject]@{ # Make object to match later
        ScopeName = $scope.name #want name of scope only
        HostName  = $_.hostname #want hostname of leases
        }
    }
}

$assets = (Import-CSV c:\script\Asset_List.csv).asset #pull asset list

$assets | ForEach-Object { $asset = $_ #Go through hostnames finding matching lease

    $leases | Where-Object { #check leases
        $_.HostName -like "${asset}*" #find hostname
    } | ForEach-Object {
        $hashtable[$asset] = $_.ScopeName #store matches
    }
}
```


Log monitoring with pythonic tail -f and process killing The purpose of this cluster of functions is to tail the log file and kill/restart processes from shell (the latter is not yet implemented).

import sys
import time
import datetime
import os
import re

def read_log(f):
    """
    Basically tail -f
    """
    logfile = open(f)
    logfile.seek(0, os.SEEK_END)
    while True:
        new_line = logfile.readline()
        if new_line:
            yield new_line
        else:
            time.sleep(0.1)

def find_error(line, base_time, line_time):
    """
    Search for ERROR keyword in log line. Return True if found, False if not.
    """
    match = re.search('.*ERROR.*', line)
    if match and line_time < base_time:
        return True
    return False

def parse_time(line):
    """
    Parse string to datetime
    """
    date_regex = r'\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}'
    date_string = re.search(date_regex, line).group(0)
    return datetime.datetime.strptime(date_string, "%Y-%m-%d %H:%M:%S")

def increase_base_time(line_time):
    """
    Return a datetime that is line_time plus one minute
    """
    return line_time + datetime.timedelta(minutes=1)

def monitor(filename):
    """
    Read from a log file.
    For each line check if log line time is later than base comparison time.
    If so, update the base time and set error count to 0.
    Check for errors in line. Increment error count if any are found.
    Check the total error count. If it's greater than five, restart the logged
    process.
    Check if process has been restarted before. If so, kill the logged process.
    """
    count = 0
    base_time = datetime.datetime.min
    log = read_log(filename)
    restarted = False
    for line in log:
        line_time = parse_time(line)
        if line_time > base_time:
            base_time = increase_base_time(line_time)
            count = 0
        if find_error(line, base_time, line_time):
            count += 1
        if count >= 5:
            if restarted:
                print("Kill the process") # A placeholder, sorry
            else:
                print("Restart the process") # Also a placeholder, sorry
                count = 0
                restarted = True

monitor(sys.argv[1])

There are a lot of if-statement for doing checks. I thought about making this class based, but it really doesn't make sense to do it (functions don't need to share states and it is a rather small program).
Is the number of if's acceptable? How could I refactor the code to make it flow a bit better?

EDIT: As this was brought up, is it necessary that the tailing function would be non blocking? As I understand I wont be doing nothing until a line is yielded. What would be the benefits of going one way or the other?

Using regex for such a simple task as finding if a line contains a string is not worth it. The built-in str.__contains__, being called by the keyword in is faster in this case.

Here are some worst case (i.e. a string that does not contain the string "ERROR") timings for the two functions:

import re

def using_in(s):
    return "ERROR" in s

def using_re(s):
    return bool(re.search('.*ERROR.*', s))

Using re is always slower in this case. It does, of course, have its uses for more complex patterns. Note that the re module also compiles the regex automatically (and caches it), so that is not a way to make it faster in this case, since it is always the same regex being used.

Let's just hope your lines are less than 10,000 characters long.

In general, I think you still have to find a good balance between not having enough functions and having too many. IMO, instead of having a increase_base_time function, I would have a global constant and inline the call:

INCREMENT = datetime.timedelta(minutes=1)

...

for line in log:
    ...
    if line_time > base_time:
        base_time = line_time + INCREMENT
        ...

If read_log is a tail -f equivalent, you could just call it tail_f. In any case, I would make the sleep time an optional parameter.

Note that your function does not ensure that the file is properly closed, which might cause issues. You should always use the with keyword to avoid this.

def read_log(f, sleep=0.1):
    """
    Basically tail -f with a configurable sleep
    """
    with open(f) as logfile:
        logfile.seek(0, os.SEEK_END)
        while True:
            new_line = logfile.readline()
            if new_line:
                yield new_line
            else:
                time.sleep(sleep)

This way the file is closed even if the user presses Ctrl+C.

You should only call your function under a if __name__ == "__main__": guard to allow importing from this script without it (trying and failing to) run:

if __name__ == "__main__":
    monitor(sys.argv[1])


Python Back-end registration validation I'm working on a personal project (Flask CRUD app) and I am currently building the user service. I am trying to use as less as libraries as possible (that's why I do not use WTF-forms for example, just for learning purposes). I am really not sure about a couple of things.
I am aware that form validation should be handled in both front and back-end (or at the very least on the back-end). In order to validate a new user, I am (currently) checking 3 things:
1) email does not already exist on the db (can only be verified on the back-end)
2) password is strong enough (can be done in both front and back end)
3) password and password_confirm matches (can be done in both front and back end)
My goal is to display a small error message on the registration page if either at least one those errors appears. Currently, I am checking option 1, 2 and 3 and the back-end but only return True if the registration is valid otherwise, False.
Since case 1 and 2 can directly be handle also on the front-end, can I display the error message using JavaScript and not from the back-end? I am still confused on what would be the best return for validate_registration (currently a Boolean).
Also, I am not quite sure my code is over-engineered, I made a RegistrationForm class just to validate a registration form. I did that because I am able to pick validations methods from my validators.py which may be used in other Classes as well.
So this is what I did so for for the registration form:
user/blueprints/routes.py
@user.route('/new-user',methods = ['POST'])
def register_user():
    form_email = request.form.get('email')
    form_password = request.form.get('psw')
    form_password_repeat = request.form.get('psw-repeat')
    registration_form = RegistrationForm(form_email, form_password, form_password_repeat).validate_registration()

    if registration_form:
        new_user = UserService().register_user(form_email, form_password)
        user_repository = UserRepository(conn, 'users')
        user_repository.add_user(new_user)
        user_repository.save()
        return "ok" #will probably change return statements later on
    return "not ok" #will probably change return statements later on

user/blueprints/forms.py
from prepsmarter.blueprints.user.validators import email_already_in_use, password_matching

class RegistrationForm():
    def __init__(self, email, pwd_1, pwd_2):
        self.email = email
        self.pwd_1 = pwd_1
        self.pwd_2 = pwd_2

    def validate_registration(self):
        is_valid = email_already_in_use(self.email) and is_strong_password(self.pwd_1) and password_matching(self.pwd_1, self.pwd_2)
        return is_valid

class LoginForm():
 # to do

user/blueprints/validators.py
import re
from email_validator import validate_email, EmailNotValidError
from prepsmarter.extensions import conn

def is_strong_password(password):
    length_error = len(password) < 8
    digit_error = re.search(r"\d", password) is None
    uppercase_error = re.search(r"[A-Z]", password) is None
    return length_error and digit_error and uppercase_error

def is_email_formated_correctly(email):
    is_correct = True
    try:
        validate_email(email)
    except EmailNotValidError:
        is_correct = False
    return is_correct

def password_matching(pwd_1, pwd_2):
    return pwd_1 == pwd_2

def email_already_in_use(email):
    sql = "SELECT CASE WHEN EXISTS ( SELECT * FROM users WHERE email = (%s)) THEN 1 ELSE 0 END;"
    cursor = conn.cursor()
    cursor.execute(sql, (email))
    res = cursor.fetchall()
    return res == 1

I have not thought carefully about all of your code and questions, but here's
one suggested technique for how to handle validation code more simply. In most
contexts, people find it easier to understand affirmative boolean logic rather
than negated logic. My guess is that the bug in is_strong_password() stems
from that phenomenon (the function tells me hi is strong, for example).
You've created 3 variables to be true when something is wrong but then combined
them under the opposite assumption. Here are two alternatives to consider:
# This alternative uses affirmative variables: True means OK. Also notice
# how the variable names are a bit more intuitive because of the
# affirmative framing. When possible, try to name boolean variables to
# convey their nature: is_X, has_X, contains_X, etc. Your current code has
# variables names implying that they each hold an "error" of some kind --
# but they do not. Finally, the usage of bool() here is not strictly
# necessary, but it does convey intent explicitly.

def is_strong_password(password):
    is_long = len(password) >= 8
    has_digit = bool(re.search(r"\d", password))
    has_upper = bool(re.search(r"[A-Z]", password))
    return is_long and has_digit and has_upper

# This alternative illustrates a handy code-layout technique
# that often works well when doing validation. This version
# is shorter (sometimes nice) but less explicit. In this
# case, I would tend to favor the short version, because the
# checks are so simple. If the complexity were higher, I would
# prefer the approach that names every check to increase clarity

def is_strong_password(password):
    return (
        len(password) >= 8 and
        re.search(r"\d", password) and
        re.search(r"[A-Z]", password)
    )

The is_email_formated_correctly() can be simplified by doing things more
directly. In addition, if validate_email() returns True/False you could
simplify further by just returning its return value.
def is_email_formated_correctly(email):
    try:
        validate_email(email)
        return True
    except EmailNotValidError:
        return False


SQL Query based on results of preceding query I am bashing my head against how to solve this puzzle. Is there any super SQL expert out there who can lend some help

I have a database with the following structure.

adjlevel | scheme | holder | client | companyID | Rate

A rate can be held in this table based on either of the following keys

level  + companyId + scheme + holder + client

OR

level  + companyId + scheme + holder

OR

level  + companyId + scheme

OR

level  + companyId

There will always be one record per level. Therefore I need to write a query that looks at all the above criteria starting with the most filters first and eliminating one criteria at a time until I find the required record.

I have written a function to do the above but it is very slow.

ALTER FUNCTION [dbo].[Hourly_Rate]
(
-- Add the parameters for the function here
@ncompanyid numeric(2),
@client varchar(100),
@scheme varchar(100),
@holder varchar(100),
@nadjlevel numeric(3),
@date_done datetime
)
RETURNS Numeric(5)
AS
BEGIN
DECLARE @cmode varchar(1),
@level numeric(1),
@counter numeric(1),
@retval Numeric(5)

set @cmode = 'T'
set @level = 1  -- @level controls which filter to apply
set @counter = 1 -- @counter = loop counter

WHILE @counter <= 9
BEGIN

SELECT @retval = bhrlyrate FROM [MYDATABASE].dbo.inv_hrrate WHERE
adjlevel = @nadjlevel
and cmode = @cmode
and companyid = @ncompanyid
and client =  case when @level <= 3 THEN @client ELSE '' END
and scheme = case when @level <= 2  THEN @scheme ELSE '' END
and holder = case when @level <= 1 THEN @holder ELSE '' END
and dworkedfrom <= case when @date_done = '30 december 1899' then dworkedfrom ELSE    @date_done END
and dworkedto >= case when @date_done = '30 december 1899' then dworkedto ELSE @date_done END

IF @@rowcount > 0  -- Break if record found
BREAK;

IF @level = 4
-- T search mode unsuccessful change to A mode,
-- Reassign a few variables
--- and continue searching
BEGIN
SET @cmode = 'A'
SET @level = 0
set @date_done = '30 december 1899'
END

S ET @counter  = @counter + 1
SET @level = @level + 1

END

-- If record is not in hourly rate table then pick default rate from company

IF @counter = 8 and @retval = 0
SELECT @retval = CO.hrly_rate FROM company CO WHERE companyid =
@ncompanyid;

RETURN @retval

END

I would pull all the possible rates and just sort by the most specific.

SELECT @retval = bhrlyrate FROM [MYDATABASE].dbo.inv_hrrate WHERE
adjlevel = @nadjlevel
and cmode = @cmode
and companyid = @ncompanyid
and (client = @client or client = '')
and (holder = @holder OR (holder = '' AND client = ''))
and (scheme = @scheme or (scheme = '' AND holder = '' AND client = ''))
and dworkedfrom <= case when @date_done = '30 december 1899' then dworkedfrom ELSE    @date_done END
and dworkedto >= case when @date_done = '30 december 1899' then dworkedto ELSE @date_done END
ORDER BY scheme DESC, holder DESC, client DESC

That would eliminate some of the looping and requerying you are doing.


A simple program using branching to guess the answer As a rookie to programming, i'm really excited about finishing my first program.

But I know I only learn by making mistakes and through constructive criticism.

I'm open to any feedback regarding my use of branching, and any way to "clean up" my code.

puts 'Hi! Pick a number between 0 and 10'
number = gets.chomp
puts 'Got it; your number is ' + number.to_s + ', right?'
check1 = gets.chomp

if
  check1 == 'yes'
  puts 'awesome. now double your number. What\'s your new number?'
else
  puts 'Oops. Either: A. I oopsie-daisied up or B. you\'re lying.'
end

number_doubled = gets.chomp
if
  number_doubled.to_i == number.to_i * 2
  puts 'cool. Now add 6. What\'s the new number?'
else
  puts 'Oopsie-daisies'
end

number_plus_6 = gets.chomp
if
  number_plus_6.to_i == number.to_i * 2 + 6
  puts 'cool; Now divide your new answer by 2; What\'s your new number?'
else
  puts 'Oopsie-daisies'
end

number_halved = gets.chomp
if
  number_halved.to_i == number_plus_6.to_i / 2
  puts 'cool. Now, if you subtract the number you originally picked, ' + number + ', you\'re answer will be 3. Is it?'
else
  puts 'Oopsie-daisies'
end

check2 = gets.chomp
if
  check2 == 'yes'
  puts 'See? I told you i was a smarty!!'
else
  puts 'hmm, i think you\'re bad at math'
end

I think in general this is fine. The only things I'd point out are stylistic really.

In Ruby, many people would prefer double-quoted strings, which allow you to: Got it; your number is #{number}, right?. Note that this also relies on an implicit #to_s on number. It also allows single quotes to be used without escaping.
Look out for variable naming. Naming things correctly is notoriously hard, even after years of experience, and number is a bit generic.
I would definitely put the if conditional on the same line as the if. If the expression is too long or a bit difficult to instinctively understand, assign its value to a variable with a meaningful name (e.g. correctly_added_six = number_plus_6.to_i == number.to_i * 2 + 6) so your condition is more easily understood (e.g. if correctly_added_six)

Good start, though!


Python Command Line Interface I wrote a little module for making quick CLIs in Python and I wanted to get your thoughts on areas of improvement (as well as to see what can break it).

One of the issues I had from the start was with looping and multi-threading, I wound up just adding a 1 second delay to make sure I've enough time to do some basic processing, but it seems a bit hackish. I'd like to perform some basic tests on the variable first (e.g. make sure it's not going to cause an overflow) and then actually copy the variable into an entirely separate memory space, although I'm not terribly knowledgeable about python's memory management practices and could use some guidance in the right direction.

A quick brief on how it works, functions which are to be made available to the user are stored in a dictionary with command:function key-pair, if a user enters a command which isn't recognized it will simply print the help screen message, right now it only has support for space separated values. It should be fairly trivial to add additional deliminator functionality like commas, dashes, etc. but I am also a bit worried about adding too many features as well and making it cumbersome to work with.

You can download the source on the GitHub repo or install with pip/pip3:

$ pip3 install -i https://test.pypi.org/simple/ pycli

And I've included the source code for reference:

#!/usr/bin/python3
"""Basic CLI in Python3"""

import threading as th
import time

cont=True
text=None

class InputThread:
    def __init__(self):
        global cont
        global text
        while True:
            self.loop_thread()
            time.sleep(1)
            text=None
            cont=True

    def loop_thread(self):
        global cont
        global text
        text = input()
        cont=False

class CLI:
    def __init__(self):
        self.cmds = {
            'help':self.help,
            }
        self.init_message = ''

    def help(self, args=None):
        """Use help 'command' for more information on a specific command."""
        if args==None:
            print("Available Commands:")
            for key in self.cmds.keys():
                print(key)
            print(self.help.__doc__)

        else:
            print(self.cmds[args[0]].__doc__)

    def set_message(self, message):
        """Sets the initialization message"""
        self.init_message = message

    def add_function(self, name, function):
        """Adds a function to the command dictionary"""
        self.cmds[name] = function

    def cli(self, a):
        global cont
        args = a.split(' ')
        if args[0] not in self.cmds.keys():
            print('Command "', args[0], '" Not Found.')
            self.help()
        else:
            if len(args) == 1:
                self.cmds[args[0]]()
            else:
                self.cmds[args[0]](args[1:])
        cont=True

    def loop(self):
        global cont
        global text
        print(self.init_message)
        th.Thread(target=InputThread, args=(), name='user_input_thread', daemon=True).start()
        while True:
            if not cont:
                self.cli(text)

if __name__ in '__main__':
    x = CLI().loop()

and here's an example CLI made using the module:

#!/usr/bin/python3

from pycli import CLI as cli
import random

def random_function(x, y):
    """Generates a random number between x and y.
    Usage:
      random x y
    """
    print(random.randint(x,y))

if __name__ in '__main__':
    x = cli()
    x.set_message("""Welcome to my super awesome python CLI!
With this new tool you can quickly generate command line
interfaces that capture user input without having to figure
out how to handle all those inputs yourself!
""")
    x.add_function('random', random_function)
    x.loop()

I've only tested this on Linux Mint (don't have Windows or Mac), so if you've got one of those OS's can you let me know if the code works as expected and any bugs?

Your script consumes a lot of CPU due to the while loops. Using just the input function in a loop should be good enough for your purpose. I can't see the need for threading in a script that is quite basic for the moment.

These:

global cont
global text

don't belong in your class.
A class is normally supposed to be self-contained and shouldn't have to rely on global variables defined elsewhere. They should go inside the class body.
On the other hand you can pass parameters to your class instantiation method or to some class functions.

By the way text is not a good choice of variable since many objects have a text property and be careful with reserved keywords.

Exception handling is lacking. Any quality script should have at least basic exception handling (ten lines of code could do).

Validation of user input is also lacking.
If I type anything after help I have a KeyError exception and your script crashes too easily. The dictionary selection is rather clumsy but see below for suggestions.

Structure: I am wondering why you split the code in two classes ?
InputThread could have been embedded in CLI as a subclass or simply implemented as a function. But they are not independent from each other.

Style: after:

if __name__ in '__main__':

the norm is to call main(), so it is customary to have a main method.

You don't call a class like that:

x = CLI().loop()

Instead:

x = CLI()

with optional parameters inside the parentheses, or you run the code in multiple steps eg:

# instantiation
x = CLI()
# or:
x = CLI(prompt="Prompt message goes here")
x.start()
x.stop()

Because you can perfectly instantiate a class without immediately running it.

Naming conventions: variable names (a, text) are not always well-chosen and meaningful.

Misc: your script does not recognize commands in uppercase. I would always convert the commands to lowercase and use strip to trim them (here the split command takes care of that).

Overall impression: I think that this is a way of reinventing the wheel. There are modules that already exist for interactive cli programs, for example PyInquirer. There is also clint and quite a few others (I have not tested and compared all of them).
So I think I would build on the more mature solutions available, unless the problem you are trying to solve cannot be addressed adequately by the solutions that already exist.

An alternative would be to use feature-rich argparse. Because it can be used not only for command line options but in functions too (I haven't done that yet though but it's worth investigating). To give you an idea have a look at this post on SO: How to pass parameters to python function using argparse?. Also have a look at the Sub-commands section for more elaborate scenarios. As you can see this module is very powerful and very often underutilized.

It seems to me that it could have fulfilled all your desired functionality, with increased flexibility since the commands could be passed in a different order, as long as the grouping of dependent parameters is respected.


How can I make this code more readable and efficient? How can I rewrite this code to make it more readable and, if possible, more efficient? The conditions are entirely necessary in this problem, but how could I rewrite those if(a != 1) and such?

celula ** verificaNo(celula ** matriz, int linhas, int colunas){
celula ** aux = matriz;
int i, j, k;
int node = 1;
int a = 0;
int b = 0;
int c = 0;

for(i = 0; i < linhas; i++){
    for(j = 0; j < colunas; j++){
        if((i == 0) && (j == 0)){
            aux[i][j].no = node;
        }
        else if((i == 0) && (j != 0)){
            if(aux[i][j-1].peso == 0 && aux[i][j].peso == 0){
                aux[i][j].no = aux[i][j-1].no;
            }
            else if(aux[i][j-1].peso != 0 && aux[i][j].peso == 0){
                for(k = j; aux[i+1][k].peso == 0 && k > 0; k--){
                    if(aux[i][k-1].peso == 0){
                        aux[i][j].no = aux[i][k-1].no;
                        a = 1;
                    }
                }
                if(a != 1){
                    node++;
                    aux[i][j].no = node;
                }
            }
        }
        else if((j == 0) && (i != 0)){
            if(aux[i-1][j].peso == 0 && aux[i][j].peso == 0){
                aux[i][j].no = aux[i-1][j].no;
            }
            else if(aux[i-1][j].peso != 0 && aux[i][j].peso == 0){
                for(k = j; aux[i][k].peso == 0 && k < colunas; k++){
                    if(aux[i-1][k+1].peso == 0){
                        aux[i][j].no = aux[i-1][k+1].no;
                        b = 1;
                    }
                }
                if(b != 1){
                    node++;
                    aux[i][j].no = node;
                }
            }
        }
        else if(aux[i][j].peso == 0 && aux[i-1][j].peso == 0 && i != 0){
            aux[i][j].no = aux[i-1][j].no;
        }
        else if(aux[i][j].peso == 0 && aux[i][j-1].peso == 0 && j != 0){
            aux[i][j].no = aux[i][j-1].no;
        }
        else if(aux[i][j].peso == 0){
            for(k = j; aux[i][k].peso == 0 && k < colunas; k++){
                if(i != 0){
                    if(aux[i-1][k].peso == 0){
                        aux[i][j].no = aux[i-1][k].no;
                        c = 1;
                    }
                }
            }
            if(c != 1){
                node++;
                aux[i][j].no = node;
            }
        }
    }
}
return aux;
}

This would become instantly more understandable if you gave a description (a comment at the start of the function) of what it was supposed to do.  You could also give the structure celula, which I'm assuming is at least:

struct celula
{
    int no;        // number of something?
    int peso;      // weight, perhaps?
};

Also you use i for line (linhas) and j for column (colunas). But l and c would be more logical and readable (but l looks like 1, so in English I'd use r for "row").  I'm generally ok with short variable names, but in such a long function i and j don't work for me.  In English I would probably use row and col.

As your function has nested loops with the outer loop doing nothing but enclosing the inner loop, I would if possible extract the whole inner loop to a separate function. But I can see that this might not be easy if those badly named variables a, b, c, loop accumulate across rows.  It is difficult to tell whether they do from a short look - I might take a longer look later...

Later...

Returning to variables a, b, c:  they are never reset once set.
Is that correct?  These variables names are meaningless and need changing.  And what does node mean - again the name needs improving.

A bigger observation is that everything below the first else contains the term aux[i][j].peso == 0.  I think these can all be factored out (so that the condition is tested at the top of the loop.

for(i = 0; i < linhas; i++){
    for(j = 0; j < colunas; j++){
        if((i == 0) && (j == 0)){
            aux[i][j].no = node;
        }
        else if(aux[i][j].peso == 0) {  // moved to here
            if((i == 0) && (j != 0)){

And also the i != 0 and j != 0 in the following lines are unnecessary, as they have been handled further up:

else if(aux[i-1][j].peso == 0 && i != 0){                     // here
    aux[i][j].no = aux[i-1][j].no;
}
else if(aux[i][j-1].peso == 0 && j != 0){                     //here
    aux[i][j].no = aux[i][j-1].no;
}
else{
    for(k = j; aux[i][k].peso == 0 && k < colunas; k++){
        if(i != 0){                                           // here


Terser syntax for first-class functions needing a context bound I have the following function that looks up a mathematical operation that can be applied to a Numeric sequence based on a String

def getAction[T : Fractional]( op : String ) : Seq[T] => String =
  op match {
    case "add" => seq => seq.sum.toString
    case "product" => seq => seq.product.toString
    case "mean" => seq => {
      implicit def intToFractional(i : Int) =
        implicitly[Fractional[T]].fromInt(i);
      (seq.sum / seq.length).toString
    }
    case "sqrt" => seq => seq.map(i => math.sqrt(i.toDouble)).toString
  }

That works fine, except I would prefer to specify the type when applying the returned function rather than at the time or retrieval.

i.e. I want

getAction("sqrt")[Double](Seq(4.0))

rather than

getAction[Double]("sqrt")(Seq(4.0))

The only way I have found to do this is by defining a new class/trait with a generic context-bound apply.

abstract class Action {
  def apply[T : Fractional](seq : Seq[T]) : String
}

Edit: Replaced with whole class rather than just the method because compilable solution was wanted.

object Main extends App {
  import scala.math.Fractional
  import Fractional.Implicits._
  import scala.language.implicitConversions

  abstract class Action {
    def apply[T : Fractional](seq : Seq[T]) : String
  }

  def getAction( op : String ) : Action =
  op match {
    case "add" =>
      new Action {
        def apply[T : Fractional](seq : Seq[T]) = seq.sum.toString
      }
    case "product" =>
      new Action {
        def apply[T : Fractional](seq : Seq[T]) = seq.product.toString
      }
    case "mean" =>
      new Action {
        def apply[T : Fractional](seq : Seq[T])= {
          implicit def intToFractional(i : Int) =
            implicitly[Fractional[T]].fromInt(i);

          (seq.sum / seq.length).toString
        }
      }
    case "sqrt" =>
      new Action {
        def apply[T : Fractional](seq : Seq[T]) =
           seq.map(i => math.sqrt(i.toDouble)).toString
      }
  }

  def getAction_v1[T : Fractional]( op : String ) : Seq[T] => String =
    op match {
      case "add" => seq => seq.sum.toString
      case "product" => seq => seq.product.toString
      case "mean" => seq => {
        implicit def intToFractional(i : Int) =
          implicitly[Fractional[T]].fromInt(i);
        (seq.sum / seq.length).toString
        }
      case "sqrt" => seq => seq.map(i => math.sqrt(i.toDouble)).toString
    }
  }

However, this seems messy to be because of all the repeated anonymous class definitions and repeated type signatures.  This makes the actual math operations less obvious than the first version.

Is there a better way?

Edit:  here is a simple test suite that shows execution of both versions
import org.scalatest.FunSuite

import org.scalatest.FunSuite

class Tests extends FunSuite {

  test("v1"){
    val act = Main.getAction_v1[Double]("sqrt")
    assert(act(Seq(4.0)) === "List(2.0)")
  }

  test("v2") {
    val act = Main.getAction("sqrt")
    assert(act[Double](Seq(4.0)) === "List(2.0)")
  }
}

How about not having to specify the type at all?

implicit class SeqWithAction[T:Fractional](seq:Seq[T]) {
  def getAction( op : String ) : String =
    op match {
      case "add" => seq.sum.toString
      case "product" => seq.product.toString
      case "mean" => {
        implicit def intToFractional(i : Int) =
          implicitly[Fractional[T]].fromInt(i);
        (seq.sum / seq.length).toString
      }
      case "sqrt" => seq.map(i => math.sqrt(i.toDouble)).toString
    }
  }
}
Seq(4.0).getAction("sqrt")


Testing if array is heap This method tests an array of integers satisfies to see if its a max binary heap. This means

each node is greater than or equal to it's children
each node has at most 2 children
the leaf nodes are filled from left to right

This is for heaps that do not use element 0. See here.

/*returns true if array satisfies heap property for heap from [START, END]*/
private boolean isHeap(final int[] heap, final int START, final int END) {
    for(int i = START; i <= Math.ceil(END / 2); i++) {
       if(2 * i + 1 <= END && heap[i] < heap[2 * i + 1])
               return false;
       else if(2 * i <= END && heap[i] < heap[2 * i])
               return false;
   }

   return true;

}

Possible bug

The method isHeap takes an array along with a start and end index. Although the code does try to take into account potential out of bounds by checking 2 * i + 1 <= end and 2 * i <= end, there is no check that end is strictly lower than heap.length. As such, out of bounds array indexes can still occur:

isHeapReview(new int[] { 0 }, 0, 1);

would throw an java.lang.ArrayIndexOutOfBoundsException: 1. There are multiple solutions depending on the intended usage of the method:

You can defend from such a case by re-defining end to be the minimum of the given end and heap.length - 1 with Math.min(end, heap.length - 1).
You can check if end is greater or equal than heap.length and, if true, throw an IllegalArgumentException.

In the same way, there is no check that start is a positive integer. Those checks should be added to it too.

Code style

Watch out your indentation style and braces. The following

if(2 * i + 1 <= END && heap[i] < heap[2 * i + 1])
        return false;
else if(2 * i <= END && heap[i] < heap[2 * i])
        return false;

doesn't use curly braces and is not indented properly. Even if they are redundant, it is best to explicitely add the curly braces, as they prevent future possible issues. Use this style instead:

if (2 * i + 1 <= end && heap[i] < heap[2 * i + 1]) {
    return false;
} else if (2 * i <= end && heap[i] < heap[2 * i]) {
    return false;
}

where the braces were added, indentation is fixed, spaces are added after if; all of this contribute to easier to read code.

Simplification

end / 2 performs integer division and will return an integer, so invoking Math.ceil on it will have no effect. You can remove it; the code already loops from 0 to end / 2 inclusive.

Also, since end is expected to be lower or equal than heap.length - 1, you can remove the 2 * i <= END check in:

if (2 * i <= END && heap[i] < heap[2 * i])

This will always be true since i maximal value is end / 2.

With this change, you can even refactor the if statement to:

if (2 * i + 1 <= END && heap[i] < heap[2 * i + 1] || heap[i] < heap[2 * i]) {
    return false;
}

without the need of an else if statement. It does make the line a tiny bit longer, but it is short enough to typically fit on a screen and is pretty direct.

Namings

Don't write the parameters in upper-case; only use this for constants. START should really be start; in the same way, END should be end.


Concatenating strings of equal length My problem is this: Given two strings, append them together and return the result. However, if the strings are different lengths, omit chars from the longer String so it is the same length as the shorter String. So "Hello" and "Hi" yield "loHi". The strings may be any length.
Is there a way to do it without using of StringBuilder like me:

public String equalConcatenation(String a, String b) {
  if(a.length() > b.length()){
    StringBuilder sb = new StringBuilder(a);
    sb.delete(0, (a.length() - b.length()));
    return sb.toString() + b;
  }
  StringBuilder sb = new StringBuilder(b);
  sb.delete(0, (b.length() - a.length()));
  return a + sb.toString();
}

You're not using StringBuilder effectively.  If you already have sb, then why not do sb.append(b).toString() instead of sb.toString() + b?  If you look at the generated bytecode for sb.toString() + b, you'll see that it actually makes another StringBuilder for you, like new StringBuilder(sb.toString()).append(b).toString().

I would write it this way instead, with no special cases:

public static String equalConcatentation(String a, String b) {
    int len = Math.min(a.length(), b.length());
    return a.substring(a.length() - len) +
           b.substring(b.length() - len);
}

I've chosen to make it a static method because it's purely a function of the two input strings, not using any object state.


Assertions about data in infrastructure code as unit tests I have some Python tests I have written, but these are for making assertions about data in infrastructure code (AWS CloudFormation templates). I am using the Unittest framework.

I have code like this:

class TestData(unittest.TestCase):

    def testConfigStackYaml(self):

        bad_list = []
        for stack in glob('*/config'):

            stack_name = os.path.dirname(stack)
            expected_file = "%s/config/%s.yaml" % (stack_name, stack_name)

            if not os.path.isfile(expected_file):
                bad_list.append(expected_file)

        assert not bad_list, "Expected config/<stack_name>.yaml files to exist: %s" % bad_list

    def testStacksetMk(self):

        bad_list = []
        for stack in glob('*/config'):

            stack_name = os.path.dirname(stack)
            expected_file = "%s/stackset.mk" % stack_name

            if not os.path.isfile(expected_file):
                bad_list.append(expected_file)

        assert not bad_list, "Expected stackset.mk file to exist: %s" % bad_list

These are just two of my test cases. I have plenty more.

As can be seen, there is a problem with this code. The for loop for each test case is repeated for each test case, resulting in code duplication and inefficient code.

I have done it this way, however, because I want each test case to yield a helpful, specific message about what is wrong.

Can anyone see a cleaner way to implement this, so that I have the best of both worlds: duplication refactored out, but I still get to have a separate test case for each logical test?

This is an interesting code.

Criticism:

def testConfigStackYaml(self):

Can we rename these functions to snake_case such as test_config_stack_yaml maybe.

assert not bad_list, "Expected config/<stack_name>.yaml files to exist: %s" % bad_list

You can access unit test asserts from `TestCas
How:

self.assertTrue(expr, msg=None)
self.assertFalse(expr, msg=None)

More information at: https://docs.python.org/3.6/library/unittest.html#unittest.TestCase.assertTrue

expected_file = "%s/config/%s.yaml" % (stack_name, stack_name)

I personally like using the new formatter. % is more unreadable compared to
"{stack_name}/config/{stack_name}".format(stack_name=stack_name)
We can also use os.path.join to join sections of a path. This makes our intention clear.

if __name__ == "__main__":
    unittest.main()

If you add this to the end of the file you can run these test files individually as a simple script.

Creating a custom assert

I recommend creating a memeber function named assert_config_exists.
Parameters: path_format, message_format

def assert_config_exists(self, path_format: str, message_format: str):
    bad_list = []
    for stack in glob('*/config'):

        stack_name = os.path.dirname(stack)
        expected_file = path_format.format(stack_name=stack_name)

        if not os.path.isfile(expected_file):
            bad_list.append(expected_file)

    self.assertFalse(bad_list, message_format.format(bad_list=bad_list))
```


Rebase values in an array to match the chart scale I have 2 long[24] arrays. Both of them contain values for each hour of a day. One of them has hit count and other queue times. Now queue times in ms are 5 digit numbers, counts are 2 digit numbers. I want to show them in one chart, because queue times are to some degree corresponding with hits, but the scale is totally different. So what I came up with is that I take the max value from one list and recalculate all the values from second array using that maximum.

var count = new long[24]; // values similar to [0, 9, 25, 65 ..]
var waitings = new long[24]; // values similar to [34111, 65321, 5003 ..]
count = NormalizeDataset(count, waitings.Max());
// ... skipped
@functions {

    private long[] NormalizeDataset(long[] count, long max)
    {
        long countMax = count.Max();
        var returnable = new long[24];
        for (int i = 0; i < count.Length; i++)
        {
            var l = (count[i] * max) / countMax;
            returnable[i] = l;

        }
        return returnable;
    }

}

Does doing it like this make sense? How is this problem usually solved?

Even though there is not much code to review a few remarks:

returnable has a hard coded length. Should your input array ever need to be larger you'd have to remember to make this larger as well. A simple

var returnable = new long[count.Length];

would eliminate any need to worry about this ever again.
Given the current data constraints it's not really a problem but multiplying by max first could yield in an overflow. It might pay off to store the scaling factor as double instead (in which case it can also be pre-calculated).
Using LINQ would make the code a bit bit more concise.

Refactored method:

private long[] NormalizeDataset(long[] count, long max)
{
    double scale = max / (double)count.Max();
    return count.Select(x => x * scale).ToArray();
}

Given that scale is probably around the order of 100 and that the scaling is more or less an arbitrary choice you could just not use double for the scale.


Switching directions in iterating 2D arrays I had to do this in a course a while back: make {1,2,3,4,5,6,7,8,9} into {{1,2,3},{6,5,4},{7,8,9}}. When doing it, I just made the inner for-loop do all the work.  But most of my friends had a Boolean and an if-statement to chose between a forward or reverse loop.

Which is more optimal and which is more readable?

private double[][] a;
public TwoD(int numRows, int numCols, double[] oneD){
    a = new double[numCols][numRows];
    int d =1;
    int x = 0;
    int i = 0;
    for (int y=0; y<numCols; y++){
        for (;x<numRows&&x>=0;x+=d){
            a[y][x] = oneD[i];
            i++;
        }
        d*=-1;
        x+=d;
    }
}

Edit: I had to have an object with a constructor which took width and height and a one dimensional array. The 1d array would be put into a 2d array as if it was a physical string being laid out (→▬▬↓←▬▬↓→▬▬).3x3 is an easy example.
1 3 9 5 4 7 3 1 5 to
1 3 9→
7 4 5←
3 1 5→

The alternative would be like (is pseudo code ok when its not the code being reviewed?)

boolean right = true;
for y
    if right
        for x++
    else
        for x--
    right= !right;

I do prefer the solution with the boolean. Also, a third possible variant would be to fill every other from left to right and remaining rows from right to left

In any case, they way you are supposed to handle arrays where size is not A*B might help you to pick the best solution for you.


Method to return a string of max length (in bytes vs. characters) In my (c#) code, I need to generate a string (from a longer string) which when UTF-8 encoded, is no longer than a given max length (in bytes).

    public static string OfMaxBytes(this string str, int maxByteLength)
    {
        return str.Aggregate("", (s, c) =>
        {
            if (Encoding.UTF8.GetByteCount(s + c) <= maxByteLength)
            {
                s += c;
            }
            return s;
        });
    }

Usage looks like:

    var shortName = longName.OfMaxBytes(32);

Does this look like a correct and sensible implementation?

It depends what you mean by correct. Consider this program:

const string Input = "a\u0304\u0308bc\u0327";
var bytes = Encoding.UTF8.GetByteCount(Input);
Console.WriteLine("{0} ({1} bytes in UTF-8)", Input, bytes);
for (var i = 0; i <= bytes; i++)
{
    var result = Input.OfMaxBytes(i);
    Console.WriteLine("{0} \"{1}\" {2}", i, result, Input.StartsWith(result, StringComparison.Ordinal));
}

Here is what your solution gives:

ā̈bç (9 bytes in UTF-8)
0 "" True
1 "a" True
2 "ab" False
3 "ā" True
4 "āb" False
5 "ā̈" True
6 "ā̈b" True
7 "ā̈bc" True
8 "ā̈bc" True
9 "ā̈bç" True

Granted, you might not come across such an input very often, but I don't think that's the result you want.

Two other points:

You are building up a lot of intermediate strings. When you catch yourself doing that, see if you can use a StringBuilder instead.
You are iterating through the entire string, regardless of the value of maxBytes.

Here is what I would suggest:

public static string OfMaxBytes(this string input, int maxBytes)
{
    if (maxBytes == 0 || string.IsNullOrEmpty(input))
    {
        return string.Empty;
    }

    var encoding = Encoding.UTF8;
    if (encoding.GetByteCount(input) <= maxBytes)
    {
        return input;
    }

    var sb = new StringBuilder();
    var bytes = 0;
    var enumerator = StringInfo.GetTextElementEnumerator(input);
    while (enumerator.MoveNext())
    {
        var textElement = enumerator.GetTextElement();
        bytes += encoding.GetByteCount(textElement);
        if (bytes <= maxBytes)
        {
            sb.Append(textElement);
        }
        else
        {
            break;
        }
    }

    return sb.ToString();
}

Which gives this output:

ā̈bç (9 bytes in UTF-8)
0 "" True
1 "" True
2 "" True
3 "" True
4 "" True
5 "ā̈" True
6 "ā̈b" True
7 "ā̈b" True
8 "ā̈b" True
9 "ā̈bç" True


Java database query logic I'm making a wrapper for querying the database so that I can more easily control the flow of database management and to make it easier for me to write data beans around my database.  This code is going to be used a ton so I'm looking to make it as clean as possible.

/**
 * Query the database.
 *
 * @param sql
 *            The query statement
 * @return List of rows returned by the query.
 */
public List<Map<String, Object>> query(String sql) {
    return this.query(sql, null);
}

/**
 * Query the database.
 *
 * @param sql
 *            The query statement
 * @param params
 *            The prepared statement parameters (in order).
 * @return List of rows returned by the query.
 */
public List<Map<String, Object>> query(String sql, Object[] params) {
    final String METHODNAME = "query";
    List<Map<String, Object>> resList = null;

    try {
        if (conn != null && !conn.isClosed()) {
            resList = new ArrayList<Map<String, Object>>();

            PreparedStatement stmt = conn.prepareStatement(sql);
            if (params != null && params.length > 0) {
                for (int i = 1; i <= params.length; i++) { // 1 based, not 0
                    stmt.setObject(i, params[i - 1]);
                }
            }

            ResultSet res = stmt.executeQuery();

            while (res.next()) {
                Map<String, Object> row = new HashMap<String, Object>();

                for (int i = 1; i <= res.getMetaData().getColumnCount(); i++) {
                    String key = res.getMetaData().getColumnName(i);
                    Object val = res.getObject(i);
                    row.put(key, val);
                    resList.add(row);
                }
            }

        } else {
            logger.log(LogLevel.ALL, CLASSNAME, METHODNAME, "Attempted to run query when "
                    + "there was no established connection to the database.");
        }
    } catch (Throwable e) {
        logger.log(LogLevel.ERROR, CLASSNAME, METHODNAME,
                "An error occurred when trying to run the query.");
    }

    return resList;
}

Resource-leak: you neither close the statement nor the result set. Depending on the database driver, this can lead to system resource leaks, blocked OS handles, etc. Always close database objects ASAP (apart from connections, which you can hold for a longer time.)

Multiple looping over metadata: you reiterate over result set metadata for each row in the result set. As you do not know the underlying implementation, this may be quite an expensive operation, there's no way to be sure. Thus, better do this once and cache the information you need.

Error handling: you catch everything, log it, and let the application continue running without any indication of an error, eventually based on incomplete data. Bad idea. Do not catch exceptions here, declare the method as "throws SQLException" (or maybe re-wrap it in MyOwnApplicationException or whatever) - just don't continue running in error conditions.

Multithreading: connections normally cannot be shared amongst multiple threads (eventually invalidating their underlying objects and doing all kinds of funny things.) I see no precautions to avoid multi-threaded access in the code.

General utility: ask yourself, what are you going to do with this map? Create objects from the contents? Create excel reports? Is your application code written to handle each and every funny datatype a driver implementor may be inclined to return via getObject()? I sincerely doubt this is really useful. For object mapping, have a look at various ORM frameworks / JPA, for reporting there are other frameworks. This looks like the re-invention of a wheel to me.


spaghetti code for lunch: refactoring a bunch of fields that always get initialized blah - not even sure if that is the correct way to say it. Basically I have this

public class XmlAccess
{
    public XmlAccess() { }

    public string ApplicationPath = "";

    public string DBServer = ConfigurationManager.AppSettings["DB2_Server"];
    public string DBName = ConfigurationManager.AppSettings["DB2_Database"];
    public string DBLogIn = ConfigurationManager.AppSettings["DB2_User"];
}

I am trying to create some seams for some unit tests and have pulled out a lot of interfaces which is great but I have this thing (code above) trying to initialize all these fields (that's just a sample - it goes on and on...) when it fires up. Anyone have any suggestions as to how to refactor this? I have never had to do this sort of major refactor of "old code" but I am sure others have. The way it is handling those variables is silly and untestable but I am not sure what to do about it and it's holding me up.

Does any code set those fields?

Turn them into properties, then extract the interface that includes the properties. I recommend against letting any code set them, if at all possible.


Put every object in a specified bucket I have this array of objects:
const dataset = [
  {name: 'Paul', age: 10},
  {name: 'Marcus', age: 5},
  {name: 'Jennifer', age: 1},
  {name: 'Linda', age: 53},
  {name: 'Mary', age: 4},
  {name: 'Rose', age: 40},
  {name: 'Peter', age: 2},
]

and an array of ranges:
const buckets = [[0, 10], [10, 20], [20, 30], [30, 60]]

This is what I would like to get:
const result = {
  "0-10": {
    res: [{name: 'Marcus', age: 5}, {name: 'Jennifer', age: 1}, {name: 'Mary', age: 4}, {name: 'Peter', age: 2}],
    count: 4
  },
  "10-20": {res: [{name: 'Paul', age: 10}], count: 1},
  "20-30": {res: [], count: 0},
  "30-60": {res: [{name: 'Linda', age: 53}, {name: 'Rose', age: 40}], count: 2},
}

So I would like to get an object where each key is the bucket range (as string) and each value is an object that contains two info: res should be an array of people whose age is inside that bucket (min <= age < max) and count is the lenght of that array.
Here is my code:
const res = buckets.reduce((acc, bucketExtent) => {
        const keyBucket = `${bucketExtent[0]}-${bucketExtent[1]}`
        const people = dataset.filter(
          (s) => s['age'] >= bucketExtent[0] && s['age'] < bucketExtent[1]
        )
        acc[keyBucket] = {
          res: people,
          count: people.length,
        }
        return acc
      }, {})

It works but there is a smarter way? I prefer not to loop everytime the dataset array because it will be very large

From a short review;

You are definitely looping indeed over every entry 4 times
This belongs in a well named function
Other than that, it is very straight forward and understandable

I was thinking that people don't live all that long, you could easily build a lookup table for every possible age. This could speed this up quite a bit.
So something like this;

const dataset = [
  {name: 'Paul', age: 10},
  {name: 'Marcus', age: 5},
  {name: 'Jennifer', age: 1},
  {name: 'Linda', age: 53},
  {name: 'Mary', age: 4},
  {name: 'Rose', age: 40},
  {name: 'Peter', age: 2},
]

const buckets = [[0, 10], [10, 20], [20, 30], [30, 60]]

function createAgeBuckets(dataset, buckets){
  const start = 0, end = 1;
  const out = {}, ages = [];

  //Create age range
  for(const bucket of buckets){
    bucketRange = `${bucket[start]}-${bucket[end]}`;
    out[bucketRange] = {res: [], count: 0};
    for(let age = bucket[start]; age < bucket[end];age++){
      ages[age] = out[bucketRange];
    }
  }

  //Create bucketses
  dataset.forEach(person => {
    ages[person.age].res.push(person);
    ages[person.age].count++;
  });

  return out;
}

console.log(createAgeBuckets(dataset, buckets));