Initialize a Sieve of Eratosthenes in Scala I am learning Scala while solving some exercises and I am currently solving an exercise where I need to initialize a Sieve of Eratosthenes.

I am using the following code:

  val sieve = Array.fill[Boolean](100)(true)

  for (p <- 2 until sieve.length if sieve(p) && math.pow(p, 2) <= sieve.length) {
    for (i <- p * 2 until sieve.length by p) {
      sieve(i) = false
    }
  }

I see some problems with this approach, for example, the outer loop still evaluates all numbers from some Y until sieve.length to test the condition sieve(p) && math.pow(p,2) <= sieve.length, although the code inside the loop is not executed for those values of p.

I think this could be solved with the following code:

 var p = 2
 while(math.pow(p, 2) <= sieve.length)
 {
   if(sieve(p))
     for (i <- p * 2 until sieve.length by p)
       sieve(i) = false

   p += 1
 }

I may be wrong, but I think I am complicating too much.

What's a good way to initialize a Sieve of Eratosthenes using Scala?

There are many other ways of writing a Sieve of Eratosthenes in Scala. Regarding this particular code, the for-comprehension is better written as:

val sieve = Array.fill[Boolean](100)(true)

for {
  p <- 2 until sieve.length
  if sieve(p) && math.pow(p, 2) <= sieve.length
  i <- p * 2 until sieve.length by p
} sieve(i) = false

Also,

  the outer loop still evaluates all numbers from some Y until
  sieve.length to test the condition sieve(p) && math.pow(p,2) <=
  sieve.length,

You could compute the square root too, though that's an expensive computation. You can use takeWhile to avoid it:

p <- 2 until sieve.length takeWhile (x => x * x < sieve.length)

Also, the starting place of the final loop can be improved:

i <- p * p until sieve.length by p

Altogether, you get this:

val sieve = Array.fill[Boolean](100)(true)

for {
  p <- 2 until sieve.length takeWhile (x => x * x < sieve.length)
  if sieve(p)
  i <- p * p until sieve.length by p
} sieve(i) = false

Now, I wouldn't use booleans for this, nor an Array. Instead, my preferred implementation (for efficiency) uses BitSet instead. For example:

val last = 100
val numbers = 2 to last
val sieve = collection.mutable.BitSet(numbers: _*)
for (p <- numbers takeWhile (x => x * x <= last) if sieve(p))
  sieve --= p * p to last by p

There are other implementations I value for their functional nature, but I'll leave that to others.


Typeahead autocomplete functionality challenge I'm working on a Talent Buddy challenge . In this challenge you have to design the autocomplete functionality of Twitter. You are given a list of queries and possible usernames. You have to return the most suited username based on the query. This whole functionality should run under 2 seconds for arrays which do not exceed 100.000 entries. Below you can find my attempt. On my computer it runs the biggest list in 0.517 seconds. But apperently it is not under 2 sec on their system. In the code bellow I did not add the big list, because it will be too much text for a question. Therefore I included the example lists.

My strategy is to divide the usernames into 26 lists (1 list for every letter in the alphabet). So this means that for each query, I do not have to go over all the user names, but only those starting with the same letter. For the rest I optimized my code with best practice rules.

Does anyone know how to make this faster?

usernames= ["james", "jBlank"]
queries= ["j", "jm", "jbl", "JB"]

import cProfile

def do_cprofile(func):
    def profiled_func(*args, **kwargs):
        profile = cProfile.Profile()
        try:
            profile.enable()
            result = func(*args, **kwargs)
            profile.disable()
            return result
        finally:
            profile.print_stats()
    return profiled_func

@do_cprofile
def typeahead(usernames, queries):

    amount_of_queries = len(queries)
    queries_lower = [query.lower() for query in queries]
    username_lower = [item.lower() for item in usernames]
    username_lower = sorted(username_lower)
    querylen = [len(query) for query in queries]

    results = [-1]*amount_of_queries
    alphabet = [[] for x in xrange(26)]
    myDict = {'a':0,'b':1,'c':2,'d':3,'e':4,'f':5,'g':6,'h':7,'i':8,'j':9,'k':10,'l':11,'m':12,'n':13,'o':14,'p':15,'q':16,'r':17,'s':18,'t':19,'u':20,'v':21,'w':22,'x':23,'y':24,'z':25}

    [alphabet[myDict[username[0]]].append(username) for username in username_lower]

    matches=[]
    for q in xrange(amount_of_queries):
        matches[:] = []
        quer = queries_lower[q]

        if querylen[q]==1:
            results[q] = alphabet[myDict[quer[0]]][0]
        else:
            matches = [item  for item in alphabet[myDict[quer[0]]] if quer==item[:querylen[q]]]
            if len(matches)>=1:
                results[q]= matches[0]

    print '\n'.join(map(str, results))

if __name__=="__main__":
    typeahead(usernames, queries);

Organisation of the function

You are doing many things in the same function which make the whole thing a bit confusing. First thing to do is to split the two main parts of the functions : one does preprocessing on usernames, one handles queries. One this is done, your code looks like :

def typeahead(usernames, queries):
    # Pre-processing on usernames
    username_lower = [item.lower() for item in usernames]
    username_lower = sorted(username_lower)

    alphabet = [[] for x in xrange(26)]
    myDict = {'a':0,'b':1,'c':2,'d':3,'e':4,'f':5,'g':6,'h':7,'i':8,'j':9,'k':10,'l':11,'m':12,'n':13,'o':14,'p':15,'q':16,'r':17,'s':18,'t':19,'u':20,'v':21,'w':22,'x':23,'y':24,'z':25}

    [alphabet[myDict[username[0]]].append(username) for username in username_lower]

    # Handling the queries
    matches=[]
    amount_of_queries = len(queries)
    results = [-1]*amount_of_queries
    queries_lower = [query.lower() for query in queries]
    querylen = [len(query) for query in queries]
    for q in xrange(amount_of_queries):
        matches[:] = []
        quer = queries_lower[q]

        if querylen[q]==1:
            results[q] = alphabet[myDict[quer[0]]][0]
        else:
            matches = [item  for item in alphabet[myDict[quer[0]]] if quer==item[:querylen[q]]]
            if len(matches)>=1:
                results[q]= matches[0]

    print '\n'.join(map(str, results))

Before we go any further, something to keep in mind

I am not quite sure that this is an issue but in your example, your print lower-case usernames even though usernames with their original case is printed on this original problem.

Making your map from letters to username a bit cleaner

