My linked list implementation in C I have been reading about pointers in C and I thought implementing a linked list would be a good exercise to test my knowledge.

Below is my code and it is 100% functional. All I am looking for is some criticism, such as how to make the functions more efficient or defining *pointerToLinkedList in a better spot.

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

//Structure represents a node of a Linked List
struct node {
    int data;
    struct node* link;
};

//Pointer to the head node
struct node *pointerToLinkedList = NULL;

//Function inserts a new node at the beginning of the linked list
void insertNodeBeginning(int aData){
    //pointer to the new node
    struct node *newNode = (struct node*) malloc(sizeof(struct node));

    //fill in data in new node
    newNode->data = aData;
    //point new node to the previous head (now second node in linked list)
    newNode->link = pointerToLinkedList;

    //adjust pointerToLinkedList accordingly with the new head
    pointerToLinkedList = newNode;
}

//Function inserts a new node at the end of the linked list
void insertNodeEnd(int aData){

    //pointer to the new node
    struct node *newNode = (struct node*) malloc(sizeof(struct node));

    //fill in data in new node
    newNode->data = aData;
    //point new node to NULL, because its last node in list
    newNode->link = NULL;

    //link second last node to the newly created last node of the linked list
    //start at beginning of list
    struct node *secondLastNode = pointerToLinkedList;
    //loop until it becomes second last
    while(secondLastNode->link != NULL){
        secondLastNode = secondLastNode->link;
    }
    //actual linking
    secondLastNode->link = newNode;
}

//Function deletes node at the beginning of the linked list
void deleteHead(){

    //get reference of the head that will be deleted
    struct node *temp = pointerToLinkedList;

    //adjust pointerToLinkedList to the new head (previously the second node)
    pointerToLinkedList = pointerToLinkedList->link;

    //free memory of the old head
    free(temp);
}

//Function deletes node at the end of the linked list
void deleteTail(){

    //get reference of the head to iterate through linked list
    struct node *temp = pointerToLinkedList;
    struct node *newTail;

    //iterate through linked list
    while(temp->link != NULL){
        newTail = temp;
        temp = temp->link;
    }

    //free memory of old tail
    free(newTail->link);

    //adjust the new tail and point it to NULL
    newTail->link = NULL;
}

Two more things apart from the criticism:

Am I using the free() function properly? Is the structure actually being removed from the dynamic memory?
What other functions apart from the four currently installed, would you recommend to implement as an exercise?

I suggest you rename link to next which is a more conventional name for the next linked list node.

struct node {
    int data;
    struct node* link;
};

Currently, you have to write the type of the node as struct node. However, if you do:

typedef struct node {
    int data;
    struct node* link;
} node;

You can allocate a new node simply by writing node my_node;
No, no, no and no. Don't you ever use global variables. As the code base grows, if something goes wrong because of a global variable, debugging will be hell.

struct node *pointerToLinkedList = NULL;

You should not cast the result of malloc:

struct node *newNode = (struct node*) malloc(sizeof(struct node));

What comes to struct node *newNode = (struct node*) malloc(sizeof(struct node)), I suggest you write instead:

malloc(sizeof *newNode)

That way, if you change the type of *newNode to, say, some_funky_struct, you don't have to rewrite malloc(sizeof(struct node)) to malloc(sizeof(struct some_funky_struct)).
There is no pointer to the tail of your list. You define one (called, say, list_tail), you can insertNodeEnd in constant time.
I suggest you rename it to list_head, since that is a conventional name for the first node of a linked list.

struct node *pointerToLinkedList = NULL;

Normally, what you would do when implementing a singly-linked list is creating a following:

typedef struct linked_list_node_t {
    void* datum;
    struct linked_list_node_t* next;
} linked_list_node;

typedef struct linked_list_t {
    linked_list_node_t* head;
    linked_list_node_t* tail;
    size_t size;
} linked_list_t;

linked_list_t* linked_list_t_alloc() {
    linked_list_t* list = malloc(sizeof *list);

    if (!list) {
        abort();
    }

    list->head = NULL;
    list->tail = NULL;
    list->size = 0;
    return list;
}

linked_list_t_operation(linked_list_t* list, ... operands) {
    ... Your implementation.
}

Alternative implementation

You could do:

singly_linked_list.h

#ifndef SINGLY_LINKED_LIST_H
#define SINGLY_LINKED_LIST_H

#include <stdlib.h>

typedef struct singly_linked_list_node_t {
    int datum;
    struct singly_linked_list_node_t* next;
} singly_linked_list_node_t;

typedef struct singly_linked_list_t {
    singly_linked_list_node_t* head;
    singly_linked_list_node_t* tail;
    size_t size;
} singly_linked_list_t;

typedef struct singly_linked_list_iterator_t {
    singly_linked_list_node_t* next_node;
} singly_linked_list_iterator_t;

/******************************************
* Allocates and returns a new empty list. *
******************************************/
singly_linked_list_t* singly_linked_list_alloc();

/**********************
* Initializes a list. *
**********************/
int singly_linked_list_init(singly_linked_list_t* list);

/*************************
* Frees the entire list. *
*************************/
void singly_linked_list_free(singly_linked_list_t* list);

/***************************
* Clears the list content. *
***************************/
void singly_linked_list_clear(singly_linked_list_t* list);

/**********************************************
* Pushes a new datum to the head of the list. *
**********************************************/
int singly_linked_list_push_front(singly_linked_list_t* list, int datum);

/**********************************************
* Pushes a new datum to the tail of the list. *
**********************************************/
int singly_linked_list_push_back(singly_linked_list_t* list, int datum);

/*****************************
* Pops the head of the list. *
*****************************/
int singly_linked_list_pop_front(singly_linked_list_t* list);

/*****************************
* Pops the tail of the list. *
*****************************/
int singly_linked_list_pop_back(singly_linked_list_t* list);

/**********************************************
* Returns the number of elements in the list. *
**********************************************/
size_t singly_linked_list_size(singly_linked_list_t* list);

/**************************************
* Returns the iterator over the list. *
**************************************/
singly_linked_list_iterator_t*
singly_linked_list_get_iterator(singly_linked_list_t* list);

/******************************************************************
* Returns non-zero value if the iterator has something to remove. *
******************************************************************/
int singly_linked_list_iterator_has_next(singly_linked_list_iterator_t* iterator);

/**************************
* Returns the next datum. *
**************************/
int singly_linked_list_iterator_next(singly_linked_list_iterator_t* iterator);

#endif /* SINGLY_LINKED_LIST_H */

singly_linked_list.c

#include "singly_linked_list.h"
#include <stdlib.h>

singly_linked_list_t* singly_linked_list_alloc()
{
    return calloc(1, sizeof(singly_linked_list_t));
}

int singly_linked_list_init(singly_linked_list_t* list)
{
    if (!list)
    {
        return 0;
    }

    list->head = NULL;
    list->tail = NULL;
    list->size = 0;
    return 1;
}

void singly_linked_list_free(singly_linked_list_t* list)
{
    if (list)
    {
        singly_linked_list_clear(list);
        free(list);
    }
}

void singly_linked_list_clear(singly_linked_list_t* list)
{
    singly_linked_list_node_t* current_node;
    singly_linked_list_node_t* next_node;

    if (!list)
    {
        return;
    }

    current_node = list->head;

    while (current_node)
    {
        next_node = current_node->next;
        free(current_node);
        current_node = next_node;
    }

    list->head = NULL;
    list->tail = NULL;
    list->size = 0;
}

int singly_linked_list_push_front(singly_linked_list_t* list, int datum)
{
    singly_linked_list_node_t* new_node;

    if (!list)
    {
        return 0;
    }

    new_node = malloc(sizeof *new_node);

    if (!new_node)
    {
        return 0;
    }

    new_node->datum = datum;

    if (list->size == 0)
    {
        list->head = new_node;
        list->tail = new_node;
    }
    else
    {
        new_node->next = list->head;
        list->head = new_node;
    }

    list->size++;
    return 1;
}

int singly_linked_list_push_back(singly_linked_list_t* list, int datum)
{
    singly_linked_list_node_t* new_node;

    if (!list)
    {
        return 0;
    }

    new_node = malloc(sizeof *new_node);

    if (!new_node)
    {
        return 0;
    }

    new_node->datum = datum;

    if (list->size == 0)
    {
        list->head = new_node;
        list->tail = new_node;
    }
    else
    {
        list->tail->next = new_node;
        list->tail = new_node;
    }

    list->size++;
    return 1;
}

int singly_linked_list_pop_front(singly_linked_list_t* list)
{
    int result;
    singly_linked_list_node_t* node;

    if (!list)
    {
        abort();
    }

    if (list->size == 0)
    {
        abort();
    }

    result = list->head->datum;
    node = list->head;
    list->head = list->head->next;
    list->size--;

    if (!list->head)
    {
        list->tail = NULL;
    }

    free(node);
    return result;
}

int singly_linked_list_pop_back(singly_linked_list_t* list)
{
    int result;
    singly_linked_list_node_t* current_node;
    singly_linked_list_node_t* previous_node;

    if (!list)
    {
        abort();
    }

    if (list->size == 0)
    {
        abort();
    }

    current_node = list->head;
    previous_node = NULL;

    while (current_node->next)
    {
        previous_node = current_node;
        current_node = current_node->next;
    }

    result = current_node->datum;

    if (!previous_node)
    {
        list->head = NULL;
        list->tail = NULL;
    }
    else
    {
        previous_node->next = NULL;
        list->tail = previous_node;
    }

    list->size--;
    free(current_node);
    return result;
}

size_t singly_linked_list_size(singly_linked_list_t* list)
{
    if (!list)
    {
        abort();
    }

    return list->size;
}

singly_linked_list_iterator_t*
singly_linked_list_get_iterator(singly_linked_list_t* list)
{
    singly_linked_list_iterator_t* iterator;

    if (!list)
    {
        return NULL;
    }

    iterator = malloc(sizeof *iterator);

    if (!iterator)
    {
        return NULL;
    }

    iterator->next_node = list->head;
    return iterator;
}

int singly_linked_list_iterator_has_next(singly_linked_list_iterator_t* iterator)
{
    if (!iterator)
    {
        abort();
    }

    return iterator->next_node != NULL;
}

int singly_linked_list_iterator_next(singly_linked_list_iterator_t* iterator)
{
    int result;

    if (!iterator)
    {
        abort();
    }

    if (!singly_linked_list_iterator_has_next(iterator))
    {
        abort();
    }

    result = iterator->next_node->datum;
    iterator->next_node = iterator->next_node->next;
    return result;
}

(Demo driver is here.)
A weakness of a singly-linked list is that we have to scan the entire list whenever popping the back in order to find the second last node. I suggest you use a doubly-linked list after all since it allows popping the back of the list in \$\Theta(1)\$.


API Client: Querying data out of an endpoint I have a simple piece of code which connects to an API, does a request and gets back the raw data.
After getting this data I create several objects (models) and persist them into a MongoDB database.

