Rspec - testing basic functionality - redundant testing? Being pretty new to testing, I was wondering how many of these tests make sense.

restaurants_controller_spec.rb

require 'spec_helper'

describe RestaurantsController do

  describe "GET #index" do
    before(:each) { get :index }
    let(:restaurants) { create_list :restaurant, 5 }

    it "populates @restaurants" do
      expect(restaurants).to match_array assigns(:restaurants)
    end
    it "renders the :dailycious view" do
      expect(response).to render_template :index
    end
  end

  describe "GET #show" do
    let(:restaurant) { create :restaurant }
    before(:each) { get :show, id: restaurant }

    it "assigns @restaurant" do
      expect(restaurant).to eq assigns(:restaurant)
    end
    it "renders :show" do
      expect(response).to render_template :show
    end
  end

  describe "GET #new" do
    before(:each) { get :new }

    context "user signed in" do
      before(:each) do
        login_user
        get :new
      end

      it "assigns @restaurant" do
        expect(assigns(:restaurant)).to be_a_new(Restaurant)
      end
      it "renders :new" do
        expect(response).to render_template :new
      end
    end

    context "user signed out" do
      it "redirects to login page" do
        expect(response).to redirect_to new_user_session_path
      end
    end
  end
end

support/controllers/controller_macros.rb

module Controllers
  module ControllerMacros
    def login_admin
      @request.env["devise.mapping"] = Devise.mappings[:admin]
      sign_in create(:confirmed_admin)
    end

    def login_user
      @request.env["devise.mapping"] = Devise.mappings[:user]
      sign_in create(:confirmed_user)
    end
  end
end

The new test makes sense to me, since it behaves differently when logged in / logged out, but do the other ones make sense as well?

Especially the index test seems superfluous. Doesn't this just check basic rails functionality? The index tests of other controllers will surely share most of the same code. Isn't this a contradiction to the DRY-Principle?

My setup includes Rspec, Capybara & FactoryGirl.

Next to the explanations I'm glad if you review this code, maybe DRY things up etc. :)

Sidenote: I adjusted my code to this tutorial.

Doesn't this just check basic rails functionality?

Yes and no. Yes in the sense that if you run the scaffold command, and follow the usual RESTful conventions, then that's what you get. But that's not Rails; it's just your specific use/application of the Rails framework.

You could easily make a controller that doesn't have an index action. Or you could make a controller where the index action renders the edit template for some strange reason. And you tests would help you verify and/or avoid that, depending on your needs.

True, there'll be repetition across controllers, but repetition in tests not quite as concerning as repetition in your app code. But you can look at RSpec's"shared examples" if you want to share common specs.

Now, RSpec is about specifications, so when you write a test that checks whether a variable has been assigned, you're specifying and checking the contract between the controller and the view; the view will expect that variable to be there, so you ensure that the controller sets it.

Similarly, when you specify that expect(response).to render_template :show it's because that's what that action is supposed to do. Yes, it's usually ok to assume that the show action will render the show template - but it's not a law.

If you run rspec with the --format documentation option, what you'll see is a detailed explanation of what your code does - or is supposed to do, e.g.:

RestaurantsController
  GET #show
    assigns @restaurant
    renders :show

And yes, that's indeed what it's supposed to do. It's the conventional setup for a show action, so you might think "that goes without saying!". And yeah, it kinda does... if you indeed wrote it that way. Maybe there's a bug.

You current code looks fine by the way. A few notes though:

it "renders the :dailycious view" do
  expect(response).to render_template :index
end

Your spec says "renders the :dailycious view", but you're actually checking something else? If the idea is that the index template renders a partial called "dailycious" then you should check that in a view spec. Right now your spec is like asking someone "Did you remember to buy milk?", and the person answering "I went to the store". Well... good for you, but that doesn't answer the question.

Also,

GET #show
  it "assigns @restaurant"

This is just me being super-duper pedantic, but consider rewording that more like "assigns the requested Restaurant record as @restaurant". If you look at the "documentation" output above, all it just tells you that something's been assigned to @restaurant. Not what it is.
Admittedly, this is not a great example, since it's pretty clear already.  But being descriptive is a good habit to get into. Try to keep that "documentation" output in mind, and ask yourself if the spec your writing is sufficiently explanatory. Just imagine that every other part of the app is written by someone else entirely, and, besides yourself, that's who your specs are for. As though they're looking at the documentation output to learn how your code works.

On a practical note, you probably don't need to create 5 restaurant records when testing the index action. It's a good idea to create more than 1 record, to ensure that the controller does load multiple records, but you don't need more than 2 records to check that. FactoryGirl has a create_pair method you can use for just this sort of thing.


Extract name, address and phone number from some web pages using multiprocessing I've written a script in Python using the multiprocessing module to scrape values from web pages (one page per subprocess). As I'm very new to multiprocessing, I'm not sure whether I did everything in the right way. It works without error, though.

Here goes the full script:

import requests
from lxml.html import fromstring
from multiprocessing import Pool

link = "https://www.yellowpages.com/search?search_terms=coffee&geo_location_terms=Los%20Angeles%2C%20CA&page={}"

def create_links(url):
    response = requests.get(url).text
    tree = fromstring(response)
    for title in tree.cssselect("div.info"):
        name = title.cssselect("a.business-name span")[0].text
        try:
            street = title.cssselect("span.street-address")[0].text
        except IndexError: street = ""
        try:
            phone = title.cssselect("div[class^=phones]")[0].text
        except IndexError: phone = ""
        print(name, street, phone)

if __name__ == '__main__':
    links = [link.format(page) for page in range(1,4)]
    with Pool(4) as p:
        p.map(create_links, links)

Any idea to make it more robust will be highly appreciated.

Proper use of a Pool p should include p.close() and p.join() (in this order).

Cases of the websites not responding should be handled. Requests should have a timeout, the timeout exception is caught, and non-200 should be handled as well.

Other than that, the script is correct for a one time extract of a few pages, but will need to be extended if you intend to produce a high volume daemon. If that is the case, usage of Pool can be questionnable and has multiple alternatives, such as using Scrapy framework scheduler or using a celery broker for high-level handling of workers (this will avoid your workers to crash entirely on exceptions, among a few other benefits).


Different class name based on condition in React I have a very simple component, which should be styled differently based on the passed condition.

I wonder if there is a better approach to refactor this piece of code?

function RandomComponent({ number, condition }) {
    let classNameToUse;

    switch (condition) {
        case 'high':
            classNameToUse = 'number number--high';
            break;
        case 'medium':
            classNameToUse = 'number number--medium';
            break;
        default:
            classNameToUse = 'number';
    }

    return
{number}
;
}

if you're just trying to decide between number--high number--medium or '' based on the condition variable, then just make a helper function that does that

then do return
{number}


Path finding function Could this be made more efficient, and/or simpler?  It's a path-finding function.  It seems to work, but do you see any potential cases where it could fail?

// Searches for a path from [start] to [end].
// Predicate [passable] should take an std::pair<int,int> and return true if the node is passable.
// Nodes in path are stored in [out].
// Return value is a pair.  With first being a bool to indicate if a path was found,
// and second an iterator for the end of the path
template<typename OutputIterator, typename PassablePR>
std::pair<bool, OutputIterator> ShortestPath(std::pair<int,int> start, std::pair<int,int> end, PassablePR passable, OutputIterator out)
{
    typedef std::pair<int,int> node;

    std::pair<bool, OutputIterator> ret(false, out);

    // queue of nodes expanding out from the starting point
    std::queue<node> q;

    // keep track of visited nodes so we don't visit them twice
    std::vector<node> visited_nodes;
    auto visited = [&visited_nodes] (node n) {
        return std::find(visited_nodes.begin(), visited_nodes.end(), n) != visited_nodes.end();
    };

    // link child nodes to parents
    std::map<node,node> path_tree;

    q.push(start);
    while(q.empty() == false)
    {
        auto parent = q.front();
        if(passable(parent) && !visited(parent))
        {
            visited_nodes.push_back(parent);

            if(parent == end)
            {
                ret.first = true;
                std::vector<std::pair<int,int>> path;
                auto i = path_tree.find(parent);
                while(i != path_tree.end())
                {
                    path.push_back(i->first);
                    parent = i->second;
                    path_tree.erase(i);
                    i = path_tree.find(parent);
                }

                // path is currently from end to start, so reverse it
                std::copy(path.rbegin(), path.rend(), ret.second);

                return ret;
            }

            auto child(parent);

            // node to the left
            --child.first;
            q.push(child);
            if(path_tree.find(child) == path_tree.end())
                path_tree[child] = parent;

            // right
            child.first += 2;
            q.push(child);
            if(path_tree.find(child) == path_tree.end())
                path_tree[child] = parent;

            // above
            --child.first;
            --child.second;
            q.push(child);
            if(path_tree.find(child) == path_tree.end())
                path_tree[child] = parent;

            // and below
            child.second += 2;
            q.push(child);
            if(path_tree.find(child) == path_tree.end())
                path_tree[child] = parent;
        }
        q.pop();
    }
    return ret;
}

Testing it out:

int main()
{
    int grid[5][5] =
    { { 0, 1, 0, 0, 0 },
      { 0, 0, 0, 1, 0 },
      { 0, 1, 0, 1, 0 },
      { 0, 1, 0, 1, 0 },
      { 0, 0, 0, 1, 0 } };

    std::vector<std::pair<int,int>> path;

    auto ret = ShortestPath(std::pair<int,int>(0,0), std::pair<int,int>(4,4),
        [&grid] (std::pair<int,int> n) -> bool {
            if(ben::WithinBoundsInclusive(std::pair<int,int>(0,0), std::pair<int,int>(4,4), n) == false)
                return false;
            return grid[n.first][n.second] == 0;
        },
        std::back_inserter(path));

    if(ret.first)
    {
        std::cout << "Path found\n";

        std::for_each(path.begin(), path.end(),
            [](std::pair<int,int> n) {
                std::cout << '(' << n.first << ',' << n.second << ")\n";
            });
    }
    else
        std::cout << "Path not found\n";
}

// keep track of visited nodes so we don't visit them twice
std::vector<node> visited_nodes;
auto visited = [&visited_nodes] (node n) {
    return std::find(visited_nodes.begin(), visited_nodes.end(), n) != visited_nodes.end();
};

Every time you have to check whether a point has been visited, this is going to scan over the entire list of visited_nodes. Better to do a std::set

// path is currently from end to start, so reverse it
std::copy(path.rbegin(), path.rend(), ret.second);

Rather then constructing the path and reversing it, you could construct the path backwards.

When it comes to constructing all of the child nodes, I think your approach is confusing. You'd be better of constructing a new node rather then reusing the node. As it is, its not clear what you are doing. That section of code is also repetitive. I'd do something like

int x_dir = {0, 0, 1, -1}
int y_dir = {1, -1, 0, 0}

for(int pos = 0; pos < 4; pos++)
{
    node child = parent;
    child.first += x_dir[pos];
    child.second += y_dir[pos];
    ...
}

Other notes:

The use of map is going to O(log), if you use a multidimensional array to hold the information for each point you can do it in O(1) time.