At the moment, you always use alphabet and myDict together. This is a good sign that something is wrong. (Also, if you really want to map letters to their index, you don't need dictionnary with hardcoded value, you just need arithmetic, ord/chr). If what you want is just to match letters with the list of corresponding usernames, you can use normal dictionnary and check if items are already present, normal dictionnary and fill with empty list from the very beginning or you could play it smart and use Python good stuff as defaultdict and setdefault :

from collections import defaultdict
mapping = defaultdict(list)
mapping2 = dict()
for username in usernames:
    low = username.lower()
    mapping[low[0]].append(low)
    mapping2.setdefault(low[0], []).append(low)

I'll assume we are using the defaultdict option from now.

In order to integrate this part of the solution in your code, let's remove the duplicated code from what we have in the part handling the queries :

for q in xrange(amount_of_queries):
    matches[:] = []
    quer = queries_lower[q]

    candidates = alphabet[myDict[quer[0]]] ### THIS IS WHAT IS HAVE ADDED
    if querylen[q]==1:
        results[q] = candidates[0]
    else:
        matches = [item  for item in candidates if quer==item[:querylen[q]]]
        if len(matches)>=1:
            results[q]= matches[0]

Replacing this with our defaultdict logic, we get :

def typeahead(usernames, queries):
    # Pre-processing on usernames
    mapping = defaultdict(list)
    for username in usernames:
        mapping[username.lower()[0]].append(username.lower())

    # Handling the queries
    matches=[]
    amount_of_queries = len(queries)
    results = [-1]*amount_of_queries
    queries_lower = [query.lower() for query in queries]
    querylen = [len(query) for query in queries]
    for q in xrange(amount_of_queries):
        matches[:] = []
        quer = queries_lower[q]

        candidates = mapping[quer[0]]
        if querylen[q]==1:
            results[q] = candidates[0]
        else:
            matches = [item  for item in candidates if quer==item[:querylen[q]]]
            if len(matches)>=1:
                results[q]= matches[0]

    print '\n'.join(map(str, results))

Please note that I took this chance to remove the list you were building with a useless list-comprehension.

Handling the queries

As you are about to iterate on queries anyway, there's no real need to build lists with information you will need. You can just iterate over queries and that will do the trick.

Also, you can initialise matches inside the loop and use enumerate if you really need the index. At this point, the part of the code looks like :

# Handling the queries
results = [-1]*len(queries)
for i,q in enumerate(queries):
    matches=[]
    low_quer = q.lower()
    l = len(low_quer)

    candidates = mapping[low_quer[0]]
    if l==1:
        results[i] = candidates[0]
    else:
        matches = [item  for item in candidates if low_quer==item[:l]]
        if len(matches)>=1:
            results[i]= matches[0]

Finally, we don't even need the indices as we can build results as we go. Also, we don't need matches in that part of the function. Things get even more concise :

# Handling the queries
results = []
for q in queries:
    low_quer = q.lower()
    l = len(low_quer)

    candidates = mapping[low_quer[0]]
    if l==1:
        results.append(candidates[0])
    else:
        matches = [item  for item in candidates if low_quer==item[:l]]
        if len(matches)>=1:
            results.append(matches[0])
        else:
            results.append('-1')

Another issue found

What if we look for "c" ? At the moment, we'll do results.append(candidates[0]) with an empty list of candidates : this will crash. This should be handled much the same way we do for longer strings : check if we do have a match. Also, this removes some unrequired logic :

# Handling the queries
results = []
for q in queries:
    low_quer = q.lower()
    l = len(low_quer)

    matches = [item for item in mapping[low_quer[0]] if low_quer==item[:l]]
    if len(matches)>=1:
        results.append(matches[0])
    else:
        results.append('-1')

If we want to play it cool, we can use the fact that in a boolean context, an empty list is considered False while a non-empty list is considered True. Also, we can use the ternary operator. Finally, because I am now using the string -1 and not the integer, we don't need the conversion to string at the end. We get :

# Handling the queries
results = []
for q in queries:
    low_quer = q.lower()
    l = len(low_quer)

    matches = [item for item in mapping[low_quer[0]] if low_quer==item[:l]]
    results.append(matches[0] if matches else '-1')

print '\n'.join(results)

Using the right function

Using startswith you don't need to do some tricky string handling :

# Handling the queries
results = []
for q in queries:
    q = q.lower()
    matches = [item for item in mapping[q[0]] if item.startswith(q)]
    results.append(matches[0] if matches else '-1')

Your final code becomes :

def typeahead(usernames, queries):
    # Pre-processing on usernames
    mapping = defaultdict(list)
    for username in usernames:
        low = username.lower()
        mapping[low[0]].append(low)
    print mapping

    # Handling the queries
    results = []
    for q in queries:
        q = q.lower()
        matches = [item for item in mapping[q[0]] if item.startswith(q)]
        results.append(matches[0] if matches else '-1')

    print '\n'.join(results)

A few things are not perfect yet : the fact that we get the lower-case name, the fact that we haven't sorted the values but I'll let your play with this.

And now for something different

The trick you have used to map first letter to potential candidates makes things at most 26 times faster. In real life, this performance boost will not be that huge because usernames are not uniform over the alphabet. You might want to generalise this trick over all letters of the words. Here are a few pages that you might find interesting :

http://en.wikipedia.org/wiki/Suffix_tree
http://en.wikipedia.org/wiki/Suffix_array
http://en.wikipedia.org/wiki/Trie


Questions on F#'s loop and cont The following tail-recursion example is taken verbatim from Chris Smith's Programming F# 3.0. If this is not the appropriate forum for this post, I will gladly move the post to the appropriate place. I did not post this in SO, because I do not have a problem to solve.

let printListRev list =
    let rec printListRevTR list cont =
        match list with
        // For an empy list, execute the continuation
        | [] -> cont()
        // For other lists, add printing the current
        // node as part of the continuation.
        | hd :: tl ->
            printListRevTR tl (fun () -> printf "%d " hd
                                         cont())

    printListRevTR list (fun () -> printfn "Done!");;

What exactly is cont() continuing. I believe I understand the concept that cont() is encapsulating the remaining work to be done, but I am still a little confused about what is going on.
This SO post contains use of the keyword loop in @Juliet 's answer. I am confused as to why loop is used in that answer and not used in the example above.

cont is continuing the computation process.

The printListRevTR function, in the process of execution, builds up a "chain" of cont functions. The very first (or last, depending on how you count) such function is the one that prints "Done!". After that, each next cont prints the current number and then calls the previous cont.

As printListRevTR goes through the list, it builds up a chain of conts, by creating each next function that calls the previous one. This chain could be visualized like this:

After the list has been exhausted, the very last iteration calls the very last (or first, depending on how you count) function in that chain, which calls the next one, which calls the next one, and so on, until the final "Done!" function is called.

As for loop - that's not a keyword, just a name that Juliet has chosen for her function.


Better ways to build a linq specification for querying or is this the only way? The code pretty much explains what I am doing here. Just wondering if anyone can think of a better way.

public class AttachmentQuery
{
    /// <summary>
    /// Initializes a new instance of the <see cref="AttachmentQuery"/> class.
    /// </summary>
    /// <param name="type">The type.</param>
    /// <param name="status">The status.</param>
    /// <param name="sort">The sort.</param>
    /// <param name="order">The order.</param>
    /// <param name="page">The page.</param>
    public AttachmentQuery(string type, string status, SortAttachment sort, SortOrder order, int? page)
    {
        IAttachmentSpecification specification = null;

        if (!string.IsNullOrEmpty(type))
        {
            specification = new AttachmentFileTypeSpecification(type);
        }

        if (!string.IsNullOrEmpty(status))
        {
            if (specification == null)
            {
                specification = new AttachmentStatusSpecification(status.AsEnum<AttachmentStatus>());
            }
            else
            {
                var spec = new AndSpecification<Attachment>(
                    specification,
                    new AttachmentStatusSpecification(status.AsEnum<AttachmentStatus>())
                );

                specification = spec as IAttachmentSpecification;
            }
        }

        if (specification == null)
        {
            specification = new AttachmentSpecification();
        }

        specification.Page = page;
        specification.Limit = Setting.AttachmentPageLimit.Value;
        specification.Sort = sort;
        specification.Order = order;

        this.Specification = specification;
    }

    /// <summary>
    /// Gets or sets the specification.
    /// </summary>
    /// <value>The specification.</value>
    public IAttachmentSpecification Specification
    {
        get;
        private set;
    }
}

Source.

How it is used.

I would definitely write it differently but exact result depends on whether your AndSpecification supports only two parameters or it can accept IEnumerable of attachments.

If it has two parameters and you expect to nest those AndSpecifications if you will have more specs to compose then I would write something like this:

var typeSpecification = string.IsNullOrEmpty(type) ? null : new AttachmentFileTypeSpecification(type);
var statusSpecification = string.IsNullOrEmpty(status) ? null : new AttachmentStatusSpecification(status.AsEnum<AttachmentStatus>());
var specificationsToCompose = new IAttachmentSpecification[] {typeSpecification, statusSpecification};

var composedSpecification = specificationsToCompose
                     .Aggregate((IAttachmentSpecification) null, (accumulator, currentSpec) =>
                         {
                             if (accumulator != null && currentSpec != null) return new AndSpecification<Attachment>(accumulator, currentSpec);
                             return accumulator ?? currentSpec;
                         })
                      ?? new AttachmentSpecification();

...

this.Specification = composedSpecification;


remove kth last element from singly-linked list - Follow up This code is a revised version of implementation which asked for an improvement. Original question is asked here:
remove kth last element from singly-linked list
credits to: Toby, Andreas, Arkadiusz
What has changed:

remove length from xllist struct
check if k is bigger than list length on the fly

Code:
#include <stdio.h>
#include <stdlib.h>
#include <stdbool.h>
#include <stdint.h>

typedef struct llnode
{
    int value;
    struct llnode *next;
} llnode;

typedef struct xllist
{
    llnode * head;
    llnode * tail;
} xllist;

bool create_node(xllist *list, int value)
{
    llnode *node = malloc(sizeof *node);
    if(!node)
    {
        return false;
    }

    node->value = value;
    node->next = NULL;
    if(!list->head)
    {
        list->head = node;
        list->tail = node;
    }
    list->tail->next = node;
    list->tail = node;
    return true;
}

bool del_element_from_last(xllist *llist, int k)
{
    //window with 2 pointers, length of k
    //prev is the prev node to the window
    llnode *prev;
    llnode *last;
    int len;  //list length

    if(llist->head)
    {
        last = llist->head;
        prev = llist->head;
        len = 1;
    }

    for(; last; last=last->next)
    {
        len++;
        if(len > k+1)
            prev = prev->next;
    }

    if(len < k) //len is smaller than k
    {
        return false;
    }

    if(len == k)  //means del 1st element from the list
    {
        llist->head = llist->head->next;
    }

    //remove first node of the window
    printf("deleted element:%d \n", prev->next->value);
    prev->next = prev->next->next;
    return true;
}

int main(void)
{
    xllist llist = {NULL, NULL};

    for(int i=0; i<100; i++)
    {
        if(!create_node(&llist, 100+i))
            printf("create fail\n");
    }

    del_element_from_last(&llist, 15);
}

In the function del_element_from_last, when removing the first element of the list, you update llist->head. However, when removing the last element of the list, you do not update llist->tail.
If keeping track of the tail of the list is only intended for building the list, and is not intended to be updated by the function del_element_from_last, then you should not pass the entire struct xllist to the function del_element_from_last. Rather, you should change the function signature to the following, so that the function only has access to the head:
bool del_element_from_last( xlnode **pp_head, int k )

According to the task description, k is guaranteed to be smaller than the list. This means that the list is guaranteed to be not empty. In that case, the line
if(llist->head)
in the function del_element_from_last is redundant, because the condition will always be true.
On the other hand, defensive programming is good programming style. Therefore, it may still be appropriate to keep it.
However, if you decide to keep it, your program should handle both cases properly. If the condition is false, your program will cause undefined behavior, because you read the uninitialized value of last. Therefore, you should probably instead immediately return false when the condition is false.
An alternative to the if statement would be to use the assert macro, like this:
assert( llist->head );
That way, you don't have to provide code to handle both cases. Instead, a diagnostic error message will appear if the variable does not have the expected value (only on debug builds though, not on release builds).


Fast Thread- and multiprocess-safe file operations in python under Linux I'm trying to implement an alternative to python built-in open(), but safe for use in multi-threaded and multiprocessing environment. I'm using advisory locking, because all file operations in my application are performed using my locked_open() function and no other process is accessing that data.

It is only required to run on modern linux distributions, no compatibility with Windows or OS X is required. Also it have to be fast, because I'm going to use it for caching operations.

The code and the tests:

from contextlib import contextmanager
import os, fcntl
import unittest
from testfixtures import TempDirectory

M_READ, M_WRITE, M_READWRITE = range(3)
MODES = (
    # open for reading
    (os.O_RDONLY, fcntl.LOCK_SH, 'rb'),
    # create for writing
    (os.O_WRONLY | os.O_CREAT | os.O_TRUNC, fcntl.LOCK_EX, 'wb'),
    # open for reading and writing
    (os.O_RDWR | os.O_CREAT, fcntl.LOCK_EX, 'r+b')
)
BLOCKING_FLAGS = (fcntl.LOCK_NB, 0)

@contextmanager
def locked_open(filename, mode=M_READ, blocking=True):
    open_mode, flock_flags, mode_str = MODES[mode]
    flock_flags = flock_flags | BLOCKING_FLAGS[blocking]

    fd = os.open(filename, open_mode)
    fcntl.flock(fd, flock_flags)
    fileobj = os.fdopen(fd, mode_str)

    yield fileobj

    fileobj.flush()
    os.fdatasync(fd)
    fcntl.flock(fd, fcntl.LOCK_UN)
    fileobj.close()

class LockedOpenTestCase(unittest.TestCase):
    filename = 'testfile.txt'
    test_data = b'random text'

    def setUp(self):
        self.tempdir = TempDirectory()
        self.filepath = os.path.join(self.tempdir.path, self.filename)

    def tearDown(self):
        self.tempdir.cleanup()

    def test_write(self):
        with locked_open(self.filepath, M_WRITE) as f:
            f.write(self.test_data)
        self.assertEqual(self.test_data, self.tempdir.read(self.filename))

    def test_read(self):
        self.tempdir.write(self.filename, self.test_data)
        with locked_open(self.filepath, M_READWRITE) as f:
            self.assertEqual(self.test_data, f.read())

    def test_read_and_write(self):
        self.tempdir.write(self.filename, self.test_data)
        with locked_open(self.filepath, M_READWRITE) as f:
            data = f.read().swapcase()
            f.seek(0)
            f.write(data)

        self.assertEqual(
            self.test_data.swapcase(),
            self.tempdir.read(self.filename))

    def test_concurrent_read(self):
        self.tempdir.write(self.filename, self.test_data)
        with locked_open(self.filepath, M_READ) as f1:
            with locked_open(self.filepath, M_READ) as f2:
                self.assertEqual(self.test_data, f1.read())
                self.assertEqual(self.test_data, f2.read())

    def test_non_blocking_read_during_write_raises(self):
        with locked_open(self.filepath, M_WRITE):
            with self.assertRaises(IOError):
                with locked_open(self.filepath, M_READ, blocking=False):
                    pass

    def test_non_blocking_write_during_read_raises(self):
        self.tempdir.write(self.filename, self.test_data)
        with locked_open(self.filepath, M_READ):
            with self.assertRaises(IOError):
                with locked_open(self.filepath, M_WRITE, blocking=False):
                    pass

The code seems to be correct, and all tests are passing, but I get the feeling that I'm reinventing the wheel here.

Now the questions:

Are there any issues with the code that I should be aware of? (besides from what I already said)
Are my tests sufficient?
Any suggestions regarding performance?
Are there any easier ways to achieve same result?

Are there any issues with the code that I should be aware of? (besides from what I already said)

If you haven't, maybe read
Everything you never wanted to know about file locking;
I guess the takeaway is that you should make sure that you're actually
getting what you want with the Python fcntl module.

The code looks good to me in general, apart from the bad error handling.

When fcntl.flock throws, the file descriptor from os.open isn't
closed.  This is generally bad and can bite you in the future.

You don't have to care about unlocking as that's done then closing the
file descriptor anyway.

And another issue you missed is that the yield has to be wrapped in a
try/catch in order for the cleanup to run,
c.f. contextmanager.

In summary it should look somewhat like this:

@contextmanager
def locked_open(filename, mode=M_READ, blocking=True):
    open_mode, flock_flags, mode_str = MODES[mode]
    flock_flags = flock_flags | BLOCKING_FLAGS[blocking]

    fd = os.open(filename, open_mode)
    try:
        fcntl.flock(fd, flock_flags)
        fileobj = os.fdopen(fd, mode_str)

        try:
            yield fileobj
        finally:
            fileobj.flush()
            os.fdatasync(fd)
    finally:
        os.close(fd)

  Are my tests sufficient?

You didn't actually test the interaction between multiple threads and
processes.  Since you're only locking a file descriptor once this will
work as it is though.

  Any suggestions regarding performance?
  Are there any easier ways to achieve same result?

You'd have to measure that, but since you're using this from a single
application only, using in- and shared-memory locks (from
multiprocessing) could be a faster alternative instead of going
through the filesystem.  But it's way more effort on your end with all
three access modes as you wrote them here and with multiple processes
you'd easily run into more failure modes - I doubt it's worth it.
That said, if you can get away with less features for locking, then you could still investigate it.


Group list of words by occurrence / inverse index of strings I'm currently working through Coding the matrix, though I'm trying to work through it using Haskell rather than Python. One of the exercises has you write a function where you take a list of strings, and then transform them into an inverse index of the words contained in each string, along with the index of the originating string:

> makeInverseIndex ["hello world", "world test", "hello test"]
fromList [("hello",fromList [0,2]),("test",fromList [1,2]),("world",fromList [0,1])]

You then implement an orSearch and an andSearch, which returns the indexes of the strings that match the search:

> orSearch ["hello", "world"] $ makeInverseIndex ["hello world", "world test", "hello test"]
fromList [0,1,2]

> andSearch ["hello", "world"] $ makeInverseIndex ["hello world", "world test", "hello test"]
fromList [0]

Here is my attempt in Haskell:

import Data.List as L
import Data.Map.Strict as M
import Data.Set as S

makeInverseIndex :: [String] -> Map String (Set Int)
makeInverseIndex = L.foldr (unionWith S.union . uncurry group) M.empty . zipWithIndex . L.map words
  where
    zipWithIndex :: [a] -> [(Int, a)]
    zipWithIndex xs = zip [0..length xs] xs

    group :: Ord k => v -> [k] -> Map k (Set v)
    group n = M.fromList . L.map (\w -> (w, S.singleton n))

orSearch :: [String] -> Map String (Set Int) -> Set Int
orSearch words =
  M.foldr S.union S.empty . pick words

andSearch :: [String] -> Map String (Set Int) -> Set Int
andSearch words index =
  M.foldr S.intersection (S.fromList [0..length index-1]) . pick words $ index

pick :: Ord k => [k] -> Map k v -> Map k v
pick keys m =
  restrictKeys m $ S.fromList keys

Prefer qualified imports for containers

The containers modules contain several functions that have the same names as their list counterpart. Therefore, they are usually included as qualified modules:

import           Data.Map.Strict (Map)
import qualified Data.Map.Strict as M
import           Data.Set (Set)
import qualified Data.Set as S

The types are imported unqualified to make the type signatures easier to read.

Next, it's a easier to exchange your types later if you provide a type synonym:

type MultiMap k v = Map k (Set v)
type WordMap      = MultiMap String Int

Now, let's have a look at your functions. zipWithIndex isn't optimal because it traverses xs twice. However the result of zip is as long as the shorter of the two lists. We can therefore simply write

zipWithIndex xs = zip [0..] xs

Note that you don't need a type signature on those local functions. Indeed, they can be misleading, because the a in zipWithIndex is not related to the a in the outer function.

Don't shadow library function names

group is a name that's already imported via Data.List. Since we now import our containers as qualified, we can simply provide our own singleton function to get rid of group:

singleton :: k -> v -> MultiMap k v
singleton k v = M.singleton k (S.singleton v)

Our makeInverseFunction would now look like this:

singleton :: k -> v -> MultiMap k v
singleton k v = M.singleton k (S.singleton v)

makeInverseIndex :: [String] -> WordMap
makeInverseIndex = foldr (M.unionWith S.union . uncurry insert) M.empty . zipWithIndex . L.map words
  where
    zipWithIndex = zip [0..]

    insert v = foldMap (flip singleton v)

Prefer functions that provide your functionality already

However, there's a function to convert a list of maps into a single map, , unionsWith:

makeInverseIndex :: [String] -> WordMap
makeInverseIndex = M.unionsWith S.union . concat . zipWith go [0..] . map words
  where
    go index ws = map (flip singleton index) ws

I admit that go is a bad name in that context. indexer might be a better one. By the way, we cannot use foldMap or mconcat here, since that wouldn't merge the map values.

Try to relax your type signatures*

orSearch is fine, although you could relax its type. Also, words is against a function name. You can use ws safely in this context.

orSearch :: (Ord k, Ord v) => [k] -> MultiMap k v -> Set v
orSearch ws =
  M.foldr S.union S.empty . pick ws

* unless it leads to performance problems or ambiguities

Prefer foldr1 instead of complicated start values

Now, andSearch is a little bit tricky. You use S.fromList [0..length index-1] in order to have a proper "zero" case. However, if the map is empty, the correct answer should be the empty set, not the complete, right?

So let's handle that case first with M.null index and then use foldr1 from Map's Foldable instance:

andSearch :: (Ord k, Ord v) => [k] -> MultiMap k v -> Set v
andSearch words index =
  | M.null sets = S.empty
  | otherwise   = foldr1 S.intersection sets
 where
   sets = pick words index

Other than that, well done. Keep in mind that there's a IntSet in Data.IntSet that might be more suitable for your use case.


Web Scraping with Python My target website is Indeed. I tried to implement the scraper such that it is convenient for the end user. I am introducing my code in the readme file on my github repo.

I am somewhat a beginner in programming so I am looking for guidance on things such as if the libraries that I used are approriate, the code itself and on generally making the script better.

Link to the README file.

import requests
from bs4 import BeautifulSoup

jobName = input('Enter your desired position: ').replace(' ', '-')
place = input("Enter the location for your desired work(City, state or zip): ")

URL = 'https://www.indeed.com/q-'+jobName+'-l-'+place.replace(' ', '-')+'-jobs.html'
page = requests.get(URL)

soup = BeautifulSoup(page.content, 'html.parser')
pages = soup.find(id='searchCountPages')
noPag = pages.text.split('of')[1].strip(' jobs').replace(',', '')

nmoPag = input(f"There are {noPag} number of pages. If you want to scrape all of them write 'Max' else write number of pages you wish to scrape: ")
if nmoPag == 'Max':
    nmoPag = noPag

for i in range(0, int(nmoPag)*10, 10):
    URL = 'https://www.indeed.com/jobs?q='+jobName+'&l='+place.replace(' ', '+')+'&start='+str(i)
    page = requests.get(URL)
    soup = BeautifulSoup(page.content, 'html.parser')
    results = soup.find(id='resultsCol')
    listings = results.find_all('div', class_='result')

    for job in listings:
        jobT = job.find('a', class_='jobtitle')
        jobL = job.find('span', class_='location').text.strip()
        jobS = job.find('div', class_='summary').text.strip()
        link = jobT['href']
        if any(any(subs in s for s in (jobT.text.strip().lower(), jobS.lower())) for subs in (jobName.split('+')[0], jobName[1])):
            print('Your job in '+jobL+' as a '+ jobT.text.strip()+
                    '.\nHere is a quick summary of your job here: '+
                        jobS+'\nLink for more information and application for the job - https://indeed.com'+link, end='\n\n\n')

What you are doing sounds reasonable overall.

It's good that you are using F-strings:

nmoPag = input(f"There are {noPag} number of pages. If you want to scrape all of them write 'Max' else write number of pages you wish to scrape: ")

But there are many other places where you don't eg:

print('Your job in '+jobL+' as a '+ jobT.text.strip()+
                    '.\nHere is a quick summary of your job here: '+
                        jobS+'\nLink for more information and application for the job - https://indeed.com'+link,

So I would upgrade the rest of the code for more consistency and better readability :)

Consistency: you are mixing lower case and upper case in some variable names eg jobName vs place. Remember that variable names are case-sensitive in Python. The practice could be dangerous and confusing. Imagine that you have jobName and jobname, it's two variables that may be assigned different values.

There is redundancy in the use of functions, for example this bit of code is repeated twice:

jobT.text.strip()

Don't repeat yourself, just assign the result of jobT.text.strip() to a variable once, and reuse it.

More repetition: www.indeed.com is hardcoded 3 times. Define a global variable for the root URL, then add the query string parameters as required.

With the urllib library you could take advantage of the URI-building functions that are available to you. See Creating URL query strings in Python. Tip: if you want to build URIs but don't fire them immediately, you can also use prepared requests.

Although in the present case, the site does not use classic query string parameters separated with a &. So you can instead use an F-string, again, with sanitized variable values:

url = f"https://www.indeed.com/q-{jobName}-l-{place}-jobs.html"

Note regarding user input: the most obvious is to check that the input is not empty. Always trim the input too, because some people might copy-paste text with extra tabs or spaces (think Excel). Maybe you could use a regex to replace multiple occurrences of whitespace with a single hyphen.

I would also add more checks to make sure that all the DOM elements you are expecting were found - because a website is subject to change at any time. When that happens, the code must alert you.

Finally, a quality script should have exception handling (10 lines of code would suffice). Catch exceptions always and log the full details to a file. This will help you a lot with debugging and troubleshooting.

As your code is on Github, some people might want to use it. If they have problems with it, it would be good if they could attach a log in their bug report, so that you get better insight into the error.

Since you are scraping a website, all sorts of errors will happen often: DNS resolution errors, timeouts, requests denied etc. Your script should handle those errors gracefully.


How do I reduce the if..else soup in this Controller function? The processing is the same, but the response type differs based on whether it's an AJAX request or not.

One way I can think of is to store all the response calls inside closure functions, and return once at the end (twice in the function overall because of the first return). That would only make it look a teensy bit better, in my subjective opinion. Not much of an improvement...

public function post_update()
{
    $submission = Input::all();
    $rules = array(
        'password' => 'required',
        'password_repeat' => 'required|same:password',
        'terms_accepted' => 'required'
    );

    $validator = Validator::make($submission, $rules);
    if (! $validator->passes()) {
        if (Request::ajax()) {
            return Response::json($validator->errors, 400);
        } else {
            return Redirect::to('signup/manual_password')->with_errors($validator);
        }
    }

    $user = Auth::user();
    $user->password = $submission['password'];
    $user->set_repeated_password($submission['password_repeat']);
    $user->password_set_manually = 1;

    try {
        $user->save_or_ex();

        if (Request::ajax()) {
            return Response::json(Session::get('_next_uri'));
        } else {
            return Redirect::to(Session::get('_next_uri'));
        }

    } catch (ValidationException $e) {

        if (Request::ajax()) {
            return Response::json($e->getErrors(), 500);
        } else {
            return Response::error(500);
        }

    }
}

Seeing I was asked to post my comment as an answer, I've set about typing this little thing up. It's, essentially, a slightly more verbose version of my initial comment.

Well, all in all, a controller action is a method, so the same "rules" should apply: 1 type of request goes to 1 distinct action method. Your method seems to be dealing with 3 distinct types of requests:

Client requests form page
Client submits form
Ajax submission of form-data

So I'd create 3 distinct actions. That way you can get rid of all those if (Request::ajax()) branches. You should still check if the request is an ajax request, but unless you're in the ajax controller, you can just redirect or return an error.
Since in 2/3 cases, you're validating the same form, you are, of course, free to stash that logic in a private helper method, or in some helper method in the model layer.

Something along these lines (I haven't got much experience with laravel, mind you):

class YourController extends BaseController
{
    public function updateForm()
    {
        //create the form, pass it to the view
    }

    public function ajaxUpdate()
    {
        if (!Request::ajax())
        {
            Response::json(
                array(
                    'message' => 'Invalid request',
                    'redirect' => 'some url'
                )
            );
        }
        $validator = $this->validateInput(Input::all());
        //process resulst
    }

    public function postUpdate()
    {
        if (Request::ajax())
        {
            Response::json(
                array(
                    'message' => 'Invalid request',
                    'redirect' => 'some url'
                )
            );
        }
        $validator = $this->validateInput(Input::all());
        //process resulst
    }

    private function validateInput($submission)
    {//No type-hint, because I'm not sure what type Input::all() returns
        $rules = array(
            'password' => 'required',
            'password_repeat' => 'required|same:password',
            'terms_accepted' => 'required'
        );
        $validator = Validator::make($submission, $rules);
    }
}

Now this may range anywhere from pretty-darn copy-paste ready to bizarre, but it's just to serve as an example of how I would set about this. To my eye, this is pretty clean code, easy to maintain, and, most important of all: Each action method deals with 1 specific request. In order not to repeat myself, I've created a private validateInput method, too, but I take it that's pretty self-evident ;-)


Basic input system for a few form fields I am attempting to port a linear function that I designed in a spreadsheet into HTML and Javascript.  It's based on linear least squares where I have to take a Matrix and multiply it by its transformation. At any rate. I am attempting to lay out a basic input system in Javascript/HTML where I can enter in variable data.  I have to take in quite a few more inputs, so I am looking for a more maintainable code pattern with less duplication.

In the name of make it work, then make it better. I came up with the following code.

Very new to javascript and the scope is hurting my brain! I am thinking that a dictionary (or similar) where I can pass that to a function then iterate over the values to update my HTML view. But again I am really new to Javascript.

The code also in JS BIN.

window.onload = function() {

    var EnameField = document.getElementById('EnameField');
    var ENameValue = undefined;
    var MnameField = document.getElementById('MnameField');
    var MNameValue = undefined;
    var CnameField = document.getElementById('CnameField');
    var CNameValue = undefined;
    var FnameField = document.getElementById('FnameField');
    var FNameValue = undefined;

    updateNameDisplay();

    setInterval(updateNameDisplay, 100);

    function updateNameDisplay() {
        var thisEValue = EnameField.value || " ";
        if (ENameValue != thisEValue) {
            document.getElementById('EnameDisplay').innerHTML = ENameValue = thisEValue;
        }

        var thisMValue = MnameField.value || " ";
        if (MNameValue != thisMValue) {
            document.getElementById('MnameDisplay').innerHTML = MNameValue = thisMValue;
        }

        var thisCValue = CnameField.value || " ";
        if (CNameValue != thisCValue) {
            document.getElementById('CnameDisplay').innerHTML = CNameValue = thisCValue;
        }

        var thisFValue = FnameField.value || " ";
        if (FNameValue != thisFValue) {
            document.getElementById('FnameDisplay').innerHTML = FNameValue = thisFValue;
        }

    }

};
<!DOCTYPE html>

      <meta charset=utf-8 />
      <title>Test Page</title>

      <form>
         How many E's <input type='text' id='EnameField'>

         How many M's <input type='text' id='MnameField'>

         How many C's <input type='text' id='CnameField'>

         How many F's <input type='text' id='FnameField'>

      </form>

      E's: ??

      M's:??

      C's: ??

      F's: ??

Handling the Problem

To make this scalable you need to generate the HTML to fit however many items you want to include. To Render it this way you need a data structure to hold what you want to render.

First Think of the data structure then what functions you need to render this data.

My Solution

I picked the following data structure.

[{
  title: String
  value: Number
}]

Here is the list of render functions I picked.

renderForm -> Render just the form
renderResults -> Render just the results
render -> Render the entire screen

And finally, we will need to capture the data changes from the form to update the data structure and then call renderResults so I created the onFormChange function.

Code

// Code goes here

const data = [{
  title: 'E',
  value: 0
}, {
  title: 'M',
  value: 0
}, {
  title: 'C',
  value: 0
}, {
  title: 'F',
  value: 0
}]

function onFormChange (value, i) {
  data[i].value = value;
  renderResults();
}

function renderForm () {
  const elForm = document.getElementById('form');
  let html = '';
  elForm.innerHTML = '';
  data.forEach((d, i) => {
    html += `
      How many ${d.title}'s
      <input type='text' value=${d.value} onchange="onFormChange( this.value, ${i})">

    `;
  })
  elForm.innerHTML = html;
}
function  renderResults () {
  const elResults = document.getElementById('results');
  let html = '';
  elResults.innerHTML = '';
  // Render Each Element
  data.forEach((d, i) => {
    html += `
      ${d.title}: ${d.value}

    `;
  })
  // Render Total
  let sum = 0;
  data.forEach((d) => {
   sum += Number(d.value) || 0
  })
  html += `

Total is ${sum}

  `
  elResults.innerHTML = html;
}

function render () {
  renderForm()
  renderResults()
}

window.onload = render;

<!DOCTYPE html>

    <form id="form">
    </form>

Final Thoughts

I did add a total div at the end of the results to show how you can also do some calculations on the entire data set when any of the values change.

Go ahead and add any additional data to the data array and have fun with it.


Print Bootstrap 3 Tabs Component using only one foreach Loop I'm using Bootstrap 3 to build a tab-panel component, based on information coming from the end-user. I don't like how there's a second foreach loop, but I don't know how else to print the HTML correctly.

Given an array structured like this:

[
    {
        "title": "Title 1",
        "content": "Lorem Ipsum"
    },
    {
        "title": "Title 2",
        "content": "Lorem Ipsum"
    }
]

And a "view" template structured like this:

<?php $panels = rwmb_meta("ssc_tabpanels") ?>
<?php if( !empty($panels) ): ?>

            <?php foreach($panels as $i => $p): ?>

                <a href="#tab-<?php echo $i ?>" data-toggle="tab"><?php echo $p['title'] ?>

            <?php endforeach; ?>

            <?php foreach($panels as $i => $p): ?>
            <div class="tab-pane" id="tab-<?php echo $i ?>"><?php echo $p['content'] ?>

            <?php endforeach; ?>

<?php endif; ?>

What suggestions do you have for removing the second foreach loop while still maintaining the correct bootstrap code?

One way would be to build the value of the tab contents in a variable during the first loop, and just print it once the loop ends.  I have not written PHP in a while nor tested this code, so use as reference.

<?php $panels = rwmb_meta("ssc_tabpanels") ?>
<?php $tabContentHtm = ''; ?>
<?php if( !empty($panels) ): ?>

        <?php
        //build the tabs
        foreach($panels as $i => $p):
        ?>

            <a href="#tab-<?php echo $i ?>" data-toggle="tab"><?php echo $p['title'] ?>

        <?php
        // build content displayed on click of each tab
        $tabContentHtm .= '
'. $p['content'] .'
\r\n';
        endforeach;
        ?>

        <?php echo $tabContentHtm; ?>

<?php endif; ?>

The rendered code should look something like this

            Title 1

            Title 2

Lorem Ipsum of Title 1

Lorem Ipsum of Title 2


Handling packets in nodejs for MMORPG Writing a MMORPG server in nodejs, I have to handle packets. These packets have a structure of

<length> <id> <data>

So what I did was use a node package called packet

And first get the packet id:

client.on('data', function(data) {
  parser.extract("l16 => length, l16 => id", function (packet) {
    if (packet.id === 300) {

    }
    if (packet.id === 301) {

    }
    if (packet.id === 302) {

    }
    if (packet.id === 303) {

    }
  });

  // parse the packet id to handle it accordingly
  parser.parse(data);
});

There are packet Id's upto 1000+ which would make my if statements very long. But each packet id contains different data and different ways to handle it.

For example packet 300 which is the handshake packet. It sends the client version to the server and the servers checks it and returns if it is correct or not, if not then client will error wrong version.

  if (packet.id === 300) {
    console.log('[recieved] packet:handshake');

    parser.extract("handshake", function (packet) {
      if (packet.gameClientVer === config.gameClientVer) {
        // do stuff
      } else {
        // do stuff
      }
    });

    // parse structure for packet 300
    parser.parse(data);
  }
});