Example of a model:

class Match:
    def __init__(self, data):
        self.data = data

    @property
    def match_id(self):
        """The Match id"""
        return self.data['attributes']['id']
    ...

Example of API CLient

class Client:
    def __init__(self):
        self.session = requests.Session()

    def request(self, endpoint, params=None):
        response = self.session.get(endpoint, timeout=TIMEOUT, params=params)
        res_code = response.status_code
        if res_code != API_OK:
            exception = API_ERRORS.get(res_code)
            raise exception
        return response.json()

Example of the controller:

class Handler(Client):
    def __init__(self, api_key):
        super().__init__()
        self.api_key = api_key

    def query_match(self, match_id, shard='default'):
        res = []
        url = f'{SHARD_URL}/{shard}/matches/{match_id}'
        res = self.request(url)
        return Match(res)

And then I have the logic for getting the data into mongoDB after calling get_player

class BuildAndStore(Handler):
    def __init__(self, mongo_client, api_key):
        super().__init__(api_key)
        self.mongo_client = mongo_client
        self.players = list_players

    def process_matches(self, list_matches):
        ret = []
        for m_i in list_matches:
            match = self.query_match(m_id)
            json_data = build_json(match) # Extracts certain class properties
            ret.append(json_data)
        mongo.insert_many('matches', ret)

Having in mind I want to start consulting the API in parallel (multiple threads or moving to async) my questions are:

Is this pythonic? Is there a better way to do so?
Currently the Handler does the query and the Model receives the payback data. Instead of proceeding passing the JSON data, should I do the query for each model within their own class, passing the match_id and inheriting from "Handler" class Match(Handler): ?

In the request method the exception variable really doesn't have to
be set before using it, just inline it and make the code more succinct:

def request(self, endpoint, params=None):
    response = self.session.get(endpoint, timeout=TIMEOUT, params=params)
    res_code = response.status_code
    if res_code != API_OK:
        raise API_ERRORS[res_code]
    return response.json()

Also, I'm guessing API_ERRORS[res_code] will also work and be more
expected for any reader.

In the query_match method res doesn't have to be initialised, that's
not pythonic actually:

def query_match(self, match_id, shard='default'):
    url = f'{SHARD_URL}/{shard}/matches/{match_id}'
    return Match(self.request(url))

The process_matches method could be a bit simpler:

def process_matches(self, list_matches):
    ret = []
    for m_i in list_matches:
        ret.append(build_json(self.query_match(m_id)))
    mongo.insert_many('matches', ret)

Or, there's the option to go for a less imperative list comprehension
too:

def process_matches(self, list_matches):
    ret = [build_json(self.query_match(m_id))
           for m_i in list_matches]]
    mongo.insert_many('matches', ret)

Regarding your second question, if I read it right, no, not a good idea
from what I can tell from the code, leaving Match as a pure data
object will serve you better in the long run.  It's better if the two
responsibilities aren't mixed.  It'll also make it more clear what's
being tested in your test cases if you can clearly distinguish between
something that just holds the data and a API client (wrapper) that does
networking (for example).


Serializing a class hierarchy to JSON I have a hierarchical object structure which needs to be converted to JSON. I have a following basic implementation of how it will be done:

#include <iostream>
using namespace std;

class base
{
    public:
    base():a(1){};
    void serialize()
    {
        cout<<"{"<<endl;
        toString();
        cout<<"}";
    };
    virtual void toString()
    {
        cout<<"\"base-a\": "<<a<<endl;
    };

    private:
    int a;
};

class derived: public base
{
    public:
    derived():b(2){};
    virtual void toString()
    {
        base::toString();
        cout<<"\"derived-b\": "<<b<<endl;
    };

    private:
    int b;
};

int main() {
    derived d;
    base& b = d;
    b.serialize();
    return 0;
}

The only problem I see here is when derived is extended then that class' toString function has to call the toString function of derived and so on.

Is calling a overridden function from overriding function a bad practice?
Is there a better way to write this code?

Note: I cannot use C++11.

Is calling a overridden function from overriding function a bad practice? Is there a better way to write this code?

If you are adding serialization support to an existing class hierarchy, then what you are doing is A-O-K.

However, if you are in the process of designing that structure, then it's indirectly considered bad practice.

The whole "subclass to extend" design pattern is frowned upon in general by modern design sensibilities.

If you want to be "modern", then your virtual functions should be pure in the base class, and your class hierarchy should rarely go beyond a single inheritance depth.

The mantra is "prefer composition to inheritance, unless you need type erasure". However, this doesn't mean that everything is fair game as soon as type erasure is needed. You should still strive to limit your use of hierarchy to the minimum required in order to achieve erasure, i.e: Base classes should be pure interface classes, nothing more.

The details on how best to do that really depends on the project, so I can't really go into details here.

Edit: As requested, a more in-depth explanation of composition vs inheritance.

Disclaimer: It's a fairly subtle, almost academic, subject.

The theory goes like this:

Apart from type erasure, there is nothing gained by inheritance that cannot be accomplished through composition.
Composition is more flexible. Primarily because you don't run the risk of running into clashing namespaces, but also because diamond patterns are easier to work with.

A good example of this is shared_ptr<> vs having a RefCounted base class at the bottom of your hierarchy.

With shared_ptr<>, which is composition-based, you get a few major benefits:

You can apply the feature to anything, even base types.
You won't run into a diamond pattern if you need to inherit from two interfaces.
You don;t pay for the feature until  you are actually using it.


Get data at beginning or before use it I have two style of code but don't know which is better or maybe you could recommend the better one.

The good part of first one is every if else branches return something and the process will stop at there but the bad part is even the Captcha is invalid I still need to query the DisalbeUser() and BlackList()

        bool isValidateCaptcha = ValidateCaptcha();

        bool isDisableUser = DisableUser();

        bool isInblackList = BlackList();

        if (isValidateCaptcha)
        {
            return Ok(new
            {
                IsSuccess = false,
                InvalidCode = LoginInvalidCode.CaptchaInvalid
            });
        }
        else if (isDisableUser)
        {
            return Ok(new
            {
                IsSuccess = false,
                InvalidCode = LoginInvalidCode.DisableAccountAndInDisableGroup
            });
        }
        else if (isInblackList)
        {
            return Ok(new
            {
                IsSuccess = false,
                InvalidCode = LoginInvalidCode.InLoginBlackList
            });
        }
        else
        {
            return Ok(new
            {
                IsSuccess = true,
            });
        }

And the second style is like below

        bool isValidateCaptcha = ValidateCaptcha();

        if (isValidateCaptcha)
        {
            return Ok(new
            {
                IsSuccess = false,
                InvalidCode = LoginInvalidCode.CaptchaInvalid
            });
        }

        bool isDisableUser = DisableUser();

        if (isDisableUser)
        {
            return Ok(new
            {
                IsSuccess = false,
                InvalidCode = LoginInvalidCode.DisableAccountAndInDisableGroup
            });
        }

        bool isInblackList = BlackList();

        if (isInblackList)
        {
            return Ok(new
            {
                IsSuccess = false,
                InvalidCode = LoginInvalidCode.InLoginBlackList
            });
        }

        return Ok(new
        {
            IsSuccess = true,
        });

Concerning

  which one is better

I would prefer the first one, as you can see your values from the beginning, then follow the if-else logic.

However, none if these are very readible!

Naming

Your variable and method names are very misleading. BlackList() looks like you call a constructor. Method names should be verbs (or start with one).

Your variable naming scheme would actually fit for your methods, as IsInBlackList() is the typical naming for methods that return boolean (true/false) values. The variables could then just be called var inBlackList = IsInBlackList().

Logic

It is not clear whether your validation cases are mutually exclusive. Can a captcha be valid and still the user be in the blacklist?

Concerning this code: if (isValidateCaptcha) ... IsSuccess = false, ...
Your variable name implies that it is true if the captcha is valid (which it should, however a better name might be isCaptchaValid). But then you set IsSuccess to false, which makes it seem like your method returns false if the captcha is true, making the logic very hard to understand without looking into the method.

Conciseness

Do you even need these variables? Your variable names are almost identical to your method names (semantically, considering the changes suggested above).

Consider this:

if (IsCaptchaValid()) // replaced the variable with the method call
{
    return Ok(new
    {
        IsSuccess = false,
        InvalidCode = LoginInvalidCode.CaptchaInvalid
    });
}

This way your code becomes shorter and more readible, and your method names should show their intent anyways.

Redundance

This part

return Ok(new
{
    IsSuccess = ...
    InvalidCode = ...
});

appears multiple times with very minor differences. You could increase readability by merging it into one.

For example

LoginInvalidCode invalidCode;
if (!IsCaptchaValid())
{
    invalidCode = LoginInvalidCode.CaptchaInvalid;
}
else if (IsUserDisabled()) {
    invalidCode = LoginInvalidCode.DisableAccountAndInDisableGroup;
}
else if (IsInBlackList()) {
    invalidCode = LoginInvalidCode.InLoginBlackList;
}

var success = invalidCode != null;

return Ok(new { IsSuccess = success, InvalidCode = invalidCode });


Is my cipher secure? This is a program that I made for encrypting text files, it uses a one time pad cipher to encrypt the files, but I do not know if there are any holes in my program that could be a vulnerability.

import os
q = 1
while q == 1:
    e = raw_input("file to encypt: ")
    #This will open a file for encryption
    o = open(e, "r")
    o1 = o.read()
    #This is the plain text to encrypt
    #'The quick brown fox jumps over the lazy dog'
    plain = o1
    #This will measure the length of the plain text
    f3 = len(plain)
    #generate random chacters as long as the text
    a1 = os.urandom(f3)
    #makes the random characters tuple format
    b = list(a1)
    b2 = list(plain)
    s = plain
    #gives the ascii value of the charters
    L = [ord(c) for c in s]
    s = a1
    a = [ord(c) for c in s]
    b = [ord(c) for c in plain]
    #adds the random digits and the plain text
    c = map(sum, zip(a,b))
    #uses Modular arithmetic if the sum is greater than 256
    x=c
    z = []
    for y in x:
        z.append(y-256 if y>=256 else y)
    z = [y-256 if y >= 256 else y for y in x]
    #converts the sum back to charter form
    cipher_text = ''.join(chr(i) for i in z)
    #makes a folder for the files
    base1 = os.path.basename(e)
    base2 = os.path.splitext(base1)[0]
    #makes a folder for the output
    p = "/Users/kyle/one_time_pad/"+base2
    print p
    if os.path.exists(p):
        print
    else:
        os.mkdir(p)

    key = a1
    #makes a file containg the key
    p = p + "/"
    f2 = p+"key.txt"
    #print f2
    if os.path.exists(f2):
        f1 = file(f2, "w")
        f1 = open(f2, "w")
        f1.write(key)
        f1.close()
    else:
        f1 = file(f2, "w")
        f1 = open(f2, "w")
        f1.write(key)
        f1.close()

    #makes a file containg the cipher text
    f3 = p+"cipher_text.txt"
    if os.path.exists(f3):
        f1 = file(f3, "w")
        f1 = open(f3, "w")
        f1.write(cipher_text)
        f1.close()
    else:
        f1 = file(f3, "w")
        f1 = open(f3, "w")
        f1.write(cipher_text)
        f1.close()

    f4 = p+"encrypt.py"
    encrypt1 = open("/Users/kyle/encrypt.py", "r")
    encrypt = encrypt1.read()
    if os.path.exists(f4):
        f1 = file(f4, "w")
        f1 = open(f4, "w")
        f1.write(encrypt)
        f1.close()
    else:
        f1 = file(f4, "w")
        f1 = open(f4, "w")
        f1.write(encrypt)
        f1.close()

    f5 = p+"decrypt.py"
    encrypt1 = open("/Users/kyle/decrypt.py", "r")
    encrypt = encrypt1.read()
    if os.path.exists(f5):
        f1 = file(f5, "w")
        f1 = open(f5, "w")
        f1.write(encrypt)
        f1.close()
    else:
        f1 = file(f5, "w")
        f1 = open(f5, "w")
        f1.write(encrypt)
        f1.close()

    print 50*"-"

