Find area of intersection between 2 squares rotated 45 degrees Ok so I am trying this task on a Code Challenge website, but I can't figure this out to do this fast enough. Because my current solutions has Time Limit Exceded Errors

This is the stated problem see images:

This is my current code:

def areaOfIntersection(shape1, shape2):
    def calc_points(shape):
        # shape = ["order(range)" , "x_center", "y_center"]
        points = []
        k = -1

        # k = Number of points in row-i in shape
        for i in range(shape[0]*2-1):
            if(i<=shape[0]-1):
                k += 2
            else:
                k -= 2

            # Add point to collection of points (x, y)
            for x in range(k):
                current_X = shape[1] + x - (k-1)/2
                current_Y = shape[2] + abs(shape[0] - i - 1) if i>shape[0]-1 else shape[2] - abs(shape[0] - i - 1)
                points.append( ( current_X , current_Y ) )
        return points

    p1 = calc_points(shape1)
    p2 = calc_points(shape2)

    # Return the length of the lists intersection
    return len(list(set(p1) & set(p2)))

Thus my Question is: How can above program complete faster?

I tried to add some logic to lessen the points to calculate but I am stuck...

My guess is I'm missing some simple math...

You can further reduce the time spent via taking advantage of the l1norm. The l1norm is the sum of the absolute values of a vector.  We represent our coordinates as a 2d vector, the l1norm of that is the absolute value of x and y.  Using this we can use the l1norm like distance (which happens to be the l2norm) for a diamond.  Calculating the l1 norm and comparing it against the order-1 of the diamond will give us whether or not the point is inside the diamond, provided the point the diamond we are trying to figure out if the point is inside of or not is centered at 0,0.

Utilizing the l1 norm we only need to look at one diamonds xy points, not two.  Because we only need to look at one, we can pick the smallest diamond to use to check overlap and translating the coordinates of the smaller one to a system where the center of the larger one is at (0,0).

This saves a lot of time, but if both of our diamonds are larger, and are nowhere close to each other we will still have to do a lot of work.  To avoid this we can use broad phase collision techniques to reduce computational time in these scenarios, using something akin to sweep and prune.  If we accomplish this via creating a range() that at first is our new translated smaller diamonds y - (order -1) and y + (order + 1).  We then conditionally swap these values if the max y values of the larger diamond are in smaller more convenient ranges, so if the furthest upward y point on the larger one is at a shorter distance than what we use as our max y range, the we will use that instead, or with the furthest downward y point on the larger one is larger than what we use for our min y range, we will use that instead.  If our smaller diamond isn't even in range of either of the larger diamonds y min and max values, our counting loop terminates as soon as it starts.

But we can go further with this idea.   We will do something similar with the x values, except we don't need to use the bounding box version of x cutoff values.  We can actually figure out the range of x on a per larger diamond y axis value.  Since our smaller x y coordinates are with respect to the larger diamond being at (0,0) y actually corresponds to the same y location relative to the center of the larger diamond is.  In fact because of the l1norm formula, we now know how to know the width (or half width) of a (0,0) centered diamond given one of its y coordinates.  We know that our y is within the larger diamonds y range from before, so the formula is (larger_order - 1) - abs(our_calculated_y_for_smaller) since abs(x) + abs(y) = order -1 we simply do algebra to figure out that abs(x) = order - 1 - abs(y) and since we only need to know the half width, we are actually looking for abs(x).

Now that we know the half width, we can use the same method above to figure out the range we should be looking at for this y level with respect to the larger diamond, instead of niavely looking within the bounding box.

But wait a second.  Now that we both know that we are only looking at values with that match the same y level on both our smaller and larger diamond, and we've been able to make sure we don't look at x's outside of the same x level why would we still need to check for each l1norm intersection?

What we can actually do is just sum up the difference between the max x range and min x range instead of using the l1norm, since we know exactly the range of values that overlap now!

now I doubt this is necessarily the fastest for very small diamond sizes, but I think this is the fastest no parallel asymptotic time algorithm for this specific problem that I could think of.  You could theoretically do this for N dimensional diamonds as well, but you'd have to include broad phase at each dimension. This should certainly be faster than both of your solutions for any large diamonds no matter the situation.

The time complexity is now bounded by only the order of the smallest diamond, as in O((smallest diamond order*2)-1).

Here is the implementation in python

def areaOfIntersection(shape1, shape2):
    # find smaller diamond
    (smaller, larger) = (shape1, shape2) if shape1[0] < shape2[0] else (shape2, shape1)

    # use the variables for the l1norm comparison for each point
    larger_offset_order = larger[0] - 1
    x_translate = -larger[1]
    y_translate = -larger[2]

    # use these variables to create each index in the diamond
    order = smaller[0]
    smaller_offset_order = order - 1
    # making relative to larger being at (0,0) instead of (shape2[1], shape2[2])
    x = smaller[1] + x_translate
    y = smaller[2] + y_translate

    # using comp order instead of larger[0] to work with logic below, since the number will represent the actual max
    # or min x or y value possible instead of one past it.
    larger_min_range_y = -larger_offset_order
    larger_max_range_y = larger_offset_order

    # overlap number
    k = 0

    # makes sure that the calculations at least take place in the same y coordinate intersection
    y_min_range = y + (-smaller_offset_order)
    y_max_range = y + (smaller_offset_order)
    y_min_range = larger_min_range_y if larger_min_range_y > y_min_range else y_min_range
    y_max_range = larger_max_range_y if larger_max_range_y < y_max_range else y_max_range

    for nmy in range(y_min_range, y_max_range + 1):
        half_width = smaller_offset_order - abs(nmy - y)
        # the difference from the y axis to the furthest diamond point, since it is already "centered"
        # no offset math is needed
        # equation abs(x) + abs(y) = order - 1, un normalized l1 norm equation, we only are about abs(x) so
        # it becomes x + abs(y) = order or x = order - 1 - abs(y), which is what we have here
        larger_half_width_at_current_y = larger_offset_order - abs(nmy)
        larger_min_range_x = -larger_half_width_at_current_y
        larger_max_range_x = larger_half_width_at_current_y
        x_min_range = (x + (-half_width))
        x_max_range = (x + half_width)  # max value in range, since exclusive
        # sets the range to make sure only iterating over possible subsection of x that could actually contain
        # a value from both
        x_min_range = larger_min_range_x if larger_min_range_x > x_min_range else x_min_range
        x_max_range = larger_max_range_x if larger_max_range_x < x_max_range else x_max_range
        # check if possible generated x offsets are out of range.
        # no longer need l1norm, since we can just sum up values in range
        if x_min_range < (x_max_range + 1):
            k += (x_max_range + 1) - x_min_range
    return k

print(areaOfIntersection([3, 0, -1], [5, 3, 0]))
# 8

The only faster method I can think of is some how accurately transforming these diamonds into AABB (Axis Aligned Bounding Boxes) and then computing the area overlap, but I'm  not sure how one would do so with out loosing out on the exact number of pixel overlap. In floating point however, I suspect this to be the correct method to measure area overlap since you aren't going to be losing out on discrete value precision. In a floating point scenario one could also use the generalized quadrilateral intersection


Validity of macro for printf() call in a corporate environment I wrote this program:

#include <stdio.h>
#define printS for(i=0;s[i];i++){printf("%c",s[i]);}

int i;
char s[] = "Hello World!\n";

void main()
{
  printS;
}

It outputs exactly what I want it to but I have no clue if this would be acceptable to use if I were working for a company.

Would this be considered a poorly written piece of code in a corporate environment?

Although the C standard can allow void main() under certain implementations, it's best to use int main() whenever possible.  See this for more info on the return type of main().
A loop is not a good use of a macro:

#define printS for(i=0;s[i];i++){printf("%c",s[i]);}

It should instead be a function:

void printS()
{
    for (i = 0; s[i]; i++)
    {
        printf("%c",s[i]);
    }
}

#define is a pre-processor directive (or a macro) that will replace the designated piece of code with something else.  While you can use this with loops, it's still discouraged as there are cleaner alternatives in C, such as functions.
Your two variables above main() are in global scope, which is almost always discouraged.  Anything in global that's not a constant can be modified anywhere in the program, which can introduce bugs and hurt maintenance.  You should keep variables as close in scope as possible, such as in functions.  You can then pass them to other functions as needed.

Assuming this is pre-C99, i should be declared right before the loop.  Otherwise, it should be initialized within the loop statement.
Functions in C that take no arguments should have a void parameter:

void main(void)

Consider adding more whitespace between operators for more readability.

Using your single-line loop as an example:

for(i=0;s[i];i++){printf("%c",s[i]);}

You could have something like this:

for (i = 0; s[i]; i++) { printf("%c", s[i]); }

Implementation with changes applied:

#include <stdio.h>

void printS(char s[])
{
    int i;
    for (i = 0; s[i]; i++)
    {
        printf("%c", s[i]);
    }
}

int main(void)
{
    char s[] = "Hello World!\n";
    printS(s);
}


Matplotlib-venn and keeping lists of the entries Having come upon the wonderful little module of matplotlib-venn I've used it for a bit, I'm wondering if there's a nicer way of doing things than what I have done so far. I know that you can use the following lines for a very simple Venn diagram:

union = set1.union(set2).union(set3)
indicators = ['%d%d%d' % (a in set1, a in set2, a in set3) for a in union]
subsets = Counter(indicators)

... but also want to have lists of entries in the various combinations of the three sets.

import numpy as np
from matplotlib_venn import venn3, venn3_circles
from matplotlib import pyplot as plt
import pandas as pd

# Read data
data = pd.read_excel(input_file, sheetname=sheet)

# Create three sets of the lists to be compared
set_1 = set(data[compare[0]].dropna())
set_2 = set(data[compare[1]].dropna())
set_3 = set(data[compare[2]].dropna())

# Create a third set with all elements of the two lists
union = set_1.union(set_2).union(set_3)

# Gather names of all elements and list them in groups
lists = [[], [], [], [], [], [], []]
for gene in union:
    if (gene in set_1) and (gene not in set_2) and (gene not in set_3):
        lists[0].append(gene)
    elif (gene in set_1) and (gene in set_2) and (gene not in set_3):
        lists[1].append(gene)
    elif (gene in set_1) and (gene not in set_2) and (gene in set_3):
        lists[2].append(gene)
    elif (gene in set_1) and (gene in set_2) and (gene in set_3):
        lists[3].append(gene)
    elif (gene not in set_1) and (gene in set_2) and (gene not in set_3):
        lists[4].append(gene)
    elif (gene not in set_1) and (gene in set_2) and (gene in set_3):
        lists[5].append(gene)
    elif (gene not in set_1) and (gene not in set_2) and (gene in set_3):
        lists[6].append(gene)

# Write gene lists to file
ew = pd.ExcelWriter('../Gene lists/Venn lists/' + compare[0] + ' & '
                    + compare[1] + ' & ' + compare[2] + ' gene lists.xlsx')

pd.DataFrame(lists[0], columns=[compare[0]]) \
    .to_excel(ew, sheet_name=compare[0], index=False)