the handshake packet is defined as:

serializer.packet("handshake", "l16 => length, l16 => id, l16 => gameClientVer, l16 => gameUpdateVer, l16 => gameDateVer");

My question is, how can I improve this?

You should compartmentalize your "handlers" for each of the packets.  Each handler should contain a canHandle method that determines whether it can process the packet and a process method that does the work.  Example:

var packet300Handler = {
    canHandle: function(packet) {
        return packet.id === 300;
    },
    process: function(packet) {
        // Do work here.
    }
};

var handlers = [packet300Handler, packet301Handler, packet302Handler];

client.on('data', function(data) {
    parser.extract('DATA', function(packet) {
        for (var i = 0; i < handlers.length; i++) {
            var handler = handlers[i];

            if (handler.canHandle(packet)) {
                handler.process(packet);
                break; // Leave loop now that something has handled the packet.
            }
        }
    });
});


Is this the right way to implement a simple hash table in C++? Does this look like a proper implementation of a hash table in C++? Any errors anyone can find?
Thanks in advance (:

template <typename T>
class HashTable
{
public:
HashTable(int amount)
{
    size = amount;
    values = new T*[size];
    for (int i = 0; i < size; i++)
    {
        *(values + i) = nullptr;
    }
}
~HashTable()
{
    if(values != nullptr)
    {
        for (int i = 0; i < size; i++)
        {
            if(*(values + i) != nullptr)
            {
                delete *(values + i);
                *(values + i) = nullptr;
            }
        }
        delete[] values;
    }
}

void addValue(const std::string & key, T value)
{
    int location = hash(key);

    if (*(values + location) == nullptr)
    {
        *(values + location) = new T;
    }

    **(values + location) = value;
}

void printValues()
{
    for (int i = 0; i < size; i++)
    {
        if (*(values + i) != nullptr)
        {
            printf("value : %i\n", **(values + i));
        }
    }
}

T* getValue(const std::string & key)
{
    return *(values + hash(key))
}

private:

int hash(const std::string & key)
{
    int value = 0;
    for (unsigned int i = 0; i < key.length(); i++)
    {
        value += key[i] * (i + 1);
    }
    return value % size;
}

int size;
T ** values = nullptr;
};

The biggest issue is, as mentioned in the comments, that you don't handle collisions. I'm not going to cover that, but I'll address some other issues.

IMO your hashtable should be templated on at least 3 types - the key type, the value type, and the hash functor.  This way you aren't limiting your users to having string keys or to rely on your hash function.

You should also use colon initialization - the data members will be default initialized at the starting curly brace of the constructor, but with colon initialization you skip the default initialization and the replacement with new values.  That would look something like this

HashTable(size_t amount) : size(amount), values(new Value*[size]) {
    for (size_t i = 0; i < size; ++i) {
        values[i] = nullptr;
    }
}

You'll notice I've replaced your ints with size_t - in these situations you don't need negative numbers.  I've also used the subscript notation instead of pointer arithmatic, which is much, much easier to read and reason about. You should strongly prefer subscript notation if it is possible to use it.

Your destructor doesn't need to check if the the values are not nullptr - there isn't a sensible reason it should be.

You should be using const functions - printValues should be, and you should provide a const version of getValue as well.

You also assume that the value type can be default initialized, which is not necessarily true. You might want to use placement new here instead. Alternatively, you could just split up the code a bit.

void addValue(const std::string & key, T value)
{
    int location = hash(key);

    if (values[location] == nullptr)
    {
        values[location] = new T(value);
    }
    else
    {
        *values[location] = value;
    }
}

This still requires that there be a copy constructor, however this is probably a less surprising requirement (most containers expect objects to be copy/move constructible).

You'll probably want to provide an iterator as well, but I'll leave that as an exercise for the reader.

Lastly, you should provide a method to find and erase, and probably resize your hash table.  Without those things the hash table doesn't have much use.


Tokenizing each document in a large document of documents I've been working on a project where I am importing a large document of documents and tokenizing each document. I then make a hashset of the tokens I have, and for each of these unique tokens I am looking to find their frequency in each of the documents I have. There are about 130000 documents total.

I've run the code and unfortunately, it's taking 150 hrs to run.  Do you have any suggestions on improving my code?

This is my Tokenizer() function, which surprisingly works okay:

static private IEnumerable<string> Tokenizer(ref StreamReader sr, ref List<string> tokenz, ref List<string> stopwords)//function for tokenizing
{
    string line;
    var comparer = StringComparer.InvariantCultureIgnoreCase;
    var StopWordSet = new HashSet<string>(stopwords, comparer);
    List<string> tokens = new List<string>();//list of strings called tokens
    while ((line = sr.ReadLine()) != null)//as long as the streamreader has something
    {
        foreach (string item in line.Split(' '))//split amongst strings
        {
            if (item.StartsWith("<") & item.EndsWith(">"))
            {
                item.Trim();//trims the item of spaces
                if (item == "</DOC>")
                {
                    //return item;
                    tokenz.Add(item);//adds the doc tags for later separation use
                }
            }
            else
            {
                string newitem;
                item.Trim();//trims the item of spaces
                if (item != "")//ensures item is not blank
                {
                    newitem = Regex.Replace(item, @"[^A-Za-z0-9]+", "", RegexOptions.IgnoreCase);//regex allows us to ignore case and remove any special characters
                    string newitem2 = newitem.ToLower();
                    {
                        if (StopWordSet.Contains(newitem2))
                        {
                        }
                        else
                        {
                            tokenz.Add(newitem2);
                        }
                        //tokens.Add(newitem.ToLower());//makes all lower case
                    }
                }
            }
        }
    }
    return tokens;
}

The real issue is here:

static public void AddToDictionaryAndCount(ref int doccounter2, ref HashSet<string> MyLexicon, ref List<string> tokens, ref Dictionary<int, int> DocFreqCounter, ref Dictionary<string, Dictionary<int, int>> MyDictionary, ref Dictionary<int, int> DocWordCounter)
{
    foreach (string item in MyLexicon)
    {
        int counter = 0;
        int secondcounter = 0;
        int doccounter = 1;
        int termcounter = 0;
        while (counter <= tokens.LastIndexOf("</DOC>"))
        {
            if (tokens[counter] == "</DOC>")
            {
                DocFreqCounter.Add(doccounter, termcounter);
                if (doccounter2 < doccounter)
                {
                    DocWordCounter.Add(doccounter, (counter - secondcounter));
                    doccounter2++;
                }
                termcounter = 0;
                secondcounter = counter;
                doccounter++;
            }
            if (tokens[counter] == item)
            {
                termcounter++;
                //words.termCount = termcounter;
            }
            counter++;
        }
        MyDictionary.Add(item, new Dictionary<int, int>(DocFreqCounter));
        DocFreqCounter.Clear();
    }
}

Tokenizer method:

You use ref incorrectly, all reference types are always passed by reference, please read the MSDN or any other online resource to understand the ref
Your code doesn't follow naming conventions: local variables, parameters should be camelCased (e.g. StopWordSet)
use string.Split(new char[0], StringSplitOptions.RemoveEmptyEntries) overload to split on all whitespace characters and remove empty strings during tokenizing, rather than trimming result
strings are immutable, so the statement item.Trim() is useless unless you assign it to something
Tokenizer method always returns an empty list since tokens (not tokenz) is only intialized and not used. It looks like tokenz parameter is redundant and tokens variable should be used instead
To improve performance it might be better to change the type of stopwords parameter to ISet<string> and use it for matching, rather than create a new StopWordSet each time
As a result you'll get a cleaner version of Tokenizer:

private static IEnumerable<string> Tokenizer(StreamReader sr, ISet<string> stopwords)//function for tokenizing
{
    string line;
    List<string> tokens = new List<string>();//list of strings called tokens

    while ((line = sr.ReadLine()) != null)//as long as the streamreader has something
    {
        foreach (string item in line.Split(new char[0], StringSplitOptions.RemoveEmptyEntries))//split amongst strings
        {
            if (item.StartsWith("<") & item.EndsWith(">"))
            {
                if (item == "</DOC>")
                    tokens.Add(item); //adds the doc tags for later separation use
            }
            else
            {
                string newitem = Regex.Replace(item, @"[^A-Za-z0-9]+", "", RegexOptions.IgnoreCase).ToLower();
                if (!stopwords.Contains(newitem))
                    tokens.Add(newitem);
            }
        }
    }
    return tokens;
}

Now let's look at the AddToDictionaryAndCount method:

naming conventions - do name your variables/parameters properly, it's absolutely not clear what is the difference between doccounter2 parameter, doccounter and secondcounter variables. Rename them to smth. like documentIndex, numberOfTermOccurances etc.
while (counter <= tokens.LastIndexOf("</DOC>")) is the killer, you're looking for "</DOC>" entry on each iteration. Cache the calculated value instead.
since you know the number of iterations and you always increment the index it's better to use for instead of while
not sure why you pass docFreqCounter as parameter, it looks like is being used as a local variable.

As to performance improvements (other than caching LastIndexOf) - currently you scan the through the tokens list for each and every item in myLexicon. Assuming that lexicon is built from tokens it would be much better to scan the tokens list only once, tracking which items you've already counted and where are the doc boundaries. Since you haven't provided the meaning of all parameters participating in this method it's hard to suggest a proper solution, but here is the first approximation:

public static Dictionary<string, Dictionary<int, int>> AddToDictionaryAndCount(List<string> tokens)
{
    var result = new Dictionary<string, Dictionary<int,int>>();
    var documentIndex = 0; //tracks the current document index

    var lastDocumentIndex = tokens.LastIndexOf("</DOC>");
    for (int i = 0; i <= lastDocumentIndex; i++)
    {
        var token = tokens[i];
        if (token == "</DOC>")
        {
            //finalize stats for the document. not sure what goes here.
            //add the logic corresponding to "doccounter2 < doccounter"
            documentIndex++;
            continue;
        }

        Dictionary<int, int> documentStats;
        if (!result.TryGetValue(token, out documentStats))
            documentStats = result[token] = new Dictionary<int, int>();

        documentStats[documentIndex]++;
    }
    return result;
}


O PC, why are you so slow? In order to test some software I've written, I need to write values to a test harness with OLE for Process Control (OPC).  The problem is, for a larger range of values I'm generating points for (say 10000), this code can take 15 seconds to execute.  I need it to work in under 1 second.

import OpenOPC
import argparse
import sys
import time
from random import randint

def get_args():
    parser = argparse.ArgumentParser(description='Record data value changes')
    parser.add_argument('range', type=int, help='range of values to record')
    parser.add_argument('time', type=int, help='time to update values in seconds')
    return parser.parse_args()

def get_range_list(amount, val, end):
    return [
        ('flcon1:DataValue~Osiris_Test_Data_' + str(i + 1) + end, randint(0, val))
        for i in range(amount)
    ]

def get_time_list(amount, end):
    return [
        ('flcon1:DataValue~Osiris_Test_Data_' + str(i + 1) + end, time.strftime('%m/%d/%Y %H:%M:%S'))
        for i in range(amount)
    ]

def main():
    args = get_args()
    opc = OpenOPC.open_client('localhost')
    opc.connect('SISCO.AXS4ICCP.3')

    print('Logging test harness value changes')
    while True:
        try:
            t0 = time.clock()
            value_list = get_range_list(args.range, 9, '.Value')
            time_list = get_time_list(args.range, '.TimeStamp')

            opc.write(value_list)
            opc.write(time_list)

            print('Wrote values')

            with open(str(args.range) + '.' + str(args.time) + '_1-iccp_harness.log', 'a') as f:
                for i in xrange(0, args.range):
                    f.write('Osiris_Test_Data_' + str(i + 1) + ', realQTimetagExtended' +
                            ', %g, %s, %s\n' % (value_list[i][1], 00, time_list[i][1]))

            t1 = time.clock()
            print t1-t0

            print('Logged values')
            time.sleep(args.time)

        except KeyboardInterrupt:
            opc.close()
            print('Closing down logger')
            sys.exit()

if __name__ == '__main__':
    main()

def get_range_list(amount, val, end):
    return [
        ('flcon1:DataValue~Osiris_Test_Data_' + str(i + 1) + end, randint(0, val))
        for i in range(amount)
    ]

You're only interested in the value of amount, not in the variable it produces itself. Have you tried using xrange instead? xrange is much faster than range.

Afar from that, I cut-out the OPC parts and ran the rest through a profiler.

python -m cProfile -s tottime CR_135184.py 1000000 1

Cut code:

import argparse
import sys
import time
from random import randint

def get_args():
    parser = argparse.ArgumentParser(description='Record data value changes')
    parser.add_argument('range', type=int, help='range of values to record')
    parser.add_argument('time', type=int, help='time to update values in seconds')
    return parser.parse_args()

def get_range_list(amount, val, end):
    return [
        ('flcon1:DataValue~Osiris_Test_Data_' + str(i + 1) + end, randint(0, val))
        for i in xrange(amount)
    ]

def get_time_list(amount, end):
    return [
        ('flcon1:DataValue~Osiris_Test_Data_' + str(i + 1) + end, time.strftime('%m/%d/%Y %H:%M:%S'))
        for i in xrange(amount)
    ]

def main():
    args = get_args()
    print('Logging test harness value changes')

    t0 = time.clock()
    value_list = get_range_list(args.range, 9, '.Value')
    time_list = get_time_list(args.range, '.TimeStamp')

    print(value_list)
    print(time_list)

    print('Wrote values')

    with open(str(args.range) + '.' + str(args.time) + '_1-iccp_harness.log', 'a') as f:
        for i in xrange(0, args.range):
            f.write('Osiris_Test_Data_' + str(i + 1) + ', realQTimetagExtended' +
                    ', %g, %s, %s\n' % (value_list[i][1], 00, time_list[i][1]))

    t1 = time.clock()
    print t1-t0

    print('Logged values')

if __name__ == '__main__':
    main()

Result (the relevant parts):

Wrote values
7.145332
Logged values
         5001665 function calls (5001624 primitive calls) in 15.432 seconds

   Ordered by: internal time

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
        1   11.660   11.660   15.299   15.299 CR_135184.py:27(main)
  1000000    1.269    0.000    1.269    0.000 {time.strftime}
  1000000    0.876    0.000    0.925    0.000 random.py:175(randrange)
        1    0.551    0.551    1.708    1.708 CR_135184.py:13(get_range_list)
        1    0.510    0.510    1.779    1.779 CR_135184.py:20(get_time_list)
  1000000    0.232    0.000    1.157    0.000 random.py:238(randint)
  1000000    0.151    0.000    0.151    0.000 {method 'write' of 'file' objects}
        1    0.120    0.120   15.432   15.432 CR_135184.py:1(<module>)
  1000000    0.049    0.000    0.049    0.000 {method 'random' of '_random.Random' objects}

Basically, you're slowing down your code by checking the time. The generating of random numbers takes some time, but get_range_list and get_time_list are surprisingly fast.

The real bottleneck? Your OPC connection. And I'm not sure whether this can be fixed at all in code.


Expanding a working query without adding multiple join statements? I have the following tables set up:

t_users table:

|----------------------------------|
|uid | name   | email              |
|----------------------------------|
|1   | Maria  | maria@example.com  |
|2   | George | george@example.com |
|----------------------------------|

t_settings table:

|---------------------------------|
|t_sid | name     | default_value |
|---------------------------------|
|1     | uicolour | #f00          |
|2     | language | en            |
|---------------------------------|

t_users_settings table:

|--------------------|
|uid | t_sid | value |
|--------------------|
|2   | 1     | #000  |
|1   | 2     | es    |
|--------------------|

What I want, is for each user, to be able to pull all of their custom settings, and also pull the default settings for anything that they haven't changed. I have the following query that works:

SELECT t_users.*, t_users_settings.*, t_settings.*
FROM t_users
LEFT JOIN t_users_settings ON t_users.uid = t_users_settings.t_uid
LEFT JOIN t_settings ON t_settings.t_sid != t_users_settings.t_sid
WHERE t_users_settings.t_sid IS NOT NULL
AND t_users_settings.t_uid IS NOT NULL
AND t_users.uid = '<input user id here>';

However, it appears that if I add more settings (Such as country), I then have to add another JOIN and AND statement. I think there is something missing something in the setup,
or I'm just not familiar enough with MySQL to format the query correctly.

Little help?

At first glance, there are some alarm bells ringing.

You have t_users_settings.t_sid IS NOT NULL AND t_users_settings.t_uid IS NOT NULL. The first seems to imply you might have NULL values in your t_users_settings.t_sid column. The second seems to defeat the purpose of doing a left join to t_users_settings in the first place.
A second concern is that you're over-generalising.
Notice that you're unable to choose a specific type for your t_users_settings.value column. This means you have to roll your own custom type checking or do without.
Yes your design allows you to add new settings "without code changes". But in reality, those settings would be meaningless without code changes to make use of them in the first place.

You could probably solve your current problems quite easily by defining your settings table with a separate column for each possible setting, and doing the minimal work of adding columns as you add new settings.

However, that said here's a suggestion based on your current design.

First you want to get all permutations of settings and users. For that you need a cross join.

SELECT  u.*, s.*
FROM    t_users u
        CROSS JOIN t_settings s

That gives all combinations with their defaults. Now you want a left outer join to find out which defaults have been overridden for each user.

SELECT  u.*, s.*
FROM    t_users u
        CROSS JOIN t_settings s
        LEFT JOIN t_users_settings us ON
            us.uid = u.uid
        AND us.t_sid = s.t_sid

Finally, cleanup it up with explicit column selection, and you can even derive the actual users_value with a COALESCE, ISNULL or CASE statement.

SELECT  u.uid,
        u.name,
        u.email,
        s.t_sid,
        s.name,
        s.default_value,
        COALESCE(us.value, s.default_value) AS users_value
FROM    t_users u
        CROSS JOIN t_settings s
        LEFT JOIN t_users_settings us ON
            us.uid = u.uid
        AND us.t_sid = s.t_sid


Numerology Calculator I've built the beginnings of a numerology calculator, and would like an experienced javascript developer to review this code.

It works as expected, but I can't help but think there is a better way to write it instead of using multiple for loops.

There are a few calculations that have to be made, so I can't see any way around it. Any direction would be great.

function addNumbers(arr, fn) {
    var arrayTotal = [],
    singleDigits = [];

    for (i = 0; i < arr.length; i++) {
        arrayTotal.push(
            fn(arr[i])
        );
    }

    for (value in arrayTotal) {
        for (num in arrayTotal[value]) {
            singleDigits.push(parseInt(arrayTotal[value][num]));
        }
    }

    for (i = 0, sum = 0; i < singleDigits.length; i++) {
        sum += singleDigits[i];
    }

    for (i = 0, numericValue = 0, sum = fn(sum); i < sum.length; i++) {
        numericValue += parseInt(sum[i]);
    }

    return numericValue;
}

function splitNumbers(item) {
    return item.toString().split('');
}

var value = addNumbers([13, 11, 1938], splitNumbers);
document.body.innerHTML = value;

map and reduce are tailor made for problems like this.

To understand the while loop in the final version of the code at the bottom of this post, it may help to consider a "longhand" version that does the same thing:

// this sets the answer we seek to the sum of all the digits
// of every number passed in via the array
finalAnswer = addDigits(combinedDigits);

// but since this sum may still be larger than a 1 digit
// number (ie, 10 or more), we need to continue the summing
// the digits until we end up with a single digit number
while ( finalAnswer > 9 ) {
  // this converts an integer to a string
  combinedDigits = '' + finalAnswer;
  // this sums the digits again
  finalAnswer = addDigits(combinedDigits);
}

Here's the final, complete version, with the more compact while loop.  We are simply combining some of the steps above.

function addDigitsRecursively(arr) {

  var combinedDigits = arr.join(''), finalAnswer;

  while ( (finalAnswer = addDigits(combinedDigits)) > 9 )
    combinedDigits = '' + finalAnswer;

  function addDigits(str) {
    return str.split('').map(function(n) {return parseInt(n)})
                        .reduce(function(a, b){return a+b;})
  }

  return finalAnswer;
}

var value = addDigitsRecursively([13, 11, 1938]);
document.body.innerHTML = value;


PHP snippet to create image gallery from folders with jpgs I would like to receive input from more experience coders regarding the potential improvements this piece of code might have. I am new to PHP development and I am sure this code looks horrible behind the eyes of someone more competent. It works, but I think there's plenty of room for improvement.

I have an images folder with several sub-directories, and inside them, different images are stored. The goal of this PHP snippet is to output HTML code in this format for each folder and files in my images folder:

examplefolder

examplefilename

examplefilename2

Here's said snippet:

<?php
    $folders = glob('images/otros/*', GLOB_ONLYDIR);
    foreach($folders as $folder) {
        echo "

".basename($folder)."

".PHP_EOL;
            $files = glob("images/otros/".basename($folder)."/*.{jpg}", GLOB_BRACE);
            foreach($files as $file) {
                echo "
".PHP_EOL."
".basename($file, ".jpg")."
".PHP_EOL."    ".PHP_EOL."
".PHP_EOL;
            }
        echo "
";
    }
?>

In particular, this one gets all folders with images inside images/otros.

Would you please offer any tips regarding best practices, or how to make my code look more readable?
Have I used any deprecated functions?
Is PHP_EOL the best way to add a newline character in order to make the output HTML look better?

Is PHP_EOL the best way to add a newline character in order to make the output HTML look better?

No, I don't think so. If you want your HTML nice on output, why not have it nice in the source code as well?

So instead of this:

echo "
".PHP_EOL."
".basename($file, ".jpg")."
".PHP_EOL."    ".PHP_EOL."
".PHP_EOL;

have this:

echo "

".basename($file, ".jpg")."

;

  Have I used any deprecated functions?

I don't think so. But generally, you can see that yourself, when you enable warnings in development (don't forget to turn them of in production though).

  Would you please offer any tips regarding best practices, or how to make my code look more readable?