This is the code that I use for decryption:

import os

q = 1
while q == 1:
    #opens the cipher text and it converts it to decimal
    cipher = raw_input("cipher text: ")
    cipher1 = open(cipher, "r")
    cipher2 = cipher1.read()
    cipher3 = [ord(c) for c in cipher2]

    #opens the key and coverts it to decimal
    key = raw_input("key: ")
    key1 = open(key, "r")
    key2 = key1.read()
    key3 = [ord(c) for c in key2]

    #subtracts the key from the cipher
    a = cipher3
    b = key3
    c = map(lambda x: (x[0]-x[1]) % 256, zip(a,b))

    #prints out the decrypted plain text
    decrypt = ''.join(map(chr,c))
    #makes a file with the decrypted output
    path1 = raw_input("out folder: ")
    name = "plain_text.txt"
    path2 = path1 + "/" + name
    if os.path.exists(path2):
        f1 = file(path2, "a")
        f1 = open(path2, "a")
        f1.write(decrypt)
        f1.close()
    else:
        f1 = file(path2, "w")
        f1 = open(path2, "w")
        f1.write(decrypt)
        f1.close()

    print 50*"-"

As you are no doubt aware, one-time pads are cryptographically secure according to Kerckhoff's principle, assuming the key is sufficiently random and secret. From a brief glance, your implementation of OTP looks correct. (But I might of course have missed something.)

However, whether the program itself has any vulnerabilities would not be relevant as long as you write the key to a file. Anyone with access to your memory or harddisk would be able to intercept the file, and therefore the key. This means the question boils down to "Is my implementation correct?", and the answer to that is "Yes", as far as I can tell.

I might be wrong, however, because your program is not very readable. Some of the things you can do to clean it up are:

1: Improve variable names. Several of your variable names are single letters. plaintext_length is way more readable than f3.

2: Split your code into logical units. Refactor into functions, and group code that logically belongs together by separating such groups with a single empty line.

3: Why on earth are you doing this?!

if os.path.exists(f2):
    f1 = file(f2, "w")
    f1 = open(f2, "w")
    f1.write(key)
    f1.close()
else:
    f1 = file(f2, "w")
    f1 = open(f2, "w")
    f1.write(key)
    f1.close()

As far as I can see, the two blocks are identical, and one of them will be executed regardless. And as Anthon points out, the first two lines in each block do the same.


Benchmarking of SQL data crunching using a common datum @nhgrif and I were curious about a couple different ways to aggregate a number of records based on a duplicate/common datum. Two different ideas will be shown. Any advice on all code but especially making it perform better is appreciated.

Note: @nhgrif did not write most of this, so if it's wrong it's my fault...

Make a bunch of data

Code:

set search_path = test;
-- create tables to seed a large number of records
drop table if exists contract_number_seed;
create table contract_number_seed(
    contract_number int
    );
insert into contract_number_seed
    (contract_number)
    select * from generate_series(1, 100000)
    ;
drop table if exists serial_number_seed;
create table serial_number_seed(
    serial_number int
    );
insert into serial_number_seed
    (serial_number)
    select * from generate_series(1, 10)
    ;
drop table if exists status_seed;
create table status_seed(
    status text
    );
insert into status_seed (status)
values (NULL), ('one'), ('two'), ('three');
-- generate a whole bunch of records from seed tables
drop table if exists seeded;
select
    contract_number_seed.contract_number,
    serial_number_seed.serial_number,
    status_seed.status
into seeded
from contract_number_seed
cross join serial_number_seed
cross join status_seed;
-- Query returned successfully: 4000000 rows affected, 11368 ms execution time.

Just for the record:

set search_path = test;
select * from seeded;
-- Total query runtime: 8056 ms.
-- 4000000 rows retrieved.

Using LEFT OUTER JOIN

PS: Capitalization is different in this; @nhgrif wrote it and I was lazy.
PPS: Table aliases are not that great but I'm just benchmarking, please forgive...

SET SEARCH_PATH = test;
SELECT
    seeded.Serial_Number,
    COUNT(one.Contract_Number) as One,
    COUNT(two.Contract_Number) as Two,
    COUNT(three.Contract_Number) as Three
FROM seeded
  LEFT JOIN seeded as one
    ON one.Contract_Number = seeded.Contract_Number
    AND seeded.Status = 'one'
  LEFT JOIN seeded as two
    ON two.Contract_Number = seeded.Contract_Number
    AND seeded.Status = 'two'
  LEFT JOIN seeded as three
    ON three.Contract_Number = seeded.Contract_Number
    AND seeded.Status = 'three'
GROUP BY seeded.Serial_Number, seeded.Status;

I found the performance of this to be abysmal:

Total query runtime: 3096218 ms. -- ~30 minutes
10 rows retrieved.

A faster way... Using case when ... then

set search_path = test;
select
        seeded.Serial_Number,
    case
        when seeded.Status = 'one' then 1
        else 0
        end,
    case
        when seeded.Status = 'two' then 2
        else 0
        end,
    case
        when seeded.Status = 'three' then 3
        else 0
        end
from seeded
group by seeded.Serial_number, seeded.Status;

Performance:

Total query runtime: 5609 ms.
40 rows retrieved.

Are there better ways to do this? I found the first way to work OK on small queries but really lack on big ones. I found the 2nd one to be fast, however any new column in seeded.Serial_Number requires a refactoring of the queries...

There is a bug in your second version. Here is one row from the result:

1;0;2;0

That is, Serial_Number 1 has zero ones, two twos, and zero threes.

But let's do a quick sanity check:

select count(*) from seeded
where serial_number = 1 and status = 'one';

This query returns 100000.

There are also rows with the same Serial_Number, which I think is not what you want.

I think you want a query like this:

select
    seeded.Serial_Number,
    sum(case when seeded.Status = 'one' then 1 else 0 end) as ones,
    sum(case when seeded.Status = 'two' then 1 else 0 end) as twos,
    sum(case when seeded.Status = 'three' then 1 else 0 end) as threes
from seeded
group by seeded.Serial_Number;

This takes ~2.1s on my machine.

Just for fun, let's look at what pgAdmin's EXPLAIN visualisation looks like for this query

versus the 30m query


Pokemon game in JavaScript I'm practicing my JavaScript skills by trying to create a Pokemon game. So far, I've made the dataset and the functions required to work the game. Can someone help me optimize the code and tell me why they made that change?

function pokemon(name, hp, critical_attack, lowest_attack, highest_attack) {
    this.name = name;
    this.hp = hp;
    this.critical_attack = critical_attack;
    this.lowest_attack = lowest_attack;
    this.highest_attack = highest_attack;
}

var Charizard = new pokemon("Charizard", 90, 30, 1, 15);
var Blastoise = new pokemon("Blastoise", 95, 28, 1, 14);
var Venasaur = new pokemon("Venasaur", 100, 26, 1, 13);

var poke_choice = new Array ("Charizard", "Blastoise", "Venasaur")

function player(type, name, pokemon) {
    this.type = type;
    this.name = name;
    this.pokemon = pokemon;
}

var user = new player("User", "Arhum", "Charizard");
console.log(window[user.pokemon].hp)

for(i = 0; i<= 2; i++) {
  if (poke_choice[i] === user.pokemon) {
    poke_choice.splice(i,1);
  }
}

var comp_choice = function() {
  return poke_choice[Math.round(Math.random())]
}

var comp = new player("Computer","Red", comp_choice());

var user_attack = function() {
  var uhigh = window[user.pokemon].highest_attack
  var ulow = window[user.pokemon].lowest_attack
  return Math.round(Math.random().toFixed(1)*uhigh)
}

var comp_attack = function() {
  var chigh = window[comp.pokemon].highest_attack
  var clow = window[comp.pokemon].lowest_attack
  return Math.round(Math.random().toFixed(1)*chigh)
}

console.log(window[comp.pokemon].name)
console.log(window[user.pokemon].name)
console.log(user_attack())
console.log(comp_attack())

function player(type, name, pokemon) {
    this.type = type;
    this.name = name;
    this.pokemon = pokemon;
}

var user = new player("User", "Arhum", "Charizard");
console.log(window[user.pokemon].hp)

There's some really strange and bad practices going on here, especially with the pokemon field. Here, you are relying on the fact that the string passed in will correspond with some variable on the global scope that is hopefully of the type pokemon.

That's not the right way to do things. Instead, player objects should be instantiated with the actual pokemon object itself:

var user = new player("User", "Arhum", Charizard);

Then, whenever you need to access information about the player's pokemon, rather than doing this:

window[user.pokemon]

You just do this:

user.pokemon

This is a lot safer, and follows practices much better.

var user_attack = function() {
  var uhigh = window[user.pokemon].highest_attack
  var ulow = window[user.pokemon].lowest_attack
  return Math.round(Math.random().toFixed(1)*uhigh)
}

var comp_attack = function() {
  var chigh = window[comp.pokemon].highest_attack
  var clow = window[comp.pokemon].lowest_attack
  return Math.round(Math.random().toFixed(1)*chigh)
}

These two functions do exactly the same exact thing, except they are for two very specific instances of a class. This is also terrible practice, as nothing is flexible here and functions not only repeat themselves but rely on two very specific objects to be, again, in the global scope.

Instead, these two functions should become one instance method of the player class:

player.prototype.attack = function() {
  var uhigh = this.pokemon.highest_attack
  var ulow = this.pokemon.lowest_attack
  return Math.round(Math.random().toFixed(1)*uhigh)
}

Then, you should use this methods instead of those other two functions.

user.attack();
comp.attack();

You may want to rename to doAttack as the wording can be confusing.

function player(type, name, pokemon) {
    this.type = type;
    this.name = name;
    this.pokemon = pokemon;
}

var user = new player("User", "Arhum", "Charizard");