Using pairs to hold co-ordinates is easy. However, I suggest that its better to have a point class with x and y members. I think it makes what is going on clearer. Additionally, the point class can encapsulate the neighboring cell logic.


Reading lines of integers into a list of lists I have a text file containing integers in different lines which I want to read into a list of lists.

So

01
11 14
45 50 09

should become

((1)
 (11 14)
 (45 50 9))

The real file is longer so I want to read it in using CL instead of transforming it using Emacs' macros etc. It is important that every line becomes exactly one list and the data is stored as integers, i.e. 09 becomes 9.

I ended up with the following (using the external library split-sequence):

(defun read-data (&optional (file "data.txt"))
  "Returns the numerical data in FILE as a list of lists."
  (with-open-file (data file)
    (loop :for line = (read-line data nil nil)
          :while line
          :collect (map 'list #'parse-integer (split-sequence #\Space line)))))

which is great because it does the job, I needed only two minutes for it and I knew exactly what I wanted to do. Nevertheless, I found the chain read-line, split-sequence and map a bit too much for this rather simple job. Is there a simpler way just using read which I am just missing?

A simple possibility could be to enclose each row between parentheses and use read-from-string (assuming that the file does not contain incorrect data):

(defun read-data (&optional (file "data.txt"))
  "Returns the numerical data in FILE as a list of lists."
  (with-open-file (data file)
    (let (*read-eval*)   ; set *read-eval* to NIL to avoid malicious code in the input file
      (loop :for line = (read-line data nil nil)
            :while line
            :collect (read-from-string (concatenate 'string "(" line ")"))))))


Conditionally creating a new vertex I have a function which gets called very often, for this test 30,720,000 times. It has to run in real-time, so performance is very important. Are there possible improvements?

// This function looks up in the vertice table wheter a vertice at that position already exists
// - If yes, return the vertice index
// - If no, create a new vertice and return the created vertice index
//
// @param cfg: Pointer to a chunkConfig struct
// @param x: x position of the vertice
// @param y: y position of the vertice
inline VERTICE_INDEX GetOrCreateVertice(float x, float y, ChunkGenerator::chunkConfig *cfg)
{
    int x_pos = int((x*POSARR_ADJ_F) + 0.5f);
    int y_pos = int((y*POSARR_ADJ_F) + 0.5f);

    int dict_index = x_pos + (y_pos * cfg->subdivisions_adj);

    VERTICE_INDEX dict_entry = cfg->vertice_pool.vertice_dict[dict_index];
    VERTICE_INDEX current_index = cfg->vertice_pool.current_vertice_index;

    if (dict_entry >= 0 && dict_entry < 65535 && current_index > 0)
        return dict_entry;

    LVecBase3f* offset      = cfg->base_position;
    LVecBase2f* dim         = cfg->dimensions;
    LVecBase2f* tex_offset  = cfg->texture_offset;
    LVecBase2f* tex_scale   = cfg->texture_scale;

    int pool_index = ((int)current_index) * 5;

    float base_scale = 1.0 / (cfg->subdivisions_f-1.0);
    float x_scaled = x * base_scale;
    float y_scaled = y * base_scale;

    cfg->vertice_pool.vertice_array[pool_index+0] = offset->get_x() + (x_scaled * dim->get_x());
    cfg->vertice_pool.vertice_array[pool_index+1] = offset->get_y() + (y_scaled * dim->get_y());
    cfg->vertice_pool.vertice_array[pool_index+2] = offset->get_z();
    cfg->vertice_pool.vertice_array[pool_index+3] = tex_offset->get_x() + (x_scaled * tex_scale->get_x());
    cfg->vertice_pool.vertice_array[pool_index+4] = tex_offset->get_y() + (y_scaled * tex_scale->get_y());

    cfg->vertice_pool.vertice_dict[dict_index] = current_index;
    cfg->vertice_pool.current_vertice_index++;
    return current_index;
}

LVecBase3f and LVecBase2f are vector-types provided by the graphics-engine I use. VERTICE_INDEX is a unsigned short, POSARR_ADJ and POSARR_ADJ_F is constant 2.

This is the chunkConfig struct:

struct chunkConfig {
    int subdivisions;
    int subdivisions_adj;
    float subdivisions_f;
    LVecBase3f *base_position;
    LVecBase2f *dimensions;
    LVecBase2f *texture_offset;
    LVecBase2f *texture_scale;
    verticePool vertice_pool;
};

struct verticePool {
    VERTICE_INDEX current_vertice_index;
    VERTICE_INDEX current_primitive_index;
    VERTICE_INDEX * vertice_dict;
    float *vertice_array;
    VERTICE_INDEX *primitive_array;
};

Performance result measured by very-sleepy: link

Version based on the suggestions made in the comments: very-sleepy, AMD CodeAnalyst, and the assembler for the slow-line: generated assembler,
I also made some arrays global, and renamed "vertice" to "vertex".

It looks to me like your function is actually doing 2 things:

It's checking to see if an entry exists in the dictionary, and if so return it
If it doesn't exist, then it creates a new one

So I would break it into 2 functions:

VERTICE_INDEX FindVerticeInDict(const float x, const float y,
    const ChunkGenerator::chunkConfig& cfg, VERTICE_INDEX& current_index);
VERTICE_INDEX GetOrCreateVertice(const float x, const float y,
    ChunkGenerator::chunkConfig *cfg);

Note that I made x and y constants because they aren't changed by either function. Also, in the Find method, I made the cfg argument const because it's not being changed there either. It is modified in GetOrCreate though, so I left it as a pointer. (It could also be a non-const reference, if you prefer that style.)

I would create a named constant for the "entry not found" value:

const VERTICE_INDEX ENTRY_NOT_FOUND = 0xFFFF;

So it would look something like this:

VERTICE_INDEX FindVerticeInDict(const float x, const float y,
    const ChunkGenerator::chunkConfig& cfg, VERTICE_INDEX& current_index)
{
    int x_pos = int((x*POSARR_ADJ_F) + 0.5f);
    int y_pos = int((y*POSARR_ADJ_F) + 0.5f);

    int dict_index = x_pos + (y_pos * cfg.subdivisions_adj);

    VERTICE_INDEX dict_entry = cfg.vertice_pool.vertice_dict[dict_index];
    current_index = cfg.vertice_pool.current_vertice_index;

    if (dict_entry >= 0 && dict_entry < ENTRY_NOT_FOUND && current_index > 0)
        return dict_entry;

    return ENTRY_NOT_FOUND;
}

For your vertex array, you really should create a struct that contains the members you're going to use rather than looking them up in a 1D array of floats. It's error prone, hard-to-read, and hard-to-maintain the way you're doing it now. I'd create a struct like this:

typedef struct Vertex {
    Point3D vertex_coord; // This might be the same as LVecBase3f?
    Point2D texture_coord; // This might be the same as LVecBase2f?
} Vertex;

Where Point3D is just:

typedef struct Point3D {
    float x;
    float y;
    float z;
} Point3D;

and Point2D just has an x and y float values. Doing this will allow you to remove the magical pool_index = ((int)current_index) * 5;, and have names for the values you're writing.

Also, you don't need to use the inline directive. The compiler can decide to ignore it, and it can decide to make something you didn't mark as inline inline. So there's not a lot of reason to use it.

You can reduce the verbosity of your code, and possibly reduce the number of pointer dereferences the compiler produces by getting the address of the vertex you want and assigning all its members through that one pointer.

Given the above, your function would now look like this:

VERTICE_INDEX GetOrCreateVertice(const float x, const float y,
    ChunkGenerator::chunkConfig *cfg)
{
    VERTICE_INDEX current_index = 0;
    VERTICE_INDEX dict_entry = FindVerticeInDict(x, y, *cfg, current_index);
    if (dict_entry != ENTRY_NOT_FOUND)
        return dict_entry;

    LVecBase3f* offset      = cfg->base_position;
    LVecBase2f* dim         = cfg->dimensions;
    LVecBase2f* tex_offset  = cfg->texture_offset;
    LVecBase2f* tex_scale   = cfg->texture_scale;

    float base_scale = 1.0 / (cfg->subdivisions_f-1.0);
    float x_scaled = x * base_scale;
    float y_scaled = y * base_scale;

    Vertex* vertex = &cfg->vertice_pool.vertice_arry [ current_index ];
    vertex->vertex_coord.x = offset->get_x() + (x_scaled * dim->get_x());
    vertex->vertex_coord.y = offset->get_y() + (y_scaled * dim->get_y());
    vertex->vertex_coord.z = offset->get_z();
    vertex->texture_coord.x = tex_offset->get_x() + (x_scaled * tex_scale->get_x());
    vertex->texture_coord.y = tex_offset->get_y() + (y_scaled * tex_scale->get_y());

    cfg->vertice_pool.vertice_dict[dict_index] = current_index;
    cfg->vertice_pool.current_vertice_index++;

    return current_index;
}


window.innerWidth - workaround for when it returns the wrong value Some JavaScript libraries relies on window innerWidth and innerHeight to do their calculations
window.innerWidth is not 100% reliable, which may lead to bugs
matchMedia().matches is 100% accurate but it returns a boolean value
come around: use matchMedia to verify window.innerWidth value; if it returns false use matchMedia to do a binary search until it finds the correct value

I faced that issue with some libs that relies on window.innerWidth. In cases where I had some media queries the return value of window.innerWidth used by the library would be off and that library would have issues with incorrect value.

I've seen that matchMedia().matches always return the correct value, but it returns a boolean value, not the width value. I've not seen a solution so far for that (maybe some of you know a better solution?), so I came up with a solution using matchMedia.

I created a function getCorrectDimension that performs a binary search around the window innerWidth (or innerHeight) to find the correct value if its value is wrong as you can see below:

const binarySearch = dim => function bin(start, end) {
    const guess = Math.floor((start + end)/2)

    // this checks if we have the correct value, if not it will keep calling itself until there's a match
    if(window.matchMedia(`(${dim}: ${guess}px)`).matches) {
        return guess
    }

    // since it is not a match, then we need to recalibrate the range and call again.
    // for that we check the boolean value using with min-width (height) rule.
    return window.matchMedia(`(min-${dim}: ${guess}px)`).matches
        ? bin(guess, end)
        : bin(start, guess)
}

const getCorrectDimension = (dim = 'width', range = 300) => {
    if(dim !== 'width' && dim !== 'height') {
        throw Error('`getCorrectDimension` accepts "width" or "height" as parameter')
    }

    let prop = 'inner' + dim.charAt(0).toUpperCase() + dim.slice(1)

    // here checks if the window.innerWidth or Height it's the correct one
    if(window.matchMedia(`(${dim}: ${window[prop]}px)`).matches) {
        return window[prop]
    }

    // here, since the value is wrong we use binarySearch to find its correct value
    const start = window[prop] - range >= 0 ? window[prop] - range : 0
    const end = window[prop] + range

    return binarySearch(dim)(start, end)

}

I have never seen innerWidth or innerHeight report the wrong value. There are a few bugs listed for chrome and firefox but they are all closed.
Could you provide any evidence?
UPDATE
Media query matches down to fractions of a CSS pixel while innerWidth returns integer CSS pixel values. This is a problem when devicePixelRatio !== devicePixelRatio | 0
I tried to find a cover all solution, but too much time need to complete so I withdraw the answers content.


Extracting information from a URI I'm completely new to C and my first project is to create a simple cURL-like HTTP request client... but first, I need to create a function that can parse a URL. I've created this function, and it seems to work quite well, but since I'm so new to the complexities of C, can someone check for any errors or to see if I can improve clock efficiency?

Example usage can be found here.

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

void filterURLstring(char *weburl, char **protocol, char **username, char **password, char **host, char **port, char **path);

int main() {
    char *text = "http://mhdfffffjddj:abc123@192.168.0.2:8888/servlet/rece/";
    char *protocol, *username, *password, *host, *port, *path;
    filterURLstring(text, &protocol, &username, &password, &host, &port, &path);
    printf("Protocol:\"%s\"\n", protocol);
    printf("Username:\"%s\"\n", username);
    printf("Password:\"%s\"\n", password);
    printf("Host:\"%s\"\n", host);
    printf("Port:\"%s\"\n", port);
    printf("Path:\"%s\"\n", path);
    free(protocol);
    free(username);
    free(password);
    free(host);
    free(port);
    free(path);
    return 0;
}

void filterURLstring(char *weburl, char **protocol, char **username, char **password, char **host, char **port, char **path) {
    char *postprotocol, *postuserinfo, *postusername, *login, *posthost, *posthostname, *hostloc, *postport;
    int isPath;
    if ((postprotocol = strstr(weburl, "://")) == NULL) {
        printf("Input URL is invalid. Exiting...\n");
        exit(1);
    }
    *protocol = malloc(sizeof(char) * (postprotocol - weburl + 1));
    strncpy(*protocol, weburl, postprotocol - weburl);
    postprotocol = (postprotocol + (*(postprotocol+3)=='/'?4:3));
    if ((postuserinfo = strstr(postprotocol, "@")) != NULL) {
        if ((postusername = strchr(postprotocol, ':')) != NULL) {
            *password = malloc(sizeof(char) * (postuserinfo - postusername + 1));
            strncpy(*password, postusername + 1, (postuserinfo - postusername - 1));
        } else {
            *password = NULL;
        }
        int userlen = (postusername?postusername:postuserinfo) - postprotocol;
        *username = malloc(sizeof(char) *  userlen + 1);
        strncpy(*username, postprotocol, userlen);
    }
    hostloc = (postuserinfo?postuserinfo + 1:postprotocol);
    if ((posthost = strchr(hostloc, '/')) == NULL) {
        posthost = hostloc + strlen(hostloc);
        isPath = 1;
    }
    if ((posthostname = strchr(hostloc, ':')) == NULL) {
        posthostname = posthost;
    }
    *host = malloc(sizeof(char) * (posthostname - hostloc + 1));
    strncpy(*host, hostloc, (posthostname - hostloc));
    if (posthostname != posthost) {
        posthostname++;
        *port = malloc(sizeof(char) * (posthost - posthostname + 1));
        strncpy(*port, posthostname, (posthost - posthostname));
    } else {
        *port = malloc(sizeof(char) * 2 + 1);
        strncpy(*port, "80", 2);
    }
    if (isPath) {
        *path = malloc(sizeof(char) * strlen(posthost));
        strncpy(*path, posthost, strlen(posthost));
    } else {
        *path = calloc(1, 1);
    }
}

I changed my code according to the answer. It can be seen here.

Think in terms of an Abstract Data Type (ADT)

@x79 is right about using a struct for this.
In fact, it makes sense to think of this in terms of an abstract data type,
as defined in Code Complete:

  An abstract data type is a collection of data and operations that work on that data.

The collection of data is the URL info, wrapped in a struct, and the operations can be:

UrlInfo * createUrlInfo(const char *);
deleteUrlInfo(UrlInfo *);
isValidUrlInfo(UrlInfo *);
printUrlInfo(UrlInfo *);

This collection of methods together form an abstract data type.
They encapsulate the operations you need,
while hiding the implementation details.
Users of a UrlInfo don't need to know how the operations work,
they just need to know what the operations are in the ADT (create / delete / isValid / print).
These methods closely collaborate with each other,
emulating class-like behavior in object oriented languages.

Armed with this ADT,
your main function can be rewritten in a more functional, natural way:

int main() {
    const char * text = "http://mhdfffffjddj:abc123@192.168.0.2:8888/servlet/rece/";
    UrlInfo * urlInfo = createUrlInfo(text);
    if (isValidUrlInfo(urlInfo)) {
        printUrlInfo(urlInfo);
    }
    deleteUrlInfo(urlInfo);
}

The purpose of the isValidUrlInfo is to move the exit statement out of the current implementation.
It's not good when a function exits in the middle of the program.
Controlling the program flow is not the job of a function parsing a URL.
The parser should just parse, and let the caller decide what to do with an invalid input.

Other improvements

port make would make sense to store in an int, not a char*

Instead of hardcoding the URL used by the program,
it would be much more useful to take it as a command line argument, for example:

int main(int argc, char ** argv) {
    const char * text = argv[1];
    UrlInfo * urlInfo = createUrlInfo(text);
    if (isValidUrlInfo(urlInfo)) {
        printUrlInfo(urlInfo);
    }
    deleteUrlInfo(urlInfo);
}

No need to do return 0 at the end of main,
the compiler automatically adds that.

This line is too tightly packed:

    int userlen = (postusername?postusername:postuserinfo) - postprotocol;

Use spaces around operators to make it more readable, like this:

    int userlen = (postusername ? postusername : postuserinfo) - postprotocol;

Suggested implementation

Putting some of the above points together,
a Url Info abstract data type and a main method using it might look like this:

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

typedef struct UrlInfo {
    char * protocol;
    char * username;
    char * password;
    char * host;
    char * port;
    char * path;
} UrlInfo;

UrlInfo * createUrlInfo(const char *);
void deleteUrlInfo(UrlInfo *);
int isValidUrlInfo(UrlInfo *);
void printUrlInfo(UrlInfo *);

int main(int argc, char ** argv) {
    int i;
    UrlInfo * urlInfo;
    for (i = 1; i < argc; ++i) {
        urlInfo = createUrlInfo(argv[i]);
        if (isValidUrlInfo(urlInfo)) {
            printUrlInfo(urlInfo);
        } else {
            printf("Input URL is invalid: %s\n", argv[i]);
        }
        deleteUrlInfo(urlInfo);
        puts("");
    }
}

UrlInfo * createUrlInfo(const char * weburl) {
    char *postprotocol, *postuserinfo, *postusername, *login, *posthost, *posthostname, *hostloc, *postport;
    UrlInfo * urlInfo = malloc(sizeof(UrlInfo));

    urlInfo->protocol = NULL;
    urlInfo->username = NULL;
    urlInfo->password = NULL;
    urlInfo->host = NULL;
    urlInfo->port = NULL;
    urlInfo->path = NULL;

    int isPath;
    if ((postprotocol = strstr(weburl, "://")) == NULL) {
        return urlInfo;
    }
    urlInfo->protocol = malloc(sizeof(char) * (postprotocol - weburl + 1));
    strncpy(urlInfo->protocol, weburl, postprotocol - weburl);
    postprotocol = (postprotocol + (*(postprotocol + 3) == '/' ? 4 : 3));
    if ((postuserinfo = strstr(postprotocol, "@")) != NULL) {
        if ((postusername = strchr(postprotocol, ':')) != NULL) {
            urlInfo->password = malloc(sizeof(char) * (postuserinfo - postusername + 1));
            strncpy(urlInfo->password, postusername + 1, (postuserinfo - postusername - 1));
        } else {
            urlInfo->password = NULL;
        }
        int userlen = (postusername?postusername:postuserinfo) - postprotocol;
        urlInfo->username = malloc(sizeof(char) *  userlen + 1);
        strncpy(urlInfo->username, postprotocol, userlen);
    }
    hostloc = (postuserinfo?postuserinfo + 1:postprotocol);
    if ((posthost = strchr(hostloc, '/')) == NULL) {
        posthost = hostloc + strlen(hostloc);
        isPath = 1;
    }
    if ((posthostname = strchr(hostloc, ':')) == NULL) {
        posthostname = posthost;
    }
    urlInfo->host = malloc(sizeof(char) * (posthostname - hostloc + 1));
    strncpy(urlInfo->host, hostloc, (posthostname - hostloc));
    if (posthostname != posthost) {
        posthostname++;
        urlInfo->port = malloc(sizeof(char) * (posthost - posthostname + 1));
        strncpy(urlInfo->port, posthostname, (posthost - posthostname));
    } else {
        urlInfo->port = malloc(sizeof(char) * 2 + 1);
        strncpy(urlInfo->port, "80", 2);
    }
    if (isPath) {
        urlInfo->path = malloc(sizeof(char) * strlen(posthost));
        strncpy(urlInfo->path, posthost, strlen(posthost));
    } else {
        urlInfo->path = calloc(1, 1);
    }
    return urlInfo;
}

void deleteUrlInfo(UrlInfo * urlInfo) {
    free(urlInfo->protocol);
    free(urlInfo->username);
    free(urlInfo->password);
    free(urlInfo->host);
    free(urlInfo->port);
    free(urlInfo->path);
    free(urlInfo);
}

int isValidUrlInfo(UrlInfo * urlInfo) {
    return urlInfo != NULL && urlInfo->protocol != NULL;
}

void printUrlInfo(UrlInfo * urlInfo) {
    printf("Protocol:\"%s\"\n", urlInfo->protocol);
    printf("Username:\"%s\"\n", urlInfo->username);
    printf("Password:\"%s\"\n", urlInfo->password);
    printf("Host:\"%s\"\n", urlInfo->host);
    printf("Port:\"%s\"\n", urlInfo->port);
    printf("Path:\"%s\"\n", urlInfo->path);
}


create static page with vuejs I have a static homepage with some routes and thought about creating this with VueJs. I am totally new to this and just know NodeJs with Handlebars. So normally I would have something like this

#page {
  margin: 30px 0;
}

  Page 1
  Page 2
  Page 3

  Current Page (depends on URL)

  Footer

And I tried to create this by using VueJs

const header = new Vue({
  el: "#header",
  methods: {
    loadPage(pageToLoad) {
      page.$data.currentPage = "Current Page: " + pageToLoad;
    }
  }
});

const page = new Vue({
  el: "#page",
  data: {
    currentPage: "Current Page: 1"
  }
});
#page {
  margin: 30px 0;
}

  Page 1
  Page 2
  Page 3

  Footer

As you can see this example works fine. Instead of rendering the whole page I just change the "main content" of the page without reloading the header and footer.

Is this the correct way to go or how would you create a page with multiple routes?

This may be an incorrect assumption, but it seems like with your proposed approach, you would need to either keep the HTML for each page within the HTML for the main page in the onclick attributes for each link or else have the links contain a key that corresponds to a mapping of keys to HTML for the pages.

There are tools like Vue Router that are built for this scenario. With Vue Router you can have the HTML templates stored in separate files and imported into the router.

One other thing I noticed in your sample VueJS code is that you have a Vue object for the header and a Vue object for the page. Those could be combined into a single object for an element that contains both elements, unless you want the header to be a separate component - then defining that as a component would be useful.

//these template values can come from external files as well
const Page1 = {
  template: '
Page 1
'
}
const Page2 = {
  template: '
Page 2
'
}
const Page3 = {
  template: '
Page 3
'
}
const routes = [{
    path: '/page1',
    component: Page1
  },
  {
    path: '/page2',
    component: Page2
  },
  {
    path: '/page3',
    component: Page3
  }
];
const router = new VueRouter({
  routes: routes
});

const viewApp = new Vue({
  el: "#app",
  router: router,
});
#page {
  margin: 30px 0;
}

    <router-link to="/page1">Page 1</router-link>
    <router-link to="/page2">Page 2</router-link>
    <router-link to="/page3">Page 3</router-link>

    <router-view></router-view>

    Footer


Extracting numeric values from a string containing key:value pairs I am writing function numericValues(text: String): List[Int] to extract patterns """([a-z]+)\s*:\s*(\d+)""" and return the list of the numeric values :

numericValues("a123 : 0 abc:123 123:abc xyz:1") // List(123, 1)

I would write numericValues like this:

def numericValues(text: String): List[Int] = {
  val regex = """([a-z]+)\s*:\s*(\d+)""".r
  regex.findAllIn(text).toList.flatMap {s =>
    PartialFunction.condOpt(s) { case regex(_, num) => num.toInt }
  }
}

I guess the condOpt invocation is redundant and I wonder how to simplify this implementation. Also, I'd appreciate comments on improvements to style and best practice.

You usually want a zero-width word boundary \b on either end of the regex, to avoid matching things like 1a:1a.

There's no need to capture [a-z]+ since you are throwing it away.

You can use lookbehind (?<=…) assertions to require a match to be preceded by whatever, without including the whatever in the match result.  This means no need for capturing parenthesis, only the integer is included in the match, and the final map is simply _.toInt. Variable-width lookbehind was introduced in Java 9; older versions have only fixed-width lookbehind.

Finally, removing the variable makes braces unnecessary.

def numericValues(text: String): List[Int] = """(?<=\b[a-z]+\s*:\s*)\d+\b""".r.findAllIn(text).toList.map(_.toInt)

With only fixed-width lookbehind, you could postprocess the matches to remove non-numerics:

….map(_.replaceAll("[^0-9]", "").toInt)

Or, more idiomatically and less hacky, just capture the digits and extract the capture groups:

"""\b[a-z]+\s*:\s*(\d+)\b""".r.findAllIn(text).matchData.toList.map(_.group(1).toInt)


Java ClassNotFoundException Handling Is there a better way to handle this ClassNotFoundException ?

private Class<?> getClass(String value)
{
    Class<?> columnClass = null;
    try
    {
        columnClass = Class.forName(StringUtils.trim(value));
    }
    catch (ClassNotFoundException ex)
    {
        if (value.contains("double") || value.contains("Double"))
        {
            columnClass = Double.class;
        }
        else if (value.contains("int") || value.contains("Int"))
        {
            columnClass = Integer.class;
        }
        else if (value.contains("bool"))
        {
            columnClass = Boolean.class;
        }
        else if (value.contains("long") || value.contains("Long"))
        {
            columnClass = Long.class;
        }
        else
        {
            log.error("FAILED. Class object is not supported: " + value, ex);
        }
    }
    return columnClass;
}

You do not provide much context for this method, so I can only give you general advice.
Logic

never use exception handling as part of the routine/regular code path in you program. Most Java VM's require significant locking and processing to generate the actual exception and its stack trace. I have seen 10% performance improvements in large commercial applications where simple pre-validation of common exceptional conditions saves a huge amount of processing resources.

What if the user enters the non-existent class name dev.null.TurningIntoZomies, they get an Integer.class back.

In the event that the Class.forname(...) fails, you do exception handling and return null. If the program asks for the same value again, it will do the full search again, and so on. If the class exists, it is not normally very slow, but, if the class does not exist, the Class.forname() has to search the entire classpath. Cache the results (of both the successes and the failures so you only have to call Class.forName() once.

Code Style

Use correct Java brace placement, at the end of the line.

Finally, you can use short-circuit return statements to make it easier:
private Class<?> getClass(String value) {
    try {
        return Class.forName(StringUtils.trim(value));
    } catch (ClassNotFoundException ex) {
        if (value.contains("double") || value.contains("Double")) {
            return Double.class;
        }
        if (value.contains("int") || value.contains("Int")) {
            return Integer.class;
        }
        if (value.contains("bool")) {
            return = Boolean.class;
        }
        if (value.contains("long") || value.contains("Long")) {
            return Long.class;
        }
        log.error("FAILED. Class object is not supported: " + value, ex);
        return null;

    }
}


Am I approaching this ASP.Net MVC/Entity Framework pattern incorrectly? I have experience is C#/ASP.NET, and I've done MVC in Ruby On Rails, so I figured making the jump to ASP.NET MVC would be a breeze.  Unfortunately, I'm second-guessing my every decision.  I'd like some more eyes on this to tell me if my way of doing things is insane.

My main question is around editing an entity.

My GET looks like this:

public ActionResult Edit(int id)
{
    var viewModel = from c in db.centres
                    // some joins are here
                    where c.id == id
                    select new CentreViewModel {
                      CentreId = c.id,
                      CentreName = c.Name,
                      etc...
                    };
    return View( viewModel );
}

And my POST is something like this:

public ActionResult Edit(CentreViewModel viewModel)
{
    // NOTE - Validation & Error handling exists, but has been omitted for berevity
    var centre = db.centres.First(c => c.id == viewModel.CentreId);
    centre = Mapper.Map<CentreViewModel, CentreEntity>(viewModel, centre);

    db.centres.ApplyCurrentValues( centre );
    db.SaveChanges();
}

Is this kinda-sorta right?  Are there any red flags here?  Is ApplyCurrentValues correct?  I see others using repository.Entry(), but I don't have a repository for my classes, near as I can tell...

Here is a link to the "official" tutorial on basic CRUD operations in ASP.NET MVC and EF.

The main difference is the validation checking, and the exception handling.  I recommend adding in the validation, and possibly the exception handling if you need it.  Don't randomly swallow exceptions and show the user some generic error message.

Other than that, they use a slightly different way of letting EF know about the entity to be updated, but I think they're accomplishing the same thing. I think the functional difference is that you are only posting back the necessary data to be updated, and having EF get the entity and update it with those changes for you. With their method, I think that they are posting back the entire entity, and having EF "reattach" the entity and mark it as modified.

So I think the differences in the update code boil down to partial vs. full entity updates.

Side Note 1

I like your way better.  In their case they are relying on the end user to postback the entire entity.  While that is a basic demo, using this method blindly opens up your application to vulnerabilities where people can update any field they want, even if the field doesn't appear in the HTML form (Think of an "IsAdmin" flag). I believe that this is known as a Mass Assignment vulnerability that bit Rails and Github last week! (Source: https://github.com/blog/1068-public-key-security-vulnerability-and-mitigation)

Side Note 2

About the comments and the LINQ syntax... I also like the expression syntax a little more, at least for short, small queries.  But when doing joins in the past, I've gone with the query syntax.  Just a personal preference.  Here is what the code would look like with the LINQ expression syntax (as opposed to the query syntax):

db.centres
    .Join(...)
    .Where(c => c.id == id)
    .Select(new CentreViewModel
        {
            CentreId = c.id,
            CentreName = c.Name,
            etc...
        };


Serial port data plotter in PyQt I am rather new in GUI programming and multi-threading apps. It is just a serial port monitor which plot serial data via pyqtgraph. There are two curves on the first plot (a and b) and one curve on the second plot (c which is (a-a_prevous)/(b-b_prevous)). How can I improve this code in general? I use Python3.5.

from PyQt4 import QtGui
from PyQt4.QtCore import QObject, pyqtSignal
import sys
import serial
import pyqtgraph
import threading
import ui_main
import numpy as np

class GraphPlotter(QtGui.QMainWindow, ui_main.Ui_GraphPlotter):

    def __init__(self):
        super().__init__()
        pyqtgraph.setConfigOption('background', 'w')
        self.a = []
        self.b = []
        self.c = [0]
        self.flag = 'a'
        self.setupUi(self)
        self.plotAB.plotItem.showGrid(True, True, 0.7)
        self.plotC.plotItem.showGrid(True, True, 0.7)
        self.monitor = SerialMonitor()
        self.monitor.bufferUpdated.connect(self.update)
        self.startButton.clicked.connect(self.monitor.start)
        self.stopButton.clicked.connect(self.monitor.stop)
        self.clearBufferButton.clicked.connect(self.clear)

    def update(self, msg):
        if self.flag == 'a':
            self.a.append(msg)
            self.flag = 'b'

        elif self.flag == 'b':
            self.b.append(msg)
            c = pyqtgraph.hsvColor(0.2, alpha=.5)
            pen2 = pyqtgraph.mkPen(color=c, width=3)
            try:
                print((self.a[-1] - self.a[-2]), (self.b[-1] - self.b[-2]))
                self.c.append((self.a[-1] - self.a[-2]) / (self.b[-1] - self.b[-2]))
                self.plotC.plot(np.arange(len(self.c)), self.c, pen=pen2, clear=True)
            except ZeroDivisionError:
                print('Division by zero')
                self.c.append(0)
            except IndexError:
                print('C is not ready')
            finally:
                self.flag = 'a'

            for y, pen in [(self.a, (255, 0, 0)), (self.b, (0, 255, 0))]:
                self.plotAB.plot(np.arange(len(y)), y, pen=pen)

    def clear(self):
        self.a = []
        self.b = []
        self.c = [0]

        self.plotAB.clear()
        self.plotC.clear()

class SerialMonitor(QObject):
    bufferUpdated = pyqtSignal(int)

    def __init__(self):
        super(SerialMonitor, self).__init__()
        self.running = False
        self.thread = threading.Thread(target=self.serial_monitor_thread)

    def start(self):
        self.running = True
        self.thread.start()

    def stop(self):
        self.running = False

    def serial_monitor_thread(self):
        while self.running is True:
            ser = serial.Serial('/dev/ttyS0', 115200)
            msg = ser.readline()
            if msg:
                try:
                    self.bufferUpdated.emit(int(msg))
                except ValueError:
                    print('Wrong data')
            else:
                pass
            ser.close()

if __name__ == "__main__":
    app = QtGui.QApplication(sys.argv)
    plot = GraphPlotter()
    plot.show()
    app.exec_()

Instead of inheriting from ui_main.Ui_GraphPlotter, I would use the PyQt4.uic module to dynamically load your ui file in the __init__ method:

import os
import pyqtgraph

from PyQt4 import QtGui
from PyQt4 import uic

curdir = os.path.dirname(os.path.abspath(__file__))

class GraphPlotter(QtGui.QMainWindow):

    def __init__(self):
        super().__init__()
        uic.loadUi(os.path.join(curdir, 'GraphPlotter.ui'), self)
        pyqtgraph.setConfigOption('background', 'w')
        # ...


Static single page app with routing I'm currently trying to write an app with Cordova and ReactJS. I haven't used ReactJS before, so I'm very confused by the mixed usage of Babel, JavaScript and JSX. I try to use JSX where I can because I like it, but I dislike Babel, which is used in many tutorials as well. I haven't quite understood what Babel is used for at all.

const { Router,
        Route,
        IndexRoute,
        Redirect,
        Link,
        IndexLink
      } = ReactRouter

var Header = React.createClass({
    render: function () {
        return (

{this.props.text}

        );
    }
});

var Content = React.createClass({
    render: function() {
      return (

{this.props.text}

      );
    }
});

var Page = React.createClass({
  getInitialState: function(){
    return {
      'title': 'Main Page' + (this.props.title ? ' - ' + this.props.title : '')
    }
  },
  render: function() {
    return (

        <Content text={this.props.text} />

    );
  }
});

var Home = React.createClass({
  getInitialState: function(){
    return {
      'title': 'Home',
      'text': <Link to="/about">About</Link>
    }
  },
  render: function(){
    return (
      <Page title={this.state.title} text={this.state.text} />
    );
  }
});

var About = React.createClass({
  getInitialState: function(){
    return {
      'title': 'About',
      'text': 'Lalal'
    }
  },
  render: function(){
    return (
      <Page title={this.state.title} text={this.state.text} />
    );
  }
});

React.render((
    <Router>
      <Route path="/" component={Home} />
      <Route path="/about" component={About} />
    </Router>
  ),
  document.getElementById('app')
);

I'm wondering if there's a better way to do this or if I'm doing everything correctly. I did my best with the classes (thinking in React stuff) and wanted to make Header a Navigation later on. Is there a good way to separate the routes, and put my views into files and components where needed?

I haven't quite understood what Babel is used for at all.

Babel is used for transpilation - the process of converting unsupported/future syntax into one that is supported at the moment. For instance, your destructuring assignment:

const { Router,
        Route,
        IndexRoute,
        Redirect,
        Link,
        IndexLink
      } = ReactRouter

This syntax is not supported by all browsers. Babel makes it possible by transpiling it into something supported by all browsers:

"use strict";

var _ReactRouter = ReactRouter;
var Router = _ReactRouter.Router;
var Route = _ReactRouter.Route;
var IndexRoute = _ReactRouter.IndexRoute;
var Redirect = _ReactRouter.Redirect;
var Link = _ReactRouter.Link;
var IndexLink = _ReactRouter.IndexLink;

In the same way, JSX is not natively supported by any browser. It just so happens that Babel also supports transpiling of JSX, making it the de facto tool to convert JSX to something the browser understands. This:

var Header = React.createClass({
    render: function () {
        return (

{this.props.text}

        );
    }
});

becomes this:

"use strict";

var Header = React.createClass({
    displayName: "Header",

    render: function render() {
        return React.createElement(
            "h1",
            { className: "title" },
            this.props.text
        );
    }
});

JSX is an optional component of React. You can write it like the transpiled version. However, it somehow just feels better to construct the DOM with markup-like syntax than with a bunch of function calls and objects.

As for your code...

'title': 'Main Page' + (this.props.title ? ' - ' + this.props.title : '')

Since you have Babel (and even without Babel), you can construct strings with template strings. The above code would be as easy as:

'title': `Main Page ${title}`

And while it is possible to put logic in the point of interpolation, I suggest moving it out for readability.

Now everything looks fine in your code except when the About and Home start to use Page. The problem with this is that when Home and About sufficiently diverge in structure, Page will hinder you from easily rewriting either. For instance, what if About needed Sidebar just beside the content body. Where will you put Sidebar? You can't put it in Page otherwise, it will appear on all components using Page. You can't also put it on About just like that because it will sit at the side of the entire page content, not beside the content.

When building componentized UIs, it's best to think of the entire thing as a bunch of very small things. Think "Home has a header, content, footer, sidebar. About has a header, content, footer, and no sidebar" and not "Home and About are pages, therefore I make a Page component". The advantage of the former is if I needed a sidebar for About, I can just pop it in there. If I want to remove the header from Home, I can just take it out.


Token Generator Using Threads I have a class that I am using to generate 10 character alphanumeric 'tokens' (Strings) and writing them to a txt file.

I would like to be able to generate as many as possible as quickly as possible without producing any duplicates. This is what I have so far:

import java.io.BufferedWriter;
import java.io.File;
import java.io.FileWriter;
import java.io.IOException;
import java.io.PrintWriter;
import java.util.Random;

public class TokenGenerator  {

  public static void main(String[] args) {

    for (;;) {
         getSaltString();
    }
 }

public static void getSaltString() {
    String SALTCHARS = "ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890";
    StringBuilder salt = new StringBuilder();
    Random rnd = new Random();

    while (salt.length() < 10) { // length of the random string.
        int index = (int) (rnd.nextFloat() * SALTCHARS.length());
        salt.append(SALTCHARS.charAt(index));
    }
    String saltStr = salt.toString();
    writer(saltStr);
}

public static void writer(String code) {
    try {
        File file = new File("codes.txt");
        PrintWriter out = new PrintWriter(new BufferedWriter(new FileWriter(file, true)));
        out.println(code);
        out.close();

    } catch (IOException e) {
       System.out.println("Dead");
    }
  }
}

I was wondering,

A: Is this an efficient way of generating the tokens?

B: How can I call this method using threads to generate them faster

Bug 1

Requirement is 'without producing any duplicates'. You have not exactly implemented this (although the chances of happening are really small)

Bug 2

You have no condition of exiting. The program will run forever, and might have inconsistent behaviour if you terminate it externally.

A: Is this an efficient way of generating the tokens?

It can be more efficient.

You can keep the File and Writers open instead of opening each time.
You can reuse the new Random() instead of creating one each time, or..
...use int randomNum = ThreadLocalRandom.current().nextInt(min, max + 1);
You can use a fixed for loop instead of checking the salt.length().

Also use try-with-resources.

B: How can I call this method using threads to generate them faster

As you write to file, this process will finally be I/O bound, so there is probably no real gain from multi-threading.


Simple Python Old Email Eraser From Inbox This is a old email eraser that deletes old unwanted email from your gmail or yahoo inbox, cause guys really we all had the nightmare of selecting and deleting 50 old emails at a time when their are thousands, well this one uses simple python imaplib library to do that, and even shows the email that its deleting with sender name And Deletes up to a certain date that we specify

try:
    import imaplib
    import argparse
    import email
    import webbrowser
except Exception as e:
    print e

def Connect_And_Login(uname, passwd, bedate):

    if uname.endswith('@gmail.com'):
        mail = imaplib.IMAP4_SSL('imap.gmail.com')
        inbox = "INBOX"
        url = 'https://www.google.com/settings/security/lesssecureapps'

    elif uname.endswith('@yahoo.com'):
        mail = imaplib.IMAP4_SSL('imap.mail.yahoo.com')
        inbox = "Inbox"
        url = 'https://login.yahoo.com/account/security#less-secure-apps'

    else:
        print 'Enter The Email Address Carefully'
        exit(0)

    try:
        mail.login(uname, passwd)

    except Exception as e:
        print "\n\nFailed to login Allow less secure apps from your email settings and try again\n\n"
        webbrowser.open(url)
        exit(0)

    Mark_emails_as_Deleted(mail, bedate, inbox)
    #print mail.list()
    return

def Mark_emails_as_Deleted(mail, bedate, inbox):
    mail.select(inbox)
    result, data = mail.search(None, '(BEFORE "'+str(bedate)+'")')

    if result == 'OK':
        for num in data[0].split():
            mail.store(num, '+FLAGS', '\\Deleted')

            res, msg_data = mail.fetch(str(num), '(RFC822)')
            for response_part in msg_data:
                if isinstance(response_part, tuple):
                    msg = email.message_from_string(response_part[1])
                    print '\nDleting.....\n'
                    for header in [ 'subject', 'to', 'from' ]:
                        print '%-8s: %s' % (header.upper(), msg[header])

        expunge_Them(mail)

    return

def expunge_Them(mail):

    mail.expunge()
    mail.close()
    mail.logout()
    return

def main():

    parser = argparse.ArgumentParser()
    parser.add_argument("uname", help="Your email username like shantanu@gmail.com")
    parser.add_argument("path", help="Enter your email password")
    parser.add_argument("bedate", help="Enter Date, Before this date all emails will be cleaned WRITE LIKE 01-Jan-2016")
    args = parser.parse_args()
    #print '(BEFORE "'+str(bedate)+'")'

    Connect_And_Login(args.uname, args.path, args.bedate)

if __name__ == "__main__":
    main()

One nice programming principle is the principle of least astonishment, which you violate with your code. Imagine someone using your module and calling Connect_And_Login. If it was me I would be very astonished that suddenly many of my emails are deleted! At least the function does not have today as a default bedate, otherwise it would be a disaster waiting to happen.

It would be a lot better if that function just returned the mail object, which the function Mark_emails_as_Deleted then takes as parameter.

mail, inbox = connect_and_login(args.uname, args.path)
mark_emails_as_deleted(mail, args.bedate, inbox)

(Before the return in Connect_And_Login was actually superfluous,  because python automatically returns None at the end of a function body

And here we are at another thing. You should use a consistent naming scheme for your variables. Python actually has an official style guide, PEP8, which programmers are encouraged to follow to improve readability. It recommends using lower_case for variables and functions (and PascalCase for classes).

It also recommends avoiding unnecessary white-space, such as between an if block and an else block, or between try and except.

Excepting an import error and then just printing the error does you no good. It will print the error, try to continue running the code, encounter wherever that module is actually needed and fail there with another cryptic error code. The output saying that it was actually an import error is buried above the stacktrace. It is better to just let it fail as an import error or at least terminate the program there.

The different email provider could be stored in a dictionary mapping to their data. This way it is more easily extendable to more providers:

supported_provider = {"gmail.com":{"mail": "imap.gmail.com",
                                   "inbox": "INBOX",
                                    "url": 'https://www.google.com/settings/security/lesssecureapps'},
                       "yahoo.com": {...}}
provider = supported_provider[uname.split("@")[-1]]
mail = imaplib.IMAP4_SSL(provider['mail'])
inbox = provider['inbox']
url = provider['url']

It is in general faster, more readable and extendable to use str.format instead of string additions:

result, data = mail.search(None, '(BEFORE "{}")'.format(bedate))

Lastly, you could use the nice functionality of argparse to validate your input as a date as explained e.g. in this answer:

parser.add_argument('bedate', type=lambda s: datetime.datetime.strptime(s, '%d-%b-%Y'), help="Enter Date, Before this date all emails will be cleaned. Format: 01-Jan-2016")

%b is the abbreviated month name *in the current locale. So in English it will beJan, Feb, ..., Dec, while e.g. in German it will beJan, Feb, ..., Dez`. I guess gmail/yahoo can actually handle this because it is also localized for the user.


Python, numpy, optimize module to handle big file With the help of some people here and there, I wrote the module below which works very well if the file targets.csv is not too big.

As soon as this file exceeds 100 MB, a MemoryError is returned.

The problem is that I used numpy to make the computations faster (and they became really fast compared to the previous versions of this module), and I think numpy requires to load the entire file into memory.

However, there should be a way to handle the MemoryError that happens there:

targets = np.array([(float(X), float(Y), float(Z))
           for X, Y, Z in csv.reader(open('targets.csv'))])

Any ideas?

Let me know if you need more details (files, etc.)

import csv
import numpy as np
import scipy.spatial
import cv2
import random

"""loading files"""

points = np.array([(int(R), int(G), int(B), float(X), float(Y), float(Z))
              for R, G, B, X, Y, Z in csv.reader(open('colorlist.csv'))])
    # load X,Y,Z coordinates of 'points' in a np.array
print "colorlist loaded"

targets = np.array([(float(X), float(Y), float(Z))
           for X, Y, Z in csv.reader(open('targets.csv'))])
    # load the XYZ target values in a np.array
print "targets loaded"

img = cv2.imread("MAP.tif", -1)
height, width = img.shape
total = height * width
# load dimensions of tif image
print "MAP loaded"

ppm = file("walladrien.ppm", 'w')
ppm.write("P3" + "\n" +  str(height) + " " + str(width) +"\n" + "255" + "\n")
# write PPM file header

"""doing geometry"""

tri = scipy.spatial.Delaunay(points[:,[3,4,5]], furthest_site=False) # True makes an almost BW picture
# Delaunay triangulation

indices = tri.simplices
# indices of vertices

vertices = points[indices]
# the vertices for each tetrahedron

tet = tri.find_simplex(targets)

U = tri.transform[tet,:3]
V = targets - tri.transform[tet,3]
b = np.einsum('ijk,ik->ij', U, V)
bcoords = np.c_[b, 1 - b.sum(axis=1)]
# find the barycentric coordinates of each point

"""looping through data"""

i = 0

for i in range(total):

    if tet[i] == -1:
        # this means that the point lies outside the convex hull

        R = G = B = 255

        ppm.write(str(R) + ' ' + str(G) + ' ' + str(B) + "\n")  # writes a pixel

    else:

        R0 = int(vertices[tet[i]][0][0])
        G0 = int(vertices[tet[i]][0][1])
        B0 = int(vertices[tet[i]][0][2])
        R1 = int(vertices[tet[i]][1][0])
        G1 = int(vertices[tet[i]][1][1])
        B1 = int(vertices[tet[i]][1][2])
        R2 = int(vertices[tet[i]][2][0])
        G2 = int(vertices[tet[i]][2][1])
        B2 = int(vertices[tet[i]][2][2])
        R3 = int(vertices[tet[i]][3][0])
        G3 = int(vertices[tet[i]][3][1])
        B3 = int(vertices[tet[i]][3][2])

        rand = random.uniform(0,1)

        BC_0 = bcoords[i][0]
        BC_1 = bcoords[i][1]
        BC_2 = bcoords[i][2]
        BC_3 = bcoords[i][3]

        i += 1

        if rand <= BC_0:
            ppm.write(str(R0) + ' ' + str(G0) + ' ' + str(B0) + "\n")  # writes a pixel

        elif rand <= BC_0 + BC_1:
            ppm.write(str(R1) + ' ' + str(G1) + ' ' + str(B1) + "\n")  # writes a pixel

        elif rand <= BC_0 + BC_1 + BC_2:
            ppm.write(str(R2) + ' ' + str(G2) + ' ' + str(B2) + "\n")  # writes a pixel

        else:
            ppm.write(str(R3) + ' ' + str(G3) + ' ' + str(B3) + "\n")  # writes a pixel

ppm.close()
print 'done'

1. The MemoryError

I can't reproduce the MemoryError. Here's the test I ran:

def make_test(n=10**7, output_file='targets.csv'):
    with open(output_file, 'w') as f:
        for _ in range(n):
            f.write('56.08401,55.19490,25.49292\n')

>>> make_test()
>>> import os
>>> os.stat('targets.csv').st_size
270000000

That's 270 megabytes of data, and it loads fine for me:

>>> import csv, numpy as np
>>> targets = np.array([(float(X), float(Y), float(Z)) for X, Y, Z in csv.reader(open('targets.csv'))])
>>> targets.shape
(10000000, 3)

So I don't really know what is going wrong for you. Can you provide the exact text of the error message and its traceback? There might be more information there.

Regardless of the cause of the MemoryError, Janne is right to suggest that using numpy.loadtxt to get the data directly from the file (with no intermediate Python list) would be better:

targets = np.loadtxt('targets.csv', delimiter=',')

However, numpy.loadtxt is still quite slow, and if this is a bottleneck for you then you should investigate pandas:

>>> from timeit import timeit
>>> timeit(lambda:np.loadtxt('targets.csv', delimiter=','), number=1)
18.912313379812986
>>> import pandas
>>> timeit(lambda:pandas.read_csv('targets.csv', delimiter=',').values, number=1)
0.5977518488653004

2. Other comments on your code

The PPM format specification says that width is given before height, but you have:

ppm.write("P3" + "\n" +  str(height) + " " + str(width) +"\n" + "255" + "\n")

Is that right? Do you really intend to transpose your image?
It's conventional to place a comment before the code that it describes. So instead of:

ppm.write("P3" + "\n" +  str(height) + " " + str(width) +"\n" + "255" + "\n")
# write PPM file header

most Python programmers would write it like this:

# Write PPM file header: see <http://netpbm.sourceforge.net/doc/ppm.html>
ppm.write("P3\n{} {}\n255\n".format(width, height))

You haven't managed to vectorize all your code. Everything up to the computation of bcoords is vectorized and will run fast, but then you have a loop over the data where you pick a random vertex corresponding to each target. This loop runs in slow Python code and so is going to be the bottleneck in your application.

See §3 below for how this part of the code can be vectorized, using
numpy.cumsum to find the cumulative sums of the barycentric coordinates, numpy.random.uniform to generate random samples for each target, and numpy.argmax to pick the vertex.
Similarly, writing the output colours to the PPM would be faster if you used numpy.savetxt to write them all at once (instead of one at a time as at present).
You compute all four barycentric coordinates for each target using the recipe from scipy.spatial.Delaunay:

bcoords = np.c_[b, 1 - b.sum(axis=1)]

but in fact you don't need the fourth coordinate since you are only going to use the cumulative sums of these coordinates, and the fourth cumulative sum is always 1.
The format of colorlist.csv is quite inconvenient, with a mixture of two types of data (colours and vertices) using a mixture of spaces and commas as delimiters:

255 63 127,35.5344302104,21.380721966,20.3661095969

Where does this file come from? If it's under your control, could you generate it in a more convenient format, for example as one file containing colours and another containing vertices? Or if you have to generate it as a single file, then could you consistently use either spaces or commas as delimiters?
Where do the targets come from? Your code implies that you have exactly one target for each pixel of the image MAP.tif, so is it the case that the targets are generated by processing this image in some way? If so, shouldn't you just generate the targets directly from MAP.tif instead of via the intermediate targets.csv?

3. Revised code

Here's the dithering algorithm, fully vectorized:

import numpy as np
import scipy.spatial

WHITE = np.array([[255, 255, 255]], dtype=np.uint8)

def dither_colors(vertices, colors, targets, default_color=WHITE):
    """Randomly dither targets onto vertices and return array of
    corresponding colors.

    Input:
    vertices:      Points in 3-space. shape=(N,3), dtype=float
    color:         RGB colors for each point. shape=(N,3), dtype=uint8
    targets:       Points in 3-space. shape=(M,3), dtype=float
    default_color: RGB color for targets not found in convex hull of
                   vertices. shape=(1,3), dtype=uint8

    Output:
    Array of colors. shape=(M,3), dtype=uint8

    Works by triangulating vertices, getting a complex of tetrahedra.
    For each target, randomly pick one of the vertices of the
    tetrahedron it is found in (weighted according to how close the
    target is to that vertex), and assign the color of that vertex to
    the target. Targets that are not found in any tetrahedron get
    assigned default_color.

    """
    N = len(vertices)
    assert vertices.shape == (N, 3)
    assert colors.shape == (N, 3)
    M = len(targets)
    assert targets.shape == (M, 3)
    assert default_color.shape == (1, 3)

    # Compute Delaunay triangulation of vertices.
    triangulation = scipy.spatial.Delaunay(vertices, furthest_site=False)

    # Find the tetrahedron containing each target (or -1 if not found)
    tetrahedra = triangulation.find_simplex(targets)

    # Affine transformation for tetrahedron containing each target
    U = triangulation.transform[tetrahedra, :3]

    # Offset of each target from the origin of its containing tetrahedron
    V = targets - triangulation.transform[tetrahedra, 3]

    # First three barycentric coordinates of each target in its tetrahedron.
    # The fourth coordinate would be 1 - b.sum(axis=1), but we don't need it.
    B = np.einsum('ijk,ik->ij', U, V)

    # Cumulative sum of barycentric coordinates of each target.
    Bsum = np.c_[B.cumsum(axis=1), np.ones(M)]

    # A uniform random number in [0, 1] for each target.
    R = np.random.uniform(0, 1, size=(M, 1))

    # Randomly choose one of the tetrahedron vertices for each target,
    # weighted according to its barycentric coordinates
    W = np.argmax(R <= Bsum, axis=1)

    # Get the color corresponding to each vertex just chosen.
    C = colors[triangulation.simplices[tetrahedra, W]]

    # Mask out the targets where we failed to find a tetrahedron.
    C[tetrahedra == -1] = default_color

    assert C.shape == (M, 3)
    return C

And here's a function that handles the input and output:

def dither_image(width, height,
                 vertices_file='colorlist.csv',
                 targets_file='targets.csv',
                 output_file='output.ppm'):
    """Read colors and vertices from vertices_file. Read targets from
    targets_file. Dither each target to a randomly chosen nearby
    vertex; output the colors for the dithered vertices to output_file
    as a PPM with the given width and height.

    """
    vertices = np.loadtxt(vertices_file, usecols=(1, 2, 3), delimiter=',')
    colors = np.loadtxt(vertices_file, usecols=(0,), delimiter=',',
                        converters={0:lambda s:s.split()}, dtype=np.uint8)
    targets = np.loadtxt(targets_file, delimiter=',')

    C = dither_colors(vertices, colors, targets)

    # Write output as image in PPM format.
    # See <http://netpbm.sourceforge.net/doc/ppm.html>
    with open(output_file, 'wb') as ppm:
        ppm.write("P3\n{} {}\n255\n".format(width, height).encode('ascii'))
        np.savetxt(ppm, C, fmt='%d')

4. Emacs bug

In the course of preparing this answer, I discovered a bug in the way Emacs handles PPM images on OS X, which I reported as bug #16683.


Optimize list comparison method? I have below Java code to compare two list values with some condition:

 public boolean compareLists(List<SortData> prevList, List<SortData> modelList) {
        if (prevList != null && modelList != null && prevList.size() == modelList.size()) {
            boolean indicator = false;

            for (SortData modelListdata : modelList) {
                if (prevList.size() > 1 && modelList.size() > 1 && prevList.size() == modelList.size()) {
                    for (SortData prevListdata : prevList) {
                        if (modelListdata.getListBoxHeaderName().equals(prevListdata.getListBoxHeaderName())) {
                            indicator = false;
                            break;
                        }
                        else{
                            indicator = true;
                        }
                    }
                }
                for (SortData prevListdata : prevList) {
                    if (prevListdata.getListBoxHeaderName() != null && modelListdata.getListBoxHeaderName() != null) {
                        if (prevList.size() == 1 && modelList.size() == 1
                                && !prevListdata.getListBoxHeaderName().equals(modelListdata.getListBoxHeaderName())) {
                            return true;
                        } else if (prevListdata.getListBoxHeaderName().equals(modelListdata.getListBoxHeaderName())
                                && prevListdata.isSortAscending() != modelListdata.isSortAscending()) {
                            return true;
                        } else if (modelListdata.getListBoxHeaderName().equals(prevListdata.getListBoxHeaderName())
                                && modelListdata.getPosition() != prevListdata.getPosition()) {
                            return true;
                        }
                    }
                }
            }

            if (indicator) {
                return true;
            }
        } else {
            return true;
        }

        return false;

    }

But I do not think this code is optimized.

The condition I am checking:

Both List not null
If both List size 1 & Name not null return true
If header Names equals but sortAcscending boolean variable not same return true.
If header Names equals but position a integer value not same return true
If both List size > 1 and bth list size equals then check same header name exist in both the List if not then return true.

I don't think the above code is optimized and I have to write two for loops which look ugly.  What can I do to optimize this code?

Some comments:

&& modelList.size() > 1

this check is redundant, in fact

if (prevList.size() > 1 && modelList.size() > 1 && prevList.size() == modelList.size()) {

this whole check doesn't depend on the for variable, and can be hoisted outside the for.

The big if that starts at the first line can be inverted to reduce nesting.


List comprehension method I have developed some Python code which revolves around two custom classes - a 'Library' class (Lib) which contains a Python list of several objects based on a 'Cas' class. I've not posted the code for either of these classes here, but all you really need to know to understand my question is that the 'Library' object contains a Python list and the 'Cas' objects contain various attributes, some of which are strings and some are values.

One of the objectives of the code is to manipulate the Python list in the Library class and return a sub-set of the 'Cas' objects based on some user driven criteria. For example, return Cas objects where a particular attribute is equal to a given string or greater than a given value.

For this purpose, I have written the following generic method filterLibrarySingle to allow me to filter the Python list in the library class (self.Lib) based on various methods (filterMethod), attributes (filterField) and values (filterValue). Within the method I'm achieving this using list comprehensions.

On profiling my code, it would appear that this method can be quite a bit of a bottle neck! Does any one have an idea of how I could speed it up?

def filterLibrarySingle(self, filterField, filterMethod, filterValue1, filterValue2=None):
    if filterMethod == 'eq':
        self.Lib = [cas for cas in self.Lib if getattr(cas, filterField) == filterValue1]

    elif filterMethod == 'lt':
        self.Lib = [cas for cas in self.Lib if getattr(cas, filterField) < filterValue1]

    elif filterMethod == 'gt':
        self.Lib = [cas for cas in self.Lib if getattr(cas, filterField) > filterValue1]

    elif filterMethod == 'le':
        self.Lib = [cas for cas in self.Lib if getattr(cas, filterField) <= filterValue1]

    elif filterMethod == 'ge':
        self.Lib = [cas for cas in self.Lib if getattr(cas, filterField) >= filterValue1]

    elif filterMethod == 'gelt':
        self.Lib = [cas for cas in self.Lib if  getattr(cas, filterField) >= filterValue1 and getattr(cas, filterField) < filterValue2]

    elif filterMethod == 'gele':
        self.Lib = [cas for cas in self.Lib if  getattr(cas, filterField) >= filterValue1 and getattr(cas, filterField) <= filterValue2]

I've wracked my brains for days on this to try and speed it up but I guess my Python knowledge simply isn't good enough!

From the comments it sounds like you don't really need to create a new list.  Using generators will avoid the overhead of ever building a new list.

Just use this pattern:

def filterLibrarySingle(self, filterField, filterMethod, filterValue1, filterValue2=None):
    if filterMethod == 'eq':
        for cas in self.Lib:
            if getattr(cas, filterField) == filterValue1:
                yield cas
    ...

And you can call the method such as:

for item in filterLibrarySingle('my_field', 'eq', 'George'):
    do_stuff(item)

If this doesn't work, then you may have to consider designing a more advanced structure to store your data so that your searches are not \$O(n)\$ but rather \$O(\log{}n)\$.


Optimizing a nested for loop over JS array of objects NOTE: I'm bringing this question up in code review rather than stack overflow since I already have a working solution. I just am looking for ways to do it better.

I have two arrays. One is an array of simple strings (i.e. A1), while the other is an array of objects (i.e. A2). I need to pluck only those objects from A2, that  have relevant keys present in A1.

Here is my implementation using a double for loop. It works, but is not elegant or efficient, I feel. How do I make this run better? Use of ext libraries such as underscore JS is allowed.

var A1  = ["1","2","3","4"];

var A2 = [
    {label:"one", value:"1"},
    {label:"two", value:"2"},
    {label:"three", value:"3"},
    {label:"four", value:"4"},
    {label:"five", value:"5"},
    {label:"six", value:"6"},
];

       var result = [];

       for(var i=0; i<A2.length; i++){
           for(var j=0; j<A1.length; j++ ){
               if(A1[i] == A2[j].value){
                   result.push( A2[j]);
               }
           }
       }

The output of the above is :

result = [
    {label:"one", value:"1"},
    {label:"two", value:"2"},
    {label:"three", value:"3"},
    {label:"four", value:"4"},
]

You can use some built in functions instead of the for loops.

var result = A2.filter(e => A1.includes(e.value));

I can't say if this is much faster, since those functions still loop through the arrays. You'll have to time this with some large input to test.

Be aware that Internet Explorer doesn't support .includes or arrow functions. IE friendly version:

var result = A2.filter(function(e) {return A1.indexOf(e.value) !== -1});


Empty abstract class enforce structure I've been looking into empty abstract classes and from what I have read, they are generally bad practice. I intend to use them as the foundation for a small search application that I am writing. I would write the initial search provider and others would be allowed to create their own providers as well. My code's intent is enforce relationships between the classes for anyone who would like to implement them.

Can someone chime in and describe why or why not this is still a bad practice and what, if any, alternatives are available?

namespace Api.SearchProviders
{
public abstract class ListingSearchResult
{
    public abstract string GetResultsAsJSON();
}

 public abstract class SearchParameters
 {
 }

 public interface IListingSearchProvider
 {
     ListingSearchResult SearchListings(SearchParameters p);
 }
 }

To build on @Jesse C. Slicer's comment above: interfaces are for defining contracts, whereas abstract classes are meant to add some base implementation code.  An abstract class without any implementation code does not add anything an interface does not already provide.  Even worse, it blocks inheriting from other (potentially useful) classes.

Relationships can be defined either way.

You are free to provide an abstract class to callers which takes care of some of the base functionality, or you can extract an abstract base class during a later re-factoring pass if you like, but your API should still stick to the interfaces as much as possible.


jQuery function to create a container div with some content I just recently started with Javascript and jQuery. My background is in Python. I would be more than happy to get some feedback if this coding style of mine is optimal, or if in JS people generally write things differently.

The current task is to build different elements in a website that are always hosted in the same container. I have written a container creation function like this:

  function create_container(type, counter) {
      var ret = $('<div />').attr({
          class: 'container-box',
      }).data({
          type: type,
          counter: counter,
      }).append(
          $('<span />').attr({
              class: 'remove'
          }).append('&times;')
      );
      return ret
  }

  var b = create_container('text', 0);
  console.log(create_container('text', 0));

This function will then be reused in different objects to create the same containing container. Type and counter will be set when the object will be initiated.

First of all, In terms of readability, when chaining jquery operations, I like to put the operation at the start of the line. When I create an Object with only one attribute, a one liner with no trailing comma is also good. And naming your jquery objects with a heading $ makes anyone understand that you're using a jquery object here :

function create_container(type, counter) {
      var $ret = $('<div />')
          .attr({class: 'container-box'})
          .data({
              type: type,
              counter: counter,
          })
          .append(
              $('<span />')
                  .attr({class: 'remove'})
                  .append('&times;')
          );
      return $ret
  }

  var $b = create_container('text', 0);
  console.log(create_container('text', 0));

Then you coud use the jquery method addClass instead of attr({class: 'foo'} :

function create_container(type, counter) {
  var $ret = $('<div />')
      .addClass('container-box')
      .data({
          type: type,
          counter: counter,
      })
      .append(
          $('<span />')
              .addClass('remove')
              .append('&times;')
      );
  return $ret
}

var $b = create_container('text', 0);
console.log(create_container('text', 0));

Finally, if you get to use es6, you could get this kind of code, which I find a lot more readable :

function create_container(type, counter) {
  const $ret = $('<div />')
      .addClass('container-box')
      .data({type, counter})
      .append(
          $('<span />')
              .addClass('remove')
              .append('&times;')
      );
  return $ret
}

const $b = create_container('text', 0);
console.log(create_container('text', 0));


"Game" Engine Requirements System I am/have been building a game for a long time now. The game is a web browser based system, and data is stored using JSON stored in RavenDB.

One thing I am very constantly running into as a problem is the notion of Requirements. Anyone who has played a game, particularly an RPG, is probably familiar with this. Requirements such as Requires Level 10 to use or Requires [Race] to use.

This has been a constant struggle for me, and I have finally worked out what I feel is a solution; But I wanted to see if there were other people who have overcome this issue and could offer any advice.

The way it works is that all of the various things in the game that can have requirements have a field...

List<Conditional> Conditions { get; set; }

Conditional is defined like this;

public class Conditionals {

    public Condition<bool> Named { get; set; }
    public Condition<double> Minimum { get; set; }
    public Condition<double> Maximum { get; set; }
    public Condition<double> Exactly { get; set; }
}

And then each of those is a type of Condition<T>, which is declared like this;

public class Condition<T> {

    public string Id { get; set; }
    public string Name { get; set; }
    public string Adjective { get; set; }
    public string Label { get; set; }
    public T Value { get; set; }
}

So then the JSON would, if I am right, look like this... assume an item that requires level 10.

var Conditionals = new List<Conditionals>{
    new Conditionals {
        Minimum = new Condition<double>{
            Id = "clue/level",
            Name = "Level",
            Adjective = ">=",
            Label = "Level is greater than or equal to 10",
            Value = 10
        }
    }
};

These will be resolved when the character is retrieved from the database using Clues. Clues are simple key/value pairs, like a dictionary. A clue has an identity and a value only.

var character = new Character {
    Level = 11,
    Clues = new List<Clue> () // clues will contain key/value pairs
};
// on database save
character.Clues.Add(new Clue {
    Id = "clue/level",
    Value = character.Level
});

This means I will have to do some hard coding, but only by coding in clues that we know the game is going to expect. This allows us to be a little more specific (I feel) and still have some freedom.

foreach (var condition in Conditionals) {
    // attempt to see if the condition can be satisfied. This code would
    // execute for all four propertyes (named, minimum, maximum, equals)
    var currentClue = character.Clues.FirstOrDefault(y => y.Id == condition.Minimum.Id);

    // attempt to resolve. this is the minimum section, so we always use >=
    var outcome = currentClue.Value >= condition.Value;
}

I am basically trying to ask if anyone else who has more experience has ever encountered this kind of issue in their own projects, and if my solution compares to any that are known to work. This has frustrated me for a long time.

I have never written code against a RavenDB, and very seldom use Json (actually, "never" is almost accurate). The way I see it, the classes that end up Json-serialized and persisted to the database, are analoguous to Entities (Entity Framework), or POCO's.

They are part of your data model.

It might seem (/be) code duplication, but if you had a domain model (which might mirror your data model... or not) and a way to "translate" a domain entity to a data entity (and vice-versa), you could very well use an enum in your domain model, and have it serialized as a string.

Basically: the shape of the data shouldn't drive, nor define, the shape of the objects your application is going to work with.

Your business logic (well, the actual game!) should work with the domain model objects, and then you'll have a data access layer that will do its job with RavenDB and Json - the business logic shouldn't be bothered with these concerns.

The Label is a good example of why presentation concerns and data concerns need to be separated: serializing and storing it is a waste, you already have all the necessary information in the other fields.

If you had a domain model Condition, it could have a Label property, with logic to build the string out of the other values.


Generating vertices of the rectified hypercube The following piece of code generates the vertices of the rectified hypercube in arbitrary dimensions. The 3D case of the rectified hypercube is a cuboctahedron with 12 vertices (±1,±1,0), (±1,0,±1), (0,±1,±1).

The code below works perfectly, I only want to know what optimizations should I consider and/or if faster approaches exist.

#include <array>
#include <vector>
#include <algorithm>

template <typename T, size_t N>
void generate_rectified_hypercube(std::vector<std::array<T, N>> &vertices)
{
    std::array<T, N> set;
    std::array<T, N> temp;

    // Initialize set { 2, 1, ..., 1 }

    set[0] = T(2);
    for (size_t i = 1; i < N; i++)
    {
        set[i] = T(1);
    }

    // Generate vertices

    for (size_t i = 0; i < N; i++)
    {
        // Modify set

        set[i] -= T(2);
        std::copy(set.begin(), set.end(), temp.begin());
        std::sort(temp.begin(), temp.end());

        // Add all possible permutations of current set

        do
        {
            vertices.push_back(temp);
        }
        while (std::next_permutation(temp.begin(), temp.end()));
    }
}

Generally I'm quite fine with your original source, looks nice.

If you are pursuing absolutely top performance (what is the reason? This looks like a function, which should be used only few times trough life cycle of application) - you would have to dig into std::next_permutation first, to see if for your special case of various sets of {-1, 0, 1} values you can generate permutations "manually" in a faster way, then the general purpose permutation generator does. Maybe there is a way, but the source will become substantially longer and more complicated.

If you want to stay with std::next_permutation, there's little room for improvement, especially without losing some readability of the source.

For example with understanding how the set is modified by last next_permutation, you can cut out some initializations and sorting like this:

#include <array>
#include <vector>
#include <algorithm>

template <typename T, size_t N>
std::vector<std::array<T, N>> generate_rectified_hypercube()
{
    std::vector<std::array<T, N>> vertices;  // result data
    std::array<T, N> set; // set to permute upon
    // Initialize set to { 0, 1, ..., 1 }
    set[0] = T(0);
    for (size_t i = 1; i < N; ++i) set[i] = T(1);

    // Generate vertices

    // Add all possible permutations of initial set (unrolled loop for i=0)
    do {
        vertices.push_back(set);
    } while (std::next_permutation(set.begin(), set.end()));

    for (size_t i = 1; i < N; ++i) { // do the rest of them

        // Modify set to be seed for next group of permutations:
        // "abusing" the known state after last std::next_permutation call
        set[i-1] = T(-1);    // Here was zero previously
        set[i] = T(0);       // Here was first 1

        // Add all possible permutations of current set
        do {
            vertices.push_back(set);
        } while (std::next_permutation(set.begin(), set.end()));

    }
    return vertices;
}

int main()
{
    auto vertices = generate_rectified_hypercube<float, 5>();
}

As you can see, I removed the temp array (std::copy not needed) and I use the know state after last next_permutation to avoid std::sort. So this is certainly more efficient, but also harder to read and comprehend.

One note about your original code:

You don't clear the vertices input, which doesn't feel right to me. If the function is named generate_rectified_hypercube, it sounds quite functional to me, so calling it twice with the same vector I would expect it to contain only single copy of vertices (your code would add another set after the first one).

For your code probably some name along add_generated_rectified_hypercube would be more precise.

In my code I avoided this completely by creating new instance of vertices inside the function, and letting the C++11-post era compilers to optimize "return by value" situation.

And you can still do twice vertices.push_back(generate_rectified_hypercube<float, 5>()); to get two sets of vertices, to simulate behaviour of your original code (if desired). So the change in API is not limiting such usage, but it shouldn't happen by accident, reusing some non empty vertices.

Another edit:

Couldn't resist from showing off, how the function can be further cleaned up for better performance (unrolling the loop for i == 0 special case, getting rid of if (0 < i) in for loop).

The real world benefit is of course useless, as the loop is done only N-times (vs permuting N size array inside, which is much more complex).

So just for your fun ;). (BTW, OCD is evil thing, even partial ... hard to dismiss some things from head)


Bash script to generate and change Java keystore passwords This is the first Bash script I've written, so I'm looking for feedback on best practices, conventions, things like that.

This script makes a few assumptions

There is a java keystore stored at ~/.keystore
There is an alias for an entry in that keystore with a value of test
Both the keystore and entry share the same initial password
That shared password is test

After that, for both the keystore and key entry, it pulls a number of bytes from /dev/urandom, Base64 encodes them, and sets that as the password.

#!/bin/bash

keystore_file=~/.keystore
config_file=~/.keystore.config
alias_name=test
initial_password=test

generate_password() {
    local password_length=$1
    local password="$(dd if=/dev/urandom bs=$password_length count=1 | base64 -w 0)"
    echo $password
}

set_keystore_password() {
    local password_length=80
    local password=$(generate_password $password_length)
    keytool -storepasswd -keystore $keystore_file -storepass $initial_password -new $password
    echo $password >> $config_file
    echo $password
}

set_key_password() {
    local keystore_password=$1
    local password_length=80
    local password=$(generate_password $password_length)
    keytool -keypasswd -keystore $keystore_file -storepass $keystore_password -alias $alias_name -keypass $initial_password -new $password
    echo $password >> $config_file
}

initialize_keystore() {
    if [ -f $config_file ]
    then
        rm $config_file
        touch $config_file
    fi

    local keystore_password=$(set_keystore_password)
    set_key_password $keystore_password
}

initialize_keystore

It is a good practice to have the script tested on shellcheck.net so that you have a convention.

In the initialise section, you are cleaning up the keystore file (if it exists). Use the shell-builtin echo and redirect to achieve this:

echo "" > $config_file

The password_length can become a global value instead of being local to set_key_passphrase.

You can avoid the double echo in the set_keystore_password by using tee:

echo "$password" | tee -a "$config_file"


Determining if a given square is a magic square I'm writing the following program for a school assignment. The prompt is as follows:

  A magic square is square grid filled with distinct positive integers such that each cell contains a different integer and the sum of the integers in each row, column and diagonal is equal.

My task is to make a boolean method, named magical() that is passed a 2d array, and will return true if those criteria are met. This is the code I wrote:

private static boolean magical(int[][] square) {
        int length = square.length;
        //This loop checks for duplicate values
        for (int i = 0; i < length; i++){
            for (int j = 0; j < length; j++){
                int x = square[i][j];
                for (int a = 0; a < length; a++){
                    for (int b = 0; b < length; b++){
                        if (a == i && b == j) continue;
                        if (x == square[a][b]) return false;
                    }
                }
            }
        }
        ArrayList<Integer> sums = new ArrayList<>();
        //This loop finds the sums of the rows and columns
        for (int i = 0; i < length; i++){
            int sum = 0;
            int sum2 = 0;
            for (int j = 0; j < length; j++){
                sum += square[i][j];
                sum2 += square[j][i];
            }
            sums.add(sum);
            sums.add(sum2);
        }
        //The next two loops find the sums of each diagonal.
        int x = 0;
        int y = 0;
        int sum = 0;
        while (x < length && y < length){
            sum += square[x][y];
            x++;
            y++;
        }
        sums.add(sum);
        sum = 0;
        x = 0;
        y = length - 1;
        while (x < length && y >= 0){
            sum += square[x][y];
            x++;
            y--;
        }
        sums.add(sum);
        return sums.stream().allMatch(s -> s.equals(sums.get(0)));
    }

Here are my questions:

Is there any way to confirm that there are no duplicate values in the array without having to nest four for loops? This seems like an incredibly inefficient system.
Is there any way to combine the two loops in the end that are used to make sure that the sum of values in each diagonal are the same? I am currently using two loops, but seeing as I am iterating through the same array, it seems inefficient to do it twice.

I am also open to any other suggestions you may have. Please note, it is given in the prompt that all arrays passed as parameters will be squares, and it is not required to test for this.

Answering your first question, yes there is a better way. Efficiency doesn't matter in your case but in this instance the more efficient thing is also easier on the eyes.

The usual idea would be to make a hash set of the elements, then you have O(1) look up of whether you have already inserted an elements, making the task O(n^2).

I'm not that familiar with Java sadly so to answer your second question I just propose the fairly imperative thing of doing everything in one traversal (I think probably it can be done with the functional constructs that Java has), like so.

private static boolean magical(int[][] square) {
    Set<Integer> seenInts = new HashSet<>();
    var leftDiagonalSum = 0;
    var rightDiagonalSum = 0;
    var rowSums = new int[square.length];
    var colSums = new int[square.length];

    for(var i = 0; i < square.length; i++) {
        for(var j = 0; j < square.length; j++) {
            var currentElement = square[i][j];

            seenInts.add(currentElement);
            leftDiagonalSum += (i == j) ? currentElement : 0;
            rightDiagonalSum += (i == square.length - 1 - j) ? currentElement : 0;
            rowSums[i] += currentElement;
            colSums[j] += currentElement;
        }
    }

    Set<Integer> sumSet = new HashSet<>();

    sumSet.add(leftDiagonalSum);
    sumSet.add(rightDiagonalSum);

    for(var i = 0; i < square.length; i++) {
        sumSet.add(rowSums[i]);
        sumSet.add(colSums[i]);
    }

    var noDuplicates = seenInts.size() == square.length * square.length;
    var allSameSums = sumSet.size() == 1;

    return noDuplicates && allSameSums;
}

Edit - While I may not know much Java, there's no reason to specify the implementation of Set used, so the type of seenInts and sumSet can just be Set