You could extract code blocks to functions
You could use more whitespace (eg around .)
You could extract duplicate function calls to well-named variables
have constants such as fixed directory names in as few places as possibly, to make changing it easier

This would give you something like this. It's not perfect, but hopefully a good start:

echo formatHTMLforDirectory('images/otros/');

/**
TODO PHPDoc comment
*/
function formatHTMLforDirectory($basedirectory) {
    $directories = glob("$basedirectory/*", GLOB_ONLYDIR);
    $html = "";
    foreach($directories as $directory) {
        $directoryName = basename($directory);
        $html .= "

$directoryName

                    ";
        $files = glob("$basedirectory/$directoryName/*.{jpg}", GLOB_BRACE);
        $html .= formatHTMLforFiles($files);
        $html .= "
";
    }
    return $hmtl;
}

/**
TODO PHPDoc comment
*/
function formatHTMLforFiles(array $files) {
    $html = "";
    foreach($files as $file) {
        $filename = basename($file, ".jpg");
            $html .+ "

$filename

";
    }
    return $hmtl;
}

Misc

I always get nervous when data from the outside is echoed unencoded, as it can lead to XSS. If you allow users to upload image files, you are already vulnerable. But even if not, you may allow this in the future, which would make you vulnerable then.
You might want to think about using a templating engine such as twig. Mixing HTML and PHP often leads to hard to read code.