Are there any types other than "User" and "Computer"? If not, then you should turn this property into a boolean value something like this:

function player(isComputer, name, pokemon) {
    this.isComputer = isComputer;
    this.name = name;
    this.pokemon = pokemon;
}

var user = new player(false, "Arhum", "Charizard");

var Charizard = new pokemon("Charizard", 90, 30, 1, 15);
var Blastoise = new pokemon("Blastoise", 95, 28, 1, 14);
var Venasaur = new pokemon("Venasaur", 100, 26, 1, 13);

var poke_choice = new Array ("Charizard", "Blastoise", "Venasaur")

Now that we're done relying on these three pokemon variables to be in the global scope, we can clean up this part of the code, too.

We don't need to explicitly store the three pokemon variables anymore with their variable names; those can just go straight into the array. However, when we do that, we can no longer access the pokemon by name so it would be best to create a map/object for this situation:

var pokeChoice = {
    Charizard: new pokemon(...),
    Blastoise: new pokemon(...),
    ...
}

With this in place, this

for(i = 0; i<= 2; i++) {
  if (poke_choice[i] === user.pokemon) {
    poke_choice.splice(i,1);
  }
}

can be simplified to:

if(pokeChoice.hasOwnProperty(user.pokemon.name)) {
    delete pokeChoice[user.pokemon.name];
}

Here is your full code now:

function pokemon(name, hp, critical_attack, lowest_attack, highest_attack) {
    this.name = name;
    this.hp = hp;
    this.critical_attack = critical_attack;
    this.lowest_attack = lowest_attack;
    this.highest_attack = highest_attack;
}
var pokeChoice = {
    Charizard: new pokemon("Charizard", 90, 30, 1, 15),
    Blastoise: new pokemon("Blastoise", 95, 28, 1, 14),
    Venasaur: new pokemon("Venasaur", 100, 26, 1, 13)
};

function player(isComputer, name, pokemon) {
    this.isComputer = isComputer;
    this.name = name;
    this.pokemon = pokemon;
}
player.prototype.attack = function() {
    var uhigh = this.pokemon.highest_attack
    var ulow = this.pokemon.lowest_attack
    return Math.round(Math.random().toFixed(1)*uhigh)
}

var user = new player("User", "Arhum", pokeChoice.Charizard);
if(pokeChoice.hasOwnProperty(user.pokemon.name)) {
    delete pokeChoice[user.pokemon.name];
}

var comp_choice = function() {
    var pokeKeys = Object.keys(pokeChoice);
    var pokeAmt = pokeKeys.length;
    return pokeChoice[pokeKeys[Math.round(Math.random() * (pokeAmt-1))]]
}
var comp = new player("Computer","Red", comp_choice());

console.log(user.attack())
console.log(comp.attack())

I made some other small changes here and there. If you have any questions, feel free to ask.

Also, the naming convection in JavaScript is:

PascalCase for classes.
camelCase for functions.

You are breaking these rules in some cases.


Asynchronous model fitting that allows termination in Python The problem

When you work with Python interactively (e.g. in an IPython shell or notebook) and run a computationally intensive operation like fitting a machine-learning model that is implemented in a native code, you cannot interrupt the operation since the native code does not return execution control to the Python interpreter until the end of the operation. The problem is not specific to machine learning, although it is typical to run a training process for which you cannot predict the training time. In case it takes longer that you expected, to stop training you need to stop the kernel and thus lose the pre-processed features and other variables stored in the memory, i.e. you cannot interrupt only a particular operation to check a simpler model, which allegedly would be fit faster.

The solution

We propose an Asynchronous Fitter design pattern that runs fitting in a separate process and communicates the results back when they are available. It allows to stop training gracefully by killing the spawned process and then run training of a simpler model. It also allows to train several models simultaneously and work in the IPython notebook during model training. Note that multithreading is probably not an option, since we cannot stop a thread that runs an uncontrolled native code.

Here is a draft implementation:

from multiprocessing import Process, Queue
import time

class AsyncFitter(object):
    def __init__(self, model):
        self.queue = Queue()
        self.model = model
        self.proc = None
        self.start_time = None

    def fit(self, x_train, y_train):
        self.terminate()
        self.proc = Process(target=AsyncFitter.async_fit_,
            args=(self.model, x_train, y_train, self.queue))
        self.start_time = time.time()
        self.proc.start()

    def try_get_result(self):
        if self.queue.empty():
            return None

        return self.queue.get()

    def is_alive(self):
        return self.proc is not None and self.proc.is_alive()

    def terminate(self):
        if self.proc is not None and self.proc.is_alive():
            self.proc.terminate()
        self.proc = None

    def time_elapsed(self):
        if not self.start_time:
            return 0

        return time.time() - self.start_time

    @staticmethod
    def async_fit_(model, x_train, y_train, queue):
        model.fit(x_train, y_train)
        queue.put(model)

Usage

It is easy to modify a code that uses scikit-learn to adopt the pattern. Here is an example:

import numpy as np
from sklearn.svm import SVC

model = SVC(C = 1e3, kernel='linear')
fitter = AsyncFitter(model)
x_train = np.random.rand(500, 30)
y_train = np.random.randint(0, 2, size=(500,))
fitter.fit(x_train, y_train)

You can check if training is still running by calling fitter.is_alive() and check the time currently elapsed by calling fitter.time_elapsed(). Whenever you want, you can terminate() the process or just train another model that will terminate the previous one. Finally, you can obtain the model by try_get_result(), which returns None when training is in progress.

The issues

As far as I understand, the training set is being pickled and copied, which may be a problem if it is large. Is there an easy way to avoid that? Note that training needs only read-only access to the training set.

What happens if someone loses a reference to an AsyncFitter instance that wraps a running process? Is there a way to implement an asynchronous delayed resource cleanup?

Two minor notes, your terminate function should just use the is_alive test you've already defined.

def is_alive(self):
    return self.proc is not None and self.proc.is_alive()

def terminate(self):
    if self.is_alive():
        self.proc.terminate()
    self.proc = None

Also in time_elapsed you should test if self.start_time is None. That matches directly to the empty value it begins with and communicates the intent better.

def time_elapsed(self):
    if self.start_time is not None:
        return 0


Is given pattern a subString of given input string? KMP implementation My implementation of Knuth Morris Pratt algorithm. Looking forward for code review comments. Let me know of there is a better way to implement this algorithm.

import static org.junit.Assert.*;

import org.junit.Test;

public class IsPatternASubString {

    public static boolean isSubString(String s, String p){

        int sIndex = 0;
        int pIndex = 0;
        boolean matchStarted = false;

        while(sIndex < s.length()-1){

            if(s.charAt(sIndex) == p.charAt(pIndex)){
                //Are in the middle of matching process and just getting started
                matchStarted = (pIndex == 0) ? true : matchStarted;

                //Increment both index pointers
                sIndex++;
                //If last char of pattern match exit returning true
                if(pIndex == p.length()-1)
                    return true;
                pIndex++;
            }else{
                //Check if you are broken in the mid way while a matching started
                //If not in stop mid way then just incr sIndex and continue
                if(!matchStarted)
                    sIndex++;
                else{
                    String suffixPrefix = isSuffixIsAlsoAPrefix(p.substring(0, pIndex));
                    if(suffixPrefix != null){
                        pIndex = suffixPrefix.length();
                    }else{
                        //Reset match found flag and pIndex - start from O index of pattern
                        pIndex = 0;
                        matchStarted = false;
                    }
                }
            }
        }
        return false;
    }

public static String isSuffixIsAlsoAPrefix(String input){

        if(input == null || input.length() == 1)
            return null;

        int mid = input.length()/2;
        int left = mid-1;
        int right = input.length()-1;
        int matchCharCount = -1;

        while(left >= 0){
            if(input.charAt(left) == input.charAt(right)){
                matchCharCount = (matchCharCount == -1) ? left+1 : matchCharCount;
                left--;
                right--;
            }else{
                matchCharCount = -1;
                left--;
            }
        }
        return (matchCharCount != -1) ? input.substring(0, matchCharCount) : null;
    }

    @Test
    public void testValidInput_1(){
        String s = "ABCBABCBABDO";
        String p = "BABD";
        assertTrue(isSubString(s, p));
    }

    @Test
    public void testValidInput_2(){
        String s = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA";
        String p = "AAAAA";
        assertTrue(isSubString(s, p));
    }

    @Test
    public void testFailure(){
        String s = "BBBBBBBBBBBBBBBB";
        String p = "AAAA";
        assertFalse(isSubString(s, p));
    }

    @Test
    public void testValidInput_3(){
        String s = "BBBBBBBBBBBBBBBBCDEFBBBBBCDEFGHBBB";
        String p = "BBBCDEFG";
        assertTrue(isSubString(s, p));
    }

}

Bug #1

If the pattern matches to the end of the string, your function returns false:

String s = "ABC";
String p = "ABC";
System.out.println(isSubString(s, p));

false

Bug #2

Your function isSuffixIsAlsoAPrefix() is off.  Given a suffix of "ABCDAC", it returns "A" which is incorrect.  This leads to the following failure:

String s = "ABCDACBCDACGX";
String p = "ABCDACG";
System.out.println(isSubString(s, p));

true

Not a true KMP implementation

Even if your code were correct, isSuffixIsAlsoAPrefix() takes \$O(m)\$ time which causes the whole algorithm to take \$O(n*m)\$ time, where \$n\$ is the length of the string and \$m\$ is the length of the pattern.  A real KMP implementation would build a partial match table first, so that instead of calling isSuffixIsAlsoAPrefix() on a mismatch, it would instead look up the new value of pIndex from the table.  This leads to an overall time of \$O(n+m)\$.

I'm wondering how you came up with your algorithm, because it doesn't resemble any Knuth Morris Pratt implementation that I've ever seen.


Splitting an array of numbers into all possible combinations This takes an array of numbers then splits it into all possible combinations of the number array of size 4 then in another array puts the leftovers.  As I want to take the difference in averages of the first column and the second.

import itertools

#defines the array of numbers and the two columns
number = [53, 64, 68, 71, 77, 82, 85]
col_one = []
col_two = []

#creates an array that holds the first four
results = itertools.combinations(number,4)

for x in results:
    col_one.append(list(x))

#attempts to go through and remove those numbers in the first array
#and then add that array to col_two
for i in range(len(col_one)):
    holder = list(number)
    for j in range(4):
        holder.remove(col_one[i][j])
    col_two.append(holder)

col_one_average = []
col_two_average = []

for k in col_one:
    col_one_average.append(sum(k)/len(k))

for l in col_two:
    col_two_average.append(sum(l)/len(l))

dif = []

for i in range(len(col_one_average)):
    dif.append(col_one_average[i] - col_two_average[i])

print dif

So for example, if I have

a = [1,2,3]

and I want to split it into an array of size 2 and 1, I get

col_one[0] = [1,2]

and

col_two[0] = [3]

then

col_one[1] = [1,3]

and

