Creating a Tkinter window that generates seven numbers from 1 - 49 and sorts them I would like to critical advice on my code:

#-----import modules-----#
from tkinter import *
from turtle import *
from random import sample , seed

#-----Setup-----#
win = Tk() # ...generates a window...
win.title("Lotto Number Generator")
#win.geometry("500x500")
win.resizable(False,False)

#-----Widget Declaration-----# ...With Labels and Buttons
Labl1 = Label(win , relief = 'groove' , width = 2)
Labl2 = Label(win , relief = 'groove' , width = 2)
Labl3 = Label(win , relief = 'groove' , width = 2)
Labl4 = Label(win , relief = 'groove' , width = 2)
Labl5 = Label(win , relief = 'groove' , width = 2)
Labl6 = Label(win , relief = 'groove' , width = 2)
LablX = Label(win , relief = 'groove' , width = 2) #This is a lucky number.
ResetBtn = Button(win , text = "Reset")
PickBtn = Button(win , text = "Pick My Lucky Numbers")

#-----Position Widgets-----#
Labl1.grid(row = 1 , column = 1 , padx = 10)
Labl2.grid(row = 1 , column = 2 , padx = 10)
Labl3.grid(row = 1 , column = 3 , padx = 10)
Labl4.grid(row = 1 , column = 4 , padx = 10)
Labl5.grid(row = 1 , column = 5 , padx = 10)
Labl6.grid(row = 1 , column = 6 , padx = 10)
LablX.grid(row = 1 , column = 7 , padx = 20)

ResetBtn.grid(row = 2 , column = 6 , columnspan = 2)
PickBtn.grid(row = 2 , column = 1 , columnspan = 5)

#-----Functions-----#
def reset(): ...That resets the labels...
    Labl1.configure(text='...')
    Labl2.configure(text='...')
    Labl3.configure(text='...')
    Labl4.configure(text='...')
    Labl5.configure(text='...')
    Labl6.configure(text='...')
    LablX.configure(text='...')
    PickBtn.configure(state = NORMAL)
    ResetBtn.configure(state = DISABLED)

def pick(): #...and can pick random numbers WITH a bonus number
    picks = sample(range(1,49) , 7)
    LablX.configure(text=picks[6])
    del picks[6]
    picks.sort()
    Labl1.configure(text=picks[0])
    Labl2.configure(text=picks[1])
    Labl3.configure(text=picks[2])
    Labl4.configure(text=picks[3])
    Labl5.configure(text=picks[4])
    Labl6.configure(text=picks[5])
    PickBtn.configure(state = DISABLED)
    ResetBtn.configure(state = NORMAL)

#-----Assign Functions-----#
ResetBtn.configure(command = reset)
PickBtn.configure(command = pick)

#-----Initialise-----#
reset()

win.mainloop()

Don't use wildcard imports

Instead of doing from tkinter import *, use import tkinter as tk and then prefix your use of tkinter objects with tk. (eg: tk.Tk(), tk.Label(...), etc)

Use an array to keep track of multiple identical widgets.

Whenever you have a number as part of a variable name, that's a code smell telling you there's probably a better way. Instead of Lab1, Lab2, etc, put the labels in a  list:

labels = []
for column in range(7):
    label = tk.Label(win , relief = 'groove' , width = 2)
    label.grid(row = 1 , column = column+1 , padx = 10
    labels.append(label)

Later, when you need to reconfigure the widget you can use the global labels like so:

for i in range(5):
    labels[i].configure(text=picks[i])

Follow PEP8

You should follow the naming conventions of PEP8. Specifically, don't use an uppercase character as the first letter of a variable name.

Don't turn off the ability of the user to resize the window

Don't do win.resizable(False,False). Users should be able to control their window size.

Use sticky appropriately

When using grid, you should almost always explicitly set the sticky attribute. It's rate to need to put something in a grid cell and not have it expand or contract to fit that cell.

Set row and column weights

When using grid, you should always give at least one row and one column a non-zero weight. If you don't want your rows and columns to grow and shrink when the user resizes the window, give the weight to an empty column to the right of all other columns, and to the row below all other rows. This will explicitly tell tkinter to allocate extra space to the edges.

Don't include unnecessary imports

It doesn't look like you're using the turtle module, so remove the import.


Creating a dictionary of strings and list of strings I have a List of objects, List<myObject> objectList, and each object contains a List of strings like so:

myObject: StringList( "A_1", "B_1", "C_1", "D_1", "E_1", "F_1" )
myObject: StringList( "A_2", "B_2", "C_1", "D_2", "E_2", "F_1" )
myObject: StringList( "A_2", "B_3", "C_1", "D_3", "E_2", "F_1" )

I'm trying to merge the lists into a dictionary of: Dictionary<string, List<string>>, where the final results will look like so:

Dictionary: { [A:1,2]  [B:1,2,3]  [C:1]  [D:1,2,3]  [E:1,2]  [F:1] }

Here is what I have done, and it does work:

Dictionary<string, List<string>> dict = new Dictionary<string, List<string>>();

foreach (myObject result in objectList)
{
    foreach (var item in result.StringList)
    {
        string Key = item.Split('_')[0];
        string Value = item.Split('_')[1];

        List<string> sValue = new List<string>();
        bool exists = dict.TryGetValue(Key, out sValue);

        if (exists && !sValue.Contains(Value))
        {
            sValue.Add(Value);
            dict[Key] = sValue;
        }
        else if (!exists)
        {
            sValue = sValue ?? new List<string>();
            sValue.Add(Value);
            dict.Add(Key, sValue);
        }
    }
}

Is there a better way to do this, where I don't have to use two foreach loops? Is there a way to do it without using any foreach loops?

I have tried using lambda but this is as far as I got:

Dictionary<string, List<string>> dict = new Dictionary<string, List<string>>();

foreach (myObject result in objectList)
{
    dict = result.StringList.Select(x => x.Split('_'))
                            .GroupBy(x => x[0])
                            .ToDictionary(x => x.Key, x => x.Select(g => g[1]).ToList());
}

The problem is that the expression keeps overwriting any existing entries in the dictionary each time I iterate through the loop.

Is there a way to keep the existing entries, groupby the Key and add to the existing list of values but don't duplicate any values?

I think the answer lies in this part of the expression:

x => x.Select(g => g[1]).ToList()

but I'm not 100% sure.

Just get rid of the outer foreach as well:

dict = objectList.SelectMany(x => x.StringList)
       .Select(x => x.Split('_'))
       .GroupBy(x => x[0])
       .ToDictionary(x => x.Key, x => x.Select(g => g[1]).Distinct().ToList());

SelectMany retrieves all the StringLists and flattens them into one single list:

"A_1", "B_1", ..., "F_1", "A_2", "B_2", ...


Loop for inserting a node into a binary search tree I am trying to implement binary search tree. One method which I am trying to implement in the most efficient and stylish way is node insertion.
I am under the impression that  while (true) is a bad practice, correct?

    while (true){
        if(n <currentNode.data){
            if (currentNode.left == null){
                currentNode.left = new node(n);
                break;
            }else{
                currentNode = currentNode.left;
            }
        }else
            if (currentNode.right == null) {
                currentNode.right = new node(n);
                break;
            } else{
                currentNode = currentNode.right;
            }
    }

and here is the whole code:

package graph;

public class BSearchTree {
    private node head = null;
    public BSearchTree(int [] entries){
        for (int a : entries){
            insert(a);
        }
    }
    public void insert(int n){
        if (head == null){
            head = new node(n);
            return;
        }
        node currentNode = head;
        while (true){
            if(n <currentNode.data){
                if (currentNode.left == null){
                    currentNode.left = new node(n);
                    break;
                }else{
                    currentNode = currentNode.left;
                }
            }else
                if (currentNode.right == null) {
                    currentNode.right = new node(n);
                    break;
                } else{
                    currentNode = currentNode.right;
                }
        }

    }

    public static void main(String[] args) {
        BSearchTree bst = new BSearchTree(new int[]{2,4,1,5});
        System.out.println(bst.toString());
    }

    private class node {
        int data = -1;
        node left = null;
        node right = null;

        public node(int n){
            data = n;
        }
    }
}

(Just checking - you want your BST to be allowed to contain duplicates? That's what your current code does.)

Edit: After fixing some mistakes kindly pointed out by @vnp, here is an implementation of insert:

    public void insert(int n) {
        final boolean RIGHT = true;
        final boolean LEFT = false;

        node previousNode = null;
        boolean previousDirection = LEFT; // arbitrary
        node currentNode = head;

        while (currentNode != null) {
            previousNode = currentNode;
            if (n > currentNode.data) {
                currentNode = currentNode.right;
                previousDirection = RIGHT;
            } else {
                currentNode = currentNode.left;
                previousDirection = LEFT;
            }
        }

        if (previousNode == null) {
            head = new node(n);
        } else if (previousDirection == LEFT) {
            previousNode.left = new node(n);
        } else {
            previousNode.right = new node(n);
        }
    }

This avoids using while(true) and break; and uses more shallow if statements, which I think makes it marginally easier to follow.


metafunction to determinate the biggest of all supplied types Can the metafunction below be implemented more elegantly, with less code? What I have now is not very elegant or smartly written.

#include <type_traits>

namespace detail
{

template <typename T, typename A, typename ...B>
struct max_type
{
  typedef typename max_type<
    typename std::conditional<(sizeof(A) > sizeof(T)), A, T>::type,
    B...
  >::type type;
};

template <typename T, typename A>
struct max_type<T, A>
{
  typedef typename std::conditional<(sizeof(A) > sizeof(T)), A, T>::type type;
};

}

template <typename A, typename... B>
struct max_type
{
  typedef typename detail::max_type<A, A, B...>::type type;
};

How about this? It’s not all that much shorter, but IMHO quite a bit simpler:

#include <type_traits>

template <typename A, typename... B>
struct max_type
{
    typedef typename max_type<A, typename max_type<B...>::type>::type type;
};

template <typename A>
struct max_type<A>
{
    typedef A type;
};

template <typename A, typename B>
struct max_type<A,B>
{
    typedef typename std::conditional<(sizeof(A) > sizeof(B)), A, B>::type type;
};


Returning the length of a 'hidden' string within each of the sentences of another string The following program is supposed to return the length of a 'hidden' string within each of the sentences of another string, using pointers and avoiding as much as possible the use of [] as a means to access strings. We were instructed on how to declare the functions, hence their declarations are a given.

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

int getHiddenString(const char * str1 , const char * str2);
int divideStringIntoSentences(char * string, char * sentences[]);
char* readLine();
int main()
{
char *ptrarr[20];
int i = 0;
printf ("Please enter a string:\n");
char *str1 = readLine();
printf ("Please enter a hidden string:\n");
char *str2 = readLine();
if (str2 == NULL) return EXIT_FAILURE;
int j = divideStringIntoSentences(str1, ptrarr);
for (; i <= j; i++)
printf ("Sentence %d contains hidden string prefix length %d\n", i,
    getHiddenString(*(ptrarr + i), str2));
free (str1);
free (str2);
return 0;
}

int getHiddenString(const char * str1 , const char * str2)
{
int hidlen = 0;
for (; *str1 != '\0' && *str2 != '\0'; str1++)
if (*str1 == *str2)
{
    hidlen++;
    str2++;
}
return (hidlen);
}

char* readLine()
{
int capacity = 1, index = 0;
char c;
char *str1 = (char*) malloc(capacity * sizeof(char));
if (str1 == NULL) return NULL;
for (c = getchar(); c != '\n' && capacity <= 100; c = getchar())
{
    if (index == capacity - 1)
    {
        str1 = (char*) realloc(str1, (++capacity) * 2 * sizeof(char));
        if (str1 == NULL) return NULL;
        capacity*=2;
    }
    *(str1 + index++) = c;
}
*(str1 + index) = '\0';
return str1;
}

int divideStringIntoSentences(char * string, char * sentences[])
{
int sentnum = 0;
*sentences = string;
for (; *string != '\0'; string++)
if (*string == '.')
{
    *string = '\0';
    *(++sentences) = (string + 1);
    sentnum++;
}
return (sentnum);
}

Some generals comments:

Define functions in reverse order to avoid prototypes, make local functions static, add void to empty parameter lists, use braces even when unnecessary, use spaces consistently around operators, define a single variable per line, return statements don't need brackets (you are inconsistent in this).

In function readLine

This function has many issues, some minor, such as casting malloc, using
sizeof(char) (sizeof char is 1 by definition) and the arbitrary limit on a
new line of 100 chars. In your variable name, str1, the '1' is unnecessary,
as you only use one string. And your default sentence allocation of 1 char is
silly - start with something reasonable, say 64 (arbitrary value).

More seriously, getchar returns an int not char.  This allows it to hold the value for EOF, the end-of-file marker.  Your for-loop should test for EOF.  If you fail to do this then if you input from a redirected file, the program will fail if EOF is reached without finding a \n.  A better loop would be, for example:

int ch;
while((ch = getchar()) != EOF) {
    if (ch == '\n') {
        break;
    }
    // do the necessary stuff
}

Your reallocation should compute the new capacity then use that in the realloc
call rather than computing it twice.  And your realloc call as it currently
stands leaks memory on failure - the original string str1 is not freed by
realloc on failure.

In function divideStringIntoSentences

You need to pass the size of sentences[] into the function to allow for overflow
detection.

In function getHiddenString

Parameter names of sentence and hidden might be more understandable than
str1, str2.  More seriously, I don't know what it is supposed to
do.  If it is supposed to find the hidden string in the sentence, then it
doesn't work.  Library function strstr would do the job for you.

In main

Although the functions handle malloc failures, main does not check for
str1 in the returned values.  Again str1 and str2 are badly named, as is
ptrarr.  The for-loop should define the loop variable and should loop up to
but not including j: for (int i = 0; i < j; ++i).  And the printout
"Sentence # contains hidden string prefix length #" is printed even if the
sentence doesn't contain the hidden string.  This make me think that the
getHiddenString function is telling us the number of consecutive chars from
the hidden string contained in the sentence, or some such.  In that case it
still doesn't work and is badly named (eg if I give it "two twa twe" and the
hidden string "twe" it says, "Sentence 0 contains hidden string prefix length 4"


Python multiprocessing messaging code I am just learning how to do some multiprocessing with Python, and I would like to create a non-blocking chat application that can add/retrieve messages to/from a database. This is my somewhat primitive attempt (note: It is simplified for the purposes of this post); however, I would like to know how close/far off off the mark I am. Any comments, revisions, suggestions, etc. are greatly appreciated.

Thank you!

import multiprocessing
import Queue
import time

# Get all past messages
def batch_messages():
    # The messages list here will be attained via a db query
    messages = [">> This is a message.", ">> Hello, how are you doing today?", ">> Really good!"]
    for m in messages:
        print m

# Add messages to the DB
def add_messages(q):

    # Retrieve from the queue
    message_to_add = q.get()

    # For testing purposes only; perfrom another DB query to add the message to the DB
    print "(Add to DB)"

# Recieve new, inputted messages.
def receive_new_message(q, new_message):

    # Add the new message to the queue:
    q.put(new_message)

    # Print the message to the screen
    print ">>", new_message

if __name__ == "__main__":

    # Set up the queue
    q = multiprocessing.Queue()

    # Print the past messages
    batch_messages()

    while True:
        # Enter a new message
        input_message = raw_input("Type a message: ")

        # Set up the processes
        p_add = multiprocessing.Process(target=add_messages, args=(q,))
        p_rec = multiprocessing.Process(target=receive_new_message, args=(q, input_message,))

        # Start the processes
        p_rec.start()
        p_add.start()

        # Let the processes catch up before printing "Type a message: " again. (Shell purposes only)
        time.sleep(1)

Don't create a new process for every task. Put a loop in add_messages; q.get will wait for the next task in the queue.
receive_new_message is not doing anything useful in your example. Place q.put(new_message) right after raw_input instead.

:

def add_messages(q):
    while True:
        # Retrieve from the queue
        message_to_add = q.get()
        print "(Add to DB)"

if __name__ == "__main__":
    q = multiprocessing.Queue()
    p_add = multiprocessing.Process(target=add_messages, args=(q,))
    p_add.start()

    while True:
        input_message = raw_input("Type a message: ")

        # Add the new message to the queue:
        q.put(input_message)

        # Let the processes catch up before printing "Type a message: " again. (Shell purposes only)
        time.sleep(1)


Damage or Heal based on the buttons you press If you press the damage button, it will damage armor before health. If you press the heal button, it will heal armor before health.

The project is here: https://github.com/austingaee/DamageHealth-Armor

Are there any improvements I can make?

import UIKit

class ViewController: UIViewController {

    @IBOutlet weak var armorBarView: UIView!

    @IBOutlet weak var healthBarView: UIView!

    var damageAmount : Float = 0.0
    var healAmount : Float = 0.0

    //Maximum Health
    var barAmount : Float = 0.0

    override func viewDidLoad() {
        super.viewDidLoad()

        damageAmount = Float(self.armorBarView.frame.size.width) * 0.10
        healAmount = Float(self.armorBarView.frame.size.width) * 0.05
        barAmount = Float(self.armorBarView.frame.size.width)
    }

    override func didReceiveMemoryWarning() {
        super.didReceiveMemoryWarning()
        // Dispose of any resources that can be recreated.
    }

    @IBAction func damageBar(_ sender: UIButton) {
        //7.63 came from how much damage was left after 10 hits
        if self.armorBarView.frame.size.width > 7.63 {
            self.armorBarView.frame.size.width -= CGFloat(damageAmount)
        } else if self.healthBarView.frame.size.width > 7.63 {
            self.healthBarView.frame.size.width -= CGFloat(damageAmount)
        }
    }

    @IBAction func healBar(_ sender: Any) {
        if self.armorBarView.frame.size.width < CGFloat(barAmount) {
            self.armorBarView.frame.size.width += CGFloat(healAmount)
            print(self.armorBarView.frame.size.width)
        } else if self.healthBarView.frame.size.width < CGFloat(barAmount) {
            self.healthBarView.frame.size.width += CGFloat(healAmount)
        }
    }

}

not so much but some tiny changes:

DRY

i just would avoid all the self.healthBarView.frame.size.width

and make methods:

getWidth
setWidth(or updateWidth because you set the diff)

Constants

the magic numbers like 0.1 0.05 10 i would define as constants so it can changed only at one place. 7.63 i would calculate

Naming

rename barAmount to maxAmount

create minAmount

Float/CGFloat

the Float / CGFloat is realy needed? just put them also into getWidth and updateWidth

else if

the else if block is not needed (same condition)

7.63

this is an value depending on your setup - it should also calculated so it not changes when you init it with different width

Resulting Code

import UIKit

class ViewController: UIViewController {

    @IBOutlet weak var armorBarView: UIView!

    @IBOutlet weak var healthBarView: UIView!

    var damageAmount : Float = 0.0
    var healAmount : Float = 0.0

    // Min/Maximum Health
    var minAmount: Float = 0.00
    var maxAmount : Float = 0.0

    // how much damage was left after this number of hits
    let minAmountHits:Float = 10

    let demageFactor: Float = 0.10
    let healFactor: Float = 0.05

    override func viewDidLoad() {
        super.viewDidLoad()

        initAmount()
    }

    override func didReceiveMemoryWarning() {
        super.didReceiveMemoryWarning()
    }

    func initAmount(){
        damageAmount = getWidth() * demageFactor
        healAmount = getWidth() * healFactor
        maxAmount = getWidth()
        minAmount = maxAmount - minAmountHits * damageAmount
    }

    @IBAction func damageBar(_ sender: UIButton) {
        if getWidth() > minAmount {
            updateWidth(-damageAmount)
        }
    }

    @IBAction func healBar(_ sender: Any) {
        if getWidth() < maxAmount {
            updateWidth(healAmount)
        }
    }

    func getWidth() -> Float {
        return Float(self.armorBarView.frame.size.width)
    }

    func updateWidth(_ amount: Float) {
        self.healthBarView.frame.size.width += CGFloat(amount)
        print(self.armorBarView.frame.size.width)
    }
}


Refactoring asynchronous JS pre-rendering code A few months ago I wrote this module but, coming back to it, I find it a bit hard to read and reason about. I want to ask community's opinion on whether this needs to be refactored, and how I could approach this.

(function (q) {

  'use strict';

  var DELAY_BEFORE_RENDERING_NEXT_PAGE_VIEW = 3 * 1000;

  window.st.viewer.Helper.PageViewFactory = (function () {
    function promiseRenderView(page, token) {
      return page.fetchUnlessReady()
        .then(token.throwIfCanceled)
        .then(function () {
          var view = new window.st.viewer.View.PageView({
            model: page
          });

          view.render();
          return view;
        });
    }

    var scheduledPage,
        scheduledPromise;

    function scheduleRenderNextView(page, token) {

      // After some time passes, prefetch and prerender next page view
      // so if it is requested (very likely), we can re-use an existing promise.

      scheduledPage = null;
      scheduledPromise = null;

      q.delay(DELAY_BEFORE_RENDERING_NEXT_PAGE_VIEW)
        .then(token.throwIfCanceled)
        .then(function () {
          return page.promiseNext();
        })
        .then(token.throwIfCanceled)
        .then(function (nextPage) {
          if (nextPage) {
            scheduledPage = nextPage;
            scheduledPromise = promiseRenderView(nextPage, window.st.Helper.CancellationToken.getNone());
          } else {
            scheduledPage = null;
            scheduledPromise = null;
          }
        })
        .catch(token.catchItself)
        .done();
    }

    function scheduleRenderView(page, token) {
      var promise = (page !== scheduledPage) ?
        promiseRenderView(page, token) :
        scheduledPromise;

      scheduleRenderNextView(page, token);

      return promise;
    }

    return {
      scheduleRenderView: scheduleRenderView
    };
  })();
})(Q);

This module exports a single function called scheduleRenderView. Its purpose is, given a page model and a cancellation token, do the following steps asynchronously:

Fully fetch page model (it may be loaded partially);
Create and render (in memory) a PageView for it;
Return this view to the caller;
Additionally, after a 3 second delay:

Request next page by calling promiseNext;
Pre-render next page by repeating steps 1-3 for it and store the pre-rendered view in case it gets requested the next time scheduleRenderView is called (very likely)

The user views pages one by one, and I wanted to anticipate the most likely scenario where she will request the next page.

I also wanted to make this optimization invisible to the calling code.

Is this code hard to comprehend? Are the simpler ways to achieve the same?

Is this code hard to comprehend? Yes.

That is because there are a lot of unknown functions: view.render, page.fetchUnlessReady, token.catchItself ,token.throwIfCanceled or window.st.Helper.CancellationToken.getNone() etc. etc.

In my mind, I would approach this more like :

function scheduleRenderView( page )
{
  //Load and display the page
  var promise = model.loadPage( page ).then( function(){
    view.renderPage( model.getPage( page ) );
  });
  //Load the next page
  model.loadPage( page.next() );
  return promise;
}

loadPage would either return immediately if we retrieved already the data once ( from cache ) or download the page model.

getPage gets a page model from cache

renderPage would do the obvious

This means I would forego the pre-rendering which really sounds like pre-mature optimization, how long could that take?

2 other minor comments:

Why access some variables through window, that does not seem to make sense
In my mind, adding functions to the window.st.viewer.Helper is terrible global variable abuse, not to mention that you probably should add it to Helper.prototype.


K&R Exercise 1-19. Reverse program input one line at a time Intro

I'm going through the K&R book (2nd edition, ANSI C ver.) and want to get the most from it: learn (outdated) C and practice problem-solving at the same time. For that reason, I'm trying to solve the exercises "hardcore" - without using any knowledge from the "future". I'll catch-up with it after I finish the book.

I believe that the author's intention was to give the reader a good exercise, to make the reader think hard about what he can do with the tools introduced, and not to skip ahead to find an easier way. For that reason, I'm only using language features introduced up to this point in the book. That also means I'm using an old C standard.

Compiling with gcc -Wall -Wextra -Wconversion -pedantic -ansi.

K&R Exercise 1-19

Write a function reverse(s) that reverses the character string s. Use it to write a program that reverses its input a line at a time.

Solution

The task explicitly states how the function should look like. For that reason, I've coded it similarly to how the author made his copy function.

The 2nd part of the challenge is to reverse the program input a line at a time, but how long can the line be? It would be too easy to set a hard limit for line length, just loop line by line and return an error if it's exceeded. I can't use dynamic memory allocation to deal with arbitrary long lines since it hasn't been introduced yet so what's left? The book already introduced all the building blocks I need for recursion, so at this point it the only solution.

Code

/* Exercise 1-19. Write a function `reverse(s)` that reverses the
 * character string `s`. Use it to write a program that reverses its
 * input a line at a time. */

#include <stdio.h>
#define BUFSIZE 10 /* line buffer size */

int getline(char line[], int maxline);
void copy(char to[], char from[]);
void reverse(char s[]);
int reverseinput(char lastchar[]);

main()
{
    char lastchar[1];

    while (reverseinput(lastchar) != 0)
        if (lastchar[0] == '\n')
            putchar('\n');
    return 0;
}

/* reverseinput: read from input until end of line is reached and then
 * print the line in reverse */
int reverseinput(char lastchar[])
{
    int len;
    char line[BUFSIZE];
    int retval;

    if ((len = getline(line, BUFSIZE)) > 0) {
        if ((lastchar[0] = line[len-1]) == '\n') {
            line[len-1] = '\0';
            retval = 1;
        }
        else
            retval = reverseinput(lastchar);
        reverse(line);
        printf("%s", line);
    }
    else
        retval = 0;
    return retval;
}

/* getline: read a line into s, return length */
int getline(char s[],int lim)
{
    int c, i;

    for (i=0; i < lim-1 && (c=getchar())!=EOF && c!='\n'; ++i)
        s[i] = c;
    if (c == '\n') {
        s[i] = c;
        ++i;
    }
    s[i] = '\0';
    return i;
}

/* copy: copy 'from' into 'to'; assume to is big enough */
void copy(char to[], char from[])
{
    int i;

    i = 0;
    while ((to[i] = from[i]) != '\0')
        ++i;
}

/* reverse: reverse a '\0' terminated string */
void reverse(char s[])
{
    int tail;
    int head;
    char c;

    /* find the last character index */
    tail = 0;
    while (s[tail] != '\0')
        ++tail;
    if (tail > 0)
        --tail;
    /* reverse the string by swapping first and last elements */
    for (head = 0; head < tail; ++head && --tail) {
        c = s[head];
        s[head] = s[tail];
        s[tail] = c;
    }
}

Test

Running $ ./ch1-ex-1-19-01 <ch1-ex-1-19-01.c | ./ch1-ex-1-19-01 prints the program code :)

Minor:

getline()

When size is pathologically small (sz <= 1), if (c == '\n') { tests the uninitialized c leading to  undefined behavior.

With sz <= 0, s[i] = '\0'; is not valid either.

I'd recommend to note the condition that sz > 1 is expected.

Further, I'd use size_t rather than int for the buffer size and re-code to handle those edge cases.

size_t getline(char s[], size_t sz) {
    int c = 0;
    size_t i;

    for (i=0; i + 1 < sz && (c=getchar())!='\n' && c!=EOF; ++i) {
        s[i] = c;
    }
    if (c == '\n') {
        s[i] = c;
        ++i;
    }
    if (i < sz) {
      s[i] = '\0';
    }
    return i;
}


Manage input values from file according to number of elements per line I was given the a test file with the following kind of content, an arbitrary number of integer pairs divided by a newline and a final line with one integer:

1 2
3 4
5

I had to use the couples as values for two data members of a series of objects and I had to put the last integer in a separate variable.

I eventually wrote a working code, but the second inner while loop is very wasteful (it is the one loading the last value).

Also, I had to include <fstream>, <sstream> and <string> just to manage this input file.

What is a simpler and more elegant way to solve this problem?

#include <iostream>
#include <fstream>
#include <sstream>
#include <vector>
#include <string>

class anObject
{
public:
    int one;
    int two;
};
int main()
{
    std::ifstream inFile;
    inFile.open("myFile.txt", std::ios::in);

    int myVar=0;
    std::string inLine;

    while (getline(inFile, inLine))
    {
        int inputA, inputB;
        std::stringstream inStream(inLine);
        std::stringstream inStreamAlt(inLine);

        while (inStream >> inputA >> inputB)
        {
            anObject inObj;
            inObj.one = inputA;
            inObj.two = inputB;
            objList.push_back(inObj);
        }

        while (inStreamAlt >> inputA)
        {
            myVar = inputA;
        }
    }
}

You can simplify your loops logic like this

while (getline(inFile, inLine)) {
    std::istringstream inStream(inLine);

    anObject inObj;
    if((inStream >> inObj.one) && (inStream >> inObj.two)) {
        objList.push_back(inObj);
    }
    else {
        myVar = inObj.one;
    }
}

relying on exactly that form of input you mentioned is given.

std::ios::in is redundant for std::ifstream:

inFile.open("myFile.txt", std::ios::in);
                     // ^^^^^^^^^^^^^^

also open() isn't explicitely needed. You can simply write:

std::ifstream inFile("myFile.txt");

class anObject
{
public:
    int one;
    int two;
};

can be simpler written as struct:

struct anObject
{
    int one;
    int two;
};


Rosalind problem "Consensus and Profile" Source: Rosalind("Consensus and Profile")

  Brief summary

                A T C C A G C T
                G G G C A A C T
                A T G G A T C T
DNA Strings     A A G C A A C C
                T T G G A A C T
                A T G C C A T T
                A T G G C A C T

            A   5 1 0 0 5 5 0 0
Profile     C   0 0 1 4 2 0 6 1
            G   1 1 6 3 0 1 0 0
            T   1 5 0 0 0 1 1 6

Consensus       A T G C A A C T

  Given: A collection of at most 10 DNA strings of equal length (at most 1 kbp) in FASTA format.

  Return: A consensus string and profile matrix for the collection. (If several possible consensus strings exist, then you may return any one of them.)

Model (cons.rb):

#!/usr/bin/env ruby
require_relative '../ie_module'

class DnaConsensus
  include ImportExport
  DNA_BASES = %w(A C G T)

  attr_reader :dna_strings, :consensus, :profile

  def initialize(source = "rosalind_#{current_dir_name}.txt")
    @dna_strings = (source =~ /txt$/ ? import_lines(source) : source).values
    @profile = build_profile
    @consensus = build_consensus
  end

  def to_s
    "#{consensus.join}\n#{stringify(profile)}"
  end

private

  def build_profile
    prof = DNA_BASES.map{|b| [b, []]}.to_h
    dna_strings.map(&:chars).transpose.each.with_object(prof) do |arr, hsh|
      hsh.merge!(hashed(arr)){ |_, oldval, newval| oldval << newval }
    end
  end

  def hashed(arr)
    hsh = arr.group_by(&:chr).map{ |k,v| [k, v.size] }.to_h
    (DNA_BASES - hsh.keys).each { |b| hsh[b] = 0 }
    hsh
  end

  def build_consensus
    dna_strings.first.length.times.with_object([]) do |index, arr|
      arr << profile.max_by{|_, list| list[index]}.first
    end
  end

end

a = DnaConsensus.new
a.export_to_file([a.to_s])

File read/write logic (ie_module.rb):

module ImportExport

  def export_to_file(result, file = "result_#{current_dir_name}.txt")
    File.open(file, 'w') do |f|
      result.each{ |val| f << "%s" % val }
    end
  end

private

  def current_dir_name
    File.basename(Dir.getwd)
  end

  def stringify(obj)
    if obj.is_a?(Hash)
    then obj.map{|k,v| "#{k}: #{v.join(' ')}"}
    else obj.map{|e| e.join(' ')}
    end.join("\n")
  end

  def import_lines(file)
    File.foreach(file).with_object({}) do |line, hsh|
      line = line.strip.sub(/^>/, '')
      $' ? hsh[line] = '' : hsh[hsh.keys.last] << line
    end
  end

end

Here is a lot of code, but #build_profile is the most "complicated" part. I know, that "alternate way" exists.
All suggestions are welcome.

I am going to ignore all the file reading code, which is extraneous to the problem, and focus just on simplifying the code to find the consensus.  It can be done in essentially a single a line, the one beginning consensus = ....  Everything else is just setting up the sample data.

transpose gets us the columns, and max_by ... count get us the most frequently occurring nucleotide:

matrix = <<EOS
A T C C A G C T
G G G C A A C T
A T G G A T C T
A A G C A A C C
T T G G A A C T
A T G C C A T T
A T G G C A C T
EOS
.split("\n").map{|x| x.split(' ')}

nucleotides = %w(A C G T)

consensus = matrix.transpose.map {|x| nucleotides.max_by {|n| x.count(n)}}

p consensus #=> ["A", "T", "G", "C", "A", "A", "C", "T"]


string length in x64 assembly (fasm) Please critique this very, very basic routine which returns the length of a given char buffer or "string."

strlen: ; NOTE: RDI IS THE DEFAULT SRC FOR SCASB
    push rdi
    push rcx
    xor rcx, rcx
    mov rcx, -1

    xor al, al
    cld

    repne scasb
    neg rcx
    sub rcx, 1
    mov rax, rcx
    pop rcx
    pop rdi
    ret

Saving rcx is usually not necessary, it is not callee-save in common calling conventions. On Linux (and similar) rdi also does not need to be saved, I guess you're using that since the Win64 calling convention does not pass an argument in rdi. You can save them anyway if you want, which can be useful if you're using custom calling conventions. Saving an even number of registers makes the stack not-16-aligned though, you will probably get away with that now, but for example if you call some function that uses XMM registers it may save them at locations that it assumes are aligned (and there are some other cases where it causes trouble).

xor rcx, rcx
mov rcx, -1

The xor is not useful, rcx does not need to be zeroed before overwriting it for correctness reasons, and simply mov-ing into a 64 (or 32) bit register already has no dependency on the previous value. By the way, when you do want to zero a 64bit register, you can use a 32bit xor since writing to the low 32 bits of a register zeroes out the top half of the 64 bit register. There is not really an immediate performance difference, but using the 32bit version often lets you save the REX prefix, unless of course one of the "numbered registers" is an operand.

Because -x - 1= ~x + 1 - 1 = ~x (using the definition of two's complement, -x = ~x + 1) and you don't use the flags set by the sub,

neg rcx
sub rcx, 1
mov rax, rcx

is equivalent to:

not rcx
mov rax, rcx

So all combined, this function could be simplified slightly to (assuming saving rdi and rcx is useful):

strlen:
    push rdi
    push rcx
    mov rcx, -1
    xor eax, eax
    repne scasb
    not rcx
    mov rax, rcx
    pop rcx
    pop rdi
    ret


Populating dropdowns that are based on others I created two dropdowns, a dadand a son that are populated based on two arrays that, hypothetically, can't be changed.

When you choose a dad with the code x it generates the son's drop-down with the options containing the code x.

var dadArray = [
  [1, 'Dad1'],
  [2, 'Dad2'],
  [3, 'Dad3']
];
var sonArray = [
  [1, 'Son1'],
  [1, 'Son2'],
  [3, 'Son3'],
  [2, 'Son4']
];

function sortArray(dad, dadID) {
  for (var i = 0; i < dad.length; i++) {
    $(dadID).html($(dadID).html() + '<option value="' + dad[i][0] + '">' + dad[i][1] + '</option>');
  }
  var selDad = $('#dadDrop option:selected').val();
  for (i = 0; i < sonArray.length; i++) {
    if (sonArray[i][0] == selDad) {
      $('#sonDrop').html($('#sonDrop').html() + '<option value="' + sonArray[i][0] + '">' + sonArray[i][1] + '</option>');
    }
  }
}

sortArray(dadArray, '#dadDrop');

$("#dadDrop").change(function() {
  $('#sonDrop').html('');
  var selDad = $('#dadDrop option:selected').val();
  for (var i = 0; i < sonArray.length; i++) {
    if (sonArray[i][0] == selDad) {
      $('#sonDrop').html($('#sonDrop').html() + '<option value="' + sonArray[i][0] + '">' + sonArray[i][1] + '</option>');
    }
  }
});

<select id="dadDrop">
</select>

<select id="sonDrop">
</select>

I made it using jQuery, but it looks huge (Probably because I'm still a newbie to JS and its libraries with amazing features). Is there a different logic that requires fewer lines? of course there is, but may you share it with me?

Your data doesn't mean much. I would suggest an array of objects so that the data will have meaning. Also, I would not recommend naming your variables based on data type. JS doesn't have strict types. You can easily assign anything to the variable. This means your dadArray may not be an array at some point.

var dads = [
  {id: 1, name: 'Dad1'},
  {id: 2, name: 'Dad2'},
];

var sons = [
  {id: 1, dadId: 1, name: 'Son1'},
  {id: 2, dadId: 1, name: 'Son2'},
  {id: 3, dadId: 3, name: 'Son3'},
  {id: 4, dadId: 2, name: 'Son4'},
];

So your operation involves:

Populating the dads dropdown.
On dad selection, populate the sons dropdown depending on the dad.

I notice that in your code there's duplicates. Let's split them up into functions to be reusable. Based on the operations, you should have something like:

function populateDads(dads){...}
function populateSons(sons){...}

Let's start first with the dads. I would discourage using .html(). While it does work, what happens is that jQuery does not clean up after itself. Handlers that are assigned to elements that are replaced will linger on your page. Suggesting you use .empty() if your current set is the parent of the elements to remove or .remove() if your current set are the elements to remove.

$('select').empty(); // Empties the options

As far as I know, jQuery's .val() should be able to extract the value from a <select>. No need to find the selected <option> from the <select>.

var selDad = $('#dadDrop').val();

If both your select boxes are static throughout the life of the page, you can simply just store their jQuery object in a variable. That way, you won't be querying the DOM every now and then.

var dadSelect = $('#dadDrop');
var sonSelect = $('#sonDrop');

There's also a neater way to construct elements in jQuery. It goes like:

$('<tagName />', { attr: value });

Running with all these, the code should look like:

var dads = [
  {id: 1, name: 'Dad1'},
  {id: 2, name: 'Dad2'},
  {id: 3, name: 'Dad3'},
];

var sons = [
  {id: 1, dadId: 1, name: 'Son1'},
  {id: 2, dadId: 1, name: 'Son2'},
  {id: 3, dadId: 3, name: 'Son3'},
  {id: 4, dadId: 2, name: 'Son4'},
];

var dadSelect = $('#dadDrop');
var sonSelect = $('#sonDrop');

function populateDads(dads){
  dadSelect.empty();
  $.each(dads, function(index, dad){
    $('<option />', {value: dad.id}).text(dad.name).appendTo(dadSelect);
  });
}

function populateSons(sons){
  sonSelect.empty();
  $.each(sons, function(index, son){
    $('<option />', {value: son.id}).text(son.name).appendTo(sonSelect);
  });
}

dadSelect.on('change', function(){
  // .val() returns a string. Coerce the value to a number using the
  // prefix "+". Acts like parseFloat most of the time.
  var selectedDad = +dadSelect.val();
  var dadSons = sons.filter(function(son){ return son.dadId === selectedDad; });
  populateSons(dadSons);
});

populateDads(dads);

// Trigger change for first load to prepopulate children
dadSelect.trigger('change');
<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.11.1/jquery.min.js"></script>
<select id="dadDrop"></select>
<select id="sonDrop"></select>


Triangle number with 500 divisors It should work but my comp can't compute it in time. It is Project Euler problem 12. Just a small bit of advice would be nice. Only at this a short time so I probably just haven't learned a good way to manipulate the situation yet.

public class HighlyDivisibleTriangularNumber{
    public static void main(String args[]){
        long count = 1, sum = 0, i;
        for(i = 1; count != 500; i++){
            sum += i;
            if(sum % 2 == 0 && sum % 3 == 0) count = 3;
            else count = 2;
            for(int j = 4; j <= sum/2; j++)
                if(sum % j == 0) count++;
        }
        System.out.println(i-1);
    }
}

Java is a just-in-time compiled language. As a side-effect of this, it is common for performance problems to happen when you have all the code in the main method. The reason is that Java often only compiles methods that are called very often, but the main method is only called once, so it is not compiled optimally.

This is one reason why 'function extraction' is often suggested (as well as the other benefits like readability).

So, consider a method that's called 'countDivisors':

private static long countDivisors(long value) {
    long count = 1;
    for (long i = 1; i <= value/2; i++) {
        if (value % i == 0) {
            count++;
        }
    }
    return count;
}

This code will be called often, and will thus be compiled optimally.

The main method also becomes simpler, with:

public static void main(String args[]){
    long count = 1, sum = 0, i = 0;
    do {
        i++;
        sum += i
        count = countDivisors(sum);
    } while (count < 500);
    System.out.println(i);
}

Now, whether this improves the performance enough, is uncertain, but it is more readable, and will be faster.

There's probably a smarter way to do this too, a better algorithm is normally what's required with Euler problems.


Hover effects for red, blue, and green boxes I've created a JQuery script to help illustrate a few points for my teaching class, but I have having trouble slimming it down.

$(document).ready(function() {
    var red = $(".small-box");
    var blue = $(".small-box2");
    var green = $(".large-box");
    red.hover(
        function () {
            red.addClass("selected-highlight");
            blue.addClass("selected-highlight2");
            green.addClass("selected-highlight3");
        },
        function () {
        red.removeClass("selected-highlight");
            blue.removeClass("selected-highlight2");
            green.removeClass("selected-highlight3");
       }
    );
    blue.hover(
        function () {
            red.addClass("selected-highlight3");
            blue.addClass("selected-highlight");
            green.addClass("selected-highlight2");
        },
        function () {
            red.removeClass("selected-highlight3");
            blue.removeClass("selected-highlight");
            green.removeClass("selected-highlight2");
       }
    );
    green.hover(
        function () {
            red.addClass("selected-highlight2");
            blue.addClass("selected-highlight3");
            green.addClass("selected-highlight");
        },
        function () {
            red.removeClass("selected-highlight2");
            blue.removeClass("selected-highlight3");
            green.removeClass("selected-highlight");
       }
    );
});

Everything works as it should, but I know there is a way to slim down the "removeClass" part to one single line. I am having trouble remembering how to do so. The idea is to whatever box you hover over is red and the colors follow accordingly in the order of: red, blue, and green.

.container-left {
    float:left;
}
.small-box {
    width:300px;
    height:300px;
    background-color:#FFF;
    border-top:1px #000 solid;
    border-left:1px #000 solid;
}
.small-box2 {
    width:300px;
    height:300px;
    background-color:#FFF;
    border-left:1px #000 solid;
    border-bottom:1px #000 solid;
    border-top:1px #000 solid;;
}
.container-right {
    float:left;
}
.large-box {
    width:300px;
    height:601px;
    background-color:#FFF;
    border:1px #000 solid;
}
.selected-highlight{
    background-color:red;
}
.selected-highlight2 {
    background-color:blue;
}
.selected-highlight3 {
    background-color:#00FF00;
}

A little shorter:

$(document).ready(function() {
    var red = $(".small-box");
    var blue = $(".small-box2");
    var green = $(".large-box");
    var boxes = red.add(blue).add(green);

    boxes.mouseleave(function() {
        boxes.removeClass("selected-highlight selected-highlight2 selected-highlight3");
    });

    red.mouseenter(
        function () {
            red.addClass("selected-highlight");
            blue.addClass("selected-highlight2");
            green.addClass("selected-highlight3");
        }
    );
    blue.mouseenter(
        function () {
            red.addClass("selected-highlight3");
            blue.addClass("selected-highlight");
            green.addClass("selected-highlight2");
        }
    );
    green.mouseenter(
        function () {
            red.addClass("selected-highlight2");
            blue.addClass("selected-highlight3");
            green.addClass("selected-highlight");
        }
    );
});


Creating a Deprecated/Obsolete behavior for methods in a JavaScript library A friend of mine has a popular open source JavaScript library which is quite well used in the community.  He is now going through a process of refactoring and applying best practices and conventions and is wanting to upper case all constructor functions.  So in the past where he may have defined a chart object, he now wants it to be called Chart to stick with convention.

Because the library is well used, he would rather warn devs when they use the old function names and eventually deprecate them.

I've recommended using a helper function :

var ObsoleteWithReplacement = function(replacementFunction, oldFnName, newFnName) {
    var wrapper = function() {
       console.warn("WARNING! Obsolete function called.  Function '" + oldFnName + "' has been deprecated, please use the new '" + newFnName + "' function instead!");

        replacementFunction.apply(this, arguments);
    }
    wrapper.prototype = replacementFunction.prototype;

    return wrapper;
}

Which could be called as follows : http://jsfiddle.net/rLNep/1/

Is this a good way to go about it?  Can you recommend a better way?

From looking at your fiddle, the first thing I would do is make sure that your function is not anonymous:

// new function implementation
var Chart = function Chart(name, data) { //See what I did there?
    this.name = name;
    this.data = data;

    console.log("Chart > constructor");
}

Then, to keep it KISS, I would

// obsolete helper function
var ObsoleteWithReplacement = function( f ) {
    var wrapper = function() {
       console.warn("This function is now called " + f.name );
       f.apply(this, arguments);
    }
    wrapper.prototype = f.prototype;
    return wrapper;
}

Because the function is no longer anonymous, you can now use f.name, you no longer have to provide it. The drawback is that the new old function name is not logged, I think any JS developer could figure that one out.

Then you can do this:

// old deprecated function
var chart = ObsoleteWithReplacement(Chart);

The name ObsoleteWithReplacement is unfortunate, I would use simply Obsolete. Also I think function Obsolete(){..} makes more sense than var Obsolete = function Obsolete(){..}, var Obsolete = function (){..} should be avoided since it creates an anonymous function.

I forked this to http://jsfiddle.net/konijn_gmail_com/8a5Ax/


Implementing C++ relational operators generator Once you define the < operator, you can have an estimation of how the rest of relational operators behave. I'm trying to implement a way to do that for my classes.

What I want is to define only the < and the rest of the operators to be defaulted implicitly. What I've got so far is this design, which I'll elaborate on further down:

template<typename T>
struct relational
{
    friend bool operator> (T const &lhs, T const &rhs) { return rhs < lhs; }
    friend bool operator==(T const &lhs, T const &rhs) { return !(lhs < rhs || lhs > rhs); }
    friend bool operator!=(T const &lhs, T const &rhs) { return !(rhs == lhs); }
    friend bool operator<=(T const &lhs, T const &rhs) { return !(rhs < lhs); }
    friend bool operator>=(T const &lhs, T const &rhs) { return !(lhs < rhs); }
};

So for a class that implements the < operator it would just take inheriting from relational to have the rest of the operators defaulted.

struct foo : relational<foo>
{
    // implement < operator here
};

Are there any alternative, better designs?
Is there a time bomb in this code?
While trying to use this code with classes that already defined one of { >, ==, !=, <=, >= } I had ambiguity problems. The implementation I opted for (also the one existing in the linked Github repository) is tweaked like so

// inside the relational struct
friend bool operator>(relational const &lhs, relational const &rhs)
{ // functions that involve implicit conversion are less favourable in overload resolution
        return (T const&)rhs < (T const&)lhs;
}

Are my problems over with this trick?

Here's a demo of the code working.

The problem I see is that it requires that the class be derived from relational, which is not a very durable design.  Instead, why not use the fact that "substitution failure is not an error" (which has the regrettable but commonly used abbreviation SFINAE). the use of real functions is preferred over the instantiation of templates.  That is, if an operator<= actually exists, the templated form will be ignored.  That might look like this:

template<typename T>
bool operator> (T const &lhs, T const &rhs) { return !(lhs <= rhs); }
template<typename T>
bool operator< (T const &lhs, T const &rhs) { return !(lhs >= rhs); }
template<typename T>
bool operator==(T const &lhs, T const &rhs) { return lhs <= rhs && lhs >= rhs; }
template<typename T>
bool operator!=(T const &lhs, T const &rhs) { return !(rhs == lhs); }
template<typename T>
bool operator<=(T const &lhs, T const &rhs) { return lhs < rhs || (!(rhs < lhs)); }
template<typename T>
bool operator>=(T const &lhs, T const &rhs) { return lhs > rhs || (!(rhs > lhs)); }

Note that the differences are that I've made each comparison operator its own template and that I've added one for < and defined <= and >= in terms of < and > respectively.

The result is that if any of the comparisons <, <=, > or >= are defined, then all of the other comparisons are also available.

Edit: I should probably have pointed out why this is different from std::rel_ops.  With std::rel_ops, in order to have all six relative operators, you must define operator< and operator==.  With this scheme, however, as long as at least one of the ordering operators is defined, all others will also be available.  This is due to the way these templates are deliberately circularly defined.  That is, > is defined in terms of <= which is defined using < which is defined using >= which is defined using >.  This arguably adds flexibility, but it also means that it's possible that the performance is not as good because a comparison may go through several templated function instantiations.

Further, if none of those operators is defined, that circular definition will mean infinite recursion at runtime until your stack is exhausted, putting this set of templates into the category of "perhaps more dangerous than clever" so Caveat lector.


Optimizing Node.js http get for AWS Lambda I have the following Node.JS (runtime v6.10) executing on AWS Lambda:

exports.handler = (event, context) => {
    send(event);
};

var flota = ['CZWF64', 'DRZY65', 'DRZY66', 'FPBS85', 'FPBS86', 'FPBS87'];

function send(json) {

    var ppu = json.vehiculo.patente.replace(/[^a-z0-9]/gi, '');

    if (flota.indexOf(ppu) == -1) {
        return;
    }

    var request = require('request');
    var url = 'http://endpointUrl';

    var gps = json.gps;
    var date = new Date(gps.fecha);

    var month = date.getMonth() + 1;
    var day = date.getDate();
    var hour = date.getHours();
    var min = date.getMinutes();

    var month = (month < 10 ? "0" : "") + month;
    var day = (day < 10 ? "0" : "") + day;
    var hour = (hour < 10 ? "0" : "") + hour;
    var min = (min < 10 ? "0" : "") + min;

    var propertiesObject = {
        aplicacion: 1802, // ID Waypoint
        imei: ppu,
        lat: gps.lat,
        lon: gps.lon,
        alt: gps.alt,
        vel: gps.speed,
        rumbo: gps.cog,
        fecha: date.getFullYear() + month + day,
        hora: hour + min
    };

    request({
        url: url,
        qs: propertiesObject
    }, function(err, response, body) {
        var success = false;
        var error;
        if (response.statusCode != 200) {
            error = new Error('Get response: ' + response.statusCode);
        } else if (err) {
                        error = new Error('[ERROR]: ' + err);
        } else if (( body.indexOf('IMEI') != -1 &&  body.indexOf('registrada') != -1) ||  body.indexOf('ERROR') != -1) {
                        error = new Error('[ERROR]: ' + body);
        } else if ('OK'.localeCompare(body) == 0) {
            success = true;
        } else {
                       error = new Error('[ERROR]: ' + body);
        }
        if (!success) {
            throw error;
        }
    });

}

And as AWS Lambda charges it's costs per execution time, I'd like to improve the above code the fastest possible execution time.

Any tips would be appreciated.

PS: Is there an actual way to profile Node.JS running in AWS Lambda to inspect the possible bottlenecks?

First approach:

This code is no longer needed, as it has been externalized to the AWS IoT rule condition that invokes the Lambda function, so less invocations are done.

var ppu = json.vehiculo.patente.replace(/[^a-z0-9]/gi, '');

if (flota.indexOf(ppu) == -1) {
    return;
}

Second improvement

Move constants out of the function:

var request = require('request');
var url = 'http://endpointUrl';

Third approach:

Changed if/else if order as HTTP code 200 should cover most cases:

if (response.statusCode == 200) {
   if ('OK'.localeCompare(body) == 0) {
       success = true;
   } else if (err) {
       error = new Error('[Response ERROR]: ' + err);
   } else if ((body.indexOf('IMEI') != -1 && body.indexOf('registrada') != -1) || body.indexOf('ERROR') != -1) {
       error = new Error('[ERROR]: ' + body);
   } else {
       error = new Error('[Unknown ERROR]: ' + body);
   }
} else {
   error = new Error('HTTP response: ' + response.statusCode);
}

Is there an actual way to profile Node.JS running in AWS Lambda to inspect the possible bottlenecks?

Node Lambdas are just small Node modules. Profiling it shouldn't be any different than any other Node app. All you need is a "wrapper" app that imports your lambda script and calls handler with a fake event and context.

Your biggest enemy here won't be your code. You can only do so little with your script actually. The only bottleneck I see is that network request and that external dependency. Just ensure that endpoint responds fast.

Instead of using the request module, consider using the built-in url.format() and http.get to construct your url and do a GET request, respectively. This would remove your only dependency, whose implementation you have no control of, and probably uses both built-in modules anyways.

Also, if you have control over that endpoint, its response needs fixing. A 200 should always be successful. There shouldn't be a 200 that carries an error. Common errors include 500 (generic server error), 400 (bad input), 401 or 403 (Auth errors). Following correct headers, this would allow you to skip all the conditionals and let the status codes explain themselves. Optional body text may be added to explain the status code.


Getting all files names and paths I'm trying to get all files paths and names to index them in my database for faster search in future. I have used multithreading and parallel classes, but I'm couldn't get the performance I expected. There is an app named "Search Everything," which indexes all my files in less than 2 minutes, but my code takes much longer. I checked Windows Task Manager and it was my hard drive limiting my code it was getting to 100% of usage. I know that's normal, but how did "Search Everything" do that? is there another way, maybe reading data from master file table or something?

void GetFiles(string dir)
        {
            Task.Factory.StartNew(() =>
            {
                try
                {
                    files.AddRange(Directory.EnumerateFiles(dir));
                    Task.Factory.StartNew(() => {
                        Parallel.ForEach(Directory.EnumerateDirectories(dir), single => {
                            GetFiles(single);
                        });
                    });
                }
                catch { }
            });
        }

I am not the expert but you are creating too many threads. If possible create 5-10 threads. Whats happening here its creating too many threads and each thready try to access the resources, probably application spending too much time in sync. Try to do this. If parent directory have 5-10 sub folder(s) create one thread for each subfolder then scan subfolder using that thread only.
If you have 100 subfolders under main folder, then create 10 threads and pick subfolders in 10 blocks each. I'll try to give you sample code.


Passing a generic function as parameter Ran into this idea and I'm curious if it's possible.

I'm trying to make a single function that can test ALL of my sorting algorithms dynamically. I want to be able to pass the sorting function as a parameter and the algorithm will use the sorting function with it's own dynamic parameters. This is an example of what I'm trying to do:

class manyFunctions{
    int[] mergeSort(int[] myArray){
        ...work here;
        return sortedArray;
    }

    int[] otherSort(int[] myArray){
        ...work here;
        return sortedArray;
        }
}

class mainClass{
   public static void main(String[]Args){
      test(manyFunctions.mergeSort);
      test(manyFunctions.otherSort);

   }

     boolean test(function someSortFunction){
        int n; // number of trials
        for (int i=0; i<=n ; i++) {
        int[] A = generateArray() // another function I made
            if (isSorted(someSortFunction(A)) = false) {
                return false;
            }
        }
     return true;
 }

I can't figure out how to do this with the little bit I know about lambda expressions and function pointers. If it's possible, this technique would come in handy.

Note: This answer is based on Java 8 and newer. This does not apply to Java 7, as Java 7 does not support Lambda
For starters, your test function is wrong in multiple places:
boolean test(function someSortFunction){//This is not how you pass functions
    int n; //this has to be initialized
    for (int i=0; i<=n ; i++) {//Depending on how n is used, you may have to use < instead of <=
        int[] A = generateArray()//Missing semi-colon
        if (isSorted(someSortFunction(A)) = false) {//Comparing is done with ==, not =
            return false;
        }
    }
    return true;
}

In order to pass a function, you can either use Consumer<Void>, Supplier<Void>, Predicate<Void> or Function<Void, Void> (don't use Void as the type if you have return values)
Supplier defines return type, Consumer defines input type, and function both. Meaning:

Use Predicate when you have a boolean return type with arguments
Use Supplier when you have a return type
Use Consumer when you have an argument
Use Function when you have both an argument and a return value

Since you have both arguments and return types, use Function. The first argument you give it is the argument it receives, and the second is the return type. For an instance, in your case, this would be:
boolean test(Function<int[], int[]> someFunction)

Using Function requires calling apply to execute the method:
int[] res = someFunction.apply(input);

Before I move on, I'd like to take a second to mention naming conventions. In Java, class names always start with an upper case letter. Instances and functions start with a lower case letter. So your classes would be:
public class ManyFunctions {...}
public class MainClass {...}

Passing methods are not done using someClass.someFunction. In your case, since you are not using static methods, you need to create an instance:
ManyFunctions functions = new ManyFunctions();

now, you pass the functions:
test(functions::mergeSort);

if you make the methods static, you can just skip the instance and use the class name directly:
test(ManyFunctions::mergeSort);

So your class would be:
class MainClass{
    public static void main(String[] args){
        ManyFunctions manyFunctions = new ManyFunctions();
        test(manyFunctions::mergeSort);//Notice the missing "()" and arguments
        test(manyFunctions::otherSort);

    }

    boolean test(Function<int[], int[]> someSortFunction){
        int n = 10;//THIS HAS TO BE INITIALIZED! Otherwise, it won't compile
        for (int i=0; i<=n ; i++) {
            int[] A = generateArray();
            if (isSorted(someSortFunction.apply(A)) == false) {//comparing is done with ==
                return false;
            }
         }
         return true;
    }
}//Don't know if it was a copy-paste problem or not, but you had a missing bracket


Function that checks if array only contains elements of a specific type The function checks whether an array is of a specific type (only contains elements of that type), the types the function accepts for checking is basically all the JavaScript types plus any types that the user may define manually (using a constructor).

What would you do to make the function more sophisticated? What would you remove or add? How would you improve or extend the function?

const checkArrayType = function checkArrayType(arr, type) {

  if (!Array.isArray(arr)) {
    throw new Error('The argument "arr" must be an array!');
  }
  else if (type !== null && type !== undefined && typeof type != 'function') {
    throw new Error('The argument "type" must be a function, "null", or "undefined"!');
  }

  // 'null' and 'undefined' require a different check than the main one
  if (type === null || type === undefined) {
    return arr.every(function(e){
      return e === type;
    });
  }

  return arr.every(function(e){
    /*
     * not using 'instanceof' because it would return 'true'
     * for cases like 'subObj instanceof Object'.
     * ---------------------------------
     * using 'Object(e)' so that the check works for primitives, see:
     * https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object
     */
    return Object.getPrototypeOf(Object(e)) === type.prototype;
  });

};

console.log(checkArrayType([1,2,3,4], Number)); // true
console.log(checkArrayType(['hello', 10], String)); // false
console.log(checkArrayType([[1, 2.5], ['a', 'b']], Array)); // true
console.log(checkArrayType([null, null, null, null], null)); // true
console.log(checkArrayType([new String('hello'), new String('world')], Object)); // false

var Foo = function Foo(){};
console.log(checkArrayType([new Foo(), new Foo()], Foo)); // true

console.log(checkArrayType({a: null}, null)); // throws error
console.log(checkArrayType([], 'abc')); // throws error

One more thing, how am I supposed to deal with long comments like the one in this snippet ?

The implementation could be written more compactly using arrow functions, for example:

if (type === null || type === undefined) {
  return arr.every(e => e === type);
}

return arr.every(e => Object.getPrototypeOf(Object(e)) === type.prototype);

There are two arr.every calls at different execution paths.
You could eliminate that duplicated logic by extracting the function parameter of arr.every to a variable:

var checker;
if (type === null || type === undefined) {
  checker = e => e === type;
} else {
  checker = e => Object.getPrototypeOf(Object(e)) === type.prototype;
}

return arr.every(checker);


Push Entity Framework exceptions to business layer I am trying to write an exception handler/parser, which I want to return from DAL to the business layer. I am writing below class because I don't want my business layer to know if I am using EF under my DAL.

Please suggest if below code adhere to SOLID principles?

using System;
using System.Collections.Generic;
using System.Data.Entity;
using System.Data.Entity.Infrastructure;
using System.Data.Entity.Validation;

namespace WebAPis
{
    public enum ExceptionType
    {
        Unknown,
        EntityModified,
        EntityValidation,
        EntityUpdateException

    }
    public class EfResponse
    {
        private Exception Exception { get; set; }
        public void ParseException(Exception ex)
        {
            this.Exception = ex;
        }
        public bool IsSuccess { get { return this.Exception == null; } }
        public int RowsModified { get; set; }
        public ExceptionType Type
        {
            get
            {
                var type = ExceptionType.Unknown;

                if (this.Exception is DbEntityValidationException)
                    return ExceptionType.EntityValidation;

                if (this.Exception is DbUpdateException)
                    return ExceptionType.EntityUpdateException;

                if (this.Exception is DbUpdateConcurrencyException)
                    return ExceptionType.EntityModified;
                // add more types here
                return type;
            }
        }

        public DbUpdateExceptionDetail GetDbUpdateExceptionDetail
        {
            get
            {
                if (this.Type != ExceptionType.EntityUpdateException)
                    throw new Exception("Invaid exception details invoked");

                var toReturn = new DbUpdateExceptionDetail()
                {
                    // Parse exception here
                    ColumnName = "test column name"
                };
                return toReturn;

            }
        }
        public ConcurrencyExceptionDetail GetConcurrencyExceptionDetail
        {
            get
            {
                if (this.Type != ExceptionType.EntityModified)
                    throw new Exception("Invaid exception details invoked");

                var toReturn = new ConcurrencyExceptionDetail()
                {
                    // Parse exception here
                    MyProperty = "test property name"
                };
                return toReturn;
            }
        }
    }

    // Exception models
    public abstract class ExceptionDetail
    {
        public List<string> Exceptions { get; set; }
    }
    public class DbUpdateExceptionDetail : ExceptionDetail
    {
        public string ColumnName { get; set; }
    }

    public class ConcurrencyExceptionDetail : ExceptionDetail
    {
        public string MyProperty { get; set; }
    }

    // example dbcontext
    public class MyDbcontext : DbContext
    {
        public EfResponse Save()
        {
            var toReturn = new EfResponse();
            try
            {
                toReturn.RowsModified = this.SaveChanges();
            }
            catch (Exception ex)
            {
                toReturn.ParseException(ex);
            }

            return toReturn;
        }
    }
}

I think this is not a good idea for the following few reasons.

You write

  I don't want my business layer to know if I am using EF under my DAL.

and yet the ExecptionType enum contains all the values strictly related to Entity Framework.
Also the name of the EfResponse object already suggests it's Entity Framework related too with its Ef prefix.
Then the public class MyDbcontext : DbContext is derived from DbContext which agian is Entity Framework.

This means that the EfResponse and the MyDbcontext aren't abstract enough and they don't separate you from either Entity Framework or any other DAL. They just use different names for some EF types.

Other things that aren't implemented correctly are the two detail properties. We don't use the Get prefix for them. Only for methods. The way they work is also a little bit strange. I first need to check the type of the exception to know which property I'm allowed to use or otherwise it'll might throw. They should return null if there is no exception of this type. The only exception they are allowed to throw is the InvalidOperationException if a EfResponse object is in the Success state.

The property RowsModified should not be publically settable. This should be a part of the constructor.

Some time ago there was a question about something similar that you might find interesting: Refactoring away from exceptions. I think what you are trying to do is virtually the same. I tried it and it's a real mess when you once start using it. You're better off with a simple repository encapsulating all db operations an their exceptions.


Carousel using a function passed a target element I've made a carousel, which uses a function that is passed a target element, and from this element it adds next, prev, next next, and prev prev classes to its siblings (causing the rotation) - what do they call this, pyramid code?

Looking at this code - it works, but can't stand know that there is probably a better way to write this:

   var render = function (cont) {
      cont.parent().find('.active').removeClass();
      cont.attr('class', 'active');
      cont.parent().children().removeClass('next prev next_next prev_prev');
      cont.next().attr('class', 'next');
      cont.prev().attr('class', 'prev');
      cont.next().next().attr('class', 'next_next');
      cont.prev().prev().attr('class', 'prev_prev');
   };

You can remove the active class along with the other classes, and you can chain the prev and next calls:

var render = function (cont) {
  cont.parent().children().removeClass('active next prev next_next prev_prev');
  cont.addClass('active');
  cont.next().addClass('next').next().addClass('next_next');
  cont.prev().addClass('prev').prev().addClass('prev_prev');
};


Template for endianness-free code, data always packed as BIG-Endian It's been a while since I've been properly grilled about my code.

I had this Idea which should probably never make it into production code but still I couldn't find anything seriously wrong with it at a glance...

All your comments are welcome.

#ifndef BITFIELDMEMBER_H
#define BITFIELDMEMBER_H

/************************************************************************
 * This is for creating Endianness-free bit-fields.
 * The values are always packed as big-endian.
 * When used for arithmetics/comparison, the values are read in the
 * proper endianness for the current CPU mode.
 *
 * Note: The fields can overlap if you want
 */

template<int firstBit, int bitSize>
struct BitFieldMember
{
    typedef BitFieldMember<firstBit, bitSize> self_t;
    typedef unsigned char uchar;
    enum {
        lastBit = firstBit + bitSize - 1,
        mask = (1 << bitSize) - 1
    };
    uchar *selfArray()
    {
        return reinterpret_cast<uchar *>(this);
    }
    const uchar *selfArray() const
    {
        return reinterpret_cast<const uchar *>(this);
    }

    /* used to read data from the field */
    /* will also work with all the operators that work with integral types */
    inline operator unsigned() const
    {
        const uchar *arr = selfArray();
        const uchar *p = arr + firstBit / 8;
        int i = 8 - (firstBit & 7);
        unsigned ret = 0;
        ret |= *p;
        while (i < bitSize)
        {
            ret <<= 8;
            ret |= *(++p);
            i += 8;
        }
        return ((ret >> (7 - (lastBit & 7))) & mask);
    }

    /* used to assign a value into the field */
    inline self_t& operator=(unsigned m)
    {
        uchar *arr = selfArray();
        m &= mask;
        unsigned wmask = ~(mask << (7 - (lastBit & 7)));
        m <<= (7 - (lastBit & 7));
        uchar *p = arr + lastBit / 8;
        int i = (lastBit & 7) + 1;
        (*p &= wmask) |= m;
        while (i < bitSize)
        {
            m >>= 8;
            wmask >>= 8;
            (*(--p) &= wmask) |= m;
            i += 8;
        }
        return *this;
    }

    inline self_t& operator+=(unsigned m)
    {
        *this = *this + m;
        return *this;
    }

    inline self_t& operator-=(unsigned m)
    {
        *this = *this - m;
        return *this;
    }

    inline self_t& operator*=(unsigned m)
    {
        *this = *this * m;
        return *this;
    }

    inline self_t& operator/=(unsigned m)
    {
        *this = *this / m;
        return *this;
    }

    inline self_t& operator%=(unsigned m)
    {
        *this = *this % m;
        return *this;
    }

    inline self_t& operator<<=(unsigned m)
    {
        *this = *this << m;
        return *this;
    }

    inline self_t& operator>>=(unsigned m)
    {
        *this = *this >> m;
        return *this;
    }

    inline self_t& operator|=(unsigned m)
    {
        *this = *this | m;
        return *this;
    }

    inline self_t& operator&=(unsigned m)
    {
        *this = *this & m;
        return *this;
    }

    inline self_t& operator^=(unsigned m)
    {
        *this = *this ^ m;
        return *this;
    }

};
#endif

Usage example:

union header
{
    unsigned char arr[2];       // space allocation, 2 bytes (16 bits)

    BitFieldMember<0, 4> m1;     // first 4 bits
    BitFieldMember<4, 5> m2;     // The following 5 bits
    BitFieldMember<9, 6> m3;     // The following 6 bits, total 16 bits
};

int main()
{
    header a;
    memset(a.arr, 0, sizeof(a.arr));
    a.m1 = rand();
    a.m3 = a.m1;
    a.m2 = ~a.m1;
    return 0;
}

This sometimes wouldn't work for fields that are larger than 24 bits... the shifting may trim the most significant bits.

Your code is good, except for two methods : operator unsigned() const and self_t& operator=(unsigned m). They are too complex, and by just looking at them, it is far from clear what they are doing.

First, you can add new values to your enum :

enum {
    lastBit = firstBit + bitSize - 1,
    mask = (1 << bitSize) - 1,
    firstBitIndex = 8 - (firstBit & 7),                         // change to proper name
    nameThisValueProperly = (7 - (lastBit & 7)) & mask,         // change to proper name
};

First modify your operator unsigned() const to this :

inline operator unsigned() const
{
    const uchar *arr = selfArray();
    const uchar *p = arr + firstBit / 8;

    unsigned result = CalculateSomeValue( p );

    return (result >> nameThisValueProperly );
}

// name this method properly
inline unsigned CalculateSomeValue( const uchar *p ) const
{
    int i = firstBitIndex;
    unsigned result = *p;

    while (i < bitSize)
    {
        result <<= 8;
        result |= *(++p);
        i += 8;
    }

    return result;
}

Of course, you need to put proper names both for enum values and method names, so when someone looks into this code says "Aha, that is what is is doing". This way, you can easy increase code clarity.

Also, by making method smaller, you can easily test them.

More about this method, you can read in Clean code by Robert Martin.

You can do something similar for the self_t& operator=(unsigned m). Break it up, until you find it clean.


build a Xbonacci function that takes a signature of X elements ,each next element is the sum of the last X elements ,and returns the first n elements this is a kata from codewars, here is the description of the kata:
think of a Quadribonacci starting with a signature of 4 elements and each following element is the sum of the 4 previous, a Pentabonacci (well Cinquebonacci would probably sound a bit more Italian, but it would also sound really awful) with a signature of 5 elements and each following element is the sum of the 5 previous, and so on.
Well, guess what? You have to build an Xbonacci function that takes a signature of X elements - and remember each next element is the sum of the last X elements - and returns the first n elements of the so seeded sequence.
and this is my solution to it:
function Xbonacci(signature,n){
 let i=0;
  let k = n - signature.length;
   while(k--){
     let sumNums = 0;
     //let newArray = [...signature];
     signature.slice(i , signature.length ).map((num)=>{
      return sumNums += num;
     })

     signature.push(sumNums);
     i++
   }
  return signature;
}

the code works well but it doesn't pass the test because of optimization. Is there any way to make this code faster or more optimized? I think the problem is the slice method but I don't what to use it instead.

I think the problem is the slice method but I don't what to use it instead.

It is, but it's not as simple as taking out the slice and using something else in its place, the problem is more fundamental than that.
This algorithm scales poorly to high values of the original length of signature, by the way let's call this value S so I don't have keep spelling it all out. There is an other approach that does not depend on S, by updating the "sum of past S values" in constant time. Instead of explicitly taking the window of S elements and summing it, the sum of the previous window can be updated: one new value enters the window (added to the sum), and one old value leaves the window (subtracted from the sum). The work done for each element of the output is only one addition and one subtraction, not S additions.


Article voting schema System I'm building has Users and Articles. Users can upvote/downvote Articles and cancel their votes if they want to. Each vote changes article's score. Score equals to sum of all vote values together.

So I created three models: User, Article, ArticleVote. Even though the most natural way for me would be to be able to do user.upvote(article), for separation of concerns reasons I've put it in ArticleVote model:

class ArticleVote < ActiveRecord::Base
  belongs_to :article
  belongs_to :user

  before_destroy :before_destroy

  validates :value, inclusion: {in: [1, -1]}

  def self.upvote(user, article)
    cast_vote(user, article, 1)
  end

  def self.downvote(user, article)
    cast_vote(user, article, -1)
  end

private

  def self.cast_vote(user, article, value)
    vote = ArticleVote.where(user_id: user.id, article_id: article.id).first_or_initialize
    vote.value = value
    vote.save!
    article.score += value
    article.save!
  end

  def before_destroy
    article.score -= value
    article.save
  end
end

So now I call upvote via ArticleVote.upvote(user, article) which doesn't look too bad, but because of Rails' behavior, this doesn't work very good.

If I want to cancel an upvote for example:

article = Article.find(528953)
# article.score = 5 at this point (for example)
vote = ArticleVote.where(user: current_user, article: article).first.destroy
# article.score = 5 !!!!
article.reload
# article.score = 4

So even though I already fetched an Article from database, and used instance of it to find ArticleVote I have to fetch it again to get the actual score value. I would expect ActiveRecord to automatically set vote.article to reference my article, so when I modify it's state, I wouldn't have to fetch it once again.

Is there a way I can avoid this unnecessary article.reload call and still have valid article.score value?

(Using Rails version 4)

Rails just isn't taking chances, so when you call vote.article it's performing a new DB query, and thus creating a new instance rather than grabbing a cache. And that's a good thing! You might run into trouble otherwise, since you call save. If it was the same Article instance everywhere, you'd suddenly be calling save on it from a non-obvious point in the code.

I.e. pretend that Rails does reuse the same Article instance, and that instance has other unsaved changes. Perhaps these changes aren't valid, and in that case, the save call in ArticleVote would fail although you're only trying to change the article's score. It would also update the article's updated_at timestamp, although that may not be relevant; an article receiving or losing a vote doesn't really mean that the article itself was updated.

So, for one I'd recommend using article.update_column(:score, article.score - value) in you before_destroy method. And, yes, you'll still need to call reload elsewhere.

However, the underlying issue is that you're modifying an article "from the outside". I'd recommend a different approach altogether, with no articles.score column in the database, and using Rails' caching API instead:

class Article
  has_many :votes, class_name: "ArticleVotes"

  # score is not part of the DB schema;
  # it's computed and cached as necessary
  def score
    # get or update the cached score
    Rails.cache.fetch(score_cache_key) { votes.sum(:value) }
  end

  def flush_score_cache
    Rails.cache.delete(score_cache_key)
  end

  # let's not use or override the built-in
  # #cache_key method for this, since that
  # relies on the updated_at timestamp.
  def score_cache_key
    "article/#{id}/score" # or something similar with the ID in it
  end
end

(In the above, you may want to handle the possibility of the record being new, and thus having a nil id.)

And in ArticleVote:

class ArticleVote
  after_commit :flush_article_score

private

  def flush_article_score
    article.flush_score_cache
  end
end

Now whenever a vote is added, removed or updated, the article's score cache will be busted, forcing a article.score to do a proper recount if/when it's called.

This also has the advantage that the article's score remains consistent with the actual votes: It's computed entirely from what's in the database.  Alternatively, you can simply increment/decrement the cached value instead of flushing it, and still avoid having a database column.

Of course, there are some drawbacks too. Since it is a cache the usual disclaimers apply (you have to be diligent about busting it when necessary, or you'll hit a stale cache, etc..). And if you've got heavy voting traffic you might end up busting the cache so often it just always does the SUM() query. In that case, you could still go with your current solution of having a database column that you increment and decrement, but I might still want to stick that in a different table to avoid the aforementioned issues with implicitly calling save on an article (or, like now, overwriting an updated score with a older one if you forget to reload). You could also consider a more "raw SQL" approach of actually incrementing or decrementing rather than writing the value directly, i.e. UPDATE ... SET score=score + 1 WHERE .... That would avoid race conditions. Optimally, you might do a bit of both: A database column holding the canonical score of an article on disk, and a memory cache you use most of the time for reads, so you only have to hit the database for writes.


Closures power and concise style vs readability and maintainability? Is large use of closures actually a bad thing for readability and maintainability of the code? For example, consider the following code with (comments removed on purpose):

$withMobile  = function($c) { return $c->getMobile() !== null; };
$extractAll  = function($c) { return $c; };
$extractId   = function($c) { return $c->getId(); };

$allUniqueCustomers = $data->customers->filter($withMobile)->map($extractAll);
$customersIds       = $allUniqueCustomers->filter($withMobile)->map($extractId)->toArray();

$isNew = function($c) use($customersIds) { return !in_array($c->getId(), $customersIds); };

foreach($data->tags as $tag) :
    foreach($tag->getCustomers()->filter($isNew)->filter($withMobile) as $newCustomer) :
        $allUniqueCustomers->add($newCustomer);
    endforeach;
endforeach;

$sanitize       = function($m) { return preg_replace("/^(\+|00)/", '', $m); };
$extractMobile  = function($c) use($sanitize) { return $sanitize($c->getMobile()); };

// Unique sanitized phone numbers of customers merged with customers from tags
$customersMobiles = $allUniqueCustomers->map($extractMobile);

It looks good to me. But i love closures.

Would you consider this a nightmare to maintain and change in future in respect of traditional more verbose programming with loops?

Lambda/anonymous functions and closures are not sophisticated enough in PHP to really be reliable at this moment, and probably wont be any time soon, if ever. They are usually too bulky to be worthwhile and typically can be replaced with a normal function or loop and be easier to read and quicker to process.

The most common way to use a lambda function, and about the only way I find acceptable, is as a callback function, which is typically called in line. But that's a matter or preference. There are other cases where they are acceptable, but, as traditional closures as seen in other languages, they fall short.

If you are planning on using a lambda function or closure more than once, then you have just defeated their purpose. As Mahn pointed out, these functions are not compiled at runtime, instead they are compiled on each use. This makes running them costly. At this point you might as well have just created a traditional function. Which, by the way, is essentially what you are doing anyways by defining them as variables before using them, only backwards. The only difference is that you are running that function twice instead of once and defining it for use in the local scope only, which is the same thing as defining a function within another function, which most people consider bad taste.

While these examples can be followed easily enough, I don't think that makes true usages "easy" to read. Sure you can figure out what's going on, but the longer they are the more difficult it becomes.

Would I consider this a nightmare? No. Would I thank you for doing this? No. In my opinion it is much better to follow a sequence of functions than read through a list of lambda functions. And no offense to Mahn, but his I would find a nightmare trying to maintain. Mostly due to his indentations though...

Also, as Mahn pointed out, why are you declaring your foreach loops that way? Traditionally that format is only used in templates.


Efficient Prime Number Generator in c++ I am trying to make an efficient prime number calculator. I've seen ones that can go up to close to 1 trillion in right around 4 hours, however I left mine on overnight and didn't get anywhere near that. I am asking this to see if anyone here knows a more efficient way for me to get up to a really high number really quickly. I also want to see if my everything else is up to standard as this is only my "2nd year" (classes where you self teach) coding.

#include <vector>
#include <iostream>

int main()
{
    std::vector<int> primes;  //vector that will have all numbers in it
    primes.push_back(3);      //push back the first 2 primes (not 2 due to never checking % 2 numbers)
    primes.push_back(5);
    for (unsigned long long i = 7;; i += 2)  //loop controlling the number that will be output
    {
        for (int j = 0; i % primes[j] != 0; ++j)  //loop that "scrolls" through my vector
        {
            if (sqrt(i) <= primes[j])  //if the square root of the number you're outputting is lower than the number you're checking against
            {
                std::cout << i << std::endl;  //output
                primes.push_back(i);
                break;
            }
            else
                continue;
        }
    }
    return 0;
}

The std::vector<> is a dynamically sized container.  The allocation strategy is implementation dependent, but for sake of the argument, assume the .capacity() of a std::vector<> starts off at 1 and doubles each time the vector's size exceeds the current capacity.  This capacity increase requires a reallocation of storage, and possibly a copying of the entire array contents to a new location.  If you are generating 1 trillion primes, you'll need 40 reallocations and will have done 1 trillion number copies over the course of the 40 reallocations.  This is wasting time.  If the operating system gets involved with virtual memory paging, you are going to suck up a lot more time.

This overhead can be eliminated by simply .reserve(size_type n)-ing the expected size of the vector ahead of time.  With sufficient memory allocated to the vector upfront, no reallocations will occur, and no copies will occur.

    for (unsigned long long i = 7;; i += 2)
    {
        for (int j = 0; i % primes[j] != 0; ++j)
        {
            if (sqrt(i) <= primes[j])
            {

When i is a prime number or semi-prime number in the order of, say, one million, you are looping over all the prime numbers upto the square root of a million, to see if any of them can divide your current number.

How many different values will sqrt(i) evaluate to over those thousand iterations?  Or phrased another way, how many times are you computing the same square root?  You may want to move that sqrt(i) calculation out of the inner loop.

    for (unsigned long long i = 7;; i += 2)

If you let this loop run over night, or even over a fort-night, will this loop ever end?  No!  i will overflow the long long and become negative, and slowly increment back towards positive numbers and repeat.  Forever is not long enough.  Use, at the very least, i > 0 as the loop test condition.

A long long has at least 64 bits. A double has only a 52 bit mantissa.  This means when you pass a large long long to sqrt( ), you will end up losing a few bits of precision, which can make your sqrt() return slightly the wrong value.  When you test sqrt(i) <= primes[j], if i is greater than 2^52, and is a perfect square, you might return a value slightly less than the correct value and fail to test the last prime value, and erroneously declare the perfect square a prime number.

You are stuffing long long values into a std:vector<int> container.  After a while, they ain't gonna fit.

You are using long long for your prime number candidates, which means you expect to find some prime numbers above 2^31.

The Prime Number Theorem tells us the density of prime numbers in that range to be around 1/21.  Or, after testing numbers up to 21 billion, you should have found around 1 billion prime numbers.

Your prime number index j is declared as an int.  An int is only guaranteed to have 16 bits.  You would need at least a long to guarantee 32 bits.  But if you hope to find prime numbers up to 2^52, you have to expect to find 2^45 primes, which even exceeds a long.  Your j index should be a long long as well.

Finally, as mentioned in the comments, look at the Sieve of Eratosthenes.


Recursively walking a tree to build up a new one Could this be better factored as a reduce?

/**
 * recursively build a nested Backbone.Model or Backbone.Collection
 * from a deep JSON structure.
 *
 * undefined for regex values
 */
exports.modelify = function modelify (data) {

    if (_.isArray(data)) {
        return new Backbone.Collection(_.map(data, modelify));
    }

    else if (_.isObject(data) && !_.isDate(data)) {

        var m = util.mapo(data, function(val, key) {
            return [key, modelify(val)];
        });

        return new Backbone.Model(m);
    }
    else {
        // data is a primitive or a date
        return data;
    }
}

var mapo = exports.mapo = _.compose(_.object, _.map);

It can indeed be factored as a reduce, but I don't know if the result is "better".

So, to anyone who still has this problem so long after the question was asked, and god knows I hated Backbone for the lack of data tree-spanning Model and Collection classes, here is the treeReduce function, analogous to Array.prototype.reduceRight but for trees, first written in a simple fashion:

Assuming that each tree object has a value property (of type a which may be anything) and a children property which is an array of child tree objects,

Assuming that f :: a -> [b] -> b in Haskell notation (i.e. to an element of type a and an array of elements of type b, f associates another b-typed element),

Assuming that ini has type [b],

function treeReduce(tree, f, ini)
{
    // If we're at a leaf, use 'ini'
    if (tree.children.length === 0) {
        return f(tree.value, ini);
    }

    // Else, recursion!
    return f(tree.value, children.map(function (child) {
        return treeReduce(child, f, ini);
    }));
}

For your problem, we have to adapt treeReduce to use _.map and to not assume that the tree has the value and children properties, etc:

// Using this, we can customize the way to get the value and children of a tree
// in our treeReduce function
function treeReduceFactory(getValue, getChildren)
{
    return function customTreeReduce(tree, f, ini)
    {
        var children = getChildren(tree);
        if (children.length === 0) {
            return f(getRoot(tree), ini);
        }

        return f(getValue(tree), _.map(children, function (child) {
            return customTreeReduce(child, f, ini);
        }));
    };
}

// Our treeReduce is defined like this:
var treeReduceForBackbone = treeReduceFactory(
    // There really is no such thing as getting the 'value' of the current node
    // in our case
    function (data) {
        return data;
    },

    // How to get the "children" of something that can be an array or an object?
    function (data) {
        // If data is an array then it is itself its own children (??!)
        if (_.isArray(data)) {
            return data;
        }
        // If it's an object then its children are in the values
        if (_.isObject(data) && !_.isDate(data)) {
            return _.map(data, function (val, key) { return [key, val]; });
        }
        // If it's neither of those, it doesn't have children
        return [];
    }
);

// Finally, we define 'modelify', and indeed it looks somewhat more elegant
exports.modelify = function (data) {
    return treeReduceForBackbone(
        data,

        function (data, modelifiedDataArray) {
            if (_.isArray(data)) {
                return new Backbone.Collection(modelifiedDataArray);
            }
            if (_.isObject(data) && !_.isDate(data)) {
                return new Backbone.Model(_.object(modelifiedDataArray));
            }
            return data;
        },

        []
    );
};

And this is where we realize that because our data tree does not have a coherent interface throughout (different ways to get the children depending on the type of the node in the tree, object or array), we end up repeating checks in the node children getter and in the f function. This could obviously be fixed with yet another modification to treeReduce, but sincerely at this point I like your initial solution better.

To note, though: an iterative version of your function, as opposed to recursive, would certainly be faster, and may counterbalance the lack of readability factor in your choice of implementation.


Query Select and Insert if not exists Are there any good ways to make this createIndexBlock() method shorter?

It looks wastefully long. I have many tables. I don't like to make these kinds of methods.

import (
    "database/sql"
    _ "github.com/go-sql-driver/mysql"
)

var db, _ = sql.Open("mysql", "xxx:xxx@(database:3306)/xx")

func createIndexBlock(blockHash string) int64 {

    rows, err := db.Query("SELECT id FROM index_blocks WHERE `hash`=? LIMIT 1", blockHash)
    if err != nil {
        panic(err.Error())
    }

    for rows.Next() {
        var id int64
        err := rows.Scan(&id)
        if err != nil {
            panic(err.Error())
        }
        rows.Close()
        return id
    }

    result, err := db.Exec("INSERT INTO index_blocks (`hash`) values (?)", blockHash)
    if err != nil {
        panic(err.Error())
    }

    id, err := result.LastInsertId()
    if err != nil {
        panic(err.Error())
    }

    return id
}

It seems to me that you are trying to retrieve the ID of a specific blockHash in the database, and if the entry does not exist, you want to create a new entry. This is where INSERT ... ON DUPLICATE KEY UPDATE comes in handy.

INSERT INTO `index_blocks` (`hash`) VALUES (?)
  ON DUPLICATE KEY UPDATE id=LAST_INSERT_ID(id);
SELECT LAST_INSERT_ID();

Here's what you can do:

func createIndexBlock(blockHash string) int64 {
    result, err := db.Exec(`INSERT INTO index_blocks (hash) VALUES (?)
        ON DUPLICATE KEY UPDATE id=LAST_INSERT_ID(id)`, blockHash)
    if err != nil {
        panic(err.Error())
    }

    id, err := result.LastInsertId()
    if err != nil {
        panic(err.Error())
    }
    return id
}

Alternatively, you can look into INSERT IGNORE, followed by a SELECT. That will work too.