A 3D Random walk simulation I was referring to the book Computational Physics by Nicholas J. Giordano and was studying the simple Random Walk model. (Pg. 169 - 173)

I consider a 3D Random Walk where the probability of moving in the positive direction is p and in the negative direction is q, in all the 3 coordinates and p + q = 1. In my model I assume:

$$ p_x=q_x =0.5,\space p_y=q_y=0.5 \space and \space p_z=q_z=0.5$$
$$l_x = l_y=l_z=\pm 1$$

where l is the step length. In my program I consider total number of walkers = N and total number of random steps taken by each walker is s. My goals are:

Plot a scatter plot : $$<x_s^2> \space vs. \space t$$
Find the slope from the regression line.

Here is my R code:

randomWalk <- function(N, s) {
  S <- 1:s
  av <- numeric(s)
  for (r in 1:N) {
    x <- 0
    y <- 0
    z <- 0
    for (i in 1:s) {
      if (runif(1)<0.5) x <- x+1
      else x <- x-1
      if (runif(1)<0.5) y <- y+1
      else y <- y-1
      if (runif(1)<0.5) z <- z+1
      else z <- z-1
      av[i] <- av[i] + (x^2+y^2+z^2)
    }
  }
  av <- av/N
  plot(S, av,xlab="No. of steps(t)", ylab="<r^2>", main="Random Walk",cex=0.4, pch=15)
  abline(lm(av~(S)-1))
  print(lm(av~(S)-1))
}
randomWalk(500, 100)