col_two[1] = [2]

After I get all those I find the average of col_one[0] - average of col_two[0].

I hope that makes sense.  I'm trying to do this for a statistics class, so if there is a 'numpy-y' solution, I'd love to hear it.

import itertools
import numpy

number = [53, 64, 68, 71, 77, 82, 85]

results = itertools.combinations(number,4)
# convert the combination iterator into a numpy array
col_one = numpy.array(list(results))

# calculate average of col_one
col_one_average = numpy.mean(col_one, axis = 1).astype(int)

# I don't actually create col_two, as I never figured out a good way to do it
# But since I only need the sum, I figure that out by subtraction
col_two_average = (numpy.sum(number) - numpy.sum(col_one, axis = 1)) / 3

dif = col_one_average - col_two_average

print dif


python: is my program optimal I wrote code in python that works slow. Because I am new to python, I am not sure that I am doing everything right. My question is what I can do optimally?
About the problem: I have 25 *.json files, each is about 80 MB. Each file just contain json strings. I need make some histogram based on data.

In this part I want create list of all dictionaries ( one dictionary represent json object):

d = [] # filename is list of name of files
for x in filename:
d.extend(map(json.loads, open(x)))

then I want to create list u :

u = []
for x in d:
  s = x['key_1'] # s is sting which I use to get useful value
  t1 = 60*int(s[11:13]) + int(s[14:16])# t1 is useful value
  u.append(t1)

Now I am creating histogram:

plt.hist(u, bins = (max(u) - min(u)))
plt.show()

Any thought and suggestions are appreciated.
Thank you!

Python uses a surprisingly large amount of memory when reading files, often 3-4 times the actual file size. You never close each file after you open it, so all of that memory is still in use later in the program.

Try changing the flow of your program to

Open a file
Compute a histogram for that file
Close the file
Merge it with a "global" histogram
Repeat until there are no files left.

Something like

u = []
for f in filenames:
    with open(f) as file:
        # process individual file contents
        contents = file.read()
        data = json.loads(contents)
        for obj in data:
            s = obj['key_1']
            t1 = 60 * int(s[11:13]) + int(s[14:16])
            u.append(t1)

# make the global histogram
plt.hist(u, bins = (max(u) - min(u)))
plt.show()

with open as automatically closes files when you're done, and handles cases where the file can't be read or there are other errors.


Make binary search tree from sorted array Here is my code for converting a sorted array to a binary search tree. Please review and let me know the improvements or some better way of solving this.

/**
     * Definition for a binary tree node.
     * public class TreeNode {
     *     int val;
     *     TreeNode left;
     *     TreeNode right;
     *     TreeNode(int x) { val = x; }
     * }
     */
    public TreeNode sortedArrayToBST(int[] nums) {
        if (nums == null || nums.length == 0) {
            return null;
        }

        // If there is only one element, make it root node and return.
        if (nums.length == 1) {
            TreeNode root = new TreeNode(nums[0]);
            return root;
        }

        // else, find the mid element and the left of it as left and right to
        // right recursively
        else {
            int length = nums.length;
            TreeNode root = new TreeNode(nums[length / 2]);
            int[] numsLeft = Arrays.copyOfRange(nums, 0, nums.length / 2);
            int[] numsRight = Arrays.copyOfRange(nums, (nums.length / 2) + 1, nums.length);
            root.left = sortedArrayToBST(numsLeft);
            root.right = sortedArrayToBST(numsRight);
            return root;

        }

    }

Simplify

The special handling for nums.length == 1 is unnecessary. This works just as well:

public TreeNode sortedArrayToBST(int[] nums) {
    if (nums == null || nums.length == 0) {
        return null;
    }
    int length = nums.length;
    TreeNode root = new TreeNode(nums[length / 2]);
    int[] numsLeft = Arrays.copyOfRange(nums, 0, nums.length / 2);
    int[] numsRight = Arrays.copyOfRange(nums, (nums.length / 2) + 1, nums.length);
    root.left = sortedArrayToBST(numsLeft);
    root.right = sortedArrayToBST(numsRight);
    return root;
}

Improve

While your solution works, some aspects can be improved:

Array copy is not cheap, it would be better to avoid
The null check is only necessary on the first call, in recursive calls it will always be false

The way to solve both of these issues is to introduce a helper method taking the start and end points of a range in the source array. Something like this, the most juicy part omitted to avoid spoiling your exercise:

public TreeNode sortedArrayToBST(int[] nums) {
    if (nums == null) {
        return null;
    }

    return sortedArrayToBST(nums, 0, nums.length);
}

private TreeNode sortedArrayToBST(int[] nums, int start, int end) {
    if (start == end) {
        return null;
    }

    // ...
}

Note that there is no more check on nums.length == 0, the start == end in the helper takes care of that. The function can be completed by adding a few lines with recursive calls, with adjusted range parameters.


Building an index of items from an iterable-of-tuples Is there a more conventional / standard / easy-to-read way of building an index of tuples from an iterable than this:

# -----------------------------------------------------------------------------
def build_index(itable_tup):
    """
    Return an index for an iterable container of tuples, all the same length.
    The last element in each tuple is treated as the payload value
    that is to be indexed, and the preceeding elements as a hierarchy
    of indices by which it is to be indexed.
    The index itself is a nested default-dictionary with len(tup)
    levels of nesting, containing a list at the innermost level
    to hold the payload.
    >>> idx = build_index((('a','b', 1), ('a','c', 2)))
    >>> import json
    >>> print(json.dumps(idx, sort_keys=True))
    {"a": {"b": [1], "c": [2]}}
    This function is a bit of an exercise in making use of the
    functional programming capabilities that come with Python,
    so the  implementation is a little more terse and opaque
    than it might otherwise be...
    For each tuple in iter_tup, we construct a slot in the index
    using the lazy initialisation capability that is provided by
    the collections.defaultdict class. All but the last element
    in the tuple are used for indexing. The last element is the
    payload which gets added to the inner list after the structure
    has been initialised. The basic capability is provided by
    the nested defaultdict data structure returned from the idx_ctor
    function, which in turn is built with the aid of the higher
    order lambda function (_partial(_dedict, ctor)).
    All this is plugged together using calls to functools.reduce,
    which is used to construct the custom index data structure; to
    add each tuple to the index, as well as to iterate over the
    input.
    """
    _dedict   = collections.defaultdict
    _chain    = itertools.chain
    _repeat   = itertools.repeat
    _reduce   = functools.reduce
    _partial  = functools.partial
    _getitem  = operator.getitem

    # Peek ahead to determine the tuple length (& reconstitute the iterator).
    iter_tup  = iter(itable_tup)
    try:
        first_tup = iter_tup.__next__()
    except StopIteration:
        return {}
    idx_depth = len(first_tup) - 1
    iter_all  = _chain([first_tup], iter_tup)

    # Create a constructor for a custom index data structure.
    idx_ctor = _reduce(lambda ctor, _: _partial(_dedict, ctor),
                       _repeat(None, idx_depth), list)

    # This function adds a sigle tuple to the custom index data structure.
    def _add_tup_to_idx(idx, tup):
        """
        Add a sigle tuple to the custom index data structure.
        """
        _reduce(_getitem, tup[:-1], idx).append(tup[-1])
        return idx

    # This iterates over all tuples adding each to the index in turn.
    return _reduce(_add_tup_to_idx, iter_all, idx_ctor())

Don't use make variables scoped to a function 'private'.
They're 'protected' so please, just remove the _ from them.

If we look at your next block.
You use iter_tup.__next__().
Don't.
Use next!
If you see __{name}__ then you should always check if there is a better way.
And so I'd use:

first = next(iterator, None)
if first is None:
    return {}

The next block to get the length and to rebuild the iterator is fine.

The reduce is fine, I dislike how static it is.
And so if I were to write this function I'd change it.
But it's fine.

I'd change _add_tup_to_idx to just a plain for loop.
Sure it means you mutate something, oh well.
It's much easier to read and understand.
I'd change it to:

dictionary = idx_ctor()
for tup in iter_all:
    reduce(_getitem, tup[:-1], dictionary).append(tup[-1])
return dictionary

Finally I'll go over a few more 'high-level' things. It's much easier to read if you use good variable names.
Take for example:

def build_index(iterator):
    iterator = iter(iterator)
    first = next(iterator)
    if first is None:
        return {}
    depth = len(first) - 1
    iterator = chain([first], iterator)

    ddict = reduce(lambda d, _: partial(defaultdict, d),
                   repeat(None, depth),
                   list)
    dictionary = ddict()
    for item in iterator:
        reduce(getitem, item[:-1], dictionary).append(item[-1])
    return dictionary

Now you seem to put emphasis on it being a hash-tree.
But what do you get if you do:

build_index((('a',), ('b',), ('c',)))

Is it a dictionary, or is it a list?
Now what do you get if you enter these:

build_index((('a', 'b'), ('b', 'c'), ('c', 'd')))
build_index(())

Yes the they return different types.
The above two are dictionary's, and the first is a list.
That is a massive edge case.

I think the design should be based around an infinite default dict.
And for it to always return a dict.
And so you could use the following recursive function:

def ddict():
    return defaultdict(ddict)

Now we just have to change your for loop.
If we slice the string into two at the second to last position,
and remove the item from the list if the second is empty:

dictionary = ddict()
for head, tail in ((i[:-2], i[-2:]) for i in iterable):
    if not tail:
        continue

Now before you say, that's not functional!
Yeah it's half functional, well the comprehension is.
It's easier to read this way, as if Python were a functional based language it'd be more like:

iterable
    .map(i => (i[:-2], i[-2:]))
    .filter(i => i[1])

Now we could do that, but it'd be icky, take:

(i for i in ((i[:-2], i[-2:]) for i in iterable) if i[0])
filter(lambda i: i[0], map(lambda i: (i[:-2], i[-2:]), iterable))

And so they all are the same, so just go for the most readable.

For the last bit, we will do what you were doing before.
But with two changes.
To allow for tail to have a length of one, we need to use extend rather than append.
and to get a list you'll have to use setdefault with a list.

This gives a quite functional function:

def build_index(iterable):
    def ddict():
        return defaultdict(ddict)
    dictionary = ddict()
    for head, tail in ((i[:-2], i[-2:]) for i in iterable):
        if not tail:
            continue
        (reduce(getitem, head, dictionary)
            .setdefault(tail[0], [])
            .extend(tail[1:]))
    return dictionary

You can also change getitem to a function that uses setdefault to remove the need for defaultdict and getitem.


Makefile and main header file for a shell My main.c includes this header file. Please let me know how I can improve it. I did my best but I don't know much about code conventions in C (I come from Java and Python).

#define _XOPEN_SOURCE 500

