Function that tests whether all items in a list are the same The program consists of two functions: the helper predicate function singleton-list? that checks whether a given list contains only one item and the function that does the actual job all-same?.

;; Predicate function: singleton-list?
;; -----------------------------------
;; The singleton-list? predicate function tests whether a list contains only a
;; single item. That is, if the list provided has got only one element in it,
;; the function returns true. In all other cases, the function returns a value
;; of false. First it checks if the object we're sent is in fact a list. If it
;; is, we check whether "cdr" returns a nonempty list. If it does, that means
;; there is more than one item in the list and thus a value of false is
;; returned.
;;
;; Usage examples:
;;     (singleton-list? '(A))     => #t
;;     (singleton-list? '(A B C)) => #f
;;     (singleton-list? '())      => #f

(define singleton-list?
    (lambda (ls)
        (and (pair? ls) (null? (cdr ls)))))

;; Predicate function: all-same?
;; -----------------------------
;; The all-same? predicate function tests whether all elements in a given list
;; are identical. Fist it checks if the list passed in is an empty list. If it
;; is, we return a value of false because the concept of sameness does not make
;; a whole lot of sense when there are no things to be compared against each
;; other. When there is only one item in a list, we return true. Otherwise,
;; we've got a list with more than one item in it. We compare the first item
;; with the second. If they're equal, we call the function recursively with our
;; list where the first element has been removed.
;;
;; Usage examples:
;;     (all-same? '(A A A)) => #t
;;     (all-same? '(A B A)) => #f
;;     (all-same? '(A))     => #t
;;     (all-same? '())      => #f

(define all-same?
    (lambda (ls)
        (cond ((null? ls)           #f) ; If empty list, return false
              ((singleton-list? ls) #t) ; If single-item list, return true
              (else                 (if (equal? (car ls) (car (cdr ls)))
                                        (all-same? (cdr ls))
                                         #f)))))

First, a note about testing: inside singleton-list? you check if the argument is a pair, while it is called by all-same? always with a non-empty list, so the test will be always true.

So you can simplify the program by eliminating the auxiliary function and replacing, inside the definition of all-same?, the second branch of the conditional with the only significant test:

((null? (cdr ls)) #t)

Then, note that your function returns a boolean value, and each branch of the cond and the inner if returns a boolean value. This is a good reason to perform a little simplification of the function, by using and and or as control structures (this is typical of the lisp languages). So the function could be redefined as:

(define all-same?
  (lambda (ls)
    (and (not (null? ls))
         (or (null? (cdr ls))
             (and (equal? (car ls) (cadr ls))
                  (all-same? (cdr ls)))))))

Finally, it may be questionable to decide that the function should return false when the list is empty, since in mathematics the universal quantifier is true even for an empty set. So, in case the function could be defined to return true even if the list is empty (“there are no elements, so there are no two element different”), we could further simplify the function in the following way:

(define all-same?
  (lambda (ls)
    (or (null? ls)
        (null? (cdr ls))
        (and (equal? (car ls) (cadr ls))
             (all-same? (cdr ls))))))


Split list into groups of n in Haskell I need to split a list into equal sublists, e. g [1..9] split into groups of 3 will be [[1, 2, 3], [4, 5, 6], [7, 8, 9]]. I have accomplished this task in the following way:

splitInGroupsOf n = takeWhile ((n ==) . length)
                  . map fst
                  . drop 1
                  . iterate (\(res, list) -> splitAt n list)
                  . (,) []

where iterate creates list of tuples with first n elements and rest of list. This way I had to use (,) [] on argument to ensure correct type, and unwrap result afterwards. My questions are

is there a better/more elegant way of performing same task?
is there some standard function I should make use of?

P.S.: I'm not sure where to ask simple Haskell-related questions and will appreciate if someone points me a better place for this than SE.

There are Data.List.Split.chunksOf and Data.List.Grouping.splitEvery implementations of this routine in specialized packages (and a number included in other application packages: search by Int -> [a] -> [[a]] signature on Hayoo).

I think splitEvery implementation is pretty elegant:

splitEvery :: Int -> [a] -> [[a]]
splitEvery _ [] = []
splitEvery n xs = as : splitEvery n bs
  where (as,bs) = splitAt n xs


Crawler for performing reverse search and writing results into a CSV file I've written a script which is able to perform reverse search in the website using the Name and Lid from a predefined CSV file. However, when the search is done then it can put the results containing Address and Phone Number adjacent to those Name and Lid creating a new CSV file. It is working errorlessly now. I tried to make the total process clean. Any suggestion to do betterment of this script will be highly appreciated. Here is the code I have tried with:

import csv
import requests
from lxml import html

with open("predefined.csv", "r") as f, open('newly_created.csv', 'w', newline='') as g:

    reader = csv.DictReader(f)
    newfieldnames = reader.fieldnames + ['Address', 'Phone']
    writer = csv.writer = csv.DictWriter(g, fieldnames = newfieldnames)
    writer.writeheader()

    for entry in reader:

        Page = "https://www.yellowpages.com/los-angeles-ca/mip/{}-{}".format(entry["Name"].replace(" ","-"), entry["Lid"])
        response = requests.get(Page)
        tree = html.fromstring(response.text)
        titles = tree.xpath('//article[contains(@class,"business-card")]')

        for title in tree.xpath('//article[contains(@class,"business-card")]'):
            Address= title.xpath('.//p[@class="address"]/span/text()')[0]
            Contact = title.xpath('.//p[@class="phone"]/text()')[0]

            print(Address,Contact)

            new_row = entry
            new_row['Address'] = Address
            new_row['Phone'] = Contact
            writer.writerow(new_row)

Here is the link to the search criteria of "predefined.csv" file.

Here is the link to the results.

There are multiple things we can do to improve the code:

variable naming - try to be consistent with PEP8 naming suggestions - for instance:

Page should probably be page - or even better url
Address would be address
Contact would be contact
f can be input_file
g can be output_file

titles variable is never used
move the url format string into a constant
you don't need writer = csv.writer = csv.DictWriter(...) - just assign the writer to the DictWriter instance directly
since you are crawling the same domain, re-using requests.Session() instance should have a positive impact on performance
use .findtext() method instead of xpath() and then getting the first item
I would also create a separate crawl function to keep the web-scraping logic separate

Here is the modified code with the above and other improvements combined:

import csv

import requests
from lxml import html

URL_TEMPLATE = "https://www.yellowpages.com/los-angeles-ca/mip/{}-{}"

def crawl(entries):
    with requests.Session() as session:
        for entry in entries:
            url = URL_TEMPLATE.format(entry["Name"].replace(" ", "-"), entry["Lid"])
            response = session.get(url)
            tree = html.fromstring(response.text)

            titles = tree.xpath('//article[contains(@class,"business-card")]')
            for title in titles:
                address = title.findtext('.//p[@class="address"]/span')
                contact = title.findtext('.//p[@class="phone"]')

                print(address, contact)

                entry['Address'] = address
                entry['Phone'] = contact
                yield entry

if __name__ == '__main__':
    with open("predefined.csv", "r") as input_file, open('newly_created.csv', 'w', newline='') as output_file:
        reader = csv.DictReader(input_file)
        field_names = reader.fieldnames + ['Address', 'Phone']

        writer = csv.DictWriter(output_file, fieldnames=field_names)
        writer.writeheader()

        for entry in crawl(reader):
            writer.writerow(entry)

(not tested)


A heap-allocated texture class for a console renderer I am working on a console renderer and I want a heap-allocated dynamic texture class. Since I am quite new to C++, I just want to check the memory management, if I destroy all pointers correctly etc.

class Texture {
private :

char** pixels; // heap allocated
int width, height;

public :

Texture(int width, int height, const char* texture) {
    this->width = width;
    this->height = height;

    pixels = new char*[width];
    for (int x = 0; x < width; x++) {
        pixels[x] = new char[height];

        for (int y = 0; y < height; y++)
            pixels[x][y] = texture[y*width+x];

    }

}

Texture(int width, int height, char fill) {

    this->width = width;
    this->height = height;

    pixels = new char*[width];
    for (int x = 0; x < width; x++) {
        pixels[x] = new char[height];

        for (int y = 0; y < height; y++)
            pixels[x][y] = fill;

    }

}

~Texture() {
    for (int x = 0; x < width; x++)
        delete[] pixels[x];

    delete pixels;
}

Texture getSubTexture(int x, int y, int width, int height) {

    Texture newtex(width, height, '?');

    for (int xx = 0; xx < width; xx++)
        for (int yy = 0; yy < height; yy++) {
            newtex.setPixel(xx, yy, getPixel(x+xx, y+yy));
        }

    return newtex;

}

void setPixel(int x, int y, char c) {
    pixels[x][y] = c;
}

char getPixel(int x, int y) {
    return pixels[x][y];
}

int getWidth() { return width; }
int getHeight() { return height; }

};

getHeight and getWidth should be const.

int getWidth() const { return width; }
int getHeight() const { return height; }

Your destructor does not properly delete the pixels pointer.  Since you allocate it with new[] you need to use delete[].

delete [] pixels;

You're storing your textures in column-major order.  Depending on how you access them, this can cause performance issues with caching.  For example, in the constructor, when you copy in the initial texture values you jump thru memory (one byte every width bytes) rather than reading it sequentially (where x would be the inner loop).

Then there's the inevitable question of why you're using manual memory management, rather than using vector.


Clipboard detector in Python The following contains my little app that detects changes to the clipboard and displays them in the GUI. I did my best using my limited knowledge of Python, but I have a feeling that I can definitely improve the program. It works, but Python's CPU usage shoots up to 20% whenever I run the program - which is due to my use of multiple threads and infinite loops I'm sure.

#! python3

#GUI
import tkinter
#needed for the clipboard event detection
import time
import threading

#listener class that inherits from Thread
class ClipListener(threading.Thread):
    #overriding Thread constructor
    def __init__(self, pause = .5):
        #from documentation: If the subclass overrides the constructor, it must make sure to invoke the base class constructor (Thread.__init__()) before doing anything else to the thread.
        super().__init__() #calls Thread class constructor first

        #initialize parameters
        self.pause = pause
        self.stopping = False

        #initialize event to communicate with main thread
        self.copyevent = threading.Event()

    #override run method
    def run(self):
        last_value =  tkinter.Tk().clipboard_get() #initialize last_value as

        #continue until self.stopping = true
        while not self.stopping:
            #grab clip_board value
            temp_value = tkinter.Tk().clipboard_get()
            #if last value is not equal to the temp_value, then (obviously) a change has occurred
            if temp_value != last_value:
                #set last value equal to current (temp) value and print
                last_value = temp_value
                print("set")
                #set the event if clipboard has changed
                self.copyevent.set()
            time.sleep(self.pause) #sleep for indicated amount of time (.5 by default)

    #override stop method to work with our paramter 'stopping'
    def stop(self):
        self.stopping = True

#GUI extends Frame, serving as main container for a root window
class GUI(tkinter.Frame):

    #constructor for GUI - intializes with a default height and width if none are given
    def __init__(self, master, ht=600, wt=800):

        #uses the parent class' constructor
        super().__init__(master, height=ht, width=wt)
        self.var = tkinter.StringVar()
        self.var.set("No copied text")
        self.pack_propagate(False) #window will use it's own width and height as parameters instead of child's dimensions
        self.pack()
        self.label = tkinter.Label(self, textvariable=self.var)
        self.label.pack()

    #method to update the label
    def update_label(self, newText):
        self.var.set(newText)
        self.label.pack()

def main():
    #good practice to have a variable to stop the loop
    running = True

    #GUI initialized
    root = tkinter.Tk()
    gui = GUI(root)

    #start thread containing Clipboard Listener
    listener = ClipListener(.100)
    listener.start()

    #loop to keep updating the program without blocking the clipboard checks (since mainloop() is blocking)
    while running:
        #update the gui
        root.update();
        #wait .1 seconds for event to be set to true
        event_set = listener.copyevent.wait(.100)
        #if true then update the label and reset event
        if event_set:
            gui.update_label(root.clipboard_get())
            listener.copyevent.clear()

#only run this program if it is being used as the main program file
if __name__ == "__main__":
    main()

Don't use threads

You don't need the complexities of threading for this problem. You can use the Tkinter method after to run a function periodically. Threading is necessary if the function you are running takes a long time, but that is not the case here.

You can use a class, but to keep this answer simple I'll only show the function.

Also, note that I use an existing window rather than tkinter.Tk() on each call. There's no need to create a new root window every time you do the check.

def check_clipboard(window):
    temp_value = window.clipboard_get()
    ...

Next, create a function that calls this function every half second:

def run_listener(window, interval):
    check_clipboard(window)
    root.after(interval, run_listener, window, interval)

To start running, simply call run_listener once, and it will continue to run every 500 milliseconds:

run_listener(root, 500)

The logic to stop and pause is roughly the same as what you have now. Create a flag, and then check that flag inside of run_check_periodically.

With this, you can remove the loop from main since tkinter already has an efficient loop:

def main():
    root = tkinter.Tk()
    gui = GUI(root)

    # start the listener
    listen_to_clipboard(root)

    # start the GUI loop
    root.mainloop()


Array to key/value JSON conversion I'm very new to Javascript...

Below is my code to convert Array to key/value JSON. Would like to know if there is a better way to achieve this or if I use any know bad practices.

An input: [10, [20.1, 20.2], 1]

The output:
{"channel_id":10, "payload":[20.1,20.2], "sequence":1}

My working code to generate the output:

let myArray = [10, [20.1, 20.2], 1]

class MyObject {
  constructor(a, b, c) {
    this.channel_id = a,
      this.payload = b,
      this.sequence = c;
  }
}

myObject = new MyObject(myArray[0], myArray[1], myArray[2])

console.log(JSON.stringify(myObject))

Not sure that a class definition makes a lot of sense here since you don’t really have any real behavior being defined in this class.
Destructuring can also help you populate variables from the input array directly.
camelCase is generally preferred in JS vs. snake_case.

For example you might simplify to something like this:

const conversionFn = ( [ channelId, payload, sequence ] ) => {
  return { channelId, payload, sequence };
};


Dealing with messy JSON API and UTF-8 encoding problems I am using an API that returns a JSON object that has "encoded_polyline" fields that tend to result in errors like this one:

  UnicodeEncodeError: 'utf-8' codec can't encode character '\udb6e' in position 2: surrogates not allowed

Since the offending fields don't offer useful data, I decided the best would be to get rid of the fields causing these errors. However, these fields are sometimes stored at the top level of the JSON object but also sometimes stored within an array at either:

jsonobject["points"]["points"]
jsonobject["laps"]["metric"]
jsonobject["laps"]["imperial"]

I think I've finally got something working to hunt down all these fields and delete them, but I have a feeling this could be done much more cleanly.

r = requests.get(url, headers = {'User-Agent':UA})
jsonobject = r.json()
if 'laps' in jsonobject and jsonobject['laps'] is not None:
    if 'imperial' in jsonobject['laps']:
        laps_array = jsonobject['laps']['imperial']
        type = 'imperial'
    if 'metric' in jsonobject['laps']:
        laps_array = jsonobject['laps']['metric']
        type = 'metric'
    if laps_array is not None:
        jsonobject['laps_correct'] = dict()
        jsonobject['laps_correct'][type] = list()
        for document in laps_array:
            #sometimes these document objects turn out to be dicts
            #and sometimes they turn out to be strings
            #even though JSON output is always the same
            #is there a better way to deal with this?
            if document.__class__.__name__ == "dict":
                document2 = document
            else:
                document2 = json.loads(document)
            if 'small_encoded_polyline' in document2 and document2['small_encoded_polyline'] is not None:
                del document2['small_encoded_polyline']
                document = document2
                #I thought this line above would modify the original
                #jsonobject since document is a dictionary so I should
                #be working with a pointer to the original object
                #but inspection of jsonobject reveals this not to be the case
                jsonobject['laps_correct'][type].append(document2)
        del jsonobject['laps']
        jsonobject['laps'] = jsonobject.pop('laps_correct')

# this can't be an elif because sometimes json objects
# have both "points" and "laps"
if 'points' in jsonobject and jsonobject['points'] is not None:
    if 'points' in jsonobject['points']:
        laps_array = jsonobject['points']['points']
        jsonobject['points_correct'] = dict()
        jsonobject['points_correct']['points'] = list()
        if laps_array is not None:
            for document in laps_array:
                if document.__class__.__name__ == "dict":
                    document2 = document
                else:
                    document2 = json.loads(document)
                    if 'small_encoded_polyline' in document2 and document2['small_encoded_polyline'] is not None:
                        del document2['small_encoded_polyline']
                        document = document2
                        jsonobject['points_correct']['points'].append(document2)
        del jsonobject['points']
        jsonobject['points'] = jsonobject.pop('points_correct')

if 'small_encoded_polyline' in jsonobject and jsonobject['small_encoded_polyline'] is not None:
    del jsonobject['small_encoded_polyline']

My two biggest worries/questions are:

How can I deal with variables that are sometimes typed as dicts and sometimes as strings even though the JSON format appears identical in both cases?
Is it really necessary to delete original key and replace it rather than updating dicts from the original key? This seems slow and clunky.

I don't see why del jsonobject[...] would be necessary before
reassigning that key.  Also, the test in the last two lines could
also be simplified to something like
jsonobject.pop('small_encoded_polyline', None) if you don't care
about that key, no?

Also:

Everything could use more error checking.  That is, check expected
types and values instead of falling through the ifs ('imperial',
'metric', what happens if it's neither, or both?).
Unless you really prefer list and dict, it's better to use the
literal syntax [] and {} (the names could be shadowed by some
definition).
The type check for dicts can be better, i.e.
isinstance(foo, dict); if you really want only dict, then it's
still better to use type(foo) is dict instead of comparing strings.
I'd also move that whole if/else into a function,
e.g. maybe_load_json or so.
The value of document after the assignments isn't used anywhere, so
it's safe to remove, though I'd rather use that variable again instead
of introducing document2.
The second branch with 'small_encoded_polyline' seems to have the
wrong level of indentation?  At least it's different from the first
one in that it's only run if the document wasn't a dict.  Assuming
that it's safe to run anyway I'll change that in the code below.
The pattern if foo in json and json[foo] is not None: could be easier
be achieved with if json.get(foo) is not None: as the return value
there defaults to None.
I'm not particularly fond of the habit to reuse input data like with
'points_correct' done here.  It would be cleaner just to have a list
points_correct and assign that to a jsonobject key (and you know,
possibly put the calculation into a separate function as well).

I might have messed up some of the logic now, but I hope you get the
idea:

def maybe_load_json(document):
    if isinstance(document, dict):
        return document
    return json.loads(document)

def cleaned_small_encoded_polylines(documents):
    result = []
    for document in documents:
        #sometimes these document objects turn out to be dicts
        #and sometimes they turn out to be strings
        #even though JSON output is always the same
        #is there a better way to deal with this?
        document = maybe_load_json(document)
        if document.get('small_encoded_polyline') is not None:
            del document['small_encoded_polyline']
            #I thought this line above would modify the original
            #jsonobject since document is a dictionary so I should
            #be working with a pointer to the original object
            #but inspection of jsonobject reveals this not to be the case
            result.append(document)
    return result

...

    r = requests.get(url, headers = {'User-Agent':UA})
    jsonobject = r.json()
    if jsonobject.get('laps') is not None:
        if 'imperial' in jsonobject['laps']:
            laps_array = jsonobject['laps']['imperial']
            type = 'imperial'
        if 'metric' in jsonobject['laps']:
            laps_array = jsonobject['laps']['metric']
            type = 'metric'
        if laps_array is not None:
            jsonobject['laps'][type] = cleaned_small_encoded_polylines(laps_array)

    # this can't be an elif because sometimes json objects
    # have both "points" and "laps"
    if jsonobject.get('points') is not None:
        if 'points' in jsonobject['points']:
            points_correct = []
            laps_array = jsonobject['points']['points']
            if laps_array is not None:
                jsonobject['points']['points'] = cleaned_small_encoded_polylines(laps_array)

    jsonobject.pop('small_encoded_polyline', None)


LeetCode 1108: Defanging an IP Address I'm posting two solutions for LeetCode's "Defanging an IP Address". If you'd like to review, please do. Thank you!
Problem
Given a valid (IPv4) IP address, return a defanged version of that IP address.
A defanged IP address replaces every period "." with "[.]".
Example 1:
Input: address = "1.1.1.1"
Output: "1[.]1[.]1[.]1"

Example 2:
Input: address = "255.100.50.0"
Output: "255[.]100[.]50[.]0"

Constraints:

The given address is a valid IPv4 address.

Code 1
#include <stdio.h>
#include <stdlib.h>

static const char* defangIPaddr(
    char* address
) {
    char* ipv4_memory = calloc(1, sizeof("###[.]###[.]###[.]###"));
    char* defanged = ipv4_memory;

    for (char* character = address; *character; character++) {
        if (*character == '.') {
            *ipv4_memory++ = '[';
            *ipv4_memory++ = '.';
            *ipv4_memory++ = ']';

        } else {
            *ipv4_memory++ = *character;
        }
    }

    return defanged;
}

int main() {

    printf ("%s \n", defangIPaddr("1.1.1.1"));
    printf ("%s \n", defangIPaddr("255.100.50.0"));

    return 0;
}

Code 2 without changing the input:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

static const char* defangIPaddr(
    const char* address
) {
    char* cloned_address = NULL;
    cloned_address = strdup(address);

    char* ipv4_memory = calloc(1, sizeof("###[.]###[.]###[.]###"));
    char* defanged = ipv4_memory;

    for (char* character = cloned_address; *character; character++) {
        if (*character == '.') {
            *ipv4_memory++ = '[';
            *ipv4_memory++ = '.';
            *ipv4_memory++ = ']';

        } else {
            *ipv4_memory++ = *character;
        }
    }

    return defanged;
}

int main() {

    printf ("%s \n", defangIPaddr("1.1.1.1"));
    printf ("%s \n", defangIPaddr("255.100.50.0"));

    return 0;
}

Watch your memory allocations and deallocations. In both cases, you've got defangIPaddr returning a const char * to heap-allocated memory, which needs to be freed by the caller... but it can't be freed, because free expects a non-const void* as its argument.
Functions that return ownership-of-a-heap-allocation to the caller should (A) return char*, not const char*, and (B) be clearly documented.
In Code 2, you're not just leaking the returned pointer; you're also leaking the strdup'ed original pointer. There's no reason to call strdup here.
What you want is simply
char *defangIPaddr(const char *address) {
    char *defanged = calloc(sizeof("###[.]###[.]###[.]###"), 1);
    char *q = defanged;
    for (const char *p = address; *p != '\0'; ++p) {
        if (*p == '.') {
            *q++ = '[';
            *q++ = '.';
            *q++ = ']';
        } else {
            *q++ = *p;
        }
    }
    return defanged;
}

I've shortened your names character and ipv4_memory to just p and q. My p is for "pointer"; it's not a character at all, so character is a misleading name for it. q is simply what comes after p.
You got the arguments to calloc in the wrong order. In practice this doesn't matter; but getting it correct is easy and free. If you're unsure about an interface, check the man page.
In your original code, you did this:
char* cloned_address = NULL;
cloned_address = strdup(address);

That's a verbose way of writing
char* cloned_address = strdup(address);

Always initialize your variables — but initialize them to the right values! Don't initialize to a nonsense or garbage value and then later assign the right value over it; just initialize with the right value to begin with.

sizeof("###[.]###[.]###[.]###") is a mildly sneaky way of computing the maximum possible length. It might be better to allocate simply 3*strlen(address)+1 bytes, so that you can't possibly buffer-overflow, even if the caller passes in ".................." as their input.
Alternatively, you could compute the exact right buffer length and malloc exactly that much.
    int len = 0;
    for (const char *p = address; *p != '\0'; ++p) {
        len += (*p == '.') ? 3 : 1;
    }
    char *defanged = calloc(len+1, 1);
    [...]

Consider throwing in an if (defanged == NULL) return NULL; just in case the allocation fails.


Making this reduce() Fibonacci generator better I've recently discovered how cool reduce() could be and I want to do this:

>>> a = [1, 1] + [0] * 11
>>> count = 1
>>> def fib(x,n):
...     global count
...     r = x + n
...     if count < len(a) - 1: a[count+1] = r
...     count += 1
...     return r
>>>
>>> reduce(fib,a,1)
610
>>> a
[1, 1, 2, 3, 5, 8, 13, 21, 34, 55, 89, 144, 233]

But this just looks so messy and almost defeats the purpose of the last line:

reduce(fib,a,1)

What would be a better way to use Python to make a Fibonacci number with reduce()?

A reduce (that's it, a fold) is not exactly the abstraction for the task (what input collection are you going to fold here?). Anyway, you can cheat a little bit and fold the indexes even if you don't really use them within the folding function. This works for Python 2.x:

def next_fib((x, y), n):
    return (y, x + y)

reduce(next_fib, xrange(5), (1, 1))[0]
#=> 8


Calculating parking fees, given start and end timestamps Since I have just started Javascript, I am trying to solve a few problems and improve myself. I encountered such a question. I tried to do everything step by step from the question, but I do not think that my code is efficient. Can someone help me develop it?

Please code for a JavaScript function that accepts two arguments that
are; parking "date and time"(timestamp) and return "date and
time"(timestamp). At an airport for car parking, the following rules
are applied.
Parking costs;
€2 for the first 20 minutes,
rising to €4 for up to 40 minutes,
rising to €6 for up to one hour,
rising to €7 for up to two hours,
rising to €9 for up to three hours,
rising to €11 for up to four hours,
rising to €13 for 4-8 hours and
rising to €15 for 8-24 hours.
€16 for the first 24 hours and after that each additional day is
charged at €9.

Here is my code:
function msToHours(milisecond) {
        let time = milisecond;

        let hour = (time / 60000) / 60;

        return hour;
    }

    //Mins to Hours Function
    function minToHours(miniute){

        let time = miniute;

        let hr = (miniute /60);

        return hr;

    }

    //Finding the nth day Function

    function add24HR(hour, cb) {

        let arr = new Array();

        for (let i = 0; i < hour; i++) {
            if (i % 24 == 0) {
                arr.push(i)
            }
        }

        return  `Your Parking Fee is £${(arr.length*cb-cb) + 16}.(${arr.length} days)`
    }

    //Main Function

    const parkingFees = (parkingDate, returnDate) => {

        //Defining dates
        var park = new Date(parkingDate)
        var returned = new Date(returnDate);

        //Variables
        var penaltyFee = 9;
        let totalPrice;

        //Time between park and return (miliseconds)
        let totalTime = returned - park

        //MiliSeconds to Hour
        let totalPark = msToHours(totalTime);

        //Mins to Hours
        if (totalPark <= minToHours(20)) {
            return `Your parking fee is only £${2}.`
        }

        else if(totalPark > minToHours(20) && totalPark <= minToHours(40)){
            return `Your parking fee is only £${4}.`
        }

        else if(totalPark > minToHours(40) && totalPark <= minToHours(60)){
            return `Your parking fee is only £${6}.`
        }
        else if(totalPark > minToHours(60) && totalPark <= minToHours(120)){
            return `Your parking fee is only £${7}.`
        }
        else if(totalPark > minToHours(120) && totalPark <= minToHours(180)){
            return `Your parking fee is only £${9}.`
        }
        else if(totalPark > minToHours(180) && totalPark <= minToHours(240)){
            return `Your parking fee is only £${11}.`
        }
        else if(totalPark > minToHours(240) && totalPark <= minToHours(480)){
            return `Your fparking fee is only £${13}.`
        }

        else if(totalPark > minToHours(480) && totalPark < minToHours(1440)){

            return `Your parking fee is only £${15}.`
        }

        else if(totalPark > minToHours(1440) && totalPark < minToHours(2880)){

            return `Your parking fee is only £${16}.`
        }

        //if totalPark > 24 HRS

        else {

            totalPrice = add24HR(totalPark, penaltyFee)

        }

        return totalPrice;

    }

    document.querySelector("body").innerHTML = (parkingFees("5/12/2020 18:30", "5/18/2020 18:30"))

The main issue I see with the code is the repetitive if/elses at the bottom. Repetitive code is annoying to write, takes some time to read, and is often a bit more likely to have bugs than DRY code. To identify the parking fee, I'd make an array of ranges and their associated fees, then find the appropriate range:
const getCost = (totalHoursParked) => {
  const costs = [
    // [Maximum number of hours, Associated cost]
    [1/3, 2],
    [2/3, 4],
    [1, 6],
    [2, 7],
    [3, 9],
    [4, 11],
    [8, 13],
    [24, 15],
  ];
  if (totalHoursParked > 24) {
    const days = Math.ceil(totalHoursParked / 24);
    return 16 + 9 * (days - 1);
  }
  const found = costs.find(([maxHours]) => totalHoursParked <= maxHours);
  return found[1];
};

Just calculate the hours stayed, then invoke the function above.
Other suggestions regarding your current code:
Always use const when you can - only use let when you must warn readers of the code that you may be reassigning the variable in the future.
Avoid var - If you're going to write in modern ES6+ syntax, great - in which case, better to avoid var, which has problems such as an unintuitive function scope and automatically assigning to a property of the global object when on the top level.
Prefer [] over new Array() -  it's shorter to write and easier to read, and using the constructor may tempt you to pass it a number, which will result in a sparse array (sparse arrays can have some very strange behaviors; better to avoid them entirely).
Prefer strict equality - better to avoid == and !=, since they have confusing coercion rules a reader of the code should not have to have memorized in order to be confident of the logic the code is implementing.
add24HR is weird While you can loop one-by-one over the hours and increment when encountering every 24th, it'd be much easier to just divide the number of hours by 24 and take the .ceil or .floor of it as needed.
Use meaningful, precise variable names that clearly indicate what the variable contains. Would someone just glancing at the script know what park means, or the difference between totalTime and totalPark, without looking at where those are defined? Probably not - better to call them something like parkDate, returnDate, totalMSParked, totalHoursParked. The penaltyFee = 9 doesn't make a whole lot of sense either except after one reads the description of the problem and some of the code. parkingFees is a function, so it should probably have a verb in its name, like calculateParkingFees.
Only use string interpolation when you need to interpolate For example, this:
return `Your parking fee is only £${4}.`

may as well be
return `Your parking fee is only £4.`

Semicolons Some of your lines have semicolons, but a lot of lines are missing semicolons too. Best to be stylistically consistent - either use semicolons or don't. Unless you're an expert, I'd highly recommend using semicolons, else you will probably occasionally run into the pitfalls of automatic semicolon insertion.
In all, I don't think there's much need in the code for anything other than a small wrapper around the getCost function:

const calculateParkingFees = (parkDateStr, returnDateStr) => {
  const parkDate = new Date(parkDateStr);
  const returnDate = new Date(returnDateStr);
  const totalMSParked = returnDate - parkDate;
  const totalHoursParked = totalMSParked / (1000 * 3600);
  return `Your parking fee is £${getCost(totalHoursParked)}.`;
}

const getCost = (totalHoursParked) => {
  const costs = [
    // [Maximum number of hours, Associated cost]
    [1/3, 2],
    [2/3, 4],
    [1, 6],
    [2, 7],
    [3, 9],
    [4, 11],
    [8, 13],
    [24, 15],
  ];
  if (totalHoursParked > 24) {
    const days = Math.ceil(totalHoursParked / 24);
    return 16 + 9 * (days - 1);
  }
  const found = costs.find(([maxHours]) => totalHoursParked <= maxHours);
  return found[1];
};

console.log(calculateParkingFees("5/12/2020 18:30", "5/18/2020 18:30")); // 6 days
console.log(calculateParkingFees("5/12/2020 18:30", "5/12/2020 19:45")); // bit more than an hour
console.log(calculateParkingFees("5/12/2020 18:30", "5/12/2020 19:00")); // 30 minutes


Specifying that one set of HTML elements controls a corresponding set How could the following code be made more concise?

HTML structure

jQuery

$('#main-nav li:eq(0)').on({
    mouseenter: function () {
        $('body section:eq(0)').find('.secondState').stop().fadeIn(600);
    },
    mouseleave: function () {
        if ($('body section:eq(0) .toggle').hasClass('current')) {
            return false;
        } else {
            $('body section:eq(0)').find('.secondState').stop().fadeOut(600);
        }
    }
});
$('#main-nav li:eq(1)').on({
    mouseenter: function () {
        $('body section:eq(1)').find('.secondState').stop().fadeIn(600);
    },
    mouseleave: function () {
        if ($('body section:eq(1) .toggle').hasClass('current')) {
            return false;
        } else {
            $('body section:eq(1)').find('.secondState').stop().fadeOut(600);
        }
    }
});
$('#main-nav li:eq(2)').on({
    mouseenter: function () {
        $('body section:eq(2)').find('.secondState').stop().fadeIn(600);
    },
    mouseleave: function () {
        if ($('body section:eq(2) .toggle').hasClass('current')) {
            return false;
        } else {
            $('body section:eq(2)').find('.secondState').stop().fadeOut(600);
        }
    }
});

The only part of the code that changes are the index values. What if you don't specific the index yourself, but use the indexof() to let it determ the index itself:

var index;
$('#main-nav li').on({
    mouseenter: function () {
        //alert($(this).index());
        index = $(this).index();
        $('body section:eq(' + index +')').find('.secondState').stop().fadeIn(600);
    },
    mouseleave: function () {
        if ($('body section:eq(' + index +') .toggle').hasClass('current')) {
            return false;
        } else {
            $('body section:eq(' + index +')').find('.secondState').stop().fadeOut(600);
        }
    }
});

I place the index into a variabe so the code doesnt have to determ the index over and over. Now i only fill the index when the mouseenters, because you will have the index of the element were  you entered with.

jsFiddle

I hope this helps.


Refactor nested null checking for multiple possible string values I am parsing an XML file and trying to get the guid of a node where it has an attribute "name" and a value of "Name" like so:

<property name="Name" guid="7eca00b3-38e0-f0e8-9b72-2b822b21de6b" />

string propertyGuid = propertyList.Where(x => x.name == "Name").FirstOrDefault()?.ToString();
if (propertyGuid == (object)null)
     throw new ArgumentNullException();

So I just found out that there are nodes that don't have a "Name" value but have a "Code" value. Which lets me check for another null value again

<property name="Code" guid="8bca00b3-38e0-f0e8-9b72-2b822b21de7a" />

string propertyGuid = propertyList.Where(x => x.name == "Name").FirstOrDefault()?.ToString();
if (propertyGuid == (object)null)
{
     propertyGuid = propertyList.Where(x => x.name == "Code").FirstOrDefault()?.ToString();
     if (propertyGuid == (object)null)
          throw new ArgumentNullException();
}

How do I refactor my code such that when another criteria (e.g "ID") is introduced in the future, I don't have to make another nested null check?

EDIT:

The check should be the other way around.

if ((object)propertyGuid == null)

You can just go with :

// Just check both case at the same time
string propertyGuid = propertyList.Where(x => x.name == "Name" || x.name == "Code").FirstOrDefault()?.ToString();
if (string.IsNullOrEmpty(propertyGuid)) // Same check as you did but written differently
{
    throw new ArgumentNullException();
}

Then for the future, I guess that you can have Guids on node you don't want to extract. You could have properties on a list or array :

List<string> possibleName = new List<string>() {"Code", "Name"};

And then calling an extract function :

Public static string GetGuid(List<string> possibleName, Whatever propertyList)
{
    string propertyGuid = string.Empty;
    foreach (string name on possibleName)
    {
        propertyGuid = propertyList.Where(x => x.name == name).FirstOrDefault()?.ToString();
        if (!string.IsNullOrEmpty(propertyGuid))
        {
             return (propertyGuid);
        }
    }
    throw new ArgumentNullException();
}

Ps : I'd like to add that if propertyList is a NodeList you won't get what you want.


Navigating around ul li and looking for the active state on the current slide How can I rewrite this in a simpler way?

$imageContainer = $($(this).parent().children()[0]).children()
currentIdx = 0
for e, idx in $imageContainer
  if e.className.indexOf('active')
    currentIdx = idx
    break

It looks like you're looking for the index of the first child of $imageContainer with the active class. (Incidentally, your code will also trigger on stuff like inactive, which might be a false positive. Be careful about your classes!)

Native selectors work fine here. Furthermore, your initial assignment doesn't do things the jQuery way -- there's no reason to initialise two separate jQuery objects. Try replacing your block with something like:

$imageContainer = $(this).parent().children().first().children()
currentIdx = $imageContainer.index($imageContainer.filter(".active"))

Better yet, instead of that monstrous .parent().children().first().children(), give it a meaningful ID or class, so you can just do $imageContainer = $(".image-container", this).

If you can guarantee that the ul will contain only lis, then you can simplify further to currentIdx = $imageContainer.filter(".active").index().

Depending on your exact situation, you may even be able to replace everything with the one liner:

currentIdx = $(this).parent().find(".active").index()


Replace anchor tags with spans, and place the href attribute into the span I'm required to perform a slight conversion on a string which contains html.

The conversion should transform a elements such that they become span elements. The conversion should also place the href attribute from the original a element into the span.

I have what I think is a working solution, but the code feels overcomplicated. I had hoped this may be possible with relatively little code, so I was wondering if anyone could suggest ways to make this code neater.

Browser compatibility is not an issue, as this will be transpiled by babel.

Here's what I have:

const testHtml = `

This is a test paragraph

  Google is here
  Msn is here

This is a heading

`

const splitByLinkEnds = html => {
  const split = html.split('')
  const allButLast = split.slice(0, -1)
  const last = split.slice(-1)[0]

  return [
    ...allButLast.map(x => `${x}</a>`),
    last
  ]
}

const splitByLinks = html => {
  const split = html.split(' `<a${x}`)
  ]

  return withReaddedOpeningTags
    .reduce((prev, curr) => [
      ...prev,
      curr.indexOf(' -1 ?
        splitByLinkEnds(curr) : curr
    ], []).flat()
}

const transformLinkToTextOnly = linkHtml => {
  const linkHref = linkHtml
    .match('href="[^"]*"')[0]
    .replace('href="', '')
    .replace('"', '')

  const htmlWithSpan = linkHtml
    .replace('<a', '<span')
    .replace('', '')

  return `${htmlWithSpan} (${linkHref})</span>`
}

const transformLinksToTextOnly = html =>
  splitByLinks(html)
    .map(x => x.indexOf(' - 1 ?
      transformLinkToTextOnly(x) : x
    ).join('')

const result = transformLinksToTextOnly(testHtml)

document.body.innerHTML += testHtml
document.body.innerHTML += '
'
document.body.innerHTML += result

Edit
I should note that the solution ought to work in node, ideally. I appreciate that the fact I include dom manipulation in the question is quite misleading, I included this more to attempt to create an easy working example that to demonstrate how the code might actually be used.

A further note, the performance of this code is not particularly important. It's only for relatively small amounts of html and gets run once when the application loads.

One thing to consider is the whole working with HTML as string. In the answer I'll assume you work within the web-browser. The web-browser can parse the HTML for you, this way you'll only have to work with nodes.

Here is an example:

const testHtml = `

This is a test paragraph

  Google is here
  Msn is here

This is a heading

`;

function transformLinksToTextOnly(htmlString) {
  const root     = document.createElement('root');
  root.innerHTML = htmlString;
  const anchors  = root.querySelectorAll('a');

  anchors.forEach(anchor => {
    const span = document.createElement('span');

    span.innerHTML = anchor.innerHTML;
    if (anchor.href)
      span.innerHTML += ` (${anchor.href})`;

    anchor
      .getAttributeNames()
      .forEach(attrName => {
        const attrValue = anchor.getAttribute(attrName);
        span.setAttribute(attrName, attrValue);
      });

    anchor.parentNode.replaceChild(span, anchor);
  });

  return root.innerHTML;
}

const result = transformLinksToTextOnly(testHtml);

document.body.innerHTML += testHtml;
document.body.innerHTML += '
';
document.body.innerHTML += result;


Creating a 2D array from a txt file I was told to create a 2D array from a txt file that had the "carModel, carColor" followed by a new line. The 2D array is 8x8 and for each time a certain carmore and carcolor appear, their respective [x][y] coordinate that represents their count goes up by 1.

So far, I have read the file, created a 2D array from the file, and have created an output with the 2D array and every slot filled with zero, but the only way I can seem to figure out to update each model,color count is if I manually make 64 if-statements to check if they appear in the list n+ times. Surely there has to be another way?

For example, When my scanner reads through the list, I need it to check if the list repeats the carmodel, carcolor and if so, update the count of that carmake and color.

This is the code I have thus far:

public static void main (String [] args) throws Exception
{
    String [][] cars = new String [8][8];
    ArrayList <String> colors = new ArrayList<>();
    colors.add("BLUE  ");
    colors.add("BLACK ");
    colors.add("BROWN ");
    colors.add("GREEN ");
    colors.add("RED   ");
    colors.add("SILVER");
    colors.add("WHITE ");
    Collections.sort(colors);
    ArrayList <String> models = new ArrayList<>();
    models.add("Escape  ");
    models.add("Explorer");
    models.add("F150    ");
    models.add("F250    ");
    models.add("Flex    ");
    models.add("Mustang ");
    models.add("Taurus  ");
    Collections.sort(models);
    cars [0][0] = "_____  ";
    for (int a = 1; a < cars.length; a++)
    {
        cars[0][a] = (models.get(a-1)) + " ";
    }
    for (int b = 1; b < cars.length; b++)
    {
        cars[b][0] = (colors.get(b-1)) + " ";
    }
    for (int fir = 1; fir < cars.length; fir++)
    {
        for (int sec = 1; sec < cars[1].length; sec++)
        {
            if (cars[fir][sec] == null)
            {
                cars[fir][sec] = "0        ";
            }
        }
    }
    for (int first = 0; first < cars.length; first++)
    {
        for (int second = 0; second < cars[first].length; second++)
        {
            System.out.print(cars[first][second]);
        }
        System.out.println();
    }
    File file = new File ("C:\\Users\\delta\\Documents\\NetBeansProjects\\SchoolWork\\cars.txt");
    Scanner sc = new Scanner(file);
    String ab = sc.nextLine();
        while (ab != null)
        {
            String [] nums = sc.nextLine().split(" ");
            for (int i = 0; i < nums.length;i++)
            {
               System.out.println(nums[i]);
            }
        }
}

The output is supposed to look something like this:

Consider using Arrays.asList("", ...) to create lists in-line.
Why are colors and models Lists while cars is an Array? It seems like you can use Arrays for all of this.
Consider using "anonymous arrays" (terrible name) to create Arrays in-line.
Consider first building the "true" data structure as int[][] car_counts = new int[7][7], a zero-indexed 2D array of counts. Then treat the task of printing it as shown in the image as a separate problem.
Your while loop at the end appears to be incorrectly indented.
That while loop should be building a data-structure that you can neatly read the file into: a list of pairs of strings. (or even a list of pairs of ints, representing indexes into models and colors.)


File selection button for Jupyter notebook Background

This is a feature that I have wanted to see in Jupyter notebook for quite some time. I'm sure that others have built similar things but I haven't been able to find them on SO, Github, CodeReview or the rest of the searchable internet.

Approach

I used ipywidgets.Button as a base class and added a traitlet called files. This traitlet receives a list of files as strings from tkinter.filedialog.askopenfilename. When the button is clicked the file dialog pops up the user selects files and then can access that list of strings as an attribute/traitlet on the button instance.

import traitlets
from ipywidgets import widgets
from IPython.display import display
from tkinter import Tk, filedialog

class SelectFilesButton(widgets.Button):
    """A file widget that leverages tkinter.filedialog."""

    def __init__(self):
        super(SelectFilesButton, self).__init__()
        # Add the selected_files trait
        self.add_traits(files=traitlets.traitlets.List())
        # Create the button.
        self.description = "Select Files"
        self.icon = "square-o"
        self.style.button_color = "orange"
        # Set on click behavior.
        self.on_click(self.select_files)

    @staticmethod
    def select_files(b):
        """Generate instance of tkinter.filedialog.

        Parameters
        ----------
        b : obj:
            An instance of ipywidgets.widgets.Button
        """
        # Create Tk root
        root = Tk()
        # Hide the main window
        root.withdraw()
        # Raise the root to the top of all windows.
        root.call('wm', 'attributes', '.', '-topmost', True)
        # List of selected fileswill be set to b.value
        b.files = filedialog.askopenfilename(multiple=True)

        b.description = "Files Selected"
        b.icon = "check-square-o"
        b.style.button_color = "lightgreen"

Example Usage

Displaying the button

my_button = SelectFilesButton()
my_button # This will display the button in the context of Jupyter Notebook

Retrieving the file list from the button.

# In a different cell of the same Jupyter Notebook You can access the file list by using the following:
my_button.files

Questions

These are just some suggestions.

1) Does this work on your machine? If so throw me an upvote! If not please specify what versions you are working with so I can extend.

2) Are there any general overall improvements that can be made?

3) Is there any redundancy that I missed?

4) Does this code follow best practices?

It is a nice piece of code. I have tried it in both jupyter notebook and jupyter lab. It works quite well in the notebook, but it does not work that well in lab.

In jupyter lab, it shows the button, but does not execute anything when you push it. I have adapted your code to be able to run it in lab.

Code:

import traitlets
from ipywidgets import widgets
from IPython.display import display
from tkinter import Tk, filedialog

class SelectFilesButton(widgets.Button):
    """A file widget that leverages tkinter.filedialog."""

    def __init__(self):
        super(SelectFilesButton, self).__init__()
        # Add the selected_files trait
        self.add_traits(files=traitlets.traitlets.List())
        # Create the button.
        self.description = "Select Files"
        self.icon = "square-o"
        self.style.button_color = "orange"
        # Set on click behavior.
        self.on_click(self.select_files)

    @staticmethod
    def select_files(b):
        """Generate instance of tkinter.filedialog.

        Parameters
        ----------
        b : obj:
            An instance of ipywidgets.widgets.Button
        """
        with out:
            try:
                # Create Tk root
                root = Tk()
                # Hide the main window
                root.withdraw()
                # Raise the root to the top of all windows.
                root.call('wm', 'attributes', '.', '-topmost', True)
                # List of selected fileswill be set to b.value
                b.files = filedialog.askopenfilename(multiple=True)

                b.description = "Files Selected"
                b.icon = "check-square-o"
                b.style.button_color = "lightgreen"
            except:
                pass
out = widgets.Output()
raw = SelectFilesButton()
widgets.VBox([raw, out])


Rate limiting function calls I wanted to write a function that when applied to another would rate limit it, but permit all calls to eventually make it through.

Comments and criticism welcome.

var MAX_RUNS_PER_WINDOW = 10;
var RUN_WINDOW = 1000;

function limit(fn) {
    var callQueue = [],
      invokeTimes = Object.create(circularQueue),
      waitId = null;

    function limited() {
        callQueue.push(() => {
            invokeTimes.unshift(performance.now())
            fn.apply(this, arguments);
        });

        if (mayProceed()) {
            return dequeue();
        }

        if (waitId === null) {
            waitId = setTimeout(dequeue, timeToWait());
        }
    }

    limited.cancel = function() {
      clearTimeout(waitId);
    };

    return limited;

    function dequeue() {
        waitId = null ;
        clearTimeout(waitId);
        callQueue.shift()();

        if (mayProceed()) {
            return dequeue();
        }

        if (callQueue.length) {
            waitId = setTimeout(dequeue, timeToWait());
        }
    }

    function mayProceed() {
        return callQueue.length && (timeForMaxRuns() >= RUN_WINDOW);
    }

    function timeToWait() {
        var ttw = RUN_WINDOW - timeForMaxRuns();
        return ttw < 0 ? 0 : ttw;
    }

    function timeForMaxRuns() {
        return (performance.now() - (invokeTimes[MAX_RUNS_PER_WINDOW - 1] || 0));
    }
}

var circularQueue = [];
var originalUnshift = circularQueue.unshift;

circularQueue.MAX_LENGTH = MAX_RUNS_PER_WINDOW;

circularQueue.unshift = function(element) {
    if (this.length === this.MAX_LENGTH) {
        this.pop();
    }
    return originalUnshift.call(this, element);
}

var printLetter = limit(function(letter) {
    document.write(letter);
});

['A', 'B', 'C', 'D', 'E', 'F', 'G',
'H', 'I', 'J', 'K', 'L', 'M', 'N',
'O', 'P', 'Q', 'R', 'S', 'T', 'U',
'V', 'X', 'Y', 'Z'].forEach(printLetter);

Not bad. Here are some suggestions:

var circularQueue = [];
var originalUnshift = circularQueue.unshift;
circularQueue.unshift = function(element) {
    if (this.length === this.MAX_LENGTH) {
        this.pop();
    }
    return originalUnshift.call(this, element);
}

I think overriding the unshift function here is an overkill here, as it's used only once. You can use a regular array in limit and define an unshift function there. Also, circularQueue won't be visible outside limit this way.

waitId = null ;
clearTimeout(waitId);

You're calling clearTimeout with null as an argument, which has no effect. You don't need to clear the timeout here anyway, it has already expired (you're inside the callback it has triggered).

function dequeue() {
    callQueue.shift()();

    if (mayProceed()) {
        return dequeue();
    }

    if (callQueue.length) {
        waitId = setTimeout(dequeue, timeToWait());
    }
}

There's a hidden do-while loop here. Since mayProceed() is known to be true when dequeue is called, this can be made a while loop:

while (mayProceed()) {
    callQueue.shift()();
}

The relation between the mayProceed() condition and the callQueue.length condition isn't obvious, which makes it hard to understand this function. It'll be easier to understand if it's obvious that the first condition is the-second-condition-and-something-else. This can be accomplished after introducing some helper functions:

function dequeue() {
    while (callsPending() && canCallNow()) {
        callQueue.shift()();
    }

    if (callsPending()) {
        waitId = setTimeout(dequeue, timeToWait());
    }
}

function limited() {
    callQueue.push(() => {
        invokeTimes.unshift(performance.now())
        fn.apply(this, arguments);
    });

It'll be simpler to just store the arguments lists in an array, instead of having an array of anonymous functions. You can update invokeTimes when you shift the callQueue in dequeue.

    if (mayProceed()) {
        return dequeue();
    }

    if (waitId === null) {
        waitId = setTimeout(dequeue, timeToWait());
    }
}

This logic is implemented in dequeue now. You can just call it.

function timeToWait() {
    var ttw = RUN_WINDOW - timeForMaxRuns();
    return ttw < 0 ? 0 : ttw;
}

The ternary expression can be replaced with Math.max(0, ttw).

limited.cancel = function() {
  clearTimeout(waitId);
};

This cancels scheduled calls to the function, but they will be performed after calling the function again. Is this intended?

The code of limit after applying these suggestions:

function limit(fn) {
    var argumentLists = [],
      invokeTimes = [],
      waitId = null;

    function dequeue() {
        while (callsPending() && canCallNow()) {
            logInvocation(performance.now());
            fn.apply(this, argumentLists.shift());
        }

        if (callsPending()) {
            waitId = setTimeout(dequeue, timeToWait());
        }
    }

    function limited() {
        argumentLists.push(arguments);
        dequeue();
    }

    limited.cancel = function() {
      clearTimeout(waitId);
    };

    return limited;

    function logInvocation(time) {
        if (invokeTimes.length === MAX_RUNS_PER_WINDOW) {
            invokeTimes.pop();
        }
        return invokeTimes.unshift(time);
    }

    function callsPending() {
        return argumentLists.length > 1;
    }

    function canCallNow() {
        return timeToWait() === 0;
    }

    function timeToWait() {
        var now = performance.now();
        var timeOfInvocation = invokeTimes[MAX_RUNS_PER_WINDOW - 1] || 0;
        var ttw = timeOfInvocation + RUN_WINDOW - now;
        return Math.max(0, ttw);
    }
}

It would be nice to make MAX_RUNS_PER_WINDOW and RUN_WINDOW arguments of limit instead of constants.


Take element of a Set collection depending on the value of another element I had to populate the fields of a Service object using a Set (HashSet) with any configuration parameters -> ConfigurationMap (Class with 2 attributes: key and value).
The problem is that the attribute quantity of Service object can be set from two distinct parameters ("service_quantity" or "publications_amount") based on the value of another parameter("type").
I've gotten it to work with the code below, but I think it's pretty messy and inefficient.
Is there any way to make it more readable considering that the methods signatures are invariable?
@Override
protected void populateServiceConfigurationData(Service theService, Set<ConfigurationMap> configurationParams) {
    boolean publishService = false;
    for (ConfigurationMap configParam : configurationParams) {
        String paramValue = configParam.getValue();
        String paramKey = configParam.getKey();
        setFieldValue(theService, paramKey, paramValue);
        if("type".equals(paramKey) && paramValue.equals("20")) {
            publishService = true;
        }
    }

    if(publishService) {
        for (ConfigurationMap configParam : configurationParams) {
            String paramKey = configParam.getKey();
            if(paramKey.equals("publications_amount")) {
                String paramValue = configParam.getValue();
                setFieldValue(theService, paramKey, paramValue);
            }
        }
    }
}

@Override
public void setFieldValue(Service service, String paramKey, String paramValue) {
    if ("type".equals(paramKey)) {
        service.setType(Integer.valueOf(paramValue));
    }
    if ("service_quantity".equals(paramKey) || "publications_amount".equals(paramKey)) {
        service.setQuantity(Integer.valueOf(paramValue));
    }
    if ("gateway_id".equals(paramKey)) {
        service.setGatewayId(paramValue);
    }
    if ("comissions".equals(paramKey)) {
        service.setCommissions(paramValue);
    }
}

I think it's pretty messy and inefficient

Let's say that it is getting messy, especially if the number of parameters (and hidden rules) keeps growing.
About being inefficient, I am not sure. Even if the input is very large, the method populateServiceConfigurationData runs in \$O(N)\$, so I don't think it will be a big problem.
My suggestions:

Readability: setting the quantity of the service depends on more than one parameter. So the "rule" to set quantity would be more evident if included in a single function, like setServiceQuantity.
Performance: the input set can be converted to a map, to easily access the parameters. The performance gain is minimal, but it should help to make the code more clear.
Design: using setFieldValue in populateServiceConfigurationData seems unnecessary. The method populateServiceConfigurationData can be completely independent to setFieldValue. More on this later.

public void populateServiceConfigurationData(Service service, Set<ConfigurationMap> configurationParams) {
    // Convert the input set to a map
    Map<String, String> params = configurationParams.stream()
            .collect(Collectors.toMap(ConfigurationMap::getKey, ConfigurationMap::getValue));

    if (params.containsKey("type")) {
        service.setType(Integer.valueOf(params.get("type")));
    }

    if (params.containsKey("gateway_id")) {
        service.setGatewayId(params.get("gateway_id"));
    }

    if (params.containsKey("comissions")) {
        service.setCommissions(params.get("comissions"));
    }

    setServiceQuantity(service, params);
}

private void setServiceQuantity(Service service, Map<String, String> params){
    boolean isPublishService = params.get("type").equals("20");
    String quantity = isPublishService ? params.get("publications_amount") : params.get("service_quantity");
    service.setQuantity(Integer.valueOf(quantity));
}

(Note: not tested, it's just to give an idea)
Now the logic for setting each property of the service is clear and can be easily extracted into functions like setServiceQuantity.
One issue is that populateServiceConfigurationData needs to know how to set the properties into the service (for example, parsing type to Integer). If you don't like this approach, the setters can be replaced with setFieldValue (as before) or let service do the parsing in its own methods.
The method setFieldValue looks fine, the only thing I can suggest is to use else if since only one condition will match per invocation.


Set up event on class and initialize state Very similar to this question, I'm curious about common patterns/best practices for handling events in coffeescript classes, particularly when you also need to call the event handlers immediately to initialize state.

Unlike the answer to the question above though, let's assume though that you do need/want a class.

Here's a contrived example:

class @Foo
  constructor ->
    @bar = new Bar()

    @checkbox = $('#my-checkbox')

    # set up the events
    @checkbox.click =>
      @bar.clickHandler(@checkbox)
      @clickHandler(@checkbox)

    # initialize state
    @bar.clickHandler(@checkbox)
    @clickHandler(@checkbox)

  clickHandler: ($elt) ->
    if $elt.is(':checked')
      # something
    else
      # something else

$ ->
  window.foo = Foo.new

Is there a usual pattern for cases like this where you want the event function called both when the event fires and when the class loads? I really hate the duplication but I'm not sure how to remove it. Any other suggestions?

I'm only vaguely familiar with coffeescript, but it seems like the issue is not that you need to call the event handler on init, it's that you need to call the logic currently residing in your event handler. If you separate your logic into reusable pieces, you can call this other method to execute your init logic. Without the complete code, it's hard to see what that logic is (by the way, incomplete code examples don't bode well for the Code Review site).

Consider something like this example though (sorry, pseudocode not coffeescript):

function init() {
    hideUserInformation();
}

function checkBoxChangeHandler(checkBox) {
    if(checkBox.checked) {
        hideUserInformation();
    } else {
        showUserInformation();
    }
}

function hideUserInformation() {
    // Some logic.
}

function showUserInformation() {
    // Some logic.
}

Now the event handler is only concerned with forwarding to the real logic, and you aren't trying to call an event handler with bogus arguments just to try to run the same code.


Translating words from one language to another My program translates words from one language to another using a language Pivot and systran.io API.

I used promises, callbacks and external API for the first time but I'm not sure that's the best way to use them. The function Translate will translate word and send a response via a callback. Then in the POST request, I used promises to chain tasks.

var express = require('express');
var request = require('request');

var router = express.Router();

router.post("/", function(req, res) {
    console.log
    var resultat
    var promise = new Promise((resolve, reject) => {
        translate(req.query.source, "en", req.query.content, function(resa) {
            resolve(resa);

        })
    }).then(function(resolve) {
        console.log(resolve);
        translate("en", req.query.target, resolve, function(resa2) {
            console.log(resa2);

        })
    });
});

function translate(source, target, content, callback) {
    let result;
    result = request("https://api-platform.systran.net/translation/text/translate?input=" + content + "&source=" + source + "&target=" + target + "&key=xxxxxxxx-783f-4f90-aea4-7fb357016647", function(err, data, body) {
        body = JSON.parse(body);
        console.log(body);

        callback(body.outputs[0].output)
    })
}
module.exports = router;

Is there a better way to write my program, which is already working?

Personally, I would have the translate call return a Promise rather than a callback and make use of async/await (presuming the version of Node you are using Node 6+) e.g.

var express = require('express');
var request = require('request');

var router = express.Router();

router.post("/", async (req, res, next) => {
  try {
    const translated = await translate(req.query.source, 'en', req.query.content);
    const result = await translate('en', req.query.target, translated);
    // presume you intend on returning something from your API?
    return res.json({ result });
  } catch (e) {
    return next(e);
  }
})

const translate = (source, target, content) =>
  new Promise((resolve, reject) =>
    request(`https://api-platform.systran.net/translation/text/translate?input=${content}&source=${source}&target=${target}&key=xxxxxxxx-783f-4f90-aea4-7fb357016647`, (err, data, body) => {
      if (err) return reject(err);

      try {
        return resolve(JSON.parse(body));
      } catch (e) {
        return reject(e);
      }
    })
  );

module.exports = router;


Test cases for an elevator simulator I'm trying to test an elevator simulator program that runs in the console and requires user interaction. At the moment, my tests look like this:

gem 'minitest', '>= 5.0.0'
require 'minitest/spec'
require 'minitest/autorun'
require_relative 'elevator'

class SingleFloorElevator < Elevator
  @@moves = ["2", "N"].each
  def get_input; @@moves.next end
end

class MultiFloorElevator < Elevator
  @@moves = ["2", "Y", "7", "Y", "4", "N"].each
  def get_input; @@moves.next end
end

class InvalidElevetorOne < Elevator
  @@moves = ["-2", "2"].each
  def get_input; @@moves.next end
end

class InvalidElevatorTwo < Elevator
  @@moves = ["2000", "5"].each
  def get_input; @@moves.next end
end

class ElevatorTest < MiniTest::Test
  def test_can_accept_and_move_to_floor
    e = SingleFloorElevator.new
    e.run
    assert_equal(e.current_floor, 2)
  end

  def test_can_change_direction
    e = MultiFloorElevator.new
    e.run
    assert_equal(e.going_down, true)
    assert_equal(e.going_up, false)
  end

  describe "it can maintain a list of floor numbers" do
    it "cannot travel below the ground floor" do
      e = InvalidElevetorOne.new
      e.enter_floor
      assert_equal(e.floors.size, 1)
      assert_equal(e.floors, [2])
    end

    it "cannot travel higher than the top floor" do
      e = InvalidElevatorTwo.new
      e.enter_floor
      assert_equal(e.floors.size, 1)
      assert_equal(e.floors, [5])
    end
  end
end

As you can see, I'm overriding the Elevator class (more important the get_input method which grabs user input from the console) multiple times according to what I need to test. It all works perfectly fine but I was wondering if there was a tidier way of doing things?

I kind of wish you had posted the elevator class as well, but it's kind of interesting how you much you can tell about the CUT (code under test) from looking at the just the tests themselves.

First, your elevator shouldn't be responsible for getting what floor it should go to from the user. Your program should tell it where to go. If you inverted control, you wouldn't have to subclass the code under test in order to test it.

Which brings me to my second point...

If you have to mock the CUT, you're no longer actually testing anything. You can't have confidence that your real class is going to behave as you expect it to, because you've very explicitly changed how it behaves. You're not testing your elevator. You're testing your mock elevators, which is no test at all.

Sure, you are testing the elevator. I know that, but certainly, this class has too many responsibilities. It shouldn't be getting user input from the console. How could you possibly reuse the elevator class in a GUI application later?


Convert letters into numbers (A=1, B=2 etc) class Names:
    def __init__(self,name):
        self.name = name
        self.numname = name
        for letter in name:
            self.numname = re.sub(letter, rosetta[letter] + ",", self.numname)
        self.numname = self.numname.split(',')[:-1]

Essentially what this does is creates an attribute for the object called 'numname' which is his/her name converted into numbers (A = 1, B = 2 This is stored in a dictionary called 'rosetta')  and stores each number as an element in a list. It works but it feels really inefficient. The problems I see here are:

self.numname is called quite a lot. (Maybe it's possible to use list comprehension here?)
re.sub is called per letter. I'd imagine this is unavoidable, however if a name had repeated letters (e.g. Matt). I tried using ''.join(OrderedDict.fromkeys(self.numname) in the third field of re.sub but then that screws up the number separation.

How can I make the creation of this list more concise/more permanent?

You might try something like:

self.numname = [rosetta[x] for x in self.name]

This creates the list of numbers directly from the letters in the name, using a list comprehension.


Haskell's `group` Function I implemented the group function:

group' :: (Ord a) => [a] -> [[a]]
group' [] = []
group' xs = takeWhile (== head' xs) (sorted xs) :
                           (group' $ dropWhile (== head' xs) (sorted xs))
                    where sorted ys   = mergesort' (ys)
                          head' (x:_) = x

Note - please assume mergesort is implemented properly.

Its signature is: [a] -> [a] where the latter list is sorted.

How's it look? Is head''s pattern matching exhaustive? It seems to me it is since group xs will only be reached if group's argument is non-empty, i.e. it'll always have a head.

You have made some uncommon stylistic choices which are not to your benefit, and a few things aren't doing what I think you think they're doing.

First, the stylistic elements. Your whitespace is excessive, there's no need to push everything that far to the right and out of line with the beginning of the RHS. I also would move the colon down to begin the next line, this is mostly a matter of personal style. The parentheses around ys are unnecessary and while harmless noise for the compiler will distract your human reader. My version would look like this.

group' xs = takeWhile (== head' xs) (sorted xs)
          : group' (dropWhile (== head' xs) (sorted xs))
    where sorted ys   = mergesort' ys
          head' (x:_) = x

Now to the meat of it. As it is, your definition of sorted is just giving a different name to mergesort'. The two uses in the main body of the function will cause sorted xs to be recomputed each time (ie twice!). The correct definition is not to create an inner function which is passed a list to be sorted as a parameter, but to bind a sorted version of the outer function's parameter.

group' xs = takeWhile (== x) sorted : group' (dropWhile (== x) sorted)
    where sorted = mergesort' xs -- Computed once, the result is used twice
          x = head sorted

You'll notice that I gave the same treatment to head', this is less of a performance hit and more a matter of consistency. If you wanted to be really pedantic you could bind the section as well, but (and this is an art not a science) I feel that this version is less readable.

group' xs = takeWhile p sorted : ...
    where ...
          p = (== head sorted)

This is still doing more work than it needs though! Once you've made the first recursive call, the tail passed to group' will always already be sorted, and doesn't need to be resorted with each recursive step. One way to handle this would be to pull the main functionality of group' into an inner function named something like group'' and change the body of group' to just be group'' (mergesort' xs). Now the list is only sorted once.

I think it makes more sense though to separate the two functions entirely. There is in fact no requirement that the list being grouped is sorted, so let's relax that constraint.

group' :: (Eq a) => [a] -> [[a]]
group' []     = []
group' (x:xs) = (x : takeWhile (== x) xs) : group' (dropWhile (== x) xs)

elementSets :: (Ord a) => [a] -> [[a]] -- Same functionality as original group'
elementSets = group' . mergeSort'

And just so you see what's happening,

> group' [2,2,1,2,2,2]
[[2,2],[1],[2,2,2]]
> elementSets [2,2,1,2,2,2]
[[1],[2,2,2,2,2]]

Note that we can now find the head of the list through pattern matching instead of calling head because we don't need to assume the list may be out of order. We do however have to remember to add that element back to the prefix of what we're returning!

There's one more improvement we can make though! As it is, our group' function will walk the prefix of the list equal to the first element twice each recursive step (once for takeWhile, and once for dropWhile). We can eliminate that inefficiency by using span which returns the prefix matching the predicate and the remainder in one go.

group' (x:xs) = (x : prefix) : group' remainder
    where (prefix, remainder) = span (== x) xs

And there we have it.

Regarding your question about whether head''s pattern matching was exhaustive, the answer is that it is not exhaustive because the definition of head' contained no case which would match every possible pattern (ie head' []). It was however "safe" to use because of how group' was defined, and the case of the empty list was handled through pattern matching at the outer level. In general you shouldn't rely on tricky business like that, it can be difficult to notice when coming back to code you or someone else has written, and can blow up on you if you have to change the definition of the function.


Removing duplication in AngularJS Factory I have multiple API calls so I created a service however I would like to find a way to remove the duplication

myAppServices.factory('apiService', function ($http, $q) {
    return {
        getDocuments: function () {
            return $http.get('/api/autodeskdocuments/1')
                .then(function (response) {
                    if (typeof response.data == 'object') {
                        return response.data;
                    } else {
                        // invalid response
                        return $q.reject(response.data);
                    }
                }, function (response) {
                    // something went wrong
                    return $q.reject(response.data);
            });
        },

        getSharePoint: function () {
            return $http.get('/api/sharepointheaderdata/1')
                .then(function (response) {
                    if (typeof response.data == 'object') {
                        return response.data;
                    } else {
                        // invalid response
                        return $q.reject(response.data);
                    }
                }, function (response) {
                    // something went wrong
                    return $q.reject(response.data);
                });
        }
    };
});

As you can see both functions has some duplication with the .then(...) Any suggestions as to make this cleaner?

You can extract the whole ajax to a function.

myAppServices.factory('apiService', function ($http, $q) {
    var get = function(endpoint) {
        return $http.get(endpoint)
                .then(function (response) {
                    if (typeof response.data == 'object') {
                        return response.data;
                    } else {
                        // invalid response
                        return $q.reject(response.data);
                    }
                }, function (response) {
                    // something went wrong
                    return $q.reject(response.data);
            });
    }
    return {
        getDocuments: function () {
            return get('/api/autodeskdocuments/1');
        },

        getSharePoint: function () {
            return get('/api/sharepointheaderdata/1');
        }
    };
});


Possible improvements to Risk board game? Enter risk!
This program will roll dice for the game of Risk. The initial input is two numbers separated by a space and if the attackers wish to do a blitz they can add one more space and an "!" to get a positive attack modifier but are committed for an all or nothing fight.
package riskdieroll;
import java.awt.Component;
import java.util.Arrays;
import java.util.Collections;
import javax.swing.JOptionPane;
public class RiskDieRoll
{
    private static Component frame;
    public static void main(String[] args)
    {
        int spcCnt, atck1, dfnc1, rollCnt, atck2 = 0, dfnc2 = 0, valHold, atckMod;
        String intInpt, Lsr;
        while(true)
        {
            boolean k = false;
            Lsr = "";
            intInpt = JOptionPane.showInputDialog("Please enter the a"
                    + "mount of attackers then the number of\ndefenders separa"
                    + "ted by a space.", atck2 + " " + dfnc2);
            spcCnt = rollCnt = 0;
            valHold = intInpt.length();
            atckMod = 6;
            if("0 0".equals(intInpt))
            {
                break;
            }
            for(int i = 0; intInpt.charAt(i) != ' '; i++)
            {
                spcCnt++;
            }
            atck2 = atck1 = (int)Double.parseDouble(intInpt.substring(0 , spcCnt));
            if(intInpt.charAt(valHold - 1) == '!')
            {
                k = true;
                atckMod = 7;
                valHold -= 2;
            }
            dfnc2 = dfnc1 = (int)Double.parseDouble(intInpt.substring(spcCnt + 1 , valHold));
            Integer[] z = new Integer[3], y = new Integer[2];
            if(atck1 < 2)
            {
                JOptionPane.showMessageDialog(frame, "INVALID ATTACK! You need"
                        + " at least two armys to make an attack!");
                atck2 = dfnc2 = 0;
            }
            else if(dfnc1 < 1)
            {
                JOptionPane.showMessageDialog(frame, "INVALID INPUT! There must"
                        + " be at least one defending army!");
                atck2 = dfnc2 = 0;
            }
            else
            {
                do
                {
                    rollCnt = 0;
                    for(int i = 0; i < 3; i++)
                    {
                        z[i] = (int)Math.ceil(Math.random() * atckMod);
                    }
                    for(int i = 0; i < 2; i++)
                    {
                        y[i] = (int)Math.ceil(Math.random() * 6);
                    }
                    Arrays.sort(z, Collections.reverseOrder());
                    Arrays.sort(y, Collections.reverseOrder());
                    while(dfnc2 > 0 && rollCnt < 2 && atck2 > 1)
                    {
                        if(y[rollCnt] >= z[rollCnt])
                        {
                            atck2--;
                        }
                        else
                        {
                            dfnc2--;
                        }
                        rollCnt++;
                    }
                }while(k && (dfnc2 > 0 && atck2 > 1));
                if(dfnc2 < 1)
                {
                    Lsr = "\n\nDefenders have lost!!";
                }
                else if(atck2 < 2)
                {
                    Lsr = "\n\nAttackers have been repelled!!!";
                }
                JOptionPane.showMessageDialog(frame, "Attacking force now at "
                        + atck2 + " (Lost " + (atck1 - atck2) + ")" + "\nDefence force now "
                        + "at " + dfnc2 + " (Lost " + (dfnc1 - dfnc2) + ")" + Lsr);
                if(!"".equals(Lsr))
                {
                    atck2 = dfnc2 = 0;
                }
            }
        }
    }
}

This code turned out to be a great exercise in refactoring, primarily using

Extract Method
Reduce Scope of Variable
Extract Class
Introduce Explaining Variable
Self Encapsulate Field

Here's my goal:

public class RiskDieRoller
{
    //////////////////////////////////////////////////////////////////////

    private static class Strength
    {
        public int attack;
        public int defence;
        public int attackMod = 6;

        public Strength() {}

        public Strength(Strength other)
        {
            this.attack = other.attack;
            this.defence = other.defence;
            this.attackMod = other.attackMod;
        }

        public void blitz()
        {
            this.attackMod = 7;
        }

        public boolean isBlitz()
        {
            return this.attackMod == 7;
        }

        public String toString()
        {
            return "Attack " + this.attack +
                   " Defence " + this.defence +
                   " AttackMod " + this.attackMod;
        }
    }

    //////////////////////////////////////////////////////////////////////

    // …

    public static void main(String[] args)
    {
        RiskDieRoller roller = new RiskDieRoller();

        Strength before = new Strength();
        while (null != (before = roller.prompt(before)))
        {
            Strength after = roller.play(before);
            boolean isDecisive = roller.report(before, after);
            before = isDecisive ? new Strength() : after;
        }
    }
}

I've renamed the class to RiskDieRoller and introduced a Strength class to help tame the proliferation of variables.  The main() function just outlines how the program flows.

It's now just a simple matter of filling in the blanks. ☺

Prompt

You failed to handle the "Cancel" button — a NullPointerException occurs.  You handle "0 0" as a special case, but what if there is gratuitous extra whitespace?  Also, if the exclamation mark is not preceded by a space, then Double.parseDouble() barfs.  Anyway, you should be calling Integer.parseInt() instead.

In any case, analyzing the string character by character is tedious, error prone, and unforgiving of variations in the user-provided input.   Just use a regular expression.

private static final Pattern INPUT_RE =
    Pattern.compile("^\\s*(\\d+)\\s+(\\d+)\\s*(!)?\\s*$");

public Strength prompt(Strength before)
{
    do
    {
        String input = JOptionPane.showInputDialog("Please enter the "
                + "number of attackers then the number of\ndefenders "
                + "separated by a space.",
                before.attack + " " + before.defence);
        if (null == input)
        {
            return null;
        }

        Matcher matcher = INPUT_RE.matcher(input);
        if (matcher.matches())
        {
            Strength s = new Strength();
            s.attack = Integer.parseInt(matcher.group(1));
            s.defence = Integer.parseInt(matcher.group(2));
            if (null != matcher.group(3))
            {
                s.blitz();
            }

            if (s.attack == 0 && s.defence == 0)
            {
                return null;
            }
            if (s.attack <= 1)
            {
                output("INVALID ATTACK! You need"
                        + " at least two armies to make an attack!");
            }
            else if (s.defence <= 0)
            {
                output("INVALID INPUT! There must"
                        + " be at least one defending army!");
            }
            else
            {
                return s;
            }
        }
    } while (true); // Repeat until input passes validation
}

private void output(String message)
{
    JOptionPane.showMessageDialog(null, message);
}

It's not necessary to have a null Component instance variable; you can just pass a literal null to .showMessageDialog().

Play

This part of the code is more or less the same as before, but with nicer variable names.  I've changed (int)Math.ceil(Math.random() * …) to Random.nextInt().

private Random random = new Random();

public Strength play(Strength before)
{
    Strength s = new Strength(before); // Return a copy
    Integer[] attackRolls = new Integer[3],
              defenceRolls = new Integer[2];

    do
    {
        for (int i = 0; i < 3; i++)
        {
            attackRolls[i] = 1 + random.nextInt(s.attackMod);
        }
        for (int i = 0; i < 2; i++)
        {
            defenceRolls[i] = 1 + random.nextInt(6);
        }
        Arrays.sort(attackRolls, Collections.reverseOrder());
        Arrays.sort(defenceRolls, Collections.reverseOrder());

        for (int rollCnt = 0; rollCnt < 2; rollCnt++)
        {
            if (s.defence <= 0 || s.attack <= 1)
            {
                return s;
            }
            if (defenceRolls[rollCnt] >= attackRolls[rollCnt])
            {
                s.attack--;
            }
            else
            {
                s.defence--;
            }
        }
    } while (s.isBlitz())
    return s;
}

Strictly speaking, you don't have to add 1 to .nextInt(), but it's more human-friendly to do so.

Report

Again, the main difference is in the management of variables.  This function now has to return true if the battle was decisive.  Arguably, that violates the single-responsibility principle, and could be improved further.

In your original code, you checked for dfnc2 > 0 to continue the battle and dfnc2 < 1 to report annihilation of the defenders.  I've changed this to s.defence <= 0 to check for termination of the battle and after.defence <= 0 to report annihilation, so that the similarity is apparent.

I think String.format() is more readable than concatenation.

public boolean report(Strength before, Strength after)
{
    String verdict = (after.defence <= 0) ?
                        "\n\nDefenders have lost!!" :
                     (after.attack <= 1)  ?
                        "\n\nAttackers have been repelled!!!" : "";

    output(String.format("Attacking force now at %d (Lost %d)\n" +
                         "Defence force now at %d (Lost %d)" +
                         "%s",
                         after.attack,
                         before.attack - after.attack,
                         after.defence,
                         before.defence - after.defence,
                         verdict));

    return !verdict.isEmpty();
}

Nitpicks

"armys" → "armies"
"a" + "mount of attackers" → "number of attackers" (because it's a countable noun)


Randomly generate a Hex Colour code and its contrasting colour using Python I am attempting to generate a new colour and its contrasting colour with this python script.

I plan to try to make a web app that is different every time it is loaded (i.e. background and text colour)

I am wondering if there is a better and more efficient way to do this?

Here is my code:

import random

values = ['0','1','2','3','4','5','6','7','8','9','10','11','12','13','14','15']
value_dict = [['A','10'],['B','11'],['C','12'],['D','13'],['E','14'],['F','15']]
start = "#"

def contrastcolour(colour):
    if colour[0] == '#':
        colour = colour[1:]
    rgb = (colour[0:2], colour[2:4], colour[4:6])
    comp = ['%02X' % (255 - int(a, 16)) for a in rgb]
    return ''.join(comp)

def startcolour():
    colour = "#"
    for i in range(6):
        x = values[random.randint(0,len(values)-1)]

        for thing in value_dict:
            if x == thing[1]:
                x = thing[0]
        colour = colour + x
    return colour

base = startcolour()
contrast = start + contrastcolour(base)

print("The colours: {0}".format([base, contrast]))

The sample output is:

The colours: ['#2EF7F3', '#D1080C']

Review

Globals should be ALL_CAPS

start

=> START
Accumulation: Loop variables that are not explicitly used within the loop block should be replaced by _ , as is standard Python style.

for i in range(6):

You don't use the i later on, so the convention is to use _ for a dummy variable.

=> for _ in range(6)
The name of the variable value_dict implies it's a dict (Python's built-in dictionary type) but it's implemented as a list.

value_dict = [['A','10'],['B','11'],['C','12'],['D','13'],['E','14'],['F','15']]
...
for thing in value_dict:
    if x == thing[1]:
        x = thing[0]

You could change this to use an actual dict like this:

val_dict = {'10' : 'A', '11' : 'B', '12' : 'C', '13' : 'D', '14' : 'E', '15' : 'F'}
...

if x in val_dict:
    x = val_dict[x]

Or better yet why not immediately convert to an hexadecimal using hex?
Check your naming. Don't be overly generic.

values #
thing  # Each of these could literally be anything
x      #

colour = colour + x
can be rewritten as colour += x
Use spaces around the comma: a,b => a, b
Use a if __name__ == '__main__' guard

Essentially your startcolour() can be rewritten as a one-liner

from random import randint
_HEX = list('0123456789ABCDEF')
...
def startcolour():
    return '#' + ''.join(_HEX[randint(0, len(_HEX)-1)] for _ in range(6))


Python infinite product generator I have the code below to get an infinite generator of the products of an iterable (e.g. for the iterable "ABC" it should return

A, B, C, AA, AB, AC, BA, BB, BC, CA, CB, CC, AAA, AAB, AAC etc.

for product in (itertools.product("ABC", repeat=i) for i in itertools.count(0)):
    for each_tuple in product:
        print(each_tuple)

How would I remove the nested for loop, or is there a better approach?

Well, you effectively have 3 nested loops including the generator comprehension, which you can reduce to two just by simplifying:

def get_products(string):
    for i in itertools.count(0):
        for product in itertools.product(string, repeat=i):
            yield product

Or if you are intent on putting it in a single line, use chain:

products_generator = itertools.chain.from_iterable(itertools.product("ABC", repeat=i)
                                                   for i in itertools.count(0))


Can I return an object on which a function is applied? I'm wondering if it is possible or if it is correct to proceed in such a way:

var initiate_shop = function(shop_navigation, shop_container_id, items_per_page) {
    return $(shop_navigation).jPages({
        containerID     : shop_container_id,
        first           : false,
        previous        : false,
        next            : false,
        last            : false,
        midRange        : 15,
        perPage         : items_per_page,
        animation       : "fadeInUp",
        links           : "blank",
        keyBrowse       : true,
        scrollBrowse    : false,
        callback        : function(pages){
            current_page = pages.current;
            total_pages = pages.count;
        }
    });
};

If the above isn't correct what could be a better way to write it?

Looks good to me, if you always want those 3 params and never want to send anything else.

Otherwise, you could improve it by receiving the non-mandatory params in an options object:

"use strict";
var initiate_shop = function(container_id, options) {
    var default_settings = {
            containerID     : container_id,
            first           : false,
            perPage         : 5,
            '...'           : '...',
            callback        : function(pages){
                current_page = pages.current;
                total_pages = pages.count;
            }
        },
        settings = $.extend(default_settings, options);
    return $(shop_navigation).jPages(settings);
};
// ...
initiate_shop('outputCell', {
    perPage:     10,
    first:       true
});

Related (hows 3 ways to assign default values to parameters):
https://stackoverflow.com/a/3672142/148412


Extension pattern in a flask controller using importlib 2.0 The first edition of the code can be found in this 1.0 version.

To summarize the problem :

I have an endpoint in flask where the work logic change according to the query
I am trying to implement an extension mechanism where the right class is loaded
The first version did not provide caching of the imported class

Based on the answer from @netme, this is a code update where I lazy load the right handler class and cache it :

./run.py:

from flask import Flask, abort
import handler

app = Flask(__name__)

handler_collection = handler.HandlerCollection()

@app.route('/')
def api_endpoint():
    try:
        endpoint = "simple" # Custom logic to choose the right handler
        handler_instance = handler_collection.getInstance(endpoint)
    except handler.UnknownEndpoint as e:
        abort(404)

    print(handler_instance, handler_instance.name)
    # Handler processing. Not yet implemented

    return "Hello World"

if __name__ == "__main__":
    app.run(host='0.0.0.0', port=8080, debug=True)

./handler.py:

# -*- coding: utf-8 -*-
import importlib

import handlers

class UnknownEndpoint(Exception):
    pass

class HandlerCollection:
    _endpoints_classes = {}

    def addClass(self, endpoint_class):
        self._endpoints_classes[endpoint_class.name] = endpoint_class

    def getClass(self, endpoint_name):
        if (endpoint_name in self._endpoints_classes):
            return self._endpoints_classes.get(endpoint_name)

        try:
            # Try to import endpoint handler from a module in handlers package
            endpoint_module = importlib.import_module(
                '.{}'.format(str(endpoint_name)),
                'handlers')
            endpoint_module.register(self)
        except ImportError as e:
            raise UnknownEndpoint('Unknown endpoint \'{}\''.format(endpoint_name)) from e

        return self._endpoints_classes.get(endpoint_name)

    def getInstance(self, endpoint_name):
        endpoint_class = self.getClass(endpoint_name)
        return endpoint_class()

./handlers/simple.py:

# -*- coding: utf-8 -*-
class SimpleWebhookHandler:
    name = "simple"

def register(handler_collection):
    handler_collection.addClass(SimpleWebhookHandler)

Each handler needs to provide a register function which allows to have different handler class name and each handler class needs to provide a class attribute name.

Can you give me your opinion on this piece of code ? Is it "pythonic" ?

Can you give me your opinion on this piece of code ? Is it "pythonic" ?

I don't really see anything non-pythonic about it.
Some minor coding style issues though:

Minor PEP8 violations:

put 2 blank lines before class and top-level function definitions
use snake_case for function names instead of camelCase

Unnecessary parentheses in if (endpoint_name in self._endpoints_classes):

A bigger issue I see is that the module loading is convoluted and fragile:

HandlerCollection.getClass tries to import a module : violates the single responsibility principle, it would be better to move this logic somewhere else
The imported module is expected to have a register function : a semantic rule, not obvious enough from the code itself. There are no docstrings either, so as it is, implementors have to dig this piece of information out of the current implementation details
The register function must pass a class that has a name property defined : semantic rule, see the previous item
The logical flow of HandlerCollection.getClass is convoluted:

try to import module if missing
pass self to module.register
module.register calls me.addClass

I suggest to move the module loading part out to a different method, perhaps HandlerRegistry.load_and_register_if_missing, and let getClass behave as a simple cache.