In the code I consider N=500 and s=100.

The plot I get is

with value of slope approximately 3. But in page 173 there is a plot with the same condition but slope approximately 1.

My question is, Is there something wrong with my code or am I misunderstanding the plot given in page 173? A review will be helpful.

Quoting page 173:

  Another obvious generalization is to allow the walker to move in three dimensions [...] For this simulation we have restricted the steps to be of unit length along either ±x, ±y, or ±z.

I think the author meant that at each time step the walker will walk in one of three random directions (x or y or z), not all three. Without making drastic changes to your code, you could replace this section:

 if (runif(1)<0.5) x <- x+1 else x <- x-1
 if (runif(1)<0.5) y <- y+1 else y <- y-1
 if (runif(1)<0.5) z <- z+1 else z <- z-1

with the following:

  direction <- sample(c("x", "y", "z"), 1)
  if (direction == "x") { if (runif(1)<0.5) x <- x+1 else x <- x-1 }
  if (direction == "y") { if (runif(1)<0.5) y <- y+1 else y <- y-1 }
  if (direction == "z") { if (runif(1)<0.5) z <- z+1 else z <- z-1 }

With this change, the slope is approximately 1, as shown in Figure 7.11.


Join strings with different delimiters and some possibly empty strings I want to construct a string from several substrings, each one of those with a different delimiter. If the string is present, the delimiter and the string should be added, if not, none of them should be added.