#ifndef OPENSHELL_OPENSHELL_H
#define OPENSHELL_OPENSHELL_H
#define    CMD_LEN        1024
#define    EXPAND_ALLOC    1024
#define    PATH_LEN    1024
#define    isBlank(ch)    (((ch) == ' ') || ((ch) == '\t'))
#define    isDecimal(ch)    (((ch) >= '0') && ((ch) <= '9'))
#define    MAX_SOURCE    10
#define    isWildCard(ch)    (((ch) == '*') || ((ch) == '?') || ((ch) == '['))
#define    CHUNK_INIT_SIZE    4
#define    DEBUG TRUE
#ifndef VERSION
#define VERSION2 "v0.1a"
#endif

#include <string.h>
#include <dirent.h>
#include <utime.h>
#include <stdbool.h>
#include <getopt.h>

/*
 * A chunk of data.
 * Chunks contain data which is allocated as needed, but which is
 * not freed until all of the data needs freeing, such as at
 * the beginning of the next command.
 */
typedef struct chunk CHUNK;

struct chunk {
    CHUNK *next;
    char data[CHUNK_INIT_SIZE];    /* actually of varying length */
};
/*
 * One entry of the command table.
 */
typedef struct {
    const char *name;

    int        (*func)(int argc, const char **argv);

    int minArgs;
    int maxArgs;
    const char *description;
    const char *usage;
} CommandEntry;

typedef int Pipe[2];

static struct option options[] = {
        {"with_param", 1, 0, 'p'},
        {"version",    0, 0, 'v'},
        {"help",       0, 0, 'h'},
        {0,            0, 0, 0}
};
struct command {
    char *const *argv;
};
extern void freeChunks(void);

extern char **str_split(char *a[], char *a_str, const char a_delim);

extern int do_checkenv(int argc, const char **argv);

extern int do_cd(int argc, const char **argv);

extern int do_add2path(int argc, const char **argv);

extern int do_exit(int argc, const char **argv);

extern int do_help(int argc, const char **argv);

extern int do_kill(int argc, const char **argv);

int spawn_proc(int in, int out, struct command *cmd);

void fork_pipes(int n, struct command *cmd);

int file_exist(char *filename);

extern bool makeArgs(const char *cmd, int *argcPtr, const char ***argvPtr, bool pipe, int g, int h);

#endif
#define VERSION "v0.160429-21-g54bb-dirty"

The above file is versioned at github. You can also review the project's Makefile:

CC = gcc
GIT_VERSION := $(shell git describe --abbrev=4 --dirty --always --tags)
CFLAGS := $(CFLAGS) -L/usr/local/include/ -L/usr/include -pedantic -std=c99 -Wall -O3 -g -DVERSION=\"$(GIT_VERSION)\" -ledit -lncurses

LDIRS = -L/usr/local/lib -L/usr/lib
LIBS =su -lcurses

shell: main.o
    $(CC) -o shell main.o errors.c util.c pipeline.c -ledit -lncurses -lcurses

main.o: main.c errors.c util.c
USERNAME := $(shell whoami >> username.txt)
GIT:= $(shell head -n -1 openshell.h > temp.txt ; mv temp.txt openshell.h;git describe --abbrev=4 --dirty --always --tags > VERSION; echo "\#define VERSION \"$(GIT_VERSION)\"" >> openshell.h)

.PHONY: clean
clean:
    rm -f *.o

As seen in the Makefile, I'm writing the git version number to the header file with a shell instruction. Maybe I should do this before I compile to be able to get a "clean" version, otherwise I will always differ from a tag just by writing the new tag.

What is the header file's name?  The point being that the name of the header file, like xopen.h should have some connection to the items inside.  Further, the objects declares and macros defines are all over the name space.  Consider some convention. Put do_...() in do.h, CommandEntry in CommandEntry.h.
Strange #define.  Did you mean #define VERSION "v0.1a"?

#ifndef VERSION
//#define VERSION2 "v0.1a"
#define VERSION "v0.1a"
#endif