pd.DataFrame(lists[1], columns=[compare[0] + ' & ' + compare[1]]) \
    .to_excel(ew, sheet_name=compare[0] + ' & ' + compare[1], index=False)

pd.DataFrame(lists[2], columns=[compare[0] + ' & ' + compare[2]]) \
    .to_excel(ew, sheet_name=compare[0] + ' & ' + compare[2], index=False)

pd.DataFrame(lists[3], columns=['All']) \
    .to_excel(ew, sheet_name='All', index=False)

pd.DataFrame(lists[4], columns=[compare[1]]) \
    .to_excel(ew, sheet_name=compare[1], index=False)

pd.DataFrame(lists[5], columns=[compare[1] + ' & ' + compare[2]]) \
    .to_excel(ew, sheet_name=compare[1] + ' & ' + compare[2], index=False)

pd.DataFrame(lists[6], columns=[compare[2]]) \
    .to_excel(ew, sheet_name=compare[2], index=False)

ew.save()

# Count the elements in each group
subsets = [len(lists[0]), len(lists[4]), len(lists[1]), len(lists[6]),
           len(lists[2]), len(lists[5]), len(lists[3])]

# Basic venn diagram
fig = plt.figure(1)
ax = fig.add_subplot(1, 1, 1)
v = venn3(subsets, (compare[0], compare[1], compare[2]), ax=ax)
c = venn3_circles(subsets)

# Annotation
ax.annotate('Total genes:\n' + str(len(union)),
            xy=v.get_label_by_id('111').get_position() - np.array([-0.5,
                                                                   0.05]),
            xytext=(0,-70), ha='center', textcoords='offset points',
            bbox=dict(boxstyle='round,pad=0.5', fc='gray', alpha=0.3))

# Title
plt.title(compare[0] + ' & ' + compare[1] + ' & ' + compare[2] +
          ' gene expression overlap')

plt.show()

So, there's basically a lot of different cases, each handled manually, and I'm wondering if there's a more "automated" / less verbose / better way of doing this. For example, can I get out the entries from the three line code snippet in the beginning somehow?

Perhaps something like the following?

values_to_sets = {a : (a in set1, a in set2, a in set3) for a in union}
sets_to_values = {}
for a, s in values_to_sets.items():
    if s not in sets_to_values:
        sets_to_values[s] = []
    sets_to_values[s].append(a)
print(sets_to_values)

This first identifies each item with a tuple indicating which sets that item belongs to. Then you flip the dictionary mapping, where each tuple maps to a list of items belonging to the combination of sets indicated in the tuple.

You could even expand this to an arbitrary number of sets:

sets = [set1, set2, set3, set4]
values_to_sets = {a : (a in s for s in sets) for a in union}


Console command tool I have created a little console tool that simply does commands that you enter.

The commands are stored in a txt file.  Right now I only got 2 commands: getTime and getCommands.

Pretty simple, but I'm not sure I'm doing anything the most optimal way, which is why I am asking here. I was thinking about creating a whole class to get the time but I wasn't sure that would make any sense (as it seems SYSTEMTIME is already a class).

Also, I was thinking about the next command I would add is something where you can input "open 'file' ", where file is the program you want to open. I googled a bit and found a function called filefindnext and filefindfirst (to search for the file), but I'm not sure if this is the best way to do it? (I was also thinking about just adding a .txt file with the dir to the predefined files)

I hope you can help clear up my code and in general make my code better.  I'm still fairly new to programming, so all constructive feedback is appreciated.

main.cpp (just a little test for my command class)

int main()
{
    command list;
    list.setCommandList();
    list.printCommands();
    while (1)
    {list.getCommand();
    }
}

commands.h

#include <iostream>
#include <fstream>
#include <string>

using namespace std;
class command
{
public:
    command();
    ~command();
    void printCommands();
    string verifyCommand(string);
    void getCommand();
    void executeCommand(string);
    void setCommandList();
private:
    std::string userinput_;
    std::ifstream file;
    string *commandlist;
    int cNr; //command number
};

commands.cpp

#include "commands.h"
#include "Time.h"
command::command(){
file.open("commands.txt");
cNr = 0;
}
command::~command()
{
    file.close();
    delete[] commandlist;
}
void command::printCommands()
{
    string line;
    cout <<"Command list:\n\n";

    for (int i = 0;i<cNr;i++)
    {
        cout <<commandlist[i] <<endl;
    }
    cout <<endl;
}
void command::setCommandList()
{
    int nrOfLines = 0;
    string cline;
    string line;
    while (getline(file,cline))
{
    nrOfLines++;
}
    file.clear();
    file.seekg(0, file.beg);
    commandlist = new string[nrOfLines];
    while (getline(file,line))
    {
        commandlist[cNr] = line;
        cNr++;
    }
}
string command::verifyCommand(string command)
{
    string line;
for (int i = 0;i<cNr;i++)
{
    if (command==commandlist[i])
        return command;
}
    return "";
}

void command::getCommand()
{
    string userinput;
    cout <<"Enter command:"<<endl;
    cin >> userinput;
    userinput_ = verifyCommand(userinput);
    if (userinput_ == "")
    {cout <<"Error, can´t find that command. Please try again"<<endl;
    getCommand();
    }
    else
    executeCommand(userinput_);
}

void command::executeCommand(string com)
{
    if (com == "getTime")
    {
        cout << string( 100, '\n' ); //clear screen
        SYSTEMTIME t;
        GetLocalTime(&t);
        cout<< "H:"<<t.wHour<<" M:"<<t.wMinute<<" S:"<<t.wSecond<<endl;
    }
    if (com == "getCommands")
    {
        cout << string( 100, '\n' );
        printCommands();
    }
}

While it is okay to put using namespace std in a local scope (such as a function), it's especially problematic to put it into a header file.  By doing this, anyone including it will be forced to have that namespace code exposed globally, and you shouldn't force that on anyone.  Although the std namespace is most commonly used, this applies to any of them.
You should be opening the file and doing user input in main(), not within the class.  You should be receiving data from a client (such as main()) and constructing an object with this data.

Move all of getCommand()'s functionality into main() and have that in your while(1) loop.  Here, you'll get the user's command and supply it to the class until the loop ends.

Another misleading thing here: the name getX() usually denotes a public "getter" function, which merely returns a data member to the client.  It's best to use this name for a function that only does this, which getCommand() does not.  As mentioned, have this functionality in main(), not within the class.  You should then no longer need this function.
Consider allowing the user to provide their own file name:

std::cout << "Filename: ";
std::string filename;
std::getline(std::cin, filename);

std::ifstream file;
file.open(filename.c_str());

You should have all of this in main() and no longer have the class manage the creation, opening, and closing of the file.  Let the user decide how they want to manage their file.  You'll also be able to terminate from main() right away if the file cannot be opened.
I'd recommend against having file.close(); in the destructor.  close() is generally unneeded as the file can close on its own, and might not work as intended here.  The destructor, in general, should primarily deallocate memory when needed.
I'm not sure why commandlist needs to be a std::string*.  This could be problematic as you're having to manage the pointer and its memory yourself, and there are better alternatives to holding this data.  In general, raw pointers should be used sparingly in C++.

In setCommandList(), you're allocating an array of std::strings to that member.  Why not have an std::vector of std::strings?  This will do all the memory management for you, so it's much safer.  You will also have access to its class functions, which could be useful.

std::vector<std::string> commandList;

This should be in place of the std::string* member, and then you can size it to nrOfLines via resize() when needed.  With this, you'll no longer need to new memory nor delete it in the destructor.  You won't even need to define your destructor anymore.
Error messages, such as the one outputted in getCommand(), should be put into std::cerr instead of std::cout.  The latter should just be used for general output.
The abbreviation and comment are pointless:

int cNr; //command number

Not only is it a very confusing abbreviation, but you cannot expect readers to see this first and then remember it throughout the code.  Just rename it to commandNumber (not very long of a name) and remove the comment.
A member function that does not modify any of its data members should be const.  This also protects against any accidental modifications and communicates to the reader that this function only performs read-only actions.

In addition, any function that receives an argument of a user-defined type (such as std::string) and is not supposed to modify it should be passed by const&.  This also prevents any accidental modifications and avoids a needless copy.

These two points apply to verifyCommand(), which is not making any modifications:

verifyCommand(std::string const& command) const {}

For printCommands():

It should be const as it does not modify any data members (see the above point):

printCommands() const {}

Leave out the label output and the extra newline.  The client shouldn't have to be forced to use them if they don't want them.  Just have them done around the function call.


Simple helper method for JUnit3 tests In JUnit 3, I cannot tell JUnit what type of exception is expected. In JUnit 4, the declaration @Test(expected=NullPointerException.class) does it.

But, unfortunately in Android development, I can use only JUnit 3. So I wrote this simple method:

public class TestingUtils {

    public static void assertExpectedException(
                           Class<? extends Exception> exceptionClass, Runnable runnable) {
        try {
            runnable.run();
            throw new AssertionFailedError(
                         "Expected exception: <" + exceptionClass.getName() + ">");
        } catch (Exception e) {
            if (!exceptionClass.equals(e.getClass())) {
                throw new AssertionFailedError(
                        "Expected exception: <" + exceptionClass.getName() + "> " +
                        "but was: <" + e.getClass().getName() + ">"
                );
            }
        }
    }
}

Using:

public void testConstructorThrowsExceptionIfArgumentIsNull() {
    TestingUtils.assertExpectedException(IllegalArgumentException.class, new Runnable() {
        @Override
        public void run() {
            new SomeTestedClass(null);
        }
    });
}

I have been though this loop before. I chose to do things in a slightly different way to you, in that instead of creating a Runnable, I instead add a try/catch block to the actual test.

Each system has pros/cons, but the differences are really cosmetic. Using a Runnable does make the JUnit logic better contained in the utility code.... but makes calling the utility test code a little bit more complicated. I am not sure which one I prefer.

I have a suggestion regarding this line here:

if (!exceptionClass.equals(e.getClass())) { ....

It is useful to be able to expect a super-type of an exception. For example, you want to trap an SQLException when you run a query, or you want to trap an IOException when you do something on a Stream..... you have the code:

TestingUtils.assertExpectedException(SQLException.class, ....);

but, the actual exception may be com.ibm.db2.jdbc.xxx.PicnicException, which is a subclass of SQLException.

Using equals() to match the exceptions is a problem.... you should use:

if (!exceptionClass.isInstance(e)) {....

One other thing, which is android specific.....

I found a bug in the Android Dalvik implementation/SDK relating to this problem... you should be setting the 'cause' for your AssertionError so that it is logged correctly. Unfortunately, the bug (which is fixed in Jelly-Bean) makes this impossible.

If you are sure you will be testing on Jelly-Bean or newer (the bug was fixed a while ago), you should also be initializing the cause on your AssertionError:

            AssertionFailedError tothrow = new AssertionFailedError(
                    "Expected exception: <" + exceptionClass.getName() + "> " +
                    "but was: <" + e.getClass().getName() + ">"
            );
            tothrow.initCause(e);
            throw tothrow;


A Tic-Tac-Toe AI based on two rules So i made a tic tac toe bot with the following rules (its not unbeatable)
    1) If bot is about to win, it will find the winning move.
    2) If bot is about to lose, it will find a move that prevents the other player from winning.
    3) if none of rule 1 or 2 is found, then it will play a random move