This is my current code that looks a bit dirty to me:

def my_join(major, minor, patch=None, pre_release=None, build=None, revision=None):

   version = '.'.join(filter(None,(major, minor, patch)))

   if pre_release: version += '-%s'  % pre_release
   if build:       version += '+b%s' % build
   if revision:    version += '.%s'  % revision

   return version

my_join('0', '1',pre_release='alpha')
'0.1-alpha'
my_join('2','3','4', build='567', revision='aabbcc440')
'2.3.4+b567.aabbcc440'

My problem is basically an extension of join 4 strings to the one if they are not empty in python with the addition of using different delimiter strings.

It doesn't look too bad to me; I'd make a few changes.

def my_join(major, minor, patch=None, prerelease=None,
            build=None, revision=None):
    version_parts = ['.'.join((major, minor, patch or ''))]

    if prerelease:
        version_parts.append('-%s' % prerelease)
    if build:
        version_parts.append('+b%s' % build)
    if revision:
        version_parts.append('.%s' % revision)

    version = ''.join(version_parts)

    return version

Instead of building the string piece-by-piece, this code adds the needed parts to a list which is then joined at the end. I also removed the overpowered call to filter, and instead used or to make the patch value be '' if patch is False-ish, as it would be if patch is None.


Project Euler 3 - Largest prime factor Problem 3 - Largest prime factor

Exercise

  The prime factors of 13195 are 5, 7, 13 and 29.

  What is the largest prime factor of the number 600851475143 ?

My solution

package pl.hubot.projecteuler.problem3;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

public class Main {
    public static void main(String[] args) {
        System.out.print(Collections.max(primeFactors()));
    }

    private static List<Long> primeFactors() {
        long n = 600851475143L;
        List<Long> factors = new ArrayList<>();
        for (long i = 2; i <= n; i++) {
            while (n % i == 0) {
                factors.add(i);
                n /= i;
            }
        }
        return factors;
    }
}

I would like ask review for my code and possible improvements. I am interested how is performance of my code.

Get rid of the list. No, really, it's as simple as that. The sequence of is in factors will be non-decreasing, so essentially you could switch

System.out.print(Collections.max(primeFactors()));

with

List<Long> factors = primeFactors();
System.out.print(factors.get(factors.size() - 1));

While the list for prime factors might come in handy for other challenges, you're just interested in the largest, so it's fine to return just a single value:

private static long largestPrimeFactor(long n) {
    long largest = -1;
    for (long i = 2; i <= n; i++) {
        while (n % i == 0) {
            largest = i;
            n /= i;
        }
    }
    return largest;
}

Since you're interested in performance, you probably want to split the for into two parts to skip the unneeded even integers:

private static long largestPrimeFactor(long n) {
    long largest = -1;

    while(n % 2 == 0) {
        largest = 2;
        n /= 2;
    }

    for (long i = 3; i <= n; i = i + 2) {
        while (n % i == 0) {
            largest = i;
            n /= i;
        }
    }
    return largest;
}

Other than that, well done, and the correct approach. But just as in your previous code, ask yourself whether you really need the whole collection.


General Retry Strategy Let’s say we copy some file using retry strategy (it might be blocked, etc.):

class Processor
{
    public void CopyData() =>
        CopyData(Try.Slow);

    public void CopyData(Try loop) =>
        loop.Execute(() =>
            File.Copy(@"c\a.txt", @"c:\b.txt"));
}

What do you think about names chosen for the following library code identifiers? Would you name them differently?

public abstract class Try
{
    public static Try Repeat(params int[] delays) => new Repeat(delays);
    public static readonly Try Never = Repeat();
    public static readonly Try Once = Repeat(0);
    public static readonly Try Slow = Repeat(0, 500, 1500, 4500, 12000);
    public static readonly Try Fast = Repeat(0, 50, 150, 450, 1200);
    public abstract void Execute(Action action);
}

class Repeat : Try
{
    IReadOnlyList<int> Delays { get; }

    public Repeat(params int[] delays)
    {
        Delays = delays;
    }

    public override void Execute(Action action)
    {
        for(int i=0; i< Delays.Count; i++)
            try
            {
                Thread.Sleep(Delays[i]);
                action();
                return;
            }
            catch
            {
                if (i == Delays.Count - 1)
                    throw;
            }
    }
}

I think this would be more useful if  the user could specify an interval and how many times he wants to retry like:

public static Try Repeat(int dalay, int count)
    => new Repeat(Enumerable.Repeat(delay, count));

or if he could specify a count and the increment function:

public static Try Repeat(int count, Func<int, int> increment)
    => new Repeat(Enumerable.Range(1, count + 1).Select(x => increment(x));

where the increment could be:

x => x * 20

I wouldn't provide such members as Fast or Slow because they are very subjective and what for you currently is slow might be in my application still too fast.

Never does not make any sense ;-) Why should I want to never try to execute something? I might as well not write the code at all if it shouldn't run :-P

One more thoght. How about specifying the retry strategy via a generic argument:

class SlowTry : Repeat
{
    public SlowTry() : base(0, 500, 1500, 4500, 12000) { }
}

the  Try becomes this:

public abstract class Try
{
    ... stays the same

    public static void Execute<TStrategy>(Action action) where TStrategy : Try, new()
    {
        new TStrategy().Execute(action);
    }
}

use:

Try.Execute<SlowTry>(() => File.Copy(@"c\a.txt", @"c:\b.txt"));

This way the user can easier specify his strategy.


PHP/Phalcon RESTful API architecture I'm building a RESTful API in PHP using the Phalcon framework. The API will consist of multiple modules (e.g. api.example.com/mail/users, api.example.com/web/users, etc). So I want to put all module specific logic at one place as much as possible.

I have a lot of programming experience but PHP is all pretty new to me. So if you could help me determine if I'm on the right track, so that this code won't give me future problems, or if I'm missing something or if the code could be abstracted even more, then that would be great.

Directory structure:

app/
    collections/
        MailCollection.php
    config/
        collections.php
    controllers/
        MailController.php
public/
    index.php

index.php

// Set path variables
$dir = dirname(__DIR__);
$appDir = $dir . '/app';

// Create autoLoader
$loader = new \Phalcon\Loader();

// Register namespaces
$loader->registerNamespaces(
    array(
        'Controllers' => $appDir . '/controllers'
    )
)->register();

// Create dependency injector
$di = new \Phalcon\DI\FactoryDefault();

// Create micro application
$app = new Phalcon\Mvc\Micro($di);

// Mount the collections
$collections = include_once($appDir . '/config/collections.php');
foreach($collections as $collection) {
    $app->mount($collection);
}

// Handle request
$app->handle();

collections.php

// Add collections
$collections[] = (
    include_once($dir . '/app/collections/MailCollection.php')
);

return $collections;

MailCollection.php

use \Phalcon\Mvc\Micro\Collection as MicroCollection;

// Setup Collection
$mail = new MicroCollection();
$mail->setHandler('Controllers\MailController', true);
$mail->setPrefix('/public/mail');

// Define routes
$mail->get('/test', 'testAction');

return $mail;

MailController.php

namespace Controllers;

class MailController extends \Phalcon\Mvc\Controller
{
    public function testAction()
    {
        echo('test');
    }
}

I believe you are on the right track.

The only thing that I would do different would be to add a library folder in which I would store all my common stuff like models, utility classes, helpers etc. I would also namespace everything so as to avoid collisions with any other application or piece of code out there.

My preferred app structure for your app would be as follows

app/
    mail/
        controllers/
            MailController.php
    web/
        controllers/
            WebController.php
library/
    MyAPI/
        models/
            Users.php   <- The model accessing the users table
        Auth.php        <- Authentication
        Controller.php  <- A master controller where all controllers inherit from
        Model.php       <- A master model that all models inherit from
        Locale.php
        Utils.php
var/
    config/
        collections.php
        db.php
        config.php
    cache/
        dummy.txt
    metadata/
        dummy.txt
    logs/
        dummy.txt
public/
    index.php

Your controllers inherit from the base controller in the library folder. In that controller you can do common tasks for all collections. The same applies to the master model.

You can remove the controllers/ folder under each module in the app folder and have them all right under mail, web etc. It's up to you.


Most pythonic way to combine elements of arbitrary lists into a single list I have a list of lists which represents the options I can chose from.

I have a sequence of indices which represent which option lists I want to take elements from, and in which order.

E.g. if I have

choices = [
    [ 1, 2, 3 ],
    [ 4, 5, 6 ],
    [ 7, 8, 9 ]
]

sequence = [ 2, 0, 1, 1 ]

I want my output to be

[7, 8, 9, 1, 2, 3, 4, 5, 6, 4, 5, 6]
#index 2  index 0  index 1  index 1

I have found three possible solutions:

choice = sum( ( choices[i] for i in sequence ), [] )

choice = reduce( operator.add, ( choices[i] for i in sequence ) )

choice = [ element for i in sequence for element in choices[i] ]

I would like to know which of these do people find the most pythonic and to know if there are any other elegant solutions.

Unless I actually needed the whole list at once, I would probably use itertools for this:

from itertools import chain

choice = chain.from_iterable(choices[i] for i in sequence)

If you do need the list, you can still use this with an explicit conversion:

choice = list(chain.from_iterable(choices[i] for i in sequence))

Note: this fits pretty close to Nobody's suggestion - here chain.from_iterable is flatten and the generator expression is making the sample.


OnKeyPress event do same stuff on multiple inputs I have 3 inputs, when I press enter on them same operations happend.

This is my code

How would you do this better?

    var __this = this;
    var a = document.getElementById("content").getElementsByTagName("more-content")[0].getElementsByTagName("input")[0];
    var b = document.getElementById("content").getElementsByTagName("more-content")[1].getElementsByTagName("input")[0];
    var b = document.getElementById("content").getElementsByTagName("more-content")[2].getElementsByTagName("input")[0];

    a.onkeypress = function (e) {
        if (e.which == 13) {
            // do same stuff here
        }
    };
    b.onkeypress = function (e) {
        if (e.which == 13) {
            // do same stuff here
        }
    };
    b.onkeypress = function (e) {
        if (e.which == 13) {
            // do same stuff here
        }
    };

There are in fact different ways to handle your request. Here's one I'd like to show as it is:

re-useable (As you can use it on any element everywhere on the page multiple times.)
decoupled from the HTML markup and from CSS classes

HTML

Add a data-*-attribute to the fields where the magic should happen:

<input type="text" name="a" value="a" data-log>

JavaScript

query those specific elements
attach the event handler to each element
run your code

The ES5 way:

var logMe = document.querySelectorAll('[data-log]');

for (var i = 0, max = logMe.length; i < max; i++) {
    logMe[i].addEventListener('keypress', function(event) {
        if (13 == event.which) {
            console.log(this.value);
        }
    }, false);
}

Try it online.

The ES6 way:

let logMe = document.querySelectorAll('[data-log]');

for (let item of logMe) {
    item.addEventListener('keypress', function(event) {
        if (13 == event.which) {
            console.log(this.value);
        }
    }, false);
}

Try it online.

That being said, you can alternatively use CSS classes as well to identify the elements:

<input class="log" type="text" name="a" value="a">

And select the elements using:

let logMe = document.getElementsByClassName('log');


Processing files with streams I'm using io.js. I have to read some files as a stream, process them with the same function, and join them back into a stream. I decided to create my own solution with Transform streams and PassThrough streams.

  "use strict";

  let fs=require('fs');
  let filePaths=['./tmp/h.txt','./tmp/s.txt'];

  let Stream = require('stream');

   class StreamProcessor  {

        constructor() {
       this.process_streams = [];
       }

       push (source_stream) {
         // Create a new Transform Stream
        let transform = new StreamTransformer();
        // Register the finish event and then pipe
        transform.processed = transform.wait.call(transform);
          source_stream.pipe(transform);
       // push the stream to the internal array
         this.process_streams.push(transform);
     }

       done (callback) {
         let streams = this.process_streams;
       // Wait for all Transform streams to finish processing
        Promise.all(
          streams.map(function(s) {return s.processed;}  )
         )
       .then ( function() {
          let combined_stream=new Stream.PassThrough();
         streams.forEach(function (stream) {
         stream.pipe(combined_stream);
       });

       // Call the callback with stream or error
       callback(null,combined_stream);
       })
        .catch(function (err) {
        callback(err);
        });
       }

     }

      class StreamTransformer extends Stream.Transform {

          constructor () {
           // call super
          super();
         }

         _transform(chunk,enc, transformed) {
           // process files here
              let data=chunk.toString();
               data=data.substring(0,data.length-2);
                this.push(data);
                 transformed();
        }

          _flush(flushed) {
              // for additonal at end
              this.push('\n');
               flushed();
             }

       wait() {
          // returns a promise that resolves, when all the data is processed;
           let stream = this;

           return new Promise(function(resolve,reject)  {
       stream.on('finish', function() {
       resolve(true); });
      stream.on('error', function(err) {
         reject(err);  });
      });

       }

    }

         ///  Now simply

          let process_stream = new StreamProcessor();

           filePaths.forEach(function (fpath) {
               let fstream = fs.createReadStream(fpath);
             process_stream.push(fstream);
         });

           process_stream.done(  function
          (err,combined_stream) {
          // Consume the combines stream
          combined_stream.pipe(process.stdout);
           });

Test files contain 'hello' and 'stream, and outputs:

     hell
     stream

I think this can be improved further.

From a good look;

Your indentation is horrendous, it makes your question hard to review. Please do consistent indentation. Use http://jsbeautifier.org/ if you cannot be bothered.
It would have been good to document what the transformer does (drop 2 last characters)
I would have one-lined this:

 _transform(chunk,enc, transformed) {
   // process files here
      let data=chunk.toString();
       data=data.substring(0,data.length-2);
        this.push(data);
         transformed();
}

to

// process files here
transform(chunk, enc, transformed) {
  this.push(chunk.toString().slice(0,-2));
  transformed();
}

If you are going to call in a handler 1 function with a signature that matches the handler like here:

stream.on('error', function(err) {
  reject(err);  });
});

then consider to use that function as the callback:

stream.on('error', reject);

When I take a step back, then I don't like the approach. You have this modern approach of the stream which is flowing data. Then you take the stream, plug it until your entire transformation is done, and then let it flow again. I might actually try to rewrite this.

I rewrote all that code, this is how streams should work: pipe your file into your transform and pipe that to stdout.

"use strict";

let fs=require('fs'),
    filePaths=['./smurf.js','./gargamel.js'];

const Stream = require('stream');
const Transform = require('stream').Transform;

const myTransform = new Transform({
  transform(chunk, encoding, callback) {
    callback( null, chunk.toString().slice(0,-2) );
  }
});

filePaths.forEach(function (path) {
  fs.createReadStream(path).pipe( myTransform ).pipe( process.stdout );
});