static struct option options[] will create a options[] with each *.c this include file is used.  Certainly not a good idea because it is static.  `static objects are best for the *.c file and not *.h header files.
Odd to have #define _XOPEN_SOURCE 500 and #define VERSION "v0.160429-21-g54bb-dirty" outside the include guard #ifndef OPENSHELL_OPENSHELL_H.  Suggest putting them inside unless you are attempting something novel.
Line spacing is odd.  Why no line space after a struct definition, yet spaces between similar function declarations.

struct command {
    char *const *argv;
};
extern void freeChunks(void);

extern char **str_split(char *a[], char *a_str, const char a_delim);

extern int do_checkenv(int argc, const char **argv);

The horizontal space is odd here - perhaps an effect of using tabs?

#define    CMD_LEN        1024
#define    EXPAND_ALLOC    1024
#define    PATH_LEN    1024
#define    isBlank(ch)    (((ch) == ' ') || ((ch) == '\t'))
#define    isDecimal(ch)    (((ch) >= '0') && ((ch) <= '9'))
#define    MAX_SOURCE    10

All UPPERCASE identifiers are often considered macros.  Code with CHUNK may confuse viewers of your code.  Re:typedef struct chunk CHUNK;
Not using all UPPERCASE for defines like with #define isBlank(ch) is a problem as a user of that macro does not benefit from the all uppercase hint that it is a macro.  That macro uses ch twice (((ch) == ' ') || ((ch) == '\t')).  Consider the effect of this usage

int ch;
while (isBlank(ch = getchar())) { ... }

Standard suggestion.  Add name and date (year) to your code.

// 2016 Programmer 400

C has a boolean _Bool.  By using <stdbool.h> , code has access to bool, true, false.  Suggest true rather than TRUE in #define    DEBUG TRUE.
Order:  In general, it is cleaner to do the define first, struct, union, typedef, then global variables and lastly function declaration.  This is highly a style issue though.
Unclear why some function have a preceding extern and others do not.  With function declarations, extern is not necessary.  To use or not is a style issue, but it should certainly be consistent within a *.h file.

// to extern or not to extern, that is the question.
extern int do_kill(int argc, const char **argv);
int spawn_proc(int in, int out, struct command *cmd);


Django template filter - sort by multiple keys I have prepared an implementation of sort_by_keys filter:

{% for donation in donations|sort_by_keys:'date,-value' %}
    <img src="{{ donation.stamp_img_url }}" />
{% endfor %}

Example data:

donations = [
    {
        date: '29-07-2014',
        value: 150,
    },
    {
        date: '07-10-2014',
        value: 450,
    },
    {
        date: '29-07-2014',
        value: 500,
    },
]

Expected result:

donations|sort_by_keys:'date,-value'

Should return:

[
    {
        date: '29-07-2014',
        value: 500,
    },
    {
        date: '29-07-2014',
        value: 150,
    },
    {
        date: '07-10-2014',
        value: 450,
    },
]

My implementation:

@register.filter
def sort_by_keys(value, keys):
    _list = list(value)

    def compare(item1, item2):
        for key in item1.__keys_for_sort.split(','):

            if key.startswith('-'):
                key = key.replace('-', '')

                if getattr(item1, key) > getattr(item2, key):
                    return -1
                elif getattr(item1, key) < getattr(item2, key):
                    return 1
            else:
                if getattr(item1, key) < getattr(item2, key):
                    return -1
                elif getattr(item1, key) > getattr(item2, key):
                    return 1

        return 0

    for item in _list:
        setattr(item, '__keys_for_sort', keys)

    _list = sorted(_list, cmp=compare)

    for item in _list:
        delattr(item, '__keys_for_sort')

    return _list

I really don't like the idea of injection special attributes like __keys_for_sort, to have possibility of using this value in comparator, but I don't know how to write this better.

keys is already visible in compare, so you can even do the splitting
outside of compare and still use it inside of it.  And the duplication
of code for the else branch should be eliminated, by only doing the
comparison once (cmp should work I guess) and then negating the
result if necessary.  This means processing all the keys before
running the sorting into a useful data-structure.  You want to compare
in the order they are specified, so a list of tuples will do fine.

Then, by using cmp instead of normal comparisons three lines per
branch are eliminated.  Also, which is maybe more important, you don't
have to worry about the comparisons and what -1/1, which personally
takes me way to long all the time while reading comparison functions
like this.

The two branches are also unnecessary, since you can just negate the
result depending on the sort key (factor in the following code).

So I arrive at this, untested of course.

@register.filter
def sort_by_keys(value, keys):
    transformed = []
    for key in keys.split(','):
        factor = 1
        if key.startswith('-'):
            key = key.replace('-', '')
            factor = -1
        transformed.append((key, factor))

    def compare(item1, item2):
        for (key, factor) in transformed:
            result = factor * cmp(getattr(item1, key), getattr(item2, key))
            if result != 0:
                return result
        return 0

    return sorted(list(value), cmp=compare)

It looks like you can't use sort directly on the input value?
Otherwise I'd suggest to do that as well.

Apart from completely compiling the sort function beforehand (using
lambdas), this seems the most compact I think.


Implementing a Simple "MoneyAccount" in CoffeeScript Please critique this CoffeeScript code. Is it idiomatic?

MoneyAccount = (balance = 0) ->
  obj = {
    withdraw: (x) ->
      if balance < x
        throw new Error("not enough funds in account")
      else
        balance -= x
        console.log "withdrew funds."
        x
    deposit: (x) ->
      if (typeof x isnt 'number' || x < 0)
        throw new Error("bad deposit amount " + x)
      else
        balance += x
    printBalance: ->
      console.log balance
  }
  obj

This seems closely tied to your other question. I posted an answer for that one, which I'll incorporate/reference here.

Calling it MoneyAccount (in "UpperCamelCase") implies that it's a constructor. But it's not quite; it's just a function. But I assume that's done on purpose in order to have "private" variables (i.e. the closed-over balance argument) while still allowing one to invoke it with or without the new keyword.

Still, there are a couple of things you could do:

You're diligent in checking the input for deposit and withdraw... but not the initial balance. So account = MoneyAccount("foo") would spell trouble right away.
But even if you check the type of the initial balance, it'd still allow someone input a negative initial balance. But the way withdraw works seems to imply that a negative balance is impossible.
Incidentally, I could also say account = MoneyAccount(1/0) and have Infinity money! Sounds nice, but perhaps not what you want.
Similar to the above, there's nothing stopping you from withdrawing or depositing a negative or infinite amount.
As mentioned in the answer I linked to, you don't need if...else branches if you throw an an exception; the function will exit if it throws.
There's no way to just get the balance, which would seem problematic for an account. You can print it, sure, but you can't just get it. If you add a method to do that, you can skip the console.log stuff (which incidentally, you're only doing for withdraw but not for deposit). Logging stuff isn't really the responsibility of an account object anyway; leave that to external code.
x should probably be named amount instead, just to be descriptive.
You might want to throw a RangeError in withdraw to be a bit more precise.
Return values are a little haphazard. deposit returns the new balance while withdraw returns the amount withdrawn.
The obj var is unnecessary; you can just leave it out entirely. And you can even leave out the curly brackets (which I personally like to do in a case like this, but it's a matter to taste, really).

In the end, I get this:

MoneyAccount = (balance = 0) ->
  # Adapted from the linked answer
  assertValidAmount = (amount) ->
    number = Number amount
    if isNaN(number)
      throw new TypeError("Expected a numeric amount")
    if not isFinite(number) or number < 0
      throw new RangeError("Expected a positive, finite amount")

  assertValidAmount balance

  # everything below will be interpreted as an object
  # and returned, even without the curly braces

  balance: -> balance

  withdraw: (amount) ->
    assertValidAmount amount
    throw new RangeError("Insufficient funds") if amount > balance
    balance -= amount

  deposit: (amount) ->
    assertValidAmount amount
    balance += amount

That should be pretty safe.


Repetitive code for different Database Entities with same implementation I have this code in LogInService

public User isValid(User user) {
        if(user == null)
            return user;

        User db_user = userDao.getUserByUsername(user.getUsername());
        if (db_user != null) {
            try {
                if (PasswordHash.validatePassword(user.getPassword(),db_user.getPassword())) {
                    return db_user;
                }
            } catch(Exception e){ e.printStackTrace(); }
        }

        return null;
    }

    public Administrator isValid(Administrator admin){
        if(admin == null)
            return admin;

        Administrator db_admin = adminDao.getAdminByUsername(admin.getUsername());

        if(db_admin != null){
            try{
                if (PasswordHash.validatePassword(admin.getPassword(),db_admin.getPassword())) {
                    return db_admin;
                }
            }catch(Exception e){ e.printStackTrace(); }
        }

        return null;
    }

Where we use a null to check if the user is valid.

My problem now is this. Users and Admins are a different entities but for them to be logged in into the system they have to login. both user and admin has the same method of logging in checking.

My question is there anyway I can refactor this repetitive code inside my Service layer class? or should create a different service layer for Admins to logged in?

Fix your method name getUser, getAdmin for starters. If you do not use the returned User object for example except for checking if it is null then consider returning boolean. But I suspect there are more fields in User and Admin.
Fix your parameter types.
Why are you passing in User if you get the user from the data store. If it is only for grouping user name and password try defining a value object.
Once you saved your User type from the burden of holding the password remove password field from the User object. The returned type of the authentication method should not contain the password but contain roles of the user etc. Passwords are an authentication concern, whereas Roles etc are an authorization concern.
Unsuccessful authentication attempt is an exceptional case. Returning null from a service in an exceptional case burdens the user of the service with remembering the null check. In this case having a NullPointerException is not the worst that could happen. Not having the exception and letting unauthenticated users access your secure resources is worse.
In the below code:

if(user == null)
     return user;

If you want to return null say so. Do not confuse the reader of your code. (Reader of the code is probably yourself in a few months.)

Why are you returning null. If null is not a valid argument, as in this case, throw an IllegalArgumentException. I will give you an example to demonstrate the importance of this. Since you did not provide the code that uses this service it will be an imaginary usage:

User user = new User();
user.setUsername(request.getParameter("username");
user.setPassword(request.getParameter("pass");

UserDb = null;
user = loginService.isValid(user); // I'm guessing usage is as horrible as this

if (userDb == null) {
   return UNSUCCESSFUL_LOGIN;
}

saveUserToSession(userDb);
return WELCOME_PAGE;

Programmer made a mistake but the user sees "Login Unsuccessfull" message for correct username and password.
Do not get the real password hash out of your datastore and do not pass around the cleartext password.

User db_user = userDao.getUserByUsername(user.getUsername());

Instead your dao method should be something like this:

User db_user = userDao.getUserByUsername(username, passwordHash);

which eventually translates to some sql or ldap or hql or whatever query like this

select blah1, blah2, blah3
 from myschema.myusertable
 where username=:username and hashedpass=:hashedpass

This reduces the chance of your hashed passwords to leak into logs or such, removes the need to pass around passwords in the clear makes it harder for you to let users know if a username is valid without knowing the password. I am not a security person, so I will not comment on its effect on the timing attacks. But need to know is a good Object Oriented design principle.
Do a shortcut return here:

    if (db_user != null) {
    //several lines of code
     }
     return null;

This should return a boolean. If it doesn't and you cannot change it or find one that does you can always wrap it.

     PasswordHash.validatePassword(user.getPassword(),db_user.getPassword())

Do not catch Exception as it is too general and include unchecked exceptions such as nullpointerexception and illegalargumentexception.

 } catch(Exception e){ e.printStackTrace(); }

Exception.printStackTrace(); is not proper exception logging. It has no use case in any production environment.

My suggestion

 public User getUser(UserCredential credential) throws SecurityException {
    if (credential == null) throw new IllegalArgumentException("credential required");

    User user = userDao.getUser(credential);

    if (user == null) throw new InvalidCredentialException();

    return user;
  }

static class UserCredential {
    //fields, all of them final
   // ..........
     UserCredential(String userName, String hashedPass) {
        //You can use org.apache.commons.lang.Validate to shorten
       //some common validations
         Validate.notNull(userName);
         Validate.notNull(hashedPass);
          this.userName = userName;
          this.hashedPass = hashedPass;
     }
    //getters no setters
   // ..........
 }

By validating the constructor arguments you catch some errors more easily and remove the need to check them again when used as a parameter type.

String userName = request.getParameter("username");

// case 1
user.setUsername(userName);
User userDb = loginService.isValid(user);

// case 2
User user = loginService.getUserValid(new UserCredential(userName, hashedPassword));

Note what happens in each case if you misspell the request parameter name. In the second case you will get an illegal argument exception close to the source of the illegal argument whereas in the first case you may tell user his password wrong or you may get some unspecified error in the depths of your ORM or JDBC driver or something.


Hodgkin-Huxley model in C++ I'm fairly new to C++ and I want to simulate the Hodgkin-Huxley neuron model with it. I have used a MATLAB implementation before and I hope that the C++ code will be faster.

Both seem to take the same time when I time them. For the C++ implementation I used the Linux time command and in MATLAB tic/toc. I let both simulations run for 1000 times with 100000 time steps each. Both take around 13-15s to complete.

I hope anybody can tell me if I have some mayor issues in my C++ implementation or if the difference between C++/MATLAB for such a task is just small.

I compiled the C++ code with g++ -std=gnu++11 -O3 -o hh hhModel.cpp

#include <iostream>
#include <cstdlib>
#include <stdio.h>
#include <math.h>
#include <array>
using namespace std;

int main()
{
    double dt = 0.01;
    int numSamples = 100000;

    // create an rectangular external stimulus
    double I[numSamples];
    for(int i = 0; i <= numSamples; i++)
        I[i] = 0;
    for(int i = 25000; i <= 75000; i++)
            I[i] = 0.01;

    // init constants
    double Vinit = -65;
    double Vref = -65;

    double Smemb = 4000;  // [um^2] surface area of the membrane
    double Cmemb = 1.0;   // [uF/cm^2] membrane capacitance density
    double Cm = Cmemb * Smemb * 1e-8;   // [uF] membrane capacitance

    double GNa = 120;
    double GK = 36;
    double GL = 0.3;
    double ENa = 125;
    double EK = -55;
    double EL = -25;

    double v[numSamples]; // [mV] membrane potential vector

    double m, h, n, aM, bM, aH, bH, aN, bN, dv_dt, dn_dt, dm_dt, dh_dt, IL, IK, INa;

    double gNa = GNa * Smemb * 1e-8;// Na conductance [mS]
    double gK = GK * Smemb * 1e-8;  // K conductance [mS]
    double gL = GL * Smemb * 1e-8;  // leak conductance [mS]

    // initial values
    v[0] = Vinit;           // initial membrane potential
    m = 0;
    h = 0;
    n = 0;

    // To compare the code to Matlab I run the numerical integration 1000 times
    for (int nn = 0; nn <= 1000; nn++){

    // Numerical integration
    for (int j = 0; j <= numSamples; j++){
        // ionic currents
        INa = gNa * m*m*m * h * (ENa - v[j]);
        IK = gK * n*n*n*n * (EK - v[j]);
        IL = gL * (EL - v[j]);

        aM = 0.1 * (v[j] - Vref -25) / ( 1 - exp(-(v[j]-Vref-25)/10));
        bM = 4 * exp(-(v[j]-Vref)/18);

        aH = 0.07 * exp(-(v[j]-Vref)/20);
        bH = 1 / ( 1 + exp( -(v[j]-Vref-30)/10 ) );

        aN = 0.01 * (v[j]-Vref-10) / ( 1 - exp(-(v[j]-Vref-10)/10) );
        bN = 0.125 * exp(-(v[j]-Vref)/80);

        // derivatives
        dv_dt = ( INa + IK + IL + I[j] ) / Cm;
        dm_dt = (1.0-m) * aM - m * bM;
        dh_dt = (1.0-h) * aH - h * bH;
        dn_dt = (1.0-n) * aN - n * bN;

        // calculate next step
        v[j+1] = v[j] + dv_dt * dt;
        m = m + dm_dt * dt;
        h = h + dh_dt * dt;
        n = n + dn_dt * dt;
    }
    }
}

Note: benchmarking C++ code over a fixed, compiler provided sample can lead to the optimizer removing all relevant code and/or replacing it by a precalculated result. The only reason this didn't happen in your test was that GCC gave up because of the complexity and not enough hints.

Implementation

I and v are declared with auto storage class. This causes them to be placed on the stack on most platforms, for which they might be too large (2 * 100000 * 8 Bytes = 1.6 MB, for comparison: standard max stack size on windows is 1 MiB). This might crash the program or cause any number of undefined behaviors.
Also, double I[numSamples]; and double v[numSamples]; would require numSamples to be a compile time constant (e.g. const or constexpr) by the C++ standard (GCC seems not to mind, though).
Checking array/loop bounds: All loop bounds are off by one (<= instead of <), which in turn causes array accesses to be out of bounds by 1 or even 2. This is undefined behavior, and can lead to crashes! (Also, it means you've run 1001 loops of the C++ code, instead of the 1000 mentioned, slightly skewing the comparison.)
A lot of variables could be made const and/or constexpr. Doing so might help the compiler optimize your code, as it gets more information to work with/doesn't have to deduce the const-ness by itself (which can be costly).
Unnecessary header #includes: contents from <iostream>, <stdio.h> and <array> are never used!
Prefer to include the adapted C++ headers over plain C headers (<cmath> and <cstdio> instead of <math.h> and <stdio.h>).
using namespace std; is considered bad practice and should be avoided.
Try to refactor code into smaller, independent units. This helps humans for readability and compilers for deducing useful traits for optimization.
Along the same line: Try to restrict variables to the smallest scope needed. Again, this helps human readers as much as compilers, as now we only have to consider them where they are reachable.
Your code still uses a lot of magic numbers. Try to extract them into appropriately named variables, so other readers of your code can reason about them easier.

Optimization

Your current code is not structured well enough for the optimizer. Why? If it could reason about it perfectly, it would remove all the code (as there are no observable side effects), running in near 0ms!

Declaring all possible variables const and moving them to the smallest scope possible helps quite a bit. Even better: Refactor the code into functions (and maybe classes) with proper const annotations!

Just this alone allowed MSVC and GCC to recognize that the whole calculations aren't needed (as they don't cause side effects by themselves), so they removed them and run-time went down to ~24 microseconds on my machine.

To get relevant results, we now need to fool the compiler a bit: Adding a little side effect (num_calls in the code below) and taking the number of loops to run as a command line argument were enough for MSVC and GCC (and clang, though not ICC). Now runtime (with all calculations!) went down from ~14 seconds (original) to ~12.5 seconds (again on my machine).

  Note: The code below has some changes that, while possibly benefiting benchmark performance, are differences to the original:

  Results of all the simulation steps aren't stored. Instead, only the latest result is kept. (The stores would probably be optimized out anyways, as they wouldn't be read from.)
  The input impulse is generated on the fly (instead of being read from an array). This causes the optimizer to generate better code for this case (no memory reads, just a branch that the hardware can predict well). I don't think the impact is large (as contiguous array accesses would be prefetched anyways), but still to be noted.
  The number of benchmark loops to run is taken from the command line arguments (argument #1). This is done to fool the optimizer into not removing/precalculating the actual calls (as it can't know how often it will run).
  To cause a visible side effect, I needed to report the number of simulation steps made. Since I had to print them anyways, I added a small snippet to measure the benchmark time and print it as well.
  I renamed a lot of variables. This was more for my understanding (and readability). For some variables, I couldn't deduce their intents, so their names remained unchanged. I also didn't move the magic numbers into variables, as I don't know what they are referring to. (I know scientific code usually tries to use formula symbols as names, but those were too similar for me to reason about, especially since I'm not too versed into that specific field.)

#include <iostream>
#include <cmath>
#include <chrono>

uint32_t num_calls = 0; // to fool optimizer

namespace constants
{
    constexpr const auto time_step = 0.01;
    constexpr const auto num_samples = 100000;

    constexpr const auto initial_voltage = -65.0;
    constexpr const auto reference_voltage = -65.0;

    constexpr const auto membrane_surface = 4000.0;  // [um^2] surface area of the membrane
    constexpr const auto membrane_capacitance_density = 1.0;   // [uF/cm^2] membrane capacitance density
    constexpr const auto membrane_capacitance = membrane_capacitance_density * membrane_surface * 1e-8;   // [uF] membrane capacitance

    constexpr const auto GNa = 120.0;
    constexpr const auto GK = 36.0;
    constexpr const auto GL = 0.3;
    constexpr const auto ENa = 125.0;
    constexpr const auto EK = -55.0;
    constexpr const auto EL = -25.0;

    constexpr const auto sodium_conductance = GNa * membrane_surface * 1e-8;// Na conductance [mS]
    constexpr const auto kalium_conductance = GK * membrane_surface * 1e-8;  // K conductance [mS]
    constexpr const auto leak_conductance = GL * membrane_surface * 1e-8;  // leak conductance [mS]
}

class simulation
{
    double membrane_potential;
    double m;
    double h;
    double n;

    double step_m(const double delta_time) const
    {
        const auto voltage_difference = membrane_potential - constants::reference_voltage;

        const auto aM = 0.1 * (voltage_difference - 25.0) / (1.0 - exp(-(voltage_difference - 25.0) / 10.0));
        const auto bM = 4.0 * exp(-voltage_difference / 18.0);

        auto const derivative_m = (1.0 - m) * aM - m * bM;

        return (m + derivative_m * delta_time);
    }

    double step_h(const double delta_time) const
    {
        const auto voltage_difference = membrane_potential - constants::reference_voltage;

        const auto aH = 0.07 * exp(-voltage_difference / 20.0);
        const auto bH = 1.0 / (1.0 + exp(-(voltage_difference - 30.0) / 10.0));

        auto const derivative_h = (1.0 - h) * aH - h * bH;

        return h + derivative_h * delta_time;
    }

    double step_n(const double delta_time) const
    {
        const auto voltage_difference = membrane_potential - constants::reference_voltage;

        const auto aN = 0.01 * (voltage_difference - 10.0) / (1.0 - exp(-(voltage_difference - 10.0) / 10.0));
        const auto bN = 0.125 * exp(-voltage_difference / 80.0);

        auto const derivative_n = (1.0 - n) * aN - n * bN;

        return n + derivative_n * delta_time;
    }

    double step_membrane_potential(const double time, const double stimulus) const
    {
        const auto sodium_current = constants::sodium_conductance * m*m*m * h * (constants::ENa - membrane_potential);
        const auto kalium_current = constants::kalium_conductance * n*n*n*n * (constants::EK - membrane_potential);
        const auto leak_current = constants::leak_conductance * (constants::EL - membrane_potential);

        auto const derivative_potential = (sodium_current + kalium_current + leak_current + stimulus) / constants::membrane_capacitance;

        return membrane_potential + derivative_potential * time;
    }
public:
    simulation(const double initial_potential, const double initial_m, const double initial_h, const double initial_n) : membrane_potential(initial_potential), m(initial_m), h(initial_h), n(initial_n) {}

    void step(const double delta_time, const double current_stimulus)
    {
        membrane_potential = step_membrane_potential(delta_time, current_stimulus);
        m = step_m(delta_time);
        h = step_h(delta_time);
        n = step_n(delta_time);

        ++num_calls; // to fool optimizer
    }

    double latest_potential() const
    {
        return membrane_potential;
    }
};

void run_simulation()
{
    auto sim = simulation{ constants::initial_voltage, 0, 0, 0 };

    for (auto j = 0; j < constants::num_samples; j++) {
        auto stimulus = 0.0;
        if (j >= 25000 && j <= 75000) stimulus = 0.01;

        sim.step(constants::time_step, stimulus);
    }
}

int main(int argc, char *argv[])
{
    using seconds = std::chrono::duration<double, std::ratio<1, 1>>;

    if(argc < 2)
    {
        std::cerr << "requires number of benchmarking loops as command line argument #1";
        return 1;
    }

    const auto num_loops = atoi(argv[1]);

    const auto start_time = std::chrono::high_resolution_clock::now();

    for (auto loop = 0; loop < num_loops; ++loop) {
        run_simulation();
    }

    const auto end_time = std::chrono::high_resolution_clock::now();
    const auto time_taken = std::chrono::duration_cast<seconds>(end_time - start_time);

    std::cout << "time taken: " << time_taken.count() << "s\n";
    std::cout << "number of loops: " << num_loops << "\n";
    std::cout << "number of simulation steps: " << (num_calls / num_loops) << "\n"; // to fool optimizer
}

Further optimization possibilities?

All compilers produce reasonable ILP (instruction level parallelism) assembly (if still fooled). There doesn't seem to be much to improve upon.
Multithreading also doesn't seem like it would be helping for this problem, as the simulation steps are dependent on previous results.
The bottleneck seems to be the exp function call, so a faster implementation might help (that's why I tried ICC, but that one is too smart and removes the calculations as a whole -.-).


Using set for unordered, unique list of elements In C++11 we have the option of using std::unordered_set if we require a list of elements that have no duplicates but order is not important.

In C++03 we don't have this options, although we can use the std::set to achieve something similar. Let's consider the following code:

std::set<ElementType, std::NotEqualTo<ElementType>> unorderedSet;
unorderedSet.insert(some_element);
unorderedSet.insert(other_element);
unorderedSet.insert(some_element);

//... more stuff with the set

This will, basically produce a set that will store the elements in the order they were inserted while preventing duplicates. Note that ElementType simply  can't  have less than or greater than operator. It doesn't make sense, semantically speaking.

Anyway, since I am using std::NotEqualTo instead of a LessThan functor, as it is requested by the set, I am not feeling very comfortable with this code. I do know that set is using something such as

if first element < second element
    //insert accordingly
else if second element < first element
    //insert accordingly
else
   //they are equal

but in the same time, theoretically, my code will be failing if the checks were more strict such as

if first element < second element AND !(second element < first element)
//...

My questions are:

There are any potentially problems that could arise from using this construct?
What other alternatives I have to achieve the same thing in a more readable form? I do not plan to write my own unordered set just for this scenario and can't really use any external libraries. Only standard library available in C++03.

Edit: I am unable to make use of "less than" operator between my elements. I can decide if they are equal or not but I can't say which one is smaller than other. With the default set functor the code won't even compile as "operator<" is not found. This is why I wanted to model the functor with equal operator in the first case.

I am also aware that it is not valid, however, the code will produce the desired results. From my understanding it will expand to something such as:

if first element != second element
    // place first element
else if second element != first element
    // this is buggy, but should never be executed; first condition will always be true, unless the elements are equal.
else
    //here the elements are equal .

My questions are:
There are any potentially problems that could arise from using this construct?

Its not valid.
The code will not work as expected as the std::set requires the comparator type to generate a strict weak ordering between elements.

What other alternatives I have to achieve the same thing in a more readable form? I do not plan to write my own unordered set just for this scenario and can't really use any external libraries. Only standard library available in C++03.

You can use std::set as an unordered container as is. If you have an unordered container it does not matter to you what order the elements are in. If they just happen to be ordered does that matter?
The reason for using std::unordred_set is that it provides faster access to the elements. You give up an ordered property to get better performance.
Container:           Insert     find
std::set             O(ln(n))  O(ln(n))
std::unordered_Set   O(1)      O(1)


Preventing duplicate submission in Rails I have a Rails app where doctors score patients on diagnosis - their score is the model Baseline. Each Baseline has a user id and a patient id and there should only be one baseline for each patient by a user. Very occasionally I have noticed that a user may create a new baseline (at which point he is then redirected to another page) but then presses the back button, be brought to the "new" action again and if he submits again, a duplicate baseline is created. Therefore I am writing some code to try to ensure this never happens.

First, I check that a baseline doesn't already exist with the current user id and the patient id at the controller "new" action.

     before_action :check_there_is_not_an_existing_baseline, only: [:new ]

and

 def check_there_is_not_an_existing_baseline
   patient =  Patient.find(params[:patient])
   @baseline = Baseline.where(user_id: current_user.id, patient_id: patient.id).first
    if @baseline
     flash[:notice] = "There was a problem with this action. Refresh the page and try again!"
     redirect_to list_patients_path(:page =>  @current_page)
    end
 end

I also check at the create action in the model

 validate :duplicate_baselines, :on => :create

  def duplicate_baselines
     check_baseline = Baseline.where(user_id: self.user_id, patient_id: self.patient_id)
     if !check_baseline.blank?
        errors.add(:base, "There was an error. Go to 'patient list' and start again") unless check_baseline.first.new_record?
     end
  end

I am trying to learn more and my suspicion is that there is good Rails way to do this. Could an expert have a look and see if there is a neater solution?

That can be done easily by Rails validation, and scope attribute:

# Baseline Model
validates :user_id, uniqueness: { scope: :patient_id }

That mean user_id can present many times in Baseline table, but only one can be connected to exact patient_id.