and i think it's highly inefficient, so if you could give optimization advice, that would be great!
class Cell(str): # a class for strings with unique ids, every time
    pass

def mediumbot_move(self, player):
    """
    medium level bot player - allows a bot with two rules to play
    1) If bot is about to win, it will find the winning move.
    2) If bot is about to lose, it will find a move that prevents the other player from winning.
    3) if none of rule 1 or 2 is found, then it will play a random move
    """
    print('Making move level "medium"')
    opponent = self.other_player[player]
    board = self.board.board # the board represented as an array like ["X", "O", " ", "O", ...]
    winning_three = set(itertools.permutations([player, player, " "])) # permutation for all the winning positions
    losing_three = set(itertools.permutations([opponent, opponent, " "])) # permutation for all the losing positions

    # the three across positions
    horizontals = [board[i:i+3] for i in range(0, 7, 3)]
    verticals   = [board[i:9:3] for i in range(3)]
    diagonals   = [board[0:9:4], board[2:7:2]]

    # if a winning pattern found win
    for pattern in (horizontals + verticals + diagonals):
        pattern = tuple(pattern)
        if pattern in winning_three:
            for winning_pattern in winning_three:
                if pattern == winning_pattern:
                    position_id = id(pattern[pattern.index(" ")]) # storing the id of the empty spot to place player on to win
                    for i in range(9): # looping through the board (array) to find the stored id, to place player
                        if id(board[i]) == position_id:
                            self.board.board[i] = Cell(player)
                            return

    # if no winning patterns found, choose a move, that would not result in a loss
    for pattern in (horizontals + verticals + diagonals):
        pattern = tuple(pattern)
        if pattern in losing_three:
            for losing_pattern in losing_three:
                if pattern == losing_pattern:
                    position_id = id(pattern[pattern.index(" ")])
                    for i in range(9):
                        if id(board[i]) == position_id:
                            self.board.board[i] = Cell(player)
                            return

    # if no patterns found, choose a random move
    available_indexes = [index for index in range(9) if board[index] == " "]
    bot_index = random.choice(available_indexes)
    self.board.board[bot_index] = Cell(player)

First of all, this is too big for one function. You have 7 levels of indentation. Split it up, or remove levels.
Second, you say "I think this is inefficient". Why? Have you timed it? How long does it take? How long do you think it should take? It is a bad idea to try to speed things up before measuring--maybe there is no problem. Maybe some other part of your program is the slow part.
If you are trying to figure out how to make your program efficient, you normally. only need to loop at the for loops:
for pattern in (horizontals + verticals + diagonals):
    for losing_pattern in losing_three:
        for i in range(9):

It should take (8 * 3 * 9)=216 iterations. Actually less, because you have if branches, but that's the maximum. You could make this smaller, yes (both of the two inner loops can be removed). But also, 216 iterations is just not that many--your computer executes a billion operations per second.
You should remove your third for loop, and every use of id, because it is confusing and hard to read, not because it's a loop.


Testing a mixin function I have a mixin function which is as follows -

function mixin(receiver, supplier) {
    for (var property in supplier) {
        if (supplier.hasOwnProperty(property)) {
            receiver[property] = supplier[property];
        }
    }
}

To test this function, I am using the ok method of Qunit -

test('Mixin Test Runner', function () {
    // using OK method
    ok(function () {
        var receiver = {
            name: 'Abul Moksud',
            age: '60'
        };

        var supplier = {
            hands: 2
        };

        mixin(receiver, supplier);

        return receiver.hasOwnProperty('hands') && (receiver.hands === 2);
    }(), "Abul Moksud now has two hands");
});

I am an absolute beginner in both unit testing and QUnit. Is my approach OK here, or is there any better option?

Your code is correct, but can be simplified. ok method usually gets an expression rather than a in-place function that is immediately evaluated.

Here is how I would write this test:

test('Mixin Test Runner', function () {
    var receiver = {
        name: 'Abul Moksud',
        age: '60'
    };

    var supplier = {
        hands: 2
    };

    mixin(receiver, supplier);

    //Assertion phase
    ok(receiver.hasOwnProperty("hands"), "hands property is defined on reciever");
    equal(receiver.hands, 2, "Abul Moksud now has two hands");
});


Defining metaclass instance methods inside of the metaclass factory instead of the metaclass itself - flawed design? I'm wondering if this is good design, or if the definition of the __init__, __call__, and __repr__ methods should be inside of mesh_class_meta itself instead of inside of the factory, mesh_class.

If it is better to for them to be defined inside of the metaclass definition, please explain specifically why? Or is there another even better way that I haven't thought of?

debugprint = print

##Metaclass for mesh data subclasses
##Receives a list of valid fields in a string (similar to namedtuple)
##Adds those fields to __slots__
class mesh_class_meta(type):
    def __new__(meta, name, bases, fields):
        if isinstance(fields, str): fields = fields.replace(',',' ').split()
        fields = tuple(fields)
        debugprint('\n-------------------------')
        debugprint('Allocating memory for new mesh_class', name)
        debugprint(meta)
        debugprint(bases)
        debugprint(fields)
        debugprint('\n')
        dctn = {}
        dctn['__slots__'] = list(fields)
        return super(mesh_class_meta, meta).__new__(meta, name, bases, dctn)

    def __init__(klass, name, bases, fields):
        debugprint('\n-------------------------')
        debugprint('Initializing class', name)
        debugprint(klass)
        debugprint(bases)
        debugprint(fields)
        debugprint('\n')
        super(mesh_class_meta, klass).__init__(name, bases, {})

##mesh_class_meta factory
##__init__, __call__, and __repr__ for the subclasses are defined here
##__call__ allows the values of slot fields to be changed by calling the class
##with a valid dictionary argument
def mesh_class(name, fields):
    new_class = mesh_class_meta(name, (object,), fields)
    def new_class__init__(self, *args, **kwargs):
        for slot, arg in enumerate(args):
            setattr(self, str(self.__slots__[slot]), arg)
        for k,v in kwargs.items():
            setattr(self, k, v)
        debugprint('\n-------------------------')
        debugprint('New', name, 'object created using', tuple(({attr: getattr(self, attr)} for attr in self.__slots__)))
        debugprint('\n')
        super(new_class, self).__init__() #Not sure if this is correct; seems to work
    new_class.__init__ = new_class__init__
    def new_class__call__(self, **kwargs):
        for k,v in kwargs.items():
            setattr(self, k, v)
        debugprint('\n-------------------------')
        debugprint('Fields in', name, 'object edited:', tuple(({k: v} for k,v in kwargs.items())))
        debugprint('\n')
    new_class.__call__ = new_class__call__
    def new_class__repr__(self):
        return str(name) + str({attr: getattr(self, attr) for attr in self.__slots__})
    new_class.__repr__ = new_class__repr__
    return new_class

##mesh classes created
Point = mesh_class('Point', 'x y')
Elpoints = mesh_class('Elpoints','i j k l')
Boundpoint = mesh_class('Boundpoint','b')

##Testing procedures##

##Create some instances
Points = [Point(p,p) for p in range(0,4,1)]
e1 = Elpoints(*tuple(p for p in Points))
b1 = Boundpoint(Points[2])

##Do some tests
assert Points[3].x == 3
Points[3](x = 4)
assert e1.l.x == 4
e1.j(y = 5)
assert Points[1].y == 5
e1.k(x=7)
assert b1.b.x == 7
assert Points[2].x == 7
e1(i = Points[3])
b1(b = Points[3])
Points[3](x=-1,y=-1)
assert e1.i.x, e1.i.y == (-1 , -1)
assert b1.b.x, b1.b.y == (-1 , -1)

It would seem to me that a simpler way (without a metaclass) to do more or less the same would be

Define a base class SlottedClass with empty __slots__.
Define  __init__, __call__, __repr__ etc. as normal instance methods in SlottedClass.
The class factory only needs to set __slots__ on the subclass it creates:

def mesh_class(name, fields):
    if isinstance(fields, str):
        fields = fields.replace(',',' ').split()
    ##Test for repeated fields
    if len(set(fields)) < len(fields):
        raise Exception
    ##Test for fields beginning with underscore
    for field in fields:
        if field[0] == '_':
            raise Exception
    new_class = type(name, (SlottedClass,), {'__slots__': fields})
    return new_class

Note by the way that list.sort() sorts in place and always returns None, so the code below does not do what it says. I've fixed it the code above.

    ##Test for repeated fields
    if list(set(fields)).sort() != list(fields).sort(): raise Exception


Go web application with a mock user repository I'm new to go and struggling to create structure of web application. I read about clean architecture and Ben Johnsons blog post about package layout. Now i want to put it all together. This is just scratch. Service abstraction looks redundant but in real project there will be services that contains more than one repository. What is your opinion in structuring project like this? And how i bootstrap it together.

import (
    "fmt"
    "html/template"
    "log"
    "net/http"
    "strconv"
)

type user struct {
    name string
}

type userRepository interface {
    getByID(id int) (*user, error)
}

type userService struct {
    userRepository userRepository
}

func (us *userService) findUser(id int) (*user, error) {
    return us.userRepository.getByID(id)
}

type mockUserRepo struct{}

func (mr *mockUserRepo) getByID(id int) (*user, error) {
    return &user{"John Doe"}, nil
}

type safeHandlerFunc func(http.ResponseWriter, *http.Request) error

type mainHandler struct {
    //session
    //logger
    view *template.Template
}

func (h *mainHandler) handle(sh safeHandlerFunc) http.HandlerFunc {
    return func(w http.ResponseWriter, r *http.Request) {
        w.Header().Set("x-custom-header", "random")
        if err := sh(w, r); err != nil {
            //return some error view
            //log error
            http.Error(w, err.Error(), http.StatusInternalServerError)
            return
        }
    }
}

type userHandler struct {
    *mainHandler
    userService *userService
}

func (uh *userHandler) getUser(w http.ResponseWriter, r *http.Request) (err error) {
    sid := r.URL.Query().Get("user_id")
    id, err := strconv.Atoi(sid)
    if err == nil {
        return
    }

    u, err := uh.userService.findUser(id)
    if err != nil {
        return
    }
    return uh.view.ExecuteTemplate(w, "user.gohtml", u)
}

func main() {
    fmt.Println("Starting web server...")

    mock := new(mockUserRepo)

    h := &mainHandler{
        view: template.Must(template.ParseGlob("views/*")),
    }
    uh := &userHandler{h, &userService{mock}}

    http.HandleFunc("/", uh.handle(uh.getUser))
    log.Fatal(http.ListenAndServe(":8888", nil))
}

To really understand the blog post, I recommend looking at its example project (https://github.com/benbjohnson/wtf - see also http branch).

Ben Johnsons posted another post detailing its steps: https://medium.com/wtf-dial/wtf-dial-domain-model-9655cd523182).

Regarding the organization of your code, it would be like this:

project.go // the exported interfaces
mock/      // the mock implementations
http/      // http handler/server
mysql/     // the mysql implementations
cmd/       // the 'glue'

A major point is that you can only import parent (sub)packages.
For instance your http subpackage can not depend on the mysql subpackage (http should only depend on the interfaces defined in project.go - project.UserService for instance).

The only exception to this rule is your main.go (or your tests).
For instance, you import project/http and project/mysql and connect them :

Since the mysql.UserService struct implements the project.UserService interface it is transparent for the http package which expects a project.UserService interface

project.go

take a look at https://github.com/benbjohnson/wtf/blob/http/wtf.go

package project

type UserID int

type User struct {
    Name string
}

type UserService interface {
    GetByID(UserID) (*User, error)
}

// HTTPService is similar

mock/user.go

take a look at https://github.com/benbjohnson/wtf/blob/http/mock/mock.go

package mock

import (
    "your/project"
)

type UserService struct {
    GetByIDFn      func(id project.UserID) *project.User, error
    GetByIDInvoked bool
}

func (s *UserService) GetByID(id project.UserID) (*project.User, error) {
    s.GetByIDInvoked = true
    return s.GetByIDFn(id)
}

mysql/user.go

Your actual implementation with a MySQL database for instance (adapt to your own user backend)

http/*.go

take a look at https://github.com/benbjohnson/wtf/tree/6d855c355488361b22b1a5ba13d9453e39141292/http

package http

import (
    "your/project"
    "net/http"
)

type HTTPService struct {
    // Here you embed some UserService Interface
    UserService project.UserService
}

func (h *HTTPService) HandleHTTP(w http.ResponseWriter, r *http.Request) {
    // Use the UserService Interface (don't care if it's mock or real)
    h.UserService.GetByID(...)
    // write response
}

cmd/mocked/main.go

Glue everything (be careful of import names conflicts!):

package main

import (
    "your/project/http"
    nethttp "net/http"
)

func main(){
    server := http.HTTPService{
        UserService: mock.UserService{},
    }

    nethttp.ListenAndServe(":8888", server)
}


Big switch statement for sorting a table I have a webpage with a table.  The table, with 10 columns, can be sorted by each column ascending or descending.

At this moment I control order by big switch:

Func<IQueryable<DeviceUsage>, IOrderedQueryable<DeviceUsage>> orderBy;
        IOrderedEnumerable<DeviceUsage> info;
        switch (sortOrder)
        {
            case "user":
                ViewBag.sortableBy = sortOrder;
                info = unitOfWork.deviceUsageRepository.Get(where, null, null, d => d.DeviceInstance, d => d.Storage, d => d.User).OrderBy(s => s.User.FullName);
                ViewBag.Desc = true;
                return PartialView(info.ToPagedList(pageNumber, 15));
            case "userDesc":
                ViewBag.sortableBy = sortOrder;
                info = unitOfWork.deviceUsageRepository.Get(where, null, null, d => d.DeviceInstance, d => d.Storage, d => d.User).OrderByDescending(s => s.User.FullName);
                ViewBag.Desc = true;
                return PartialView(info.ToPagedList(pageNumber, 15));
            case "manufacturer":
                ViewBag.sortableBy = sortOrder;
                orderBy = q => q.OrderBy(o => o.DeviceInstance.Device.Manufacturer1.Name);
                break;
            case "manufacturerDesc":
                ViewBag.sortableBy = sortOrder;
                orderBy = q => q.OrderByDescending(o => o.DeviceInstance.Device.Manufacturer1.Name);
                break;
            .
            .
            .
            default:
                ViewBag.sortableBy="";
                orderBy=q => q.OrderBy(o => o.DeviceInstance.Device.CatalogNo);
                break;
        }

This code works but looks not nice.  I think it's the longest switch I ever wrote.
Can it be someway upgraded or leave it as it is?

Explanation why there is difference in logic:

As you can see in user I'm sorting by Fullname which isn't in db ( its created locally in DAL from FirstName & LastName).  @Nick's approach is ok BUT adding a new Desc variable is a bit problematic because there is only the possibility to sort by ONE row at a time. So I would need to store it somewhere in the last sorting order.

As a quick improvement you should move the line ViewBag.sortableBy... out of your case statement and only set it in the default.

Secondly, I'm noticing there seem to be two different kinds of logic going on here: whether somebody's a user or a manager, etc. or whether the order is ascending or descending. It may be a lot easier to do something like this:

    Func<IQueryable<DeviceUsage>, IOrderedQueryable<DeviceUsage>> orderBy;
    IOrderedEnumerable<DeviceUsage> info;
    ViewBag.sortableBy = ascending? sortField : sortField + "desc";

    switch (sortField)
    {
        case "user":

            var deviceUsage = unitOfWork.deviceUsageRepository.Get(where, null, null, d => d.DeviceInstance, d => d.Storage, d => d.User);
            var orderFunction = s => s.User.FullName;
            ViewBag.Desc = true;
            info = ascending? deviceUsage.OrderBy(orderFunction); : deviceUsage.OrderByDescending(orderFunction);
            return PartialView(info.ToPagedList(pageNumber, 15));
        case "manufacturer":
            var orderFunction = o => o.DeviceInstance.Device.Manufacturer1.Name;
            orderBy = ascending? q => q.OrderBy(orderFunction) : q => q.OrderByDescending(orderFunction)
            break;
        .
        .
        .
        default:
            ViewBag.sortableBy="";
            orderBy=q => q.OrderBy(o => o.DeviceInstance.Device.CatalogNo);
            break;
    }

This code obviously requires you to split out whether an order is ascending or not beforehand. This can be done with string manipulation if necessary, but there is probably an easier way using code you haven't shown us.


Picking seats in a matrix I'm trying to write a Python class that creates a matrix of zeros, then uses a random number generator to pick 'seats' on the matrix. It changes the zero in that seat to a one, until the matrix is all ones. Can someone critique/correct my code? (I also want the 'agent' to check its surroundings on the matrix, and try 3 times to find a seat that is 2 seats away from any ones.)

import random
import numpy as np
#cs is grid size or seats, ns is number of agents
class Seats():
    def __init__(self, cs, ns):
        self.cs = cs
        self.ns = ns
        self.seats = np.zeros([self.cs, self.cs],dtype=int)

    def foundseat(self):
        foundseat = False
        tries = 0
        while foundseat == False and tries <= 3:
            x = random.randint(0, self.cs)
            y = random.randint(0, self.cs)
            if self.seats[x][y] < 1:
                self.seats[x][y] = 1
                foundseat = True
            else:
                tries += 1

   def goodseat(self, x,y):
    empty_neighbors = 0
    for neighbor_x in range(x-1,x+1):
        for neighbor_y in range(y-1,y+1):
            if self.seats[neighbor_x][neighbor_y] == 0:
                empty_neighbors = empty_neighbors + 1

Docstring: You should include a docstring at the beginning of every method/class/module you write. This will help documentation identify what your code is supposed to do.
Simplify boolean comparison: Instead of evaluating if found == False, you should evaluate the variable itself, if not found:, since it contains a boolean value.
Simplify variable addition: Instead of doing x = x + 1, you should do x += 1, since it's more compact and easier to read. You switch between these two in your program, but it good to stay consistent with one practice.
Variable Naming: You did a good job of naming the variables snake_case, but the method names are lacking. This is a link to PEP-8 Naming Conventions, which can provide more insight.

Updated Code

"""
Module Docstring:
A description about your program goes here
"""

import random
import numpy as np

class Seats():
    """
    A class for containing an array of seats, finding seats,
    and counting good seats
    """
    def __init__(self, cs, ns):
        """
        Seats Class Init Method

        :param cs: The size of the grid of seats
        :param ns: The number of agents

        """
        self.cs = cs
        self.ns = ns
        self.seats = np.zeros([self.cs, self.cs], dtype=int)

    def found_seat(self):
        """
        Attempts to find an open seat in the array `self.seats`
        """
        found = False
        tries = 0
        while not found and tries <= 3:
            x = random.randint(0, self.cs)
            y = random.randint(0, self.cs)
            if self.seats[x][y] < 1:
                self.seats[x][y] = 1
                found = True
            else:
                tries += 1

    def good_seat(self, x, y):
        """
        Determines the amount of empty neighbors around the seat

        :param x: The X coordinate of the seat
        :param y: The Y coordinate of the seat

        """
        empty_neighbors = 0
        for neighbor_x in range(x - 1, x + 1):
            for neighbor_y in range(y - 1, y + 1):
                if self.seats[neighbor_x][neighbor_y] == 0:
                    empty_neighbors += 1


Web crawler that charts stock ticker data using matplotlib I've built a web crawler using the BeautifulSoup library that pulls stock ticker data from CSV files on Yahoo finance, and charts the data using matplotlib.  I'm wondering if there are any ways to improve the code I've written, because there are some parts that I think could be a lot better.

import urllib.request
from matplotlib import pyplot as plt
from bs4 import BeautifulSoup
import requests

def chartStocks(*tickers):

    # Run loop for each ticker passed in as an argument
    for ticker in tickers:

        # Convert URL into text for parsing
        url = "http://finance.yahoo.com/q/hp?s=" + str(ticker) + "+Historical+Prices"
        sourceCode = requests.get(url)
        plainText = sourceCode.text
        soup = BeautifulSoup(plainText, "html.parser")

        # Find all links on the page
        for link in soup.findAll('a'):
            href = link.get('href')
            link = []
            for c in href[:48]:
                link.append(c)
            link = ''.join(link)

            # Find the URL for the stock ticker CSV file and convert the data to text
            if link == "http://real-chart.finance.yahoo.com/table.csv?s=":
                csv_url = href
                res = urllib.request.urlopen(csv_url)
                csv = res.read()
                csv_str = str(csv)

                # Parse the CSV to create a list of data points
                point = []
                points = []
                curDay = 0
                day = []
                commas = 0
                lines = csv_str.split("\\n")
                lineOne = True
                for line in lines:
                    commas = 0
                    if lineOne == True:
                        lineOne = False
                    else:
                        for c in line:
                            if c == ",":
                                commas += 1
                            if commas == 4:
                                point.append(c)
                            elif commas == 5:
                                for x in point:
                                    if x == ",":
                                        point.remove(x)
                                point = ''.join(point)
                                point = float(point)
                                points.append(point)
                                day.append(curDay)
                                curDay += 1
                                point = []
                                commas = 0
                points = list(reversed(points))

                # Plot the data
                plt.plot(day,points)
                plt.ylabel(ticker)
                plt.show()

Composing small functions

chartStocks would be more readable if you split it into a handful of smaller functions, roughly like this:

def chartStocks(*tickers):
    for ticker in tickers:
        page = getTickerPage(ticker)
        csv_url = findCSVUrl(page)
        csv = getCSV(csv_url)
        day, points = parseCSV(csv)
        plot_data(ticker, day, points)

        # Or, if you're allergic to temporary variables:
        day, points = parseCSV(getCSV(findCSVUrl(getTickerPage(ticker))))

This approach lets you clearly see the "pipeline" your data is passing through, and lets you test and reuse smaller pieces individually.

Arguably it would be even cleaner to define def chartStock(ticker) to handle the case of one ticker, so that chartStocks is just

def chartStocks(*tickers):
    for ticker in tickers:
        chartStock(ticker)

The only thing to be careful of here is to make sure to design your functions to handle errors properly -- either check that each return value is not None before calling the next function, or allow them to take None as a parameter and just return nothing in that case.

str.startswith

This:

# Find all links on the page
for link in soup.findAll('a'):
    href = link.get('href')
    link = []
    for c in href[:48]:
        link.append(c)
    link = ''.join(link)

    # Find the URL for the stock ticker CSV file and convert the data to text
    if link == "http://real-chart.finance.yahoo.com/table.csv?s=":
        # ...

could be simplified with str.startswith:

def findCSVUrl(soupPage):
    CSV_URL_PREFIX = 'http://real-chart.finance.yahoo.com/table.csv?s='
    for link in soupPage.findAll('a'):
        href = link.get('href', '')
        if href.startswith(CSV_URL_PREFIX):
            return href

I've also provided a not-found value of '' so that if link has no href, startswith won't get called on None.

Skipping first row

Instead of using a lineOne flag when looping over lines:

lineOne = True
for line in lines:
    if lineOne == True:
        lineOne = False
    else:
         # continue parsing line...

you can just start from after the first row with a slice:

for line in lines[1:]:
    # ... continue parsing line

CSV parsing

Python has a built-in CSV parsing module that could simplify a lot of your parsing. It would do the splitting-by-commas for you and return either a list or dict of fields for each row, depending on what you ask for. You would end up with roughly this:

def parseCSV(csv_text):
    csv_rows = csv.reader(csv_text.split('\n'))
    days = []
    points = []

    for day, row in enumerate(csv_rows):
        close = float(row[4])
        days.append(day)
        points.append(close)

    return days, points

where the enumerate function would give you the same zero-based days list as you currently have.

Actually, since it seems like days will just be the list [0 .. len(points)], you could skip the enumerate and simply define days after you've parsed all the points, and toss in a list comprehension if you'd like for good measure:

def parseCSV(csv_text):
    csv_rows = csv.reader(csv_text.split('\n'))

    points = [float(row[4]) for row in csv_rows]
    days = list(range(len(points)))

    return days, points


Checking 160,000 records in a database for changes I am checking over 160,000 records in my database every 24 hours to see if there were changes in the incoming data coming from API and my existing data in the database. If there were I am updating those, otherwise I'm doing nothing. Checking 189 days takes 10 seconds and 160,000 about 2.5 hours.

What can I improve in my code to speed this up?

$listings = Array('23169','23567','23114','5487'); //Rooms to check (189 days in each room)
$count_listings = count(listings); //count the rooms
$continue = TRUE;
$x = 0;

while ($continue == TRUE && $x < $count_listings) {
    foreach ($listings as $list) {
        //Generates a link from the looped @list
        $url = $this->generate_link($list);
        $results_page = $this->http_get_contents($url, $this->generate_proxy()); //JSON output
        $calendar_month = json_decode($results_page, true); //Into Array

        $data = Array();

        foreach ($calendar_month['calendar_months'] as $month) {
            foreach ($month['days'] as $day) {
                $data[] = $day; //Getting each day
            }
        }
        //Super_unique removes duplicate days
        foreach ($this->super_unique($data) as $day_info) {
            $data_listing = Array(
                'list_id'           => $list,
                'list_price'        => $day_info['price']['local_price'],
                'list_currency'     => $day_info['price']['local_currency'],
                'list_date'         => $day_info['date'],
                'list_available'    => $this->check_list_available((int)$day_info['available'])
            );
            //First get incoming result and search in the database
            $check = $this->db->select('list_price, list_available')
                ->from('airbnb_lists_price')
                ->where("(list_id = '{$data_listing['list_id']}' and list_date = '{$data_listing['list_date']}')")
                ->get();
            //If the record exists
            if ($check->num_rows() > 0) {
                foreach ($check->result() as $row) {
                    //If the record info was changed update
                    if ($row->list_price != $data_listing['list_price'] || $row->list_available != $data_listing['list_available']) {
                        $this->db->where("(list_id = '{$data_listing['list_id']}' and list_date = '{$data_listing['list_date']}')");
                        $this->db->update('airbnb_lists_price', $data_listing);
                    }
                    else {
                    //do nothing
                    }
                }
            }
        }
        $x++;
    }
    if ($x > $count_listings) {
        $continue = FALSE;
    }
}

There is multiple things that can be optimized in this code.

It is pretty obvious that most time is spent in HTTP requests to remote API and in database queries, so these points should be first in list of optimizations.

Your code doesn't tells much about what is coming from remote API and you didn't tell if you really need to run all requests for this API each day. If amount of requests for remote API can be somehow reduced - you should do it. If not - you need to think about how "fresh" information from this API you need to have, maybe part of information can be cached and re-used.

Besides this, if you need information from remote API on daily basis - you may think about refactoring API requests loop into separate service that  will run during whole day and store received information in some local data storage (e.g. database or some cache). Then your primary script will not need to spend time waiting for remote API responses and will run much faster.

If you know that significant amount of information that came from remote API updates less often than daily and you need to process only information that really changed - you should think about hashing. For example you can query API (even in this loop), clean it up from any information that may change at every request (e.g. request date) and, calculate hash from it (e.g. sha1) and store pair (url=>hash) somewhere. Then on the next iteration of script run you will be able to compare hash from new API response with already available hash and if they're same - completely skip data comparison with database.

About database queries - you should check that your query uses database indexes, it may significantly improve query performance.

And less intensive, but still useful optimizations is to avoid repetitive creation of things that you need to create once. For example:

Do you need to call generate_proxy() at each request?
You should use prepared SQL queries if your library supports them
You may collect information that should be updated into database into local array and then send it into database as a single transaction at the end of data processing - it may be faster.


Generation of products of power Given positive integers a,b,c and limit, I want to generate all products having the form
$$p^aq^br^c \leq limit$$ less than limit (where p,q,r are distinct primes). a,b and c are not necessary distinct.

I tried something like:

primes= [ 2,3,5,...] # primes up to 10**8
[ (p**a)*(q**b)*(r**c) for p in primes for q in primes for r in primes if (p**a)*(q**b)*(r**c) <= limit ]

But it is very slow because len(primes)(=5761455) is high.

Then I tried a very ugly code for printing values.
It generates all values for p < q < r (p, q, r primes)

def generate3(a,b,c,limit):
    global primes
    i1 = 0
    i2 = 1
    i3 = 2
    while i1 < len(primes) and i2 < len(primes) and i3 < len(primes) and (primes[i1]**a)*(primes[i2]**b)*primes[i3]**c < limit:
        print(str(i1)+" "+str(i2)+str(i3))
        while i1 < len(primes) and i2 < len(primes) and i3 < len(primes) and (primes[i1]**a)*(primes[i2]**b)*primes[i3]**c< limit:
            while i1 < len(primes) and i2 < len(primes) and i3 < len(primes) and (primes[i1]**a)*(primes[i2]**b)*primes[i3]**c< limit:
                print(str(primes[i1])+" "+str(primes[i2])+" "+str(primes[i3])+" "+str((primes[i1]**a)*(primes[i2]**b)*(primes[i3]**c)))
                i3 = i3 + 1
            i2 = i2 +1
            i3 = i2 +1
        i1 = i1 + 1
        i2 = i1 + 1
        i3 = i2 + 1

Is there a more pythonic way to do this?
Is there a better method for generating these products of powers quickly?

global

there is no need to make primes a global. You only read from it, but don't assign to it, you can use it as is. It will be even faster if you make primes a local variable by passing it in as a parameter, so Python uses the  LOAD_FAST bytecode instead of LOAD_GLOBAL. Since primes is called, indexed and sliced a lot, this can make a difference.

while condition

i1 < len(primes) and i2 < len(primes) and i3 < len(primes) and .... Since i1<i2<i3, only i3 < len(primes) is needed. If you use len(primes) so often, it pays to make it a local variable.

return, don't print

your method immediately prints the results. In general it is better to split the calculation and presentation, so to let the method return or yield the values, and another method do the presentation

looping

I suggest you watch the talk Loop like a Pro by David Baumgold. It's recommended material for every python programmer.

Instead of looping over the indices, you can loop over the primes-list immediately.

For p, and the first index (i), you can loop over enumerate(primes)

Then you can use this index i to slice primes to only include the elements with an index larger than p and go on to r, so you arrive at the following, naive implementation:

def generate4_naive(a, b, c, limit):
    for i, p in enumerate(primes):
        for j, q in enumerate(primes[i+1:], i+1):
            for r in primes[j+1:]:
                product = p**a * q**b * r**c
                if product < limit:
                    yield p, q, r, product

If you include the early breaks, you arrive at something like this:

def generate4(a, b, c, limit):
    for i, p in enumerate(primes):
        for j, q in enumerate(primes[i+1:], i+1):
            sub_product = p**a * q**b
            for r in primes[j+1:]:
                product = sub_product * r**c
                if product > limit:
                    break
                yield p, q, r, product
            if sub_product * primes[j+1]**c > limit:
                break
        if p ** a * primes[i+1] **b * primes[i+2] ** c > limit:
            return

then I also used primes as a local variable by changing the signature: def generate4b(a, b, c, limit, primes):

Performance:

limit = 1_000_000
%timeit tuple(generate3(2,3,4, limit=limit))

16.6 µs ± 166 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)

%timeit tuple(generate3b(2,3,4, limit=limit, primes=primes))

16.2 µs ± 138 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)

%timeit tuple(generate4(2,3,4, limit=limit))

14.2 µs ± 643 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)

%timeit tuple(generate4b(2,3,4, limit=limit, primes=primes))

7.72 ms ± 516 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)

%timeit list(generate4_naive(2,3,4, 1_000)) # primes also only to 1000

940 ms ± 84.9 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)


Observable storage - revised Previous question:

JavascriptObservable Storage

The idea is to have some storage interface that exists of different buckets - known as Eagles:

Eagle('bucket1');
Eagle('bucket2');

Every Eagle data is stored as Feathers.

var e = Eagle('fooEagle');
e.set('myFeather',{
    'foo' : 'bar';
});

These Feathers can be observed for changes (nothing more at this moment):

var f = e.get('myFeather');
f.on('change',function(f, key) {
    console.log(key+ ' index of the Feather was changed to'+ f.get(key)));
});
f.set('foo','newer foo');

You could also retrieve data in a long chain like this:

Eagle('fooEagle').get('myFeather').get('foo');

Does this seem like a good approach? If so, how would you implement events like 'delete'. And would it be useful to let those events bubble up to the Eagle? If so, how?

window.Eagle = (function(){
    "use strict";

    var _eagleInstances = [],
        _featherInstances = [];

    var Feather = (function() {
        var _data = {},
            _observers = {};

        var F = function(data) {
            this._id = _featherInstances.length;
            _featherInstances[this._id] = this;
            _data[this._id] = data;
        }

        F.prototype.has = function(key) {
            return _data[this._id].hasOwnProperty(key);
        }

        F.prototype.get = function(key) {
            return this.has(key) ? _data[this._id][key] : undefined;
        }

        F.prototype.set = function(key, data) {
            _data[this._id][key] = data;
            this.notify('change', key);
            return this;
        }

        F.prototype.on = function(action, callable) {
            if ( ! _observers.hasOwnProperty(action) ) {
                _observers[action] = [];
            }

            _observers[action].push(callable);
        }

        F.prototype.notify = function(action, info) {
            if ( _observers.hasOwnProperty(action) ) {
                for ( var i = 0; i < _observers[action].length; i++ ) {
                    _observers[action][i](this, info);
                }
            }
        }

        return F;
    })();

    var Eagle = (function(){
        var _feathers = {};

        var E = function() {
            this._id = _eagleInstances.length;
            _feathers[this._id] = {};
            _eagleInstances[this._id] = this;
        }

        E.prototype.has = function(featherIdentifier) {
            return _feathers[this._id].hasOwnProperty(featherIdentifier);
        }

        E.prototype.get = function(featherIdentifier) {
            if ( !this.has(featherIdentifier) ) {
                this.set(featherIdentifier, {});
            }
            return _featherInstances[_feathers[this._id][featherIdentifier]];
        }

        E.prototype.set = function(featherIdentifier, data) {
            var f = new Feather(data);
            _feathers[this._id][featherIdentifier] = f._id;
            return f;
        }

        return E;
    })();

    return function(eagleIdentifier) {
        if ( !_eagleInstances.hasOwnProperty(eagleIdentifier) ) {
            _eagleInstances[eagleIdentifier] = new Eagle();
        }
        return _eagleInstances[eagleIdentifier];
    }

})();

I am just going to comment on a few things because I don't think I have a great high level understanding of your code is meant to do (not that is bad for any reason).

In general I would avoid adding scope to variables if it is not needed, for example instead of declaring _data that is available to the whole F class and reference it with a special id I would just put a new data object on each instance.  Keep in mind limiting the scope of variables would apply for _featherInstances and _eagleInstances as well but there could be some business reason for it that I don't understand.

     var F = function(data) {
                this._id = _featherInstances.length;
                _featherInstances[this._id] = this;
                this._data = data;
            }

        F.prototype.has = function(key) {
            return this._data.hasOwnProperty(key);
        }
   ...

Another note to make is has might not work as you expected it.  For example:

new F({key: undefined}).has('key') would return true


Create Lists for Drop-Down Menus in Different Sheet I'm currently working with a workbook utilized at my work that has the following code.  It currently takes about 10 seconds to complete, and needs to be executed several times throughout utilizing the workbook.  Is there any way to speed up this process?

Right now, the user specifies enters the drop down items needed in Sheet2, the code creates an additional sheet named "Filter Builder", enters the data in a column, which is then copied into the corresponding cell in the Filter Options sheet, and finally pulled by Sheet1 to create a drop down list.

Sub Macro1()

'TURNS OFF THE SCREEN UPDATING SO IT BRING YOU TO THE FILTER OPTIONS TAB AFTER THE MACRO IS COMPLETE
Application.ScreenUpdating = False

'FILTERS OUT THE ROWS THAT DO NOT HAVE "SPECIAL VALIDATION" AND HIDES THEM
ActiveSheet.Range("$A$1:$AF$5150").AutoFilter Field:=5, Criteria1:="Special Validation"

'ADDS ANOTHER SHEET FOR THE DATA TO BE ENTERED INTO
Sheets.Add After:=Sheets(Sheets.Count)

'NAMES THE SHEET AS "FILTER BUILDER"
ActiveSheet.Name = "Filter Builder"

'PASTES THE DATA IN FILTER BUILDER THEN DELIMITS IT
Sheets("Checklist Builder").Columns("F:F").EntireColumn.Hidden = False
Sheets("Checklist Builder").Columns("F:F").Copy Sheets("Filter Builder").Columns("A:A")
Sheets("Checklist Builder").Columns("G:G").Copy Sheets("Filter Builder").Columns("B:B")

'DELIMITS THE DATA SO THE TEXT SPLIT UP IN TO DIFFERENT COLUMNS, DIVIDED UP BY COMMAS
Application.CutCopyMode = False
Columns("B:B").TextToColumns Destination:=Range("B1"), DataType:=xlDelimited, _
    TextQualifier:=xlDoubleQuote, ConsecutiveDelimiter:=False, Tab:=False, _
    Semicolon:=False, Comma:=True, Space:=False, Other:=False, FieldInfo _
    :=Array(Array(1, 1), Array(2, 1), Array(3, 1), Array(4, 1)), TrailingMinusNumbers:= _
    True

'SELECTS THE DATA TO TRANSPOSE IT
Sheets("Filter Builder").Rows("2:1150").Select

'COPIES AND TRANSPOSES THE DATA IN THE "FILTER BUILDER" EXCEL SHEET TO "FILTER OPTIONS"
Application.CutCopyMode = False
Selection.Copy
 Sheets("Filter Options").Range("K1").PasteSpecial Paste:=xlPasteValues, Operation:=xlNone, SkipBlanks _
    :=False, Transpose:=True

'AUTO FITS THE COLUMNS IN THE EXCEL SHEET
Sheet3.Cells.EntireColumn.AutoFit

'TURNS OFF THE DISPLAY ALERT TO DELETE THE SHEET
Application.DisplayAlerts = False

'DELETES THE SHEET
Sheets("Filter Builder").Delete

'HIDES COLUMN 'F' IN THE CHECKLIST BUILDER AGAIN
Sheets("Checklist Builder").Columns("F:F").EntireColumn.Hidden = True

'TURNS THE DISPLAY ALERTS BACK ON
Application.DisplayAlerts = True

'TAKES AWAY THE AUTO FILTER THAT WAS APPLIED IN THE "CHECKLIST BUILDER" SHEET
Sheet2.Range("$A$1:$AF$150").AutoFilter Field:=5

'GOES BACK TO THE CHECKLIST BUILDER SHEET
Sheets("Checklist Builder").Select
Range("A1").Select

Application.ScreenUpdating = True

End Sub

This will be hard to get right without the actual workbook.

Stop adding and removing Sheets("Filter Builder"); just hide it.
Avoid Select or Activate. Reference: Excel VBA Introduction Part 5 - Selecting Cells (Range, Cells, Activecell, End, Offset)
By working with the data in arrays, you can avoid filtering & unfiltering you data as well as hiding and unhiding the columns.
Use With block to qualify your ranges. Reference: Excel VBA Introduction Part 12 - With Statements
Replace your static ranges ,e.g. .Range("$A$1:$AF$5150"),  dynamic ranges ,e.g. .Range(.Range("A" & .Rows.Count).End(xlup),"AF1"))

Here are some inconsistencies

The initial ActiveSheet, Sheet2 and Sheets("Checklist Builder") all appear to be the same Worksheet.  Pick 1 and use it in a With block. (e.g. With Sheet2)
Range("$A$1:$AF$5150").AutoFilter - .Range("$A$1:$AF$150").AutoFilter

This should work "As Is" but it is hard to say without the actual Workbook.  You will have to add a Worksheet, name it "Filter Builder", hide it and forget about it.  I would guess that it should take less than 1 sec to execute.

Sub Macro1()

'TURNS OFF THE SCREEN UPDATING SO IT BRING YOU TO THE FILTER OPTIONS TAB AFTER THE MACRO IS COMPLETE
    Application.ScreenUpdating = False

    Dim data As Variant, results As Variant
    Dim count As Long, x As Long
    With Sheets("Checklist Builder")
        data = .Range("$A$1:$AF$5150").Value
        ReDim results(1 To UBound(data), 1 To 2)
        'Fill array with the values
        For x = 1 To UBound(data)
            If count = 1 Or UCase(data(x, 7)) = "SPECIAL VALIDATION" Then
                count = count + 1
                results(count, 1) = data(x, 7)
                results(count, 2) = data(x, 8)
            End If
        Next
    End With

    'DELIMITS THE DATA SO THE TEXT SPLIT UP IN TO DIFFERENT COLUMNS, DIVIDED UP BY COMMAS

    With Sheets("Filter Builder")
        .Cells.Clear
        .Range("A1").Resize(UBound(results), UBound(results, 2)).Value = results

        .Columns("B:B").TextToColumns Destination:=Range("B1"), DataType:=xlDelimited, TextQualifier:=xlDoubleQuote, ConsecutiveDelimiter:=False, Tab:=False, Semicolon:=False, Comma:=True, Space:=False, Other:=False, FieldInfo:=Array(Array(1, 1), Array(2, 1), Array(3, 1), Array(4, 1)), TrailingMinusNumbers:=True

        .Rows("2:1150").Copy
        Sheets("Filter Options").Range("K1").PasteSpecial Paste:=xlPasteValues, Operation:=xlNone, SkipBlanks _
                                                                                :=False, Transpose:=True
        Application.CutCopyMode = False
    End With
    Application.ScreenUpdating = True

End Sub


High Performance Blocking Queue C# I am developing a high-performance(real-time) system, it requires message passing between its component. speed and accuracy is at most priority. I have explored a few options. ConcurrentQueue<T> is super fast BlockingCollection<T> provides a better API. also BlockingCollection<T> waits for the producer to produce the message. the problem is BlockingCollection<T> is not fast enough for the job. also, it uses locks internally which may lead to locking contention. so I tried to develop a queue that provides blocking behavior(waits for the producer to produce message) and does not use lock internally. here what I have come up with

    public class BlockingConcurrentQueue<T> : IDisposable
    {
        private readonly ConcurrentQueue<T> _internalQueue;
        private AutoResetEvent _autoResetEvent;
        private long _consumed;
        private long _isAddingCompleted = 0;
        private long _produced;
        private long _sleeping;

        public BlockingConcurrentQueue()
        {
            _internalQueue = new ConcurrentQueue<T>();
            _produced = 0;
            _consumed = 0;
            _sleeping = 0;
            _autoResetEvent = new AutoResetEvent(false);
        }

        public bool IsAddingCompleted
        {
            get
            {
                return Interlocked.Read(ref _isAddingCompleted) == 1;
            }
        }

        public bool IsCompleted
        {
            get
            {
                if (Interlocked.Read(ref _isAddingCompleted) == 1 && _internalQueue.IsEmpty)
                    return true;
                else
                    return false;
            }
        }

        public void CompleteAdding()
        {
            Interlocked.Exchange(ref _isAddingCompleted, 1);
        }

        public void Dispose()
        {
            _autoResetEvent.Dispose();
        }

        public void Enqueue(T item)
        {
            _internalQueue.Enqueue(item);

            if (Interlocked.Read(ref _isAddingCompleted) == 1)
                throw new InvalidOperationException("Adding Completed.");

            Interlocked.Increment(ref _produced);
            if (Interlocked.Read(ref _sleeping) == 1)
            {
                Interlocked.Exchange(ref _sleeping, 0);
                _autoResetEvent.Set();
            }
        }

        public bool TryDequeue(out T result)
        {
            if (Interlocked.Read(ref _consumed) == Interlocked.Read(ref _produced))
            {
                Interlocked.Exchange(ref _sleeping, 1);
                _autoResetEvent.WaitOne();
            }

            if (_internalQueue.TryDequeue(out result))
            {
                Interlocked.Increment(ref _consumed);
                return true;
            }
            return false;
        }
    }

I seek your expertise to review

Sequence Guarantee
Any Performance improvement
Am I doing anything wrong which may break in race condition

This question has a number of misconceptions

You are unlikely to develop a real-time system using .NET because of
unbounded GC times
ConcurrentQueue<T> is better than BlockingCollection<T> not because
it does not use locks. It actually uses more locks, just in a
smarter way.
ConcurrentQueue<T> provides worse API exactly because it is faster.
Most of the missing APIs are missing because there is no way of
implementing them efficiently and in a thread-safe manner

Your code has a number of weaknesses:

I don't see any reasonable use for IsAddingCompleted and friends
I am not sure there are no race conditions. For example, lets assume we have produced == consumed == 0. Thread1 adds an item and is just before incrementing produced. Thread2 calls TryDequeue, since produced==consumed, it goes through and stops right before changing sleeping. Thread1 continues, changes produced to 1, checks if sleeping is 0, which it is and quits without setting the event. Thread2 continues to Wait(). Now we have a task and an eteranlly-waiting client.
There is no way to abort a waiting TryDequeue and finish gracefully


An array of values. looping many times to calculate mean, closest matching array value, and then finding its index I have an array of values. (cols)

I want to find out which index of cols contains the value closest to the mean of cols.

Can I combine these functions so that I'm not repeating loops?

let mean = cols.reduce((prev, curr) => prev + curr) / cols.length;
let closest = cols.reduce((prev, curr) => (curr - mean) < (prev - mean) ? curr : prev);
let meanCol = cols.indexOf(closest);

You can remove the last line searching for the index since you can get the index in the reduce method.

Btw, you forgot to add Math.abs to deal with positive value when comparing.

Demo:

const cols = [10, 20, 30, 40, 50, 60, 70, 80, 90, 100]

const mean = cols.reduce((prev, curr) => prev + curr, 0) / cols.length;

const { value, index } = cols.reduce((prev, curr, i) => {
    // You have deal in absolute
    if (Math.abs(curr - mean) < Math.abs(prev.value - mean)) {
        return { value: curr, index: i }
    } else {
        return prev
    }
}, { value: cols[0], index: 0 });

console.log({ mean, value, index })


A simple card game simulator This is a homework question, and I have written the code but wasn't sure if I had picked the right data structure for this job and minimised time complexity. Can anyone give me some feedback, anything about space complexity or potential bugs that can occur is appreciated as well.
Problem Description

MagicTheGathering
Aim: deal with a series of command (SUMMON, KILL, HEAL). Each SUMMON
command will create a monster and each monster has a initial HP value.
Each KILL command will kill the monster with the lowest HP. And each
HEAL command applies to every monster in your list.
Input: N, the number of commands. Followed by N lines of commands.
Output: Each KILL command requires you to print the HP of the monster
that was killed. At the end of the series of commands, print the HP of
the remaining. monsters.
Input example:
8
SUMMON 30
SUMMON 20
SUMMON 50
HEAL 5
KILL
SUMMON 7
KILL
HEAL 10

Output example:
25
7
45 65

Attempt
import java.util.*;
import java.util.stream.*;

class  Yugioh {
    public static void main(String [] args) {
        Scanner sc = new Scanner(System.in);
        int count = sc.nextInt();
        PriorityQueue<Integer> queue = new PriorityQueue(Comparator.naturalOrder());
        for (int i =0;i<count;i++) {
            String cmd = sc.next();
            if (cmd.equals("SUMMON")) {
                int health = sc.nextInt();
                queue.add(health);
            } else if (cmd.equals("HEAL")) {
                int health = sc.nextInt();
                PriorityQueue<Integer> newQueue = new PriorityQueue<>(Comparator.naturalOrder());
                List<Integer> data = queue.stream().map(x -> x + health).collect(Collectors.toList());
                newQueue.addAll(data);
                queue = newQueue;
            } else if (cmd.equals("KILL")) {
                System.out.println(queue.poll());
            }
        }
        while (!queue.isEmpty()) {
            System.out.println(queue.poll());
        }
    }
}

You should use an ENUM or a class for the list of commands.

E.G:

public ENUM Command
{
    private String value;

    SUMMON("Summon"),
    ATTACK("Attack"),
    HEAL("Heal")

    public Command(String value) { this.value = value; };
}

// String cmd = Commands.valueOf(sc.next());
// if (cmd == Command.HEAL) ...
// ( Or better yet, use a switch statement)

You should also be dealing with invalid input. (E.g in the default of the switch).

You've mentioned Magic the gathering, but your class name is Yugioh. As we all know, those are two very different games.

The variable 'health' is declared twice, I don't think the name makes sense for 'Summon'.

The case for 'Heal' looks overly complicated. You shouldn't need to create a 'newQueue'. Try to take away X amount of health for each item (either using a for-loop or lambdas). No need to re-order the queue.


Processing XML configuration which stores regular expressions and format strings for a documentation tool I'm investigating some feature for the ScalaDoc tool, which would allow library writers to link to documentation created by third party tools like JavaDoc.

My idea is to have some (XML) configuration which specifies which package prefixes belong to which vendor and how to create a valid URL from the the class name, stored in the XML as regular expressions and format strings.

I only need one public method getLink(entity: String): Option[String] which either returns Some(validURL) or None, if the package name is not supported by the given configuration. Everything else can be changed as much as I want if it improves the code.

Consider this code:

object ExternalReferences2 {
  import java.util.regex._
  import collection.mutable._

  private object Mapping {
    def fromXml(mapping: scala.xml.NodeSeq) = {
      new Mapping(mapping \ "vendor" text, mapping \ "match" text, mapping \ "format" text)
    }
  }
  private case class Mapping(vendor: String, matches: String, format: String) {
    private val pattern = Pattern.compile(matches)
    private var currentMatcher: Matcher = null

    def hasValue(entity: String) = {
      currentMatcher = pattern.matcher(entity);
      currentMatcher.matches
    }

    def getValue = {
      val range = 0 until currentMatcher.groupCount()
      val groups = range
                     .map (currentMatcher.group(_))
                     .filterNot (_ == null)
                     .map (_.replace('.', '/'))
      format.format(groups: _*)
    }
  }

  private val config =
    <external-links>
      <mapping>
        <vendor>OpenJDK</vendor>
        <match>{ """^(javax?|sunw?|com.sun|org\.(ietf\.jgss|omg|w3c\.dom|xml\.sax))(\.[^.]+)+$""" }</match>
        <format>{ "http://download.oracle.com/javase/7/docs/api/%s.html" }</format>
      </mapping>
    </external-links>

  private def lookUp(entity: String) =
    (config \ "mapping").view
      .map(m => Mapping.fromXml(m))
      .find(_.hasValue(entity))
      .map(_.getValue)

  private val links: Map[String, Option[String]] = Map[String, Option[String]]()
    .withDefault(entry => {val result = lookUp(entry); links += ((entry, result)); result})

  def getLink(entity: String) = links(entity)
}

How can I improve that code?

The necessity to call hasValue before getValue is pretty bad, but I don't want to create the matcher twice.
Which environment variables do I have to use so that the tool can pick up the configuration from the current directory or from an arbitrary place with a command line switch?
Is there a better way to do simple caching than using withDefault to mutate the underlying map?
Any idea on how to improve lookUp?
Are there any ideas for better names? I'm not happy with Mapping, Link, Reference ... things are all over the place.

Any idea on how to improve lookUp?

Use a concurrent cache.  The point is, regardless how good or bad you lookup function is, nothing beats a cache-hit performance wise..

  Is there a better way to do simple caching than using withDefault to
  mutate the underlying map?

getOrElseUpdate(key, value)


TCP retarder for network delay simulation I would like to simulate delay over network, for that I have implemented a simple retarder (=introduce random delay while maintaining order of messages) using Boost to handle sending itself. It appears to work as intended. However, since parallel programming is not trivial especially when coupled with networking, I would like to hear feedback more experienced C++ programmer. Mostly, I am concerned about the parallelization(deadlocks, resource leaks) and possible side effects on the communication I might have missed.

class SendRetarder
{
    typedef std::pair<session_ptr, message_t> args_t;//to whom and what
    std::queue<args_t> message_queue;//messages to be sent
    std::mutex m;
    std::condition_variable cv;
    const size_t max_wait = 10000;

    void send_loop()
    {
        while (true)
        {
            boost::system::error_code ec;
            args_t args;
            size_t wait;
            {
                std::unique_lock<std::mutex> lk(m);
                cv.wait(lk, [&] { return !message_queue.empty(); });
                wait = (rand() % max_wait) / message_queue.size();//to prevent unchecked growing
                args = message_queue.front();
                message_queue.pop();
            }
            std::this_thread::sleep_for(std::chrono::milliseconds(wait));
            boost::asio::write(args.first->socket_, boost::asio::buffer(&args.second, sizeof(message_t)), ec);
            if (ec) std::cerr << ec.message().c_str() << std::endl;
        }
    }

public:
    SendRetarder() {
        std::thread(&SendRetarder::send_loop, this).detach();
    }

    void async_send(session_ptr client, message_t& msg)
    {
        {
            std::lock_guard<std::mutex> lk(m);
            message_queue.push(args_t(client, msg));
        }
        cv.notify_one();
    }
};

where message_t is the contents of the message consisting of integer types and session_ptr is a smart pointer wrapper around socket shared with client itself:

struct message_t {
    msg_id_t message_id;
    tag_t message_tag;
    sender_t sender_id;
    union { uint32_t value; bool success; } data;
}

typedef std::shared_ptr<Session> session_ptr;
class Session : public std::enable_shared_from_this<Session> {
    tcp::socket socket_;
    sender_t name_;
    //...
}

I see a number of things that may help you improve your code.

Consider using existing tools

First, let me note that I've had a need for a similar thing before, but what I eventually used was tc which was tremendously useful for my testing and required essentially no additional code on my part.  In my case, although neither of the devices I was testing ran Linux, I put a Linux box between the devices and did all of the data logging from there.  Not only did this allow me to arbitrarily delay some number of packets, but also allowed the deliberate injection of bit errors, etc. which very much expanded the possible test repertoire.

Make sure you have all required #includes

The code uses a number of things from both the standard library and from boost but doesn't show the required #includes.  Carefully consider which #includes are part of the interface (and belong in the .h file) and which are part of the implementation and provide them to users (and reviewers!) of the code.

Lock shared resources before use

The send_loop() routine prints any error message to std::cerr but does so without a lock.  This is a problem because std::cerr is a shared resource.  If this thread is in the middle of printing a message when another thread writes, you may get interleaved messages.  While this is not fatal, it's easily avoided by controlling access to std::cerr via a mutex.

Don't launch a thread without considering object lifetime

The SendRetarder constructor simply launches a detached thread with this line:

std::thread(&SendRetarder::send_loop, this).detach();

However, this has a major problem.  What if the object is destroyed after this thread is launched?  Everything within the send_loop() routine that relies on object variables is going to be potentially using de-allocated memory and non-existent objects!  Prevent this by using shared_from_this and a std::shared_ptr instead of just passing this.

Provide a destructor

What happens to the detached thread if the object is destroyed?  Nothing, at the moment, and that's a problem.  If the object is destroyed, it seems to me that either there should be a way to signal the detached thread that it's time to quit.  One simple way to do that is to close the underlying socket which is one thing that should probably be done in a destructor.

Consider using a better random number generator

If you are using a compiler that supports at least C++11, consider using a better random number generator.  In particular, instead of rand, you might want to look at std::uniform_int_distribution and friends in the <random> header.

Consider future users

The line that writes the message  to the socket looks like this:

boost::asio::write(args.first->socket_,
                   boost::asio::buffer(&args.second, sizeof(message_t)), ec);

That works right now, but what happens if you decide to have a variable size message?  For example, consider this:

using message_t = std::string;

Now the sizeof() operator will probably return the size of a pointer rather than the size of the actual object which will lead to maddenly difficult-to-find bugs.  I'd recommend adding a size() method to your existing message_t object and then changing the write to look like this:

boost::asio::write(args.first->socket_,
                   boost::asio::buffer(&args.second, args.second.size())), ec);

Then if the message_t is ever changed to something else, it will be required to implement a size() also.  One way to enforce that, if needed, would be to have a message_t base class and have a pure virtual size() but that may be overkill for your purposes.


Making a "remember login" function I'm trying to make a safe "remember me / auto login" function on my site and as I'm just a hobby programmer I would like someone professional to take a look at my code this far. This site will probably never see the light of day I'm just doing this because I find it fun and learning, but I still like to do it correct. I have read this.

Login.php

$query = $db->prepare('SELECT id, username, password FROM users WHERE username = ? OR email = ?');
$query->execute(array($_POST['username'], $_POST['username']));
$row = $query->fetch();

if ($row and password_verify($_POST['password'], $row['password'])) {

    // Remember?
    if (isset($_POST['remember'])) {

        $selector = base64_encode(random_bytes(9));
        $authenticator = random_bytes(33);

        setcookie('remember', $selector.':'.base64_encode($authenticator), time() + 864000);

        $query = $db->prepare('INSERT INTO auth_tokens (selector, token, userid, expires) VALUES (?, ?, ?, ?)');
        $query->execute(array(   $selector, hash('sha256', $authenticator), $row['id'], date('Y-m-d\TH:i:s', time() + 864000)  ));
    }

    $_SESSION['userid'] = $row['id'];
    $_SESSION['username'] = $row['username'];

    exit(header('Location: /'));
}

This is my logout:

session_destroy();

if (!empty($_COOKIE['remember'])) {
    setcookie('remember', '', time() - 1000);
    $db->exec('DELETE FROM auth_tokens WHERE userid = '.$_SESSION['userid']);
}

exit(header('Location: '.SITE_URL));

And this is my code in index.php to check if the user has the cookie and log them in:

if (empty($_SESSION['userid']) && !empty($_COOKIE['remember'])) {

    list($selector, $authenticator) = explode(':', $_COOKIE['remember']);

    $query = $db->prepare('SELECT * FROM auth_tokens WHERE selector = ?');
    $query->execute(array($selector));
    $row = $query->fetch();

    if (hash_equals($row['token'], hash('sha256', base64_decode($authenticator)))) {
        $_SESSION['userid'] = $row['userid'];
    }
}

One thing I need to do is check if the cookie has expired and log them out, but other than that what do you think? Am I missing something?

You are doing good. You are referencing a good example.
The flow will be as below:

Once the user logs in and have checked "remember me", set the cookie with a unique token for user.
Next time when the user comes back, check the cookie with the token value in DB.
If it matches, allow the user to login.
If the token is missing redirect to login.

Ref:
http://jaspan.com/improved_persistent_login_cookie_best_practice


Redundant DataContext ? - LINQ to SQL This seems like a pretty basic question, so bear with me...Say I have two (or many) methods in the same class that need a LINQ to SQL DataContext. Additionally, I am needing the DataContext in other classes as well. Does it make sense to create the new DataContext every time?

Should I be creating this DataContext in a different data access class and then inheriting it on each page? I feel like this is the way it should be done, but I'm still learning LINQ to SQL and just looking for some best practices. Can anyone point me to good examples of this?

Thanks,

Code for reference:

protected void LoadProducts()
{
    StoreDataClassesDataContext db = new StoreDataClassesDataContext();
    var query = from p in db.Products

            select new
            {
             p.ProductID,
             p.Description,
             p.Price
            };

    dgProducts.DataSource = query;
    dgProducts.DataBind();
}

protected void LoadSales()
{
    StoreDataClassesDataContext db = new StoreDataClassesDataContext();
    var query = from s in db.Sales

                select new
                {
                s.SaleID,
                s.SaleDate,
                s.Total
                };

    dgSales.DataSource = query;
    dgSales.DataBind();
}

I can't make a thorough review right now, but here's a quick fix that should help you structure things up.

What you have is probably working, but you're mixing-up presentation with data concerns. I don't think these methods should be returning void. Picture this:

protected IEnumerable<SalesInfo> LoadSales()
{
    using (var db = new StoreDataClassesDataContext())
    {
        var result = db.Sales.Select(s => new SalesInfo {
                                                SaleId = s.SaleID,
                                                SaleDate = s.SaleDate,
                                                Total = s.Total })
                             .ToList();
        return result;
    }
}

What you get is a bunch of SalesInfo objects that contain your query results. Bind that to your UI instead.

It's probably normal you're re-creating the context everytime, you want these things to be as short-lived as possible.


Bubble sort in Scala I wanted to implement a bubble sort in Scala following the steps described in: http://www.bbc.co.uk/education/guides/z22wwmn/revision/3

import scala.collection.mutable.MutableList

val result = bubbleSort(List(3,2,4,1,5))

def bubbleSort(list: List[Int]): (List[Int]) = {
  def doIteration(list: List[Int]): (List[Int], Boolean) = {
    val mutableList = MutableList(list: _*)
    val numberOfPairs = 0 until mutableList.length - 1
    for (pair <- numberOfPairs) {
      if (list(pair) > list(pair + 1)) {
        val tempOld = list(pair + 1)
        mutableList(pair + 1) = list(pair)
        mutableList(pair) = tempOld
      }
    }
    val hasSwaps = list != mutableList.toList
    (mutableList.toList, hasSwaps)
  }

  var result = doIteration(list)
  while(result._2){
    result = doIteration(result._1)
  }
  result._1
}

I am aware I have used a few things which are not idiomatic in Scala such as:

MutableList
while loop
var

I would appreciate suggestions for incremental improvements to make the code more functional and idiomatic Scala.

Below I've written up an implementation of a recursive bubble sort that addresses the main concerns you mentioned. The areas your code can be improved matches what you instinctively pointed out in your bullet points.

Whenever you use a List or MutableList in Scala remember that it is in fact a linked list and so accessing an element takes linear time. This could add a significant number of operations to an implementation of bubble sort that utilizes a List. As a substitute I used an Array which offers constant time access to elements. It is also worth mentioning that the Array container is mutable in Scala even if it is declared as a val.

We can kill two birds with one stone by rewriting the while loop as a recursive procedure. The two birds are while and var and our trusty stone is recursion.

def bubbleSort(arr: Array[Int]): Array[Int] = {

  val hasSwaps =
    for {
      i <- (1 until arr.length)
    } yield {
      if (arr(i - 1) > arr(i)) {
        val tmp    = arr(i)
        arr(i)     = arr(i - 1)
        arr(i - 1) = tmp
        true
      }
      else false
    }

  if (hasSwaps.reduce(_ || _)) bubbleSort(arr)
  else arr
}


Retry logic for delegates I created this class which supposed to take a method and try to execute it. If it fails lets say 10 times, it will throw the original exception thrown by the delegate.

Do you have any ideas for improvement about that code?

I am particularly interested, in where to put this code. Right now it is within a FileHandler assembly because the code was used for serialization.
But since it can be used generally I want to move it to a different assembly. Unfortunately I did not come up with a better name (other than "Utility" or "Helper"). Unfortunately I did not come with a really meaningful name...maybe something related to "invoke" or so?

public static class FileHandler
{
  private const int MAX_TRIES = 10;
  private const int WAIT_FOR_RETRY_DELAY = 250;

  public static T ExecuteFuncWithRetry<T>(Func<T> func, string errorMessage)
  {
    var retryFunc = new FuncWithRetry<T>(errorMessage, func);
    var ret = retryFunc.PerformAction();
    return ret;
  }

  private class FuncWithRetry<T>
  {
    private readonly string _errorMessage;

    private readonly Func<T> _func;

    public FuncWithRetry(string errorMessage, Func<T> func)
    {
      _errorMessage = errorMessage;
      _func = func;
    }

    public T PerformAction()
    {
      var tries = 1;
      Exception exception = null;
      do
      {
        try
        {
          var retval = _func();
          return retval;
        }
        catch (Exception e)
        {
          exception = e;
          Thread.Sleep(WAIT_FOR_RETRY_DELAY);
        }
        finally
        {
          tries++;
        }
      } while (tries <= MAX_TRIES);

      throw new Exception(_errorMessage, exception);
    }
  }
}

Assemblies and naming

Assemblies are units of deployment -- you should put this code in the assembly it's being used in; don't over-think it. Use namespaces, not assemblies, for logical partitioning of code, and don't create a new assembly just because some code may be used elsewhere. Wait until it is used elsewhere.

However

However, I would question the usefulness of such a function.

You catch an Exception, which is generally not recommended -- what happens if it's an OutOfMemoryException? But you're forced to catch an Exception as you have no way of knowing which exceptions could be thrown.

You throw an Exception, forcing callers in turn to catch an Exception. And only the last exception is kept around (in the inner exception) -- what if some exceptions require special handling, or logging? All the information contained in them is lost. As @CodesInChaos commented, an AggregateException would be better.

What if it's being called on the UI thread, and it wouldn't make sense to call Thread.Sleep between retries?

The problem with this method is that it tries to be too general. Retry logic will often have very specific requirements that can't be abstracted away into one function. For instance, say I'm getting exceptions that contain HTTP status codes. If I'm getting 503 Service Unavailable, I may want to try again with exponential back-off. But if it's a 400 Bad Request, there is something wrong with my code and I should log it without retrying.