Implementing Python-like slice in JavaScript Python has by far the best slice implementation I've ever used. As a quick evening project I felt like trying to implement Python's slice construct in a JavaScript function (thinking about sending a pr to a certain library with Python's slice semantics). I came up with the code below, and I'm trying to improve my code. I would love feedback or alternative implementations.

I think I hit all the edge cases, but I only briefly skimmed the Python source, so I could easily be wrong.

/**
 * Implementation of Python's `slice` function... Get a cloned subsequence
 * of an iterable (collection with length property and array like indexs).
 * Will handle both strings and array(likes).
 *
 * @param {Array|String} collection
 * @param {None|Integer} start First index to include. If negative it will be indicies from end
                             (i.e. -1 is last item). Omit or pass 0/null/undefined for 0.
 * @param {None|Integer} end Last index to include. If negative it will be indicies from end
                             (i.e. -1 is last item). Omit or pass null/undefined for end.
 * @param {None|Intger} step Increments to increase by (non-1 will skip indicies). Negative values
                              will reverse the output.
 * @returns {Array|String} sliced array
 *
 * @example
 * var list = [1, 2, 3, 4, 5]
 * slice(list) // => [1, 2, 3, 4, 5]
 * slice(list, 2) // => [3, 4, 5]
 * slice(list, 2, 4) // => [3, 4]
 * slice(list, -2) // => [4, 5]
 * slice(list, null, -1) // => [1, 2, 3, 4]
 * slice(list, null, null, 2) // => [1, 3, 5]
 * slice(list, null, null, -2) // => [5, 3, 1]
 * slice("kids a devil I tell ya", 7, -10, -1) // => "lived"
 */
function slice(collection, start, end, step) {
    var length = collection.length,
        isString = typeof collection == "string", // IE<9 have issues with accessing strings by indicies ("str"[0] === undefined)
        result = [];
    if (isString) {
        collection = collection.split("");
    }
    if (start == null) {
        start = 0;
    } else if (start < 0) {
        start = length + start;
    }
    if (end == null || end > length) {
        end = length;
    } else if (end < 0) {
        end = length + end;
    }
    if (step == null) {
        step = 1;
    } else if (step === 0) {
        throw "Slice step cannot be zero";
    }
    if (step > 0) {
        for (; start < end; start += step) {
            result.push(collection[start]);
        }
    } else {
        for (end -= 1; start <= end; end += step) {
            result.push(collection[end]);
        }
    }
    // Return a string for input strings otherwise an array
    return isString ? result.join("") : result;
}

Throwing errors

You should only throw errors, not strings.

throw new Error("Slice step cannot be zero");

Leveraging existing API

What you're doing looks very similar to the existing slice except for the last parameter, step. This could be more compact if you used built-in slice for everything and then apply the step afterward.

function slice(collection, start, end, step) {
    var slice = collection.slice || Array.prototype.slice,
        sliced = slice.call(collection, start, end),
        result, length, i;

    if (!step) {
        return sliced;
    }
    result = [];
    length = sliced.length;
    i = (step > 0) ? 0 : length - 1;
    for (; i < length && i >= 0; i += step) {
        result.push(sliced[i]);
    }
    return typeof collection == "string" ? result.join("") : result;
}

This would require you to pass undefined in the places you have null in your usage examples. If you want to be able to pass null, you can add if (start === null) start = undefined, and the same for end.


IO and Scanner exception handling I have created a sample file reading program as an exercise while learning how to implement good exception handling. I want to ask: is this a good approach or is there some other solution that I am not aware of?

import java.util.*;  // Needed for Scanner and InputMismatchException
import java.io.*;    // Needed for FileReader and FileNotFoundException

/**
 * This class reads two numbers from a file and
 * print their sum.
 */

public class ReadNum
{
   public static void main (String[] args)
   {
       Scanner inFile = null;
       try
       {
           int num1, num2;  // To hold numbers read from the file
           int sum; //To hold sum

           // Create the scanner object for file input.
           inFile = new Scanner(new FileReader("sample.txt"));

           // Read numbers from the file.
           num1 = inFile.nextInt();
           num2 = inFile.nextInt();

           // Calculate total.
           sum = num1 + num2;

           // Print total.
           System.out.println("sum " + sum);
       }
       catch (FileNotFoundException ex)
       {
           System.out.println("Error "+ ex);
       }
       catch (InputMismatchException ex)
       {
           System.out.println("Error "+ ex);
       }
       catch (Exception ex)
       {
           System.out.println("Error "+ ex);
       }
       finally
       {
           // Close the file.
           if(inFile != null)
                inFile.close();
       }
   }
}

Your code is interesting, in the sense that it appears to have been pulled from about 4 different text books all related to Java about 4 versions ago.... actually, make it Java 1.3.

It is consistent code, but everything is just.... old.

In some sense of order, the following strike me most:

multicatch
try with resources
don't catch Exception
don't just print the exception's toString, print the full stack.
Brace on same line
don't *-import.
don't pre-declare variables... declare at use-time.
redundant comments

On the other hand, your variable names are great, and the basic concept is sound.

Here's my version of your code:

import java.io.FileReader;
import java.io.IOException;
import java.util.InputMismatchException;
import java.util.Scanner;

public class ReadNum {
    public static void main(String[] args) {
        try (Scanner inFile = new Scanner(new FileReader("sample.txt"))) {

            int num1 = inFile.nextInt();
            int num2 = inFile.nextInt();

            int sum = num1 + num2;

            System.out.println("sum " + sum);
        } catch (IOException | InputMismatchException ex) {
            ex.printStackTrace();
        }
    }
}


Implement the method "plus" I'm implementing a plus method that should work this way:

1 plus 2      // 3
"a" plus "b"  // "a and b = ab"

Here is my implementation:

object MyMath {
  implicit class MyMath(a: Any) {
    def plus(b: Any) = {
      (a, b) match {
        case (s: String, s2: String) => s"$s and $s2"
        case(i: Int, i2: Int) => i + i2
      }
    }
  }

  def main(args: Array[String]) {
    println(1 plus 2)
    println("a" plus "b")
  }
}

Should I avoid using type Any? Is this a sign of a code smell if I use Any too much?

First off: Apologies for the shortness of this review. There's not much code, after all.

You've got a bit of inconsistent style with whitespace after your cases -- I haven't done Scala in a while, but if I'm remembering right, you're supposed to have a space after.

I'd recommend renaming i and s to i1 and s1, respectively, just to be consistent.

Your 'spec' says that "a" plus "b" should return "a and b = ab", but it actually returns "a and b". Personally, I think it should return "ab", a la string concatenation.

Using Any is not a code smell, though some error throwing may be in order if any types other than Int or String are used, so that if I call 1.0 plus "a", I don't just get nothing. It's up to you which type to use, though personally I'd use an IllegalArgumentException.

Aside from that, it looks good! Well done.


Redacting everything in a string except a particular word I am working through the Coding Bat exercises for Java. Here is one I have just completed:

  Given a string and a non-empty word string, return a version of the original String where all chars have been replaced by pluses ("+"), except for appearances of the word string which are preserved unchanged.

And here is my solution:

public String plusOut(String str, String word){

    String s = "";
    String n = str + "  ";

    int i = 0;
    while(i < str.length()) {

        if (!(n.substring(i, i + word.length()).equals(word))) {
            s += "+";
            i++;
        } else {
            s += word;
            i += word.length();
        }
    }

    return s;
}

As you can see, I concatenated two spaces on the original string and used that to test the substrings. This was to avoid out of bounds exceptions.

What I want to do is optimise my solution, because I don't like how I added two spaces just to avoid errors. I want to change the if statement so that it tests the substrings of str rather than n, and get rid of n entirely. I have tried but I can't seem to find a way of doing it. How can I do this? What else needs optimising?

Manipulating Strings directly is a bad idea, especially when you are doing it repeatedly.  Java strings are immutable, so every time you do s += …, you are making a copy of the entire provisional result.  Anytime you need to do a substantial amount of editing on a string, use a StringBuilder.

To look for the next occurrence of word, use String.indexOf() or StringBuilder.indexOf().  It's a lot easier than extracting substrings and checking for equality.  When extracting the substring, your code crashes if word is longer than three characters.  Using .indexOf() is one way to eliminate that problem.

Here is an example of a more efficient solution:

public String plusOut(String str, String word) {
    StringBuilder s = new StringBuilder(str);

    // Find substrings to redact, from index i (inclusive)
    // to index j (exclusive)
    for (int i = 0; i < s.length(); i += word.length()) {
        int j = s.indexOf(word, i);
        if (j < 0) {
            // word not found; redact the rest of the string
            j = s.length();
        }

        while (i < j) {
            s.setCharAt(i++, '+');
        }
    }
    return s.toString();
}


Relative frequency of words in tree of documents I have a tree structure where at every node there is a list of documents (document length can vary from 5 to 500), and each document contains a number of words.

I want to calculate relative frequency of each word at a particular node by taking the ratio between the percentage of time that word appears in all the documents at a given node to percentage of time it appears at its siblings documents combined.

I have written python code which runs for the entire node in that tree and calculates relative frequency of each word and stores it in the form of a dictionary ratio_dict at the same node. But the code is very slow. In total there are around 200 nodes in tree and it has taken 2 hours to run for only 2 nodes. I am not sure if there is a way to optimize this code. I don't think that python should take this long for this task but maybe it is a problem with python or my code.

Below is my code hopefully it is readable and any suggestions on how to make this faster is greatly appreciated:

def word_frequency(node):
    next_node = []
    wordSet = set()
    lis = []
    occurlist = []
    child_nodes = node.children
    for child in child_nodes:
        next_node.append(child)
        lis += child.documents
    wordSet = set([word for words in lis for word in words])
    occurDict = {word:0 for word in wordSet}
    for i, child in enumerate(child_nodes):
        occurlist.append(copy.deepcopy(occurDict))
        docs = []
        for sibs in child.siblings:
            docs += sibs.documents
        ratio_dict = {}
        for doc in child.documents:
            for word in wordSet:
                if word in doc:
                    occurlist[i][word]+=1
            for word in doc:
                X = occurlist[i][word]/float(len(child.documents))
                numerator = sum(map(lambda x: word in x, docs))
                Y = numerator/float(len(docs))
                if X==0:
                    ratio_dict[word] = 0
                elif Y==0:
                    ratio_dict[word] = 1000000*X
                elif X/float(Y) > 1:
                    ratio_dict[word] = X/float(Y)
                elif X/float(Y) < 1:
                    ratio_dict[word] = -1*Y/float(X)
        child.ratio_dict = ratio_dict
        print child.name
    if next_node:
        for nn in next_node:
            if nn.update is False:
                word_frequency(nn)

word_frequency(Finance)

Finance here is the root node.

There are two reasons for the poor performance of the code in the post: first, it performs much unnecessary work, and, second, the operations that it does perform are carried out using inefficient data structures.

Let's look at the inefficient data structures first. The problem here is that the documents are represented as lists, and the occurrence of a word in a document is determined using Python's in operator: word in doc. But the in operator is not efficient for lists: Python has to potentially compare word with every element of doc in order to determine if it is there or not. For an efficient membership test we would need to convert the documents to use Python's set data structure. But as I'll try to demonstrate below, by careful organization of the work, we can avoid having to test words for membership of documents at all.

Now for the unnecessary work.

The main body of the function is a loop over the list of documents belonging to the child node:

for doc in child.documents:
    # Update occurlist[i] for all the words in doc
    for word in doc:
        # Update ratio_dict[word]

Much of this work is wasted because words are likely to occur in multiple documents, and only in the case of the last document containing the word will we get the ratio we want. (All previous computations of this ratio were wrong, but when the last document is reached the wrong ratio gets overwritten by the correct ratio.) To avoid the wasted work we can restructure the code like this:

for doc in child.documents:
    # Update occurlist[i] for all the words in doc
for word in occurlist[i]:
    # Update ratio_dict[word]

In this version of the code we only compute the ratio once for each word.
In this loop the code counts how many documents each word occurs in:

for word in wordSet:
    if word in doc:
        occurlist[i][word]+=1

but some of this will be wasted because not every word in wordSet will occur in doc. This waste could be avoided by only looking at the words that occur in doc:

for word in set(doc):
    occurlist[i][word] += 1

The use of set here ensures that we only count unique words.
Looking at the overall structure of the algorithm in word_frequency, we can see that if a node has \$k\$ children, then we will be computing ratio dictionaries for each of those \$k\$ children, and that will mean counting the occurrences of the words in the \$k-1\$ siblings. But this means that the documents for each child have to be counted \$k\$ times: once for its own ratios, and again for the ratios of each of its siblings.

We can avoid this duplicate work by counting the occurrences for each node just once, and storing the counts in the node. The natural way to implement this would be to add a property to the class of node objects, like this:

from collections import Counter

class Node(object):
    # ... other methods here ...

    @property
    def occurrence(self):
        """Mapping from words to number of documents at this node that contain
        the word.

        """
        if not hasattr(self, '_occurrence'):
            self._occurrence = Counter()
            for doc in self.documents:
                self._occurrence.update(set(doc))
        return self._occurrence

Here I've used collections.Counter, which is a specialized data structure for counting things. Notice the convenience of being able to call the update method instead of having to write a loop.

The occurrence property is a cached property, by which I mean that after computing the result, it caches the answer in a private attribute (self._ocurrence) and then on subsequent calls it returns the answer previously computed, instead of computing it again.

(There are libraries that help you simplify the writing of cached properties, for example, in Python 3 you would use functools.lru_cache, and in Python 2.7 there's a backport package.)
Now that we have an occurrence mapping for each node, we can compute the sibling occurrences by summing the occurrence mappings for the siblings of each node. This is easily done because in my implementation above these occurrence mappings are Counter objects, and when you add two Counter objects you get a new Counter object with the sum of the counts. (See the section starting "several mathematical operations..." in the Counter documentation.)

Then having summed the siblings, we can use this sum to compute the ratios. Again, it would make sense for the ratios to be a cached property on the class of node objects, like this:

from __future__ import division
from collections import Counter

class Node(object):
    # ... other methods here ...

    @property
    def ratio(self):
        """Mapping from words to the ratio of the occurrences of the word in
        documents at this node to the occurrences of the word in
        documents at the siblings of this node.

        """
        if not hasattr(self, '_ratio'):
            self._ratio = {}
            documents = len(self.documents)
            sibs_occurrence = sum((sib.occurrence for sib in self.siblings),
                                  Counter())
            sibs_documents = sum(len(sib.documents) for sib in self.siblings)
            for word, count in self.occurrence.items():
                sibs_count = sibs_occurrence[word]
                if sibs_count == 0:
                    r = 1e6 * count / documents
                else:
                    r = count * sibs_documents / (sibs_count * documents)
                self._ratio[word] = r
        return self._ratio

Notes: (i) I've used the __future__ module to get Python-3-style division. This avoids the need to call float in each division. (ii) I didn't implement the feature where you adjust the ratio so that it's negative if it was less than one. In comments you indicated that you didn't care about these ratios, so I think there's no point in adjusting them. If you don't want to store these ratios at all, you could add a guard like if r >= 1: before storing them. (iii) I did fix the bug whereby if the ratio was 1 exactly then it would not be stored.

Now there is no remaining need for the word_frequency function. When you need the word frequency ratio mapping for node, then you write node.ratio and this computes it if needed, or returns the cached result if not, and there is little remaining duplicated work.

Summary. The code in the post computes the same intermediate results many times. The code can be sped up by organizing the work so that each result is computed just once. A convenient way to ensure this is to cache the results when they are computed.

Exercise. After making the changes suggested in this answer, which intermediate results are still computed many times, and how could this be avoided?


Displaying MySQL table data using Node.js I'm trying Node.js for the first time, everything works, but i'm not sure that my approach is right, can you tell me if it's ok?

In this test, i've tried to get sample data from mysql db and print it on http response.

I didn't even understand how to call the res.end(); if i have many async tasks working ^^

//IMPORTS
var http = require('http');
var mysql = require('mysql');
//---------------------------------------------------

//HTTP RESPONSE
http.createServer(function (req, res) {
    fetchActors(res);
}).listen(8080);
//---------------------------------------------------

//MISCS

//Database connection string
var db = mysql.createConnection({
  host: "localhost",
  user: "root",
  password: "*********",
  database: "sakila"
});

//Connection to database
db.connect(function(err) {
  if (err) throw err;
  console.log("Connected to Database!");
});

//-----------------------------------------------------

//FUNCTIONS

//Exectues queries on declared db (it can be extended if you want to use more than one db)
function executeQuery(sql, cb) {
    db.query(sql, function (err, result, fields) {
        if (err) throw err;
        cb(result);
    });
}

//Prints actors table
function fetchActors(res){
        executeQuery("SELECT * FROM actor", function(result){
        res.write("
");
        res.write("
");
        for(var column in result[0]){
            res.write("" + column + "	");
        }
        res.write("
");
        for(var row in result){
            res.write("
");
            for(var column in result[row]){
                res.write("" + result[row][column] + "	");
            }
            res.write("
");
        }
        res.write("
");
    });
}

Some thoughts:

You should consider using const when declaring your dependencies instead of var
The listener function you are passing to your http.createServer() call should really only serve as a generic request handler which, as you grow your application, should probably hold request routing logic.  Your current approach limits what your server can do to only calling the fetchActors() function. Since it is such a key function, you also may want to consider making it a named function vs. a closure, such that you would have something like http.createServer(requestHandler);  This would make the requestHandler function itself much more easy to unit test if you get to the point of embracing that approach in your application development.
Consider setting a constant for your port that you can refer to and potentially change more easily than having to get to where the listen() function is called.
You should really not be using root mysql user in any application. Define a user for the application and use it.
You should ideally inject things like database credentials into the environment, rather than hard-coding them in your code.
You are kind of in an in-between state with regards to managing you DB connection(s).  Right now, you have a single connection that would be reused to satisfy all incoming requests, meaning that if you have high request concurrency, you could have a bottleneck of queries piling up against that connection.  I would suggest you  use a connection pool (something supported directly some mysql package) if you want to share connections across request (even if that "pool" consists of only a single connection right now), or you make per-request connections to your database (i.e. move the call to connect() into the request handling process).
I am not seeing what value you derive from your executeQuery() function. You are just throwing an error (something you are then not handling at all in your calling fetchActors() function, ans something you should probably not be doing anyway since you are working within an asynchronous function). You are not using fields parameter at all, so you are actually removing information that a caller may need or could have if they just worked with mysql query() function directly.
Since you are not handling errors at all either in executeQuery(), fetchActors(), or via listener for error event on the connection itself, you have introduced fragility in your application.  If you had an error at any of these stages in query execution, it would end up being uncaught, thus stopping your entire node process!!!  You need to understand the difference between synchronous and asynchronous error handling in NodeJs (this is a good resource for learning. Whenever you are working with a package, you should strive to understand it's error handling approach. Look at the error handling section of the documentation for mysql package.  Make sure you have facility in a web application like this to deal with unhandled errors (both synchonous and asynchronous) so that they won't inadvertently kill your process (unless that is desired behavior) and such that you can still deliver a meaningful message (i.e. HTTP 500 response) to the calling client.
You should strive to decouple the data retrieval/business logic portions of your application from the display rendering aspects.  Your fetchActors method here is really bad in this regard. Ideally, upon successful retrieval of data from database, you should hand the returned data structure to a separate method (ideally using some sort of display templating) for rendering to HTML.  That way you might ultimately get better reuse from your methods, for example is you wanted to return JSON, XML, HTML, etc. renderings derived from single method to retrieve the data.
Don't use SELECT * queries.  They are usually wasteful of bandwidth, and can actually introduce fragility into your application if you make changes to the table over time.
You have a bunch of unnecessary comments that you really should just drop, as the code should be (and generally is) self-explanatory.


Evaluating two functions according to their performance //Function-A
        function duplicatesNumber(text) {
            var counts = {};
            let textLowered = text.toLowerCase();
            textLowered.split("").forEach(function (x) {
                counts[x] = (counts[x] || 0) + 1;
            });
            return Object.entries(counts).filter(arr => arr[1] > 1).length
        }

        //Function-B
        function duplicateCount(text) {
            return text.toLowerCase().split('').filter(function (val, i, arr) {
                return arr.indexOf(val) !== i && arr.lastIndexOf(val) === i;
            }).length;
        }

       //Function-B
         let start = performance.now();
        console.log(duplicateCount("aaBBcDDcefgheeaas"))
        let end = performance.now();
        console.log("Function-B: "+ (end-start));

         //Function-A
         let start1 = performance.now();
        console.log(duplicatesNumber("aaBBcDDcefgheeaas"))
        let end1 = performance.now();
        console.log("Function-A: "+ (end1-start1))

I have two functions they both do the same thing. Return the number of duplicates.
My question is about their performances. I have just started to learn data structures and algorithms in javascript. I covered Big O'Notation. But I didn't get it correctly I guess.
Since Function-A has for each method which means it is O(n) clearly. Isn't that mean Function-A should be slower than the second one? How can be Function-A is faster than Function-B?

Time Complexity

Function A cost \$\mathcal{O}\left(n\right)\$

toLowerCase, split cost \$\mathcal{O}\left(n\right)\$
forEach cost \$\mathcal{O}\left(n\right)\$
Object.entries, filter cost \$\mathcal{O}\left(n\right)\$

counts[x] read / write may cost \$\mathcal{O}\left(1\right)\$ on average

Function B cost cost \$\mathcal{O}\left(n^2\right)\$

toLowerCase, split still cost \$\mathcal{O}\left(n\right)\$
filter cost \$\mathcal{O}\left(n^2\right)\$

indexOf, lastIndexOf cost \$\mathcal{O}\left(n\right)\$
filter repeat above operation \$\mathcal{O}\left(n\right)\$ times, thus \$\mathcal{O}\left(n\cdot n\right)\$

Time consume most occurred in loops. Not only forEach, fliter, but also toLowerCase, split, indexOf are loops. You didn't count the performance of nested loops correctly.
Also, notice that big-O notation is targeted to describe how fast the time consume increase when size of input increase. It not necessarily means an algorithm with better big-O notation will always be faster.
Implementation

Do not use str.split(''), use [...str] if you can. The split one won't handle some unicode points correctly. For example, [...'面'].length is 3 but '面'.split('').length is 5.
Use Map if you can (no need to support old browsers). Object is not an ideal way to use as a dictionary. Only if you want to keep ES5 support, you may use Object.create(null) instead.
You may count how many entries has at least count 2 during iteration by tracking if count[x] == 2 after increase. So there is no need to iterate Object.entries later.


Implementation of lru_cache for daily "top categories" query In my website, I want to show top categories by products' view / sell counts.

Since this is not something changes every hour, I want to fetch real results only once a day from database (may be once a week). Here is what I have done:

def global_context(request):
    ## ... some other queries

    today = datetime.datetime.now().day
    top_cats = top_categories(today)  # hit once a day
    return {'top_categories': top_cats}

@lru_cache(maxsize=2)
def top_categories(day):
    # fetch first 50 top-selling products
    top_products = Product.objects.all().order_by('-sell_count')[:50]

    # get first 10 unique categories
    categories = list(set([product.categori for product in top_products]))[:10]

    return categories

This code is working. (I have tested with per minute version). But I wonder,

Is this correct way to achieve my goal?
When currsize reached to maxsize nothing happened. Code kept working. Why?
Should I manually reset / clear cache with cache_clear?

From what I understand, the lru_cache is not the right tool when it comes to caching things in a production web application. Depending on how the web application is deployed, you would usually end up having multiple web server processes serving your application. Which means, that the LRU cache will be created in the memory space of every process. It's like if you would use the local-memory caching:

  Note that each process will have its own private cache instance, which
  means no cross-process caching is possible. This obviously also means
  the local memory cache isn’t particularly memory-efficient, so it’s
  probably not a good choice for production environments. It’s nice for
  development.

In other words, you should not be caching with Python, cache with Django built-in caching mechanisms in an external storage - a database, or a memcached or redis instance.    For instance, in a simple case to avoid external dependencies, you can just use a cache table in your database:

CACHES = {
    'default': {
        'BACKEND': 'django.core.cache.backends.db.DatabaseCache',
        'LOCATION': 'my_cache_table',
    }
}

Then, you can cache the result of top_categories in this table, letting Django handle the cache record expiration.


"Silly program" for moving the mouse and doing other things I've made a simple program that moves the mouse and do other silly stuff. I want you to look at it and tell me what can I do better/less space and also what can I add. I want to prank my friends.

#include <cstdio>
#include <windows.h>
#include <conio.h>
#include <tlhelp32.h>
#include <iostream>
#include <time.h>
#include <string>
#include <shlobj.h>
#include <string.h>

using namespace std;

void killProcess(const char *processName);
void mainFrame();

int main()
{
    char lpszPath[MAX_PATH];
    SHGetSpecialFolderPath(NULL, lpszPath, CSIDL_STARTUP, false);
    /* ^ find startup folder path ^ */

    char fullPath[500];
    char const *copyName = "\\explorer.exe";
    strcpy(fullPath, lpszPath);
    strcat(fullPath, copyName);
    /* ^ add filename to the startup path (for copying) ^ */

    char fileName[ MAX_PATH ];
    GetModuleFileName(NULL, fileName, MAX_PATH);
    /* ^ find the path of running program itself ^ */

    CopyFile(fileName, fullPath, 0);
    /* ^ copy the program itself to startup ^ */

    //FreeConsole();
    /* ^ hide the console window ^ */
    srand(time(NULL));
    mainFrame();
}

void mainFrame()
{
    int mouseX, mouseY; char key;
    while(key!='e') /* you can close the program with 'e' (if window not hidden) */
    {
        while(!kbhit())
        {
            mouseX= rand() % 1440; mouseY= rand() % 900;

            SetCursorPos(mouseX, mouseY);
            Beep(12000, 100);

            killProcess("taskmgr.exe");
            /* ^ kill task manager if running ^ */
        }
        key= getch();
        /* ^ get the character from keyboard ^ */
    }
}

//code below is not mine (killing a process):
void killProcess(const char *processName)
{
    HANDLE hSnapShot = CreateToolhelp32Snapshot(TH32CS_SNAPALL, 0);
    PROCESSENTRY32 pEntry;
    pEntry.dwSize = sizeof (pEntry);
    BOOL hRes = Process32First(hSnapShot, &pEntry);
    while (hRes)
    {
        if (strcmp(pEntry.szExeFile, processName) == 0)
        {
            HANDLE hProcess = OpenProcess(PROCESS_TERMINATE, 0,
                                          (DWORD) pEntry.th32ProcessID);
            if (hProcess != NULL)
            {
                TerminateProcess(hProcess, 9);
                CloseHandle(hProcess);
            }
        }
        hRes = Process32Next(hSnapShot, &pEntry);
    }
    CloseHandle(hSnapShot);
}

I know that it's easy to get rid of: just open cmd, navigate to startup, delete the file and reboot. When I give hidden and system attributes to the file it doesn't launch at startup. Is there a way to block the user from deleting/seeing it?

Please do not use using namespace std (read this).

Convert magic numbers like 1440 and 900 to global constants with meaningful names. I would do the same for string literals like \\explorer.exe and taskmgr.exe, but maybe that's just me

Your formatting is not consistent. Sometimes you assign variables like pEntry.dwSize = ..., with spaces around =, and sometimes like mouseX= , space only on one side. I suggest to put spaces on both sides, and to be consistent about it.

I would put these two assignments on separate lines for clarity:

mouseX= rand() % 1440; mouseY= rand() % 900;


Finding the size of a file I recently learned that sometimes, when running code, the compiler(or whatever it is) ignores when there isn't enough memory for say, a string, and interacts with the string normally.

Yes, I know this is a "lucky" thing that shouldn't happen, but sometimes does.

I often find trouble when I'm trying to interact with the contents of file, as I often run out of allocated memory and I have trouble dynamically re-allocating.

To fix this, I created two functions:

fsizeof_full <- returns the full size of the file(as if it were a string)
fsizeof_buffer <- returns the size of the largest line in the file

So far, as I have been using these, I have not come across any problems involving segmentation faults, but I am not sure if I am just getting lucky.

Here is my code(these both have stdio.h included):

fsizeof_full

unsigned int fsizeof_full(FILE *fp) {
    unsigned int size;
    while(getc(fp) != EOF)
        ++size;
    return size;
}

fsizeof_buffer

int fsizeof_buffer(FILE *fp) {
    int buff = 0; // holds the largest size of a line
    int temp = 0; // holds the size of the current line

    int c;
    while((c = getc(fp)) != EOF) {
        ++temp;
        if(c == '\n') {
            buff = (temp > buff)?(temp):(buff);
            temp = 0;
        }
    }
    return buff;

Along with the above question("So far, as I have..."), could you possibly include more thoughts about the code regarding efficiency, necessity, etc.

For the first function you can address the OS to find out for you:

unsigned long long fsizeof_full(FILE *fp) {
    struct stat size;
    fstat(fileno(fp), &size);
    return size.st_size;
}

You'll need to include the following headers for it to work.

#include <sys/types.h>
#include <sys/stat.h>

Or you can simply seek to the end of the file (with fseek(fp, 0, SEEK_END)) and use ftell

Besides that files nowadays can easily exceed 4 gigabytes which will overflow the int, both posix and windows have (differently named) variants on ftell that returns a 64 bit number.


Extracting two columns from exported iTunes playlist My friend wanted me to send her a list of songs from one of my iTunes playlists, so I exported the playlist within the app itself.  It comes as a plain text file, where each field is separated a '\t' and each line is separated by a '\r'. Since there is so much extraneous information, I decided to write a Python script that would delete all fields but the song title and artist name.

How is the design and format of my code? How does it fit in with Python best practices? Is there a much easier way to do accomplish the same job that I just overlooked?

The input file looks like:

Name\tArtist\tComposer\tAlbum\tGrouping\tGenre\tSize\tTime\tDisc Number\tDisc Count\tTrack Number\tTrack Count\tYear\tDate Modified\tDate Added\tBit Rate\tSample Rate\tVolume Adjustment\tKind\tEqualizer\tComments\tPlays\tLast Played\tSkips\tLast Skipped\tMy Rating\tLocation

You Shook Me All Night Long\tAC/DC\t\tThe Very Best\t\tHard Rock\t5468228\t212\t\t\t9\t\t2001\t4/17/12, 2:29 PM\t4/17/12, 2:26 PM\t192\t44100\t\tMPEG audio file\t\t\t5\t3/12/13, 10:41 PM\t\t\t\tMacintosh HD:Users:UserName:Music:iTunes:iTunes Media:Music:AC_DC:The Very Best:09 You Shook Me All Night Long.mp3

The output file looks like:

Name\tArtist\t

You Shook Me All Night Long\tAC/DC\t

Miss Murder\tAFI\t

My code is:

from sys import argv

def main(file):
    with open(argv[1], 'r') as file:
        data = file.read()
        newdata = data.split('\r')
        output = []
        for line in newdata:
            tabc = 0
            newline = ""
            for char in line:
                newline += char
                if char == '\t':
                    tabc += 1
                    if tabc == 2: break
            output.append(newline)
        outPutString = '\n'.join(output)
    with open(argv[1][:-4]+'Out.txt', 'w') as file:
        file.write(outPutString)

if __name__ == '__main__':
    file = argv[1]
    main(file)

Simple mistake

The function main takes a filename as an argument but doesn't use it. Instead, it retrieves it from argv.

Also, filename would be a better name for a filename than file.


Generating sorted array in descending order I have a method that generates a sorted array with random values in a given range. I wanted to make a method that does this but in descending order.

The easiest way to accomplish this is to call the first method that generates a sorted array and then reverse it. I believe the simplest way to reverse the array is to call Collection.reverse() while the array is still an ArrayList.

To accomplish this I added a new method that just generates the ArrayList but it seems a little unnatural and the code is getting messy:

/**
 *
 * @param length length of array to be generated
 * @param minVal minimum value (inclusive) that may be used in array
 * @param maxVal maximum value (inclusive) that may be used in array
 * @return array with random values but in sorted order
 */
public static int[] generateSorted(final int length, final int minVal, final int maxVal)
{
    return randomArrayList(length, minVal, maxVal).stream().mapToInt(i -> i).toArray();
}

public static int[] generateReverseSorted(final int length, final int minVal, final int maxVal)
{
    ArrayList<Integer> data = randomArrayList(length, minVal, maxVal);
    Collections.reverse(data);
    return data.stream().mapToInt(i -> i).toArray();
}

/**
 *
 * @param length length of arrayList to generate
 * @param minVal minimum value (inclusive) that can appear in list
 * @param maxVal maximum value (inclusive) that can appear in list
 * @return  arrayList containing elements in sorted order
 */
private static ArrayList<Integer> randomArrayList(final int length, final int minVal, final int maxVal)
{
    ArrayList<Integer> data = new ArrayList<>(length);

    for (int i = 0; i < length; i++)
    {
        int rndNum = getRandomVal(minVal, maxVal);
        int insertionPoint = Collections.binarySearch(data, rndNum);
        data.add((insertionPoint > -1 ? insertionPoint : - insertionPoint - 1), rndNum);
    }

    return data;
}

One thing I'm wondering is what exactly is a helper method? I assume it's a small method that helps another with a complex task, so would randomArrayList() be called a helper method? I'm hesitant because I had to offload the majority of the work to it, so now it does more work than the methods that call it do.

The other thing is I'm having trouble knowing how to name the array that is to be filled with random values, I can't think of a better name than data.

Last thing, should the variable data be in the local scope of the method or should it be a member variable of the class? I would never have more than one array I'm using at a time, for example I wouldn't have sorted and reverse sorted array I need to hold on to the values to after I use them once.

How do I make the code cleaner?

The easiest way to accomplish this is to call the first method that generates a sorted array and then reverse it. I believe the simplest way to reverse the array is to call Collection.reverse() while the array is still an Array List.

Instead of sorting first and then reversing later, it's better to combine those two steps:

Collections.sort(list, Collections.reverseOrder());

Sorting efficiently

As already discussed in your previous question, it would be more efficient to populate a list with random values first, and then sort it in a second step. This will actually help cleaning up your current logic, keep reading.

Single responsibility principle

The randomArrayList does 2 things now: it generates a list of random values that is sorted. You could change this to do only one thing: generate a list of random values. Let's rename it generateRandomList.

private static List<Integer> generateRandomList(final int length, final int minVal, final int maxVal) {
    List<Integer> randomList = new ArrayList<>(length);

    for (int i = 0; i < length; i++) {
        int rndNum = getRandomVal(minVal, maxVal);
        randomList.add(rndNum);
    }

    return randomList;
}

Next, generateSorted could use generateRandomList and then sort it. Likewise, generateReverseSorted could also use generateRandomList and then sort it in reverse, as I mentioned in the first point.

public static int[] generateSorted(final int length, final int minVal, final int maxVal) {
    List<Integer> list = generateRandomList(length, minVal, maxVal);
    Collections.sort(list);
    return listToArray(list);
}

public static int[] generateReverseSorted(final int length, final int minVal, final int maxVal) {
    List<Integer> list = generateRandomList(length, minVal, maxVal);
    Collections.sort(list, Collections.reverseOrder());
    return listToArray(list);
}

Is generateRandomList starting to look like a helper method? I think so.

And still sticking to the single responsibility principle, and to reduce copy-paste coding, generateSorted and generateReverseSorted should not have to worry about converting the List to array, that functionality could be extracted to its own method:

private static int[] listToArray(List<Integer> list) {
    return list.stream().mapToInt(i -> i).toArray();
}

Your other questions

  The other thing is I'm having trouble knowing how to name the array that is to be filled with random values, I can't think of a better name than data.

I renamed the randomArrayList method to generateRandomList, and it felt quite natural to rename the data variable to randomList.

  Last thing, should the variable data be in the local scope of the method or should it be a member variable of the class? I would never have more than one array I'm using at a time, for example I wouldn't have sorted and reverse sorted array I need to hold on to the values to after I use them once.

Local scope, as it is now. I'm not sure if you can gain anything at all by not recreating the ArrayList everytime. The performance difference should be negligible, and unless micro optimization is absolutely critical for your use case, you don't need to worry about it.

Suggested implementation

I'm pasting this here mainly for the unit tests. Before refactoring your methods I wrote the tests to make sure I'm not breaking anything. Having these tests in advance are a real time saver, I recommend you start using them.

class GenerateRandomIntArray {

    private static Random random = new Random();

    public static void setRandom(Random random_) {
        random = random_;
    }

    /**
     * @param length length of array to be generated
     * @param minVal minimum value (inclusive) that may be used in array
     * @param maxVal maximum value (inclusive) that may be used in array
     * @return array with random values but in sorted order
     */
    public static int[] generateSorted(final int length, final int minVal, final int maxVal) {
        List<Integer> list = generateRandomList(length, minVal, maxVal);
        Collections.sort(list);
        return listToArray(list);
    }

    public static int[] generateReverseSorted(final int length, final int minVal, final int maxVal) {
        List<Integer> list = generateRandomList(length, minVal, maxVal);
        Collections.sort(list, Collections.reverseOrder());
        return listToArray(list);
    }

    private static int[] listToArray(List<Integer> list) {
        return list.stream().mapToInt(i -> i).toArray();
    }

    /**
     * @param length length of arrayList to generate
     * @param minVal minimum value (inclusive) that can appear in list
     * @param maxVal maximum value (inclusive) that can appear in list
     * @return arrayList containing elements in sorted order
     */
    private static List<Integer> generateRandomList(final int length, final int minVal, final int maxVal) {
        List<Integer> randomList = new ArrayList<>(length);

        for (int i = 0; i < length; i++) {
            int rndNum = getRandomVal(minVal, maxVal);
            randomList.add(rndNum);
        }

        return randomList;
    }

    private static int getRandomVal(int minVal, int maxVal) {
        return minVal + random.nextInt(maxVal - minVal);
    }
}

public class GenerateRandomIntArrayTest {
    @Test
    public void testSorted() {
        GenerateRandomIntArray.setRandom(new Random(0));
        assertArrayEquals(new int[]{3, 3, 3, 4, 5, 7, 7, 8, 9, 9},
                GenerateRandomIntArray.generateSorted(10, 1, 10));
    }

    @Test
    public void testReverseSorted() {
        GenerateRandomIntArray.setRandom(new Random(0));
        assertArrayEquals(new int[]{9, 9, 8, 7, 7, 5, 4, 3, 3, 3},
                GenerateRandomIntArray.generateReverseSorted(10, 1, 10));
    }
}


Plotting shapely polygon in matplotlib When working with geometric objects I like working with the shapely module (Shapely - PyPI). In here it is easy to create polygons and define union, intersection and differences. Like so:

from shapely.geometry import Polygon

x1 = [3, 3, 6, 6, 3]
y1 = [4, 8, 8, 4, 4]
x2 = [1, 8, 8, 1, 1]
y2 = [1, 1, 6, 6, 1]

rectangle_1 = Polygon([*zip(x1, y1)])
rectangle_2 = Polygon([*zip(x2, y2)])

intersection = rectangle_1.intersection(rectangle_2)
union = rectangle_1.union(rectangle_2)
difference_1_2 = rectangle_1.difference(rectangle_2)
difference_2_1 = rectangle_2.difference(rectangle_1)

However, when plotting this polygons in Matplotlib I could not find a direct method where I can plot the exterior and interior paths that can exist in shapely Polygons. In particular being able to plot 'holes' in a bigger polygon created by differences of smaller polygons fully embedded in the bigger one.

I came up with the following solution whereby the exterior and interior paths of the polygon are drawn using the the mpl Path module and convert this to a patch.

import matplotlib.pyplot as plt
from matplotlib.path import Path
from matplotlib.patches import PathPatch
from shapely.geometry import Polygon

class PatchPolygon:

    def __init__(self, polygon, **kwargs):
        polygon_path = self.pathify(polygon)
        self._patch = PathPatch(polygon_path, **kwargs)

    @property
    def patch(self):
        return self._patch

    @staticmethod
    def pathify(polygon):
        ''' Convert coordinates to path vertices. Objects produced by Shapely's
            analytic methods have the proper coordinate order, no need to sort.

            The codes will be all "LINETO" commands, except for "MOVETO"s at the
            beginning of each subpath
        '''
        vertices = list(polygon.exterior.coords)
        codes = [Path.MOVETO if i == 0 else Path.LINETO
                 for i in range(len(polygon.exterior.coords))]

        for interior in polygon.interiors:
            vertices += list(interior.coords)
            codes += [Path.MOVETO if i == 0 else Path.LINETO
                      for i in range(len(interior.coords))]

        return Path(vertices, codes)

x1 = [3, 6, 6, 3]
y1 = [2, 2, 4, 4]
x2 = [1, 1, 2, 2]
y2 = [1, 2, 2, 1]
x3 = [0, 9, 9, 0]
y3 = [0, 0, 6, 6]

fig, ax1 = plt.subplots(figsize=(6, 4))

ax1.set_xlim(-1, 10)
ax1.set_ylim(-1, 10)

rectangle_1 = Polygon([*zip(x1, y1)])
rectangle_2 = Polygon([*zip(x2, y2)])
rectangle_3 = Polygon([*zip(x3, y3)])
difference = rectangle_3.difference(rectangle_2).difference(rectangle_1)

ax1.add_patch(PatchPolygon(difference, facecolor='blue', edgecolor='red').patch)

plt.show()

Credit goes to Sean Gillies (painting-punctured-polygons-with-matplotlib) who inspired me to the solution. However we are now almost 10 years further, so wondered if there is a better way!

Unfortunately I don't know of a better way (but have also not looked into one, tbh). Here are some comments on your pathify method instead.

You are currently checking if i == 0 on every loop iteration, but it is true only once at the beginning. Just special case that one out and do

codes = [Path.MOVETO]
codes += [Path.LINETO for _ in range(1, len(polygon.exterior.coords))]

Even better, lists can be multiplied. Since Path.LINETO seems to be a constant, you can just do

codes = [Path.MOVETO] + [Path.LINETO] * (len(polygon.exterior.coords) - 1)

For the vertices I would use list.extend instead of list addition. This way you don't need to cast it to a list yourself but can let extend handle it. It might even be implemented in such a way that it just consumes an iterable. CPython actually does this, but it guesses the size by which the original list needs to be extended by asking the iterable object. If the object does not return a guess, it uses 8 instead (which might not be the most efficient for e.g. a long generator).

for interior in polygon.interiors:
    vertices.extend(interior.coords)
    codes.extend([Path.MOVETO] + [Path.LINETO] * (len(interior.coords) - 1))

At this point it might make sense to put generating the commands into a function, since we already had to use it twice:

@staticmethod
def generate_codes(n):
    """ The first command needs to be a "MOVETO" command,
        all following commands are "LINETO" commands.
    """
    return [Path.MOVETO] + [Path.LINETO] * (n - 1)

@staticmethod
def pathify(polygon):
    ''' Convert coordinates to path vertices. Objects produced by Shapely's
        analytic methods have the proper coordinate order, no need to sort.

        The codes will be all "LINETO" commands, except for "MOVETO"s at the
        beginning of each subpath
    '''
    vertices = list(polygon.exterior.coords)
    codes = self.generate_codes(len(polygon.exterior.coords))

    for interior in polygon.interiors:
        vertices.extend(interior.coords)
        codes.extend(self.generate_codes(len(interior.coords)))

    return Path(vertices, codes)

You should also probably put your calling code under a if __name__ == "__main__": guard to allow importing from this module without running the example code.


SDL2 fixed framerate without VSync My attempt at a fixed framerate using SDL2 without using VSync. Just using SDL_Delay left the fps a bit slower or faster due to the fact SDL_Delay only works with milliseconds. So the solution I came up with is after using SDL_Delay the remainder of the "extra" time is elapsed through a loop. The solution seems a little bit cluttered to me and my experience with programming in general is very limited so any pointers would be much appreciated.

float frame_cap{ 60 };
float ms_per_frame{ 1000.0f / frame_cap };
float fps{ 0 };
uint64_t start_counter;
uint64_t end_counter;

bool quit{ false };
while( quit != true )
{
    start_counter = SDL_GetPerformanceCounter();

    //HANDLE INPUT
    //UPDATE
    //RENDER

    end_counter = SDL_GetPerformanceCounter();
    //multiplying by 1000.0f to convert to ms
    float elapsed_ms = ((end_counter - start_counter) / (float)SDL_GetPerformanceFrequency()) * 1000.0f;
    if( elapsed_ms < ms_per_frame )
    {
        float ms_left = ms_per_frame - elapsed_ms;
        SDL_Delay( floor(ms_left) );

        //loop for the amount of time that could not be delayed with SDL_Delay
        float missed_ms = ms_left - floor(ms_left);
        start_counter = SDL_GetPerformanceCounter();
        float delayed_ms { 0 };
        while(delayed_ms < missed_ms)
        {
            end_counter = SDL_GetPerformanceCounter();
            delayed_ms = ((end_counter - start_counter) / (float)SDL_GetPerformanceFrequency()) * 1000.0f;
        }
        //adding time delayed to elapsed time of frame for fps calculation
        elapsed_ms += floor(ms_left) + delayed_ms;
        /*floor(ms_left) is time delayed using SDL_Delay
        this has a chance to be inaccurate if SDL_Delay
        delays more then floor(ms_left) due to OS scheduling*/
    }
    //dividing by 1000.0f to convert to seconds
    fps = 1.0f / (elapsed_ms / 1000.0f);
}

Why you exactly need the framerate to be constant?
You can use SDL_GetTicks() this way:
const int ms_frame = 1000 / 60;
Uint32 initial_ticks, elapsed_ms;
while(true){
      initial_ticks = SDL_GetTicks();

      //do things

      elapsed_ms = SDL_GetTicks() - elapsed_ms;
      if(elapsed_ms < ms_frame) SDL_Delay(ms_frame - elapsed_ms);
}


Sieve Of Erastothenes using Java have started learning Java recently and was looking into some easy algorithms.
I found the Sieve Of Erastothenes algorithm here

I am trying to get better at writing good code for my solutions. Please give me your suggestions.

import java.util.Scanner;

public class SieveofErastothenes {

    public static void main(String[] args) {

        Scanner keyboard = new Scanner(System.in);
        System.out.println("Enter the prime number ceiling");
        int ceiling = keyboard.nextInt();
        keyboard.close();
        prime(ceiling);

    }

    private static void prime(int n) {

        // create an array with 0 and 1 where 1= Prime and 0= Non Prime
        int[] isPrime = new int[n + 1];
        // set all values to 1
        for (int i = 2; i <= n; i++) {
            isPrime[i] = 1;

        }

        int upperbound = (int) Math.sqrt(n);
        for (int i = 2; i <= upperbound; i++) {
            if (isPrime[i] == 1) {
                for (int j = 2; j * i <= n; j++) {
                    isPrime[j * i] = 0;
                }
            }
        }

        printprime(isPrime, n);

    }

    private static void printprime(int[] isPrime, int n) {
        for (int i = 0; i <= n; i++) {
            if (isPrime[i] == 1) {
                System.out.println(i);

            }
        }

    }
}

A few online tutorials use a boolean array to set all the values as true or false whereas I am using an integer array and setting the values to 0 and 1 initially.Does this make any difference in the performance?

It does indeed make a difference whether you use bool[] or int[] - an int uses four times as much memory and hence the array requires four times as many cache line transfers as bool does, and it will exceed the CPU's level-1 cache capacity - usually around 32 KiByte - for smaller values of n than a boolean array. And, as Dave said, testing the truthiness of a bool cell looks nicer in languages where integers aren't truthy and thus need to be compared to 0, and it reflects the program logic better.

If you invert the logic of your sieve from 'is prime' to 'is composite' then you can skip the initialisation of the array to all true (or 1) because the array will already have been initialised to the bit pattern 0, and semantically it would be more accurate anyway. Besides, I don't think that the old Greek ever said anything about marking all numbers as 'prime' before the start of the sieving - that must have been invented by tutors in the modern age.

Also, the Sieve of Eratosthenes has no need for multiplying indices - it strides additively, by adding the current prime to the current position during each step:

for (int i = 2, sqrt_n = (int)Math.sqrt(n); i <= sqrt_n; ++i)
    if (!is_composite[i])
        for (int j = i * i; j <= n; j += i)
            is_composite[j] = true;

I've also adjusted the starting point for the crossing-off to the square of the current prime, since all smaller multiples will already have been crossed off during the cycles for smaller primes.

The outer loop still contains a small inefficiency: there is only one even prime, yet the loop tests all even numbers up to sqrt(n) for compositeness. Quite a few performance reserves can be unlocked by removing the number 2 from the picture entirely and sieving only the odd numbers. The biggest gain comes from removing half of all 'hops' (crossings-off) in the inner loop and from the halved memory pressure.

Last but not least, the performance will drop significantly when the sieve array gets appreciably bigger than the L1 cache size of the CPU (typically 32 KiByte), and it will drop even further when the L2 cache size is exceeded. If you need decent performance in ranges beyond the L1 cache size then you might want to consider segmented sieving, i.e. working the sieve range in cache-sized blocks.

This is quite simple to implement, and definitely simpler than windowed sieving; you can find an example in my answer to Find prime positioned prime number. There you can also see that odds-only sieving is just as simple as plain sieving; it just requires a bit of care when dealing with indexes (i.e. clarity whether a given value is supposed to be a number or a bit index).

Many coding challenges that deal with primes require the sieving of millions of numbers instead of small handfuls, and so an investment in studying suitable 'prime technology' can pay off handsomely. Moreover, this study can tell you lot about the efficiency of basic mechanisms in your language - be it bool[] vs BitSet (bool[] wins hands-down in Java, C# and C++, if you have an eye on cache sizes), or the performance cost of syntactic sugar.


Do you want to be a super (voxel) hero? I have implemented an upgrades system in my Unity3d game, and I am pretty sure that I am not doing things in an optimal way. Any advice regarding best practices would be much appreciated.

The idea is that when the player collects a powerup, their run speed and jump speed increases.  Collect enough powerups, and you can jump around the level like the incredible hulk.

Play the game here: Super Voxel

The powerup is a basic Sphere object with its Sphere Collider Is Trigger checkbox activated.  It has this script attached:

SphereTriggerScript.cs

public class SphereTriggerScript : MonoBehaviour {

    public GameObject sphereObject;

    private GameObject playerObject;

    void Start() {
        playerObject = GameObject.Find("Player1");
    }

    void OnTriggerEnter (Collider other) {

        Destroy(sphereObject);

        PlayerScript script = playerObject.GetComponent<PlayerScript>();
        script.jumpForce += 1;
        script.runSpeed += 1;
    }
}

In the scene I have a basic Player1 prefab with this script attached:

PlayerScript.cs

public class PlayerScript : MonoBehaviour {

    public int runSpeed;
    public int jumpForce;

    void Start () {
        runSpeed = 20;
        jumpForce = 10;
    }
}

The powerup references this script with GameObject.Find. I do understand that this is suboptimal, however the powerups are created programmatically at run time, so it is not possible to simply drag and drop the Player1 game object onto the powerup prefab.

I have modified the default FPSController prefab (specifically the FirstPersonController script) to add a public GameObject field that I populate with the Player1 prefab.  Then when I need to get the jump or run speed, I access that script like so:

PlayerScript script = playerObject.GetComponent<PlayerScript>();
m_MoveDir.y = script.jumpForce;

Finally, I am doing something a bit funny to position the powerup orbs properly.  When creating them, I use a RayCast to position them at the "top" of the building:

int randomNumber = random.Next(0, 100);
if (randomNumber > 80) {

    int maxPossibleHeight = 250;
    RaycastHit hit;
    Ray ray = new Ray(new Vector3(x * tileSize, maxPossibleHeight, y * tileSize), Vector3.down);

    if (tileObject.GetComponentInChildren<MeshCollider>().Raycast(ray, out hit, 2.0f * maxPossibleHeight)) {

        int randomHeightOffset = random.Next(2, 12);

        float height = hit.point.y;
        GameObject powerup = (GameObject)GameObject.Instantiate (Resources.Load (powerupPrefabName));
        powerup.transform.position = new Vector3(x * tileSize, height + randomHeightOffset, y * tileSize);
    }
}

In your PlayerScript class, you have two variables, runSpeed, and jumpForce, and you initialize them in Start, like this:

void Start () {
    runSpeed = 20;
    jumpForce = 10;
}

This isn't necessary. Rather, you can do something like this:

public class PlayerScript : MonoBehaviour {
    public int runSpeed = 20;
    public int jumpForce = 10;
}

This will also mean that the variables runSpeed and jumpForce will have the default values of 20 and 10 in the Unity Editor as well.

In addition, in your SphereTriggerScript class, you also don't need to initialize playerObject in Start as well.

In your SphereTriggerScript class, in the method OnTriggerEnter, you never do a check to make sure that the collider is actually the Player GameObject. In order to do this, you'll have to wrap the contents of SphereTriggerScript.OnTriggerEnter in this if statement:

if(other.gameObject.name == playerObject.name)
{
    ...
}

This ensures that in case any other GameObject happens to collide with a power-up, that it won't be destroyed.

The UnityEngine namespace comes with a static class Random, which I prefer over System.Random while using Unity3D. Rather than having to instantiate System.Random, I can just call the methods directly from UnityEngine.Random.

Compare:

System.Random random = new System.Random();
int randomNumber = random.Next(0, 100);

To:

int randomNumber = (int)Random.Range(0, 100);

Other than the above, I just have a few small nitpicks:

The general style for storing an instantiated GameObject is the below example, not containing the Gameobject. prefix, and using as for a cast instead.

GameObject gameObject = Instantiate( ... ) as GameObject;

In addition, C# braces should be placed on the next line, not like Java braces, as seen in the below example.

if( ... )
{
    ...
}

Finally, you have a few magic numbers scattered across your code, so here's a small list of places containing magic numbers:

  script.jumpForce += 1;

  script.runSpeed += 1;

  int randomNumber = random.Next(0, 100);

  int randomHeightOffset = random.Next(2, 12);

Other than that, your code looks pretty nice! I'm excited to see where this game goes!


Two methods to prompt the user to enter the width and length of a rectangle I have two similar methods for setting dimensions of a rectangle. At first, I had them combined into one longer method that set width and length. I read about keeping code modular that performs one task at a time. So split it into two methods. Also, I have separate methods for displaying messages. Am I going overboard breaking methods into smaller ones?

private void userInputWidth() {
    Scanner reader = new Scanner(System.in);
    double loopCondition = 0;
    while (loopCondition <= 0) {
        displayEnterWidth();
        double nextDouble = reader.nextDouble();
        loopCondition = nextDouble;
        if (nextDouble <= 0) {
            displayErrorMessage();
        } else {
            this.rectangleWidth = nextDouble;
            break;
        }
    }
}

private void displayEnterWidth() {
    System.out.println("Please enter the width of the rectangle ");
}

2nd Method

private void userInputLength() {
    Scanner reader = new Scanner(System.in);
    double loopCondition = 0;
    while (loopCondition <= 0) {
        displayEnterLength();
        double nextDouble = reader.nextDouble();
        loopCondition = nextDouble;
        if (nextDouble <= 0) {
            displayErrorMessage();
        } else {
            this.rectangleLength = nextDouble;
            break;
        }
    }
}

private void displayEnterLength() {
    System.out.println("Please enter the length of the rectangle");
}

Breaking methods down into less than five lines is generally a sign that you have gone a bit too far (But not always!). This Software Engineering post is a good overview.

Instead of breaking your methods into userInputWidth and userInputHeight, I would recommend creating a helper method named getPositiveInput that accepts a prompt string and using that instead.

For example:

private double getPositiveInput(String prompt) {
    Scanner reader = new Scanner(System.in);
    double result;
    do {
        System.out.println(prompt);
        result = reader.nextDouble();
        if (result <= 0) {
            System.out.println("Enter a number greater than zero.");
        }
    } while (result <= 0)
    return result;
}

This makes it possible to reduce the duplication of your code without breaking your methods down so far that you have to have one for every property on your class.

The trick is what the definition of "one task" is. Deciding what each method should do to keep the code modular is made much easier by experience. I've found the following guidelines help.

Describe the method out loud, if you say "and" or "then" more than once, the method is probably doing too much and should be split up.
If you say the same thing for more than one method, extract the code that does that thing into it's own method.
If you catch yourself copy/pasting code stop and put it in a method.

Here's an example:

Given the following code (combined your methods to get what I presume was the original method), here's how I would describe the method.

private void userInputDimensions() {
    Scanner reader = new Scanner(System.in);
    double loopCondition = 0;
    while (loopCondition <= 0) {
        System.out.println("Please enter the width of the rectangle ");
        double nextDouble = reader.nextDouble();
        loopCondition = nextDouble;
        if (nextDouble <= 0) {
            System.out.println("Enter a number greater than zero.");
        } else {
            this.rectangleWidth = nextDouble;
            break;
        }
    }

    loopCondition = 0;
    while (loopCondition <= 0) {
        System.out.println("Please enter the length of the rectangle");
        double nextDouble = reader.nextDouble();
        loopCondition = nextDouble;
        if (nextDouble <= 0) {
            System.out.println("Enter a number greater than zero.");
        } else {
            this.rectangleLength = nextDouble;
            break;
        }
    }
}

Prompt the user for the width of the rectangle until they enter a number greater than zero
Set the rectangleWidth to the number
Prompt the user for the length of the rectangle until they enter a number greater than zero
Set the rectangleLength to the number

Notice that the duplicate "code" here is the prompt and validation input. By contrast, consider the following code using the getPositiveInput method.

private void userInputDimensions() {
    this.rectangleWidth = this.getPositiveInput("Please enter the width of the rectangle");
    this.rectangleLength = this.getPositiveInput("Please enter the length of the rectangle");
}

This method can be described by "set the width and length of the rectangle to the user input" and does not need to be broken down further.


Project Euler 62: cubic permutations, logic Problem 62 of Project Euler goes

  The cube, 41063625 (3453), can be permuted to produce two other cubes: 56623104 (3843) and 66430125 (4053). In fact, 41063625 is the
  smallest cube which has exactly three permutations of its digits which
  are also cube.

  Find the smallest cube for which exactly five permutations of its digits are cube.

Now I have written the following code to handle the problem. In essence I iterate over the cubes, and sort the numbers from highest to lowest. These values I use as keys in a dictionary. When the count reaches the desired amount (5) i break the loop.

Well that is almost correct, or it was my first code. It gave me the correct answer, however someone made the following remark:

  Even if the chain of permutations is 5, it is possible that you will
  find permutations later on that increases the count.

To overcome this silly issue i do the following: if the count is right i add them to a list of possible candidates. Once the number of digits in the current cube is larger than the number of digits in the candidates, i iterate over the candidates. Here i again check the number of cube permutations (it is still stored in the dictionary) and find the smallest one. If none of the candidates are valid, we clear the candidate list and start over.

It seems like quite a convoluted way of doing things.. and I do not like the logic of my try's and exceptions. I also think there is a logic break if the best permutation is the one where we go from $n$ to $n+1$ digits, since then it will be added to the list of permutations, and then removed.

Question: Is there a clearer way to code my reasoning in this problem? Does my code cover the corner / limit cases?

When I look at the code of others solving the same excercise their code looks so tiny and compact in comparison to mine =(

# Sorts a number from highest to lowest
def num2highest(num):
    string = sorted(str(num))
    rev_string = string[::-1]
    return int(''.join(map(str, rev_string )))

def cube_perms(perms, limit):
    checked = dict()
    cube_lst = []
    candidates = []
    len_cand = len(str(limit**3)) + 1

    for i in range(min(10**6, limit)):
        cube = i**3
        low_perm = num2highest(cube) #Sort the cube

        # Add sorted cube as a key in dict
        if checked.has_key(low_perm):
            count, cube_lst = checked[low_perm]
            cube_lst.append(i)
        else:
            count = 0
            low_cube = cube
            cube_lst = [i]
        checked[low_perm] = count+1, cube_lst #Increases the count of one

        # If the count is -1 of the wanted number of perms
        # add it to a list of possible candidates (since the count might increase)
        if count == perms - 1:
            if low_perm not in candidates:
                candidates.append(low_perm)
            len_cand = len(str(cube))

        # If the number of digits of the current cube is greater than the candidates
        # iterate over the possible candidates
        if len(str(cube)) > len_cand and len_cand < (len(str(limit**3)) + 1):
            the_one = choosen(candidates, perms, checked)
            if the_one > 0:
                count, cube_lst = checked[the_one]
                return sorted(cube_lst)
            else:
                candidates = set()
                len_cand = len(str(limit**3)) + 1

# Iterates over the possible candidates, finding the smallest one
def choosen(candidates, perms, checked):
    choosen = 0
    minimal = candidates[0]
    for cand in candidates:
        count, cubes = checked[cand]
        if count == perms:
            temp_minimal = min(cubes)
            if temp_minimal < minimal:
                    minimal = temp_minimal
                    choosen = cand
    return choosen

if __name__ == '__main__':

    print cube_perms(5, 10**6)

Return Value

The question is asking for the smallest cube (127035954683), your code returns the list of values whose cubes permute to that ([5027, 7061, 7202, 8288, 8384]), so it's technically wrong.

Signature

The problem as described suggests a function that takes a single argument as input: the number. You additionally require a limit. What's the limit for? How do you know that the limit you picked isn't too small? You happen to pick a good limit, but what if we needed to go up in digits? What if we wanted to find the smallest cube with 7 permutations?

try/catch

You have this code:

try:
    return choosen
except:
    return 0

It took me a lot of squinting to actually realize what it is you were catching. Ohhh... NameError on choosen. And then you have the same thing in the interior - you're catching not an exception but an undefined name. That is awkward code to understand.

First, you can just up-front declare choosen = 0, that lets you drop the last try/except block completely.

Next, you can up-front declare minimal to be None and change that block to be:

if minimal is None or temp_minimal < minimal:
    minimal = temp_minimal

Or you could declare minimal to be sys.maxint. Also there, you're assigning to a variable named min_cand but never reading it, so not sure what that is for.

Algorithm

I think you're overthinking the problem somewhat. You have the right approach with keeping track of a dictionary with the sorted permutation. But then you kind of lose me with the digit counting. I chuckled at the_one = chosen(...), but I have no idea what that function is doing.

Here's my thoguht process. When our count for a particular permutation reaches the goal count, we have to verify that there aren't any future cubes that would end up here. That remark is correct. However, we know exactly which things we need to test - so we should just explicitly test those:

To start with:

def my_cube_perm(num_perms):
    def as_str(x):
        return ''.join(sorted(str(x)))

    counts = collections.defaultdict(int)
    smallest = {}

    for i in itertools.count(start=1): # no limit!
        cube = i**3
        cube_str = as_str(i**3)
        counts[cube_str] += 1
        smallest.setdefault(cube_str, cube)

        if counts[cube_str] == num_perms:
           # ok what, now?

The next smallest possible cube that could produce cube_str is i+1. The largest possible cube that produce it comes from reverse-sorting cube_str and taking its cube root. So just walk that range:

max_possible = int(int(''.join(reversed(cube_str)))**(1/3.))
if not any(as_str(p**3) == cube_str
           for p in xrange(i+1, max_possible+1)):
    return smallest[cube_str]

This is ~2.3x faster (38ms vs 87ms), but more importantly I think is easier to follow the logic and is much shorter.

Algorithm Update

N3buchadnezzar makes a good point in that the first solution I found isn't necessarily the smallest. We could have 5 permutations of one cube all fall in between the first and next 4 permutations of another. So we have to keep track of some more state. If we find a value that we think is the answer - we save it and the length of its cube. Once we get to a cube with more digits, if we have anything saved, return at that point.

Full solution looks like this:

def my_cube_perm(num_perms):
    def as_str(x):
        return ''.join(sorted(str(x)))

    counts = collections.defaultdict(int)
    smallest = {}

    best = None
    best_len = 0

    for i in itertools.count(start=1):
        cube = i**3
        cube_str = as_str(cube)

        # got an answer that is definitely shorter?
        if best is not None and len(cube_str) > best_len:
            return best

        counts[cube_str] += 1
        smallest.setdefault(cube_str, cube)

        if counts[cube_str] == num_perms:
            # check all the possibilities!
            max_possible = int(int(''.join(reversed(cube_str)))**(1/3.))
            if not any(as_str(p**3) == cube_str
                       for p in xrange(i+1, max_possible)):
                # update our best solution - if appropriate
                if best is None or smallest[cube_str] < best:
                    best = smallest[cube_str]
                    best_len = len(cube_str)

This increases runtime to 46ms. But then again, runtime of an incorrect algorithm doesn't really matter. And still ~1.9x faster.


Python GCD implementations I wrote 2 different implementations to find gcd of 2 big numbers. The simple one using Euclid's algorithm always beat the Binary algorithm in term of speed. Is my implementation of Binary algorithm bad?

a = 5**100 - 1
b = 5**120 - 1

def gcd_euclid_recursive(a, b):
    if b == 0: return a
    return gcd_euclid_recursive(b, a % b)

def gcd_binary_recursive(a, b):
    if a == b: return a
    if a == 0: return b
    if b == 0: return a
    if a & 1 == 0: # a is even
        if b & 1 == 1: # b is odd
            return gcd_binary_recursive(a >> 1, b)
        else: # both a and b are even
            return gcd_binary_recursive(a >> 1, b >> 1) << 1

    if b & 1 == 0: # a is odd and b is even
        return gcd_binary_recursive(a, b >> 1)

    if (a > b):
        return gcd_binary_recursive((a-b) >> 1, b)
    return gcd_binary_recursive((b-a) >> 1, a)

Your implementation is as efficient as it can be in python. Because python is an interpreted language, every statement that is executed takes a long time compared to something like c.

At the bottom of this answer is an altered version of your code that times 1000 evaluations of both of the functions. It also records the total amount of time spent evaluating the first three lines of the binary algorithm.

Below is the output of the code:

Euler:             0.0010025501251220703
Binary1:           0.2012491226196289
First three lines: 0.04787778854370117

The amount of time spent checking for a==b, a==0, and b==0 takes longer than the entire Euler algorithm. It should be noted that the binary algorithm is only supposed to be faster when dealing with "regular" arithmetic. From the Wikipedia page on the binary gcd algorithm:

"For arbitrary-precision arithmetic, neither the Euclidean algorithm nor the binary GCD algorithm are fastest, as they both take time that is a quadratic function of the number of input digits."

first_three_lines = 0

def gcd_euclid_recursive(a, b):
    if b == 0: return a
    return gcd_euclid_recursive(b, a % b)

def gcd_binary_recursive(a, b):
    global first_three_lines
    t = time()
    if a == b: return a
    if a == 0: return b
    if b == 0: return a
    first_three_lines += time() - t

    if a & 1 == 0: # a is even
        if b & 1 == 1: # b is odd
            return gcd_binary_recursive(a >> 1, b)
        else: # both a and b are even
            return gcd_binary_recursive(a >> 1, b >> 1) << 1

    if b & 1 == 0: # a is odd and b is even
        return gcd_binary_recursive(a, b >> 1)

    if (a > b):
        return gcd_binary_recursive((a-b) >> 1, b)
    return gcd_binary_recursive((b-a) >> 1, a)

a = 5**100 - 1
b = 5**120 - 1

from time import time
euler_time = 0
for i in range(1000):
    t = time()
    x = gcd_euclid_recursive(a,b)
    euler_time += time() - t

bin_time = 0
for i in range(1000):
    t = time()
    x = gcd_binary_recursive(a,b)
    bin_time += time() - t

print("Euler:            ",euler_time)
print("Binary1:          ",bin_time)
print("First three lines:",first_three_lines)
```


Program in JAVA to generate random alphanumeric string This code will generate a pseudorandom alphanumeric string of given length.
I would welcome suggestions on how to make it more random. Beyond that, have I made any convention violations, Exception cases, and the like?
Also, is there any way to make it faster?

public class Test
{
    public static String getRandomAlphaNum(int length)
    {
        String charstring = "abcdefghijklmnopqrstuvwxyz0123456789";
        String randalphanum = "";
        double randroll;
        char randchar;
        for
        (double i = 0; i < length; i++)
        {
            randroll = Math.random();
            randchar = '@';
            for
            (int j = 1; j <= 36; j++)
            {
                if
                (randroll <= (1.0 / 36.0 * j))
                {
                    randchar = charstring.charAt(j - 1);
                    break;
                }
            }
            randalphanum += randchar;
        }
        return randalphanum;
    }
}

for
        (double i = 0; i < length; i++)

And related loops should have the "for" on the same line (as this is a common coding convention.)

(randroll <= (1.0 / 36.0 * j))

This doesn't have to be a double; instead, the random number can be generated as an integer (to select which element from the array.)

randchar = '@';

Unless the random string is not random, I would not initialize the variables with sample data. I'd just leave them blank and then adjust the loop to always run at least once (a do-while loop) so that it becomes initialized.

for
            (int j = 1; j <= 36; j++)
            {
                if
                (randroll <= (1.0 / 36.0 * j))
                {
                    randchar = charstring.charAt(j - 1);
                    break;
                }
            }

I would remove the inner if-statement and un-hardcode the values so it can work with strings with any size. Applying these suggestions, it can be simplified to:

import java.util.Random;
class Main {
  public static void main(String[] args) {
    int strLen = 100;
    String randString = "";
    Random r = new Random();
    String[] chars = "abcdefghijklmnopqrstuvwxyz0123456789".split("");
    while (randString.length() < strLen)
        randString += chars[randBetween(r, 0, chars.length - 1)];

    System.out.println(randString);
  }

  /*
  Generates a random number from min to max inclusive
  */
  public static int randBetween(Random r, int min, int max) {
    return r.nextInt((max - min) + 1) + min;
  }
}

This approach is not optimal as the string is constantly being appended to, meaning that the string has to be re-copied every iteration.

Java introduced Streams, which allows reading forever from certain generators. Knowing this, we can read a stream of random numbers up until the string length that the user wants, and then get the character at the random string length:

import java.util.Random;
class Main {
  public static void main(String[] args) {
    int strLen = 100;
    String chars = "abcdefghijklmnopqrstuvwxyz0123456789";

    StringBuilder randomOutput = new StringBuilder();
    new Random().ints(strLen, 0, chars.length())
                .forEach(c -> randomOutput.append(chars.charAt(c)));

    System.out.println(randomOutput);
  }
}

StringBuilder is used to append the random character as it doesn't have to be re-copied for every loop iteration.


Random Word Splitter I wrote a word splitting function. It splits a word into random characters. For example if input is 'runtime' one of each below output possible:

['runtime']

['r','untime']

['r','u','n','t','i','m','e'] ....

But it's runtime is very high when I want to split 100k words do you have any suggestions to optimize or write it smarter.

def random_multisplitter(word):
    from numpy import mod
    spw = []
    length = len(word)
    rand = random_int(word)
    if rand == length:       #probability of not splitting
        return [word]

    else:
        div = mod(rand, (length + 1))#defining division points
        bound = length - div
        spw.append(div)
        while div != 0:
            rand = random_int(word)
            div = mod(rand,(bound+1))
            bound = bound-div
            spw.append(div)
        result = spw
    b = 0
    points =[]
    for x in range(len(result)-1): #calculating splitting points
        b=b+result[x]
        points.append(b)
    xy=0
    t=[]
    for i in points:
        t.append(word[xy:i])
        xy=i
    if word[xy:len(word)]!='':
        t.append(word[xy:len(word)])
    if type(t)!=list:
        return [t]
    return t

Most of your variable names seem good, however xy explains nothing.

You should normally have two space's around operators.
b = b + result[x] is more readable than b=b+result[x],
which looks at a glance like a variable.

Why do you need to import numpy to do a simple mod?
Python comes with mod, %.

You use the word result,
which I would use to imply that that will be the result of a function.

You shouldn't import things anywhere apart from the beginning of the file.
I nor anyone else wants to traverse you entire program to find that you import numpy's mod.
We want to know at the begging you use it.
Also this can lead to bugs where you think you have imported numpy, but you haven't.

Your programs overall ability to be read with ease is quite low.

I would recommend splitting the function into two.
A 'feeder' and a 'consumer'.

First I love generators, and you can write one that reduces the complexity of this program.

The algorithm that I use is to find the start and stops of the split.
Then I replace start with stop and add a random amount to the stop.
It really is that simple. To do the majority of your program.

def get_numbers(length):
    start, stop = 0, 0
    while stop < length:
        start, stop = stop, stop + random.randint(1, length)
        yield start, stop

This is simple, it will count until it gets to or past the length, and will return them.
You can think of it like building an array that looks like [(start, stop), (start, stop), ...].

This is a near drop in replacement for the entire program until for i in points:.
However I use a lazy approach for that for loop.
Where yours is a one dimensional list of index.

If you wanted it to build an array instead then it would look like the following block.
However it's not advised, as it will lead to bad memory usage, and it will take slightly longer.

def get_numbers(length):
    list_ = []
    start, stop = 0, 0
    while stop < length:
        start, stop = stop, stop + random_int(length)
        list_.append((start, stop))
    return list_

Then you can now split the string.
To do this I will loop through the above generator and yield split strings.

def random_multisplitter(word):
    for start, stop in get_numbers(len(word)):
        yield word[start:stop]

I use Python's amazing split operation just like you did.
However as you want random_multisplitter to return a list, not a generator.
You will change it.
But a generator is a better choice for the 100k words input.
This is as then you will have a smaller memory consumption.

def random_multisplitter(word):
    return [word[start:stop] for start, stop in get_numbers(len(word))]

If not faster, it will at least be smaller and nicer to look at.

I thought that the program would be larger, and so, a small one function version would be:

from random import randint

def random_multisplitter(word):
    length = len(word)
    start, stop = 0, 0
    while stop < length:
        start, stop = stop, stop + randint(1, length)
        yield word[start:stop]

# Generator
random_multisplitter('runtime')
# List
list(random_multisplitter('runtime'))

Some people may dislike that I allow the slice to go above the maximum length of the list. Just to avoid any potential confusion about that, it's safe to use, but can seem weird.

>>> 'abcde'[0:20]
'abcde'
>>> 'abcde'[20]
IndexError: string index out of range

Due to this you may wish to change the assignment of stop to the following:

start, stop = stop, randint(stop + 1, length)

Or when you are returing the values change it to:

yield start, min(stop, length)

The former will have a higher amount of splits to the end of the string. Where as the unmodified version and latter version will prominently have two or three main splits on small strings.

As this is tagged performance, It's probably best if we have a speed test.
The code I use to test the speed is:

word = ' ' * 100000

def time_it(fn, name):
    n = 100
    t0 = time.time()
    for _ in range(n):
        fn(word)
    t1 = time.time()
    print name, '=', t1-t0

This uses time and so just take the results with a bit of salt.

Also, I can't test your original code.
random_int is not defined in the example.

And finally I used start, stop = stop, stop + random.randint(1, 2) On the 'Slow' ones, and start, stop = stop, stop + random.randint(1, length) on the 'Fast' ones.

Other Answer = 75.2860000134
Slow Generator = 12.1919999123
Slow List = 13.236000061
Fast Generator = 0.00200009346008
Fast List = 0.0019998550415

Keep in mind that Generators perform better when they aren't converted to a list straight off the bat.


Result class which wraps another object I have a result class which wraps another object if successful or an error if failure

public class Result
{
    private Result() { }

    public bool Success { get; private set; }
    public string Error { get; private set; }
    public MyClass MyClass { get; private set; }

    public static Result CreateSuccess(MyClass myClass)
    {
        return new Result { Success = true, MyClass = myClass };
    }
    public static Result CreateFailure(string error)
    {
        return new Result { Error = error };
    }

    // ... see below
}

1. Is this a reasonable way to go about this?

I also am considering putting an implicit conversion to bool

public static implicit operator bool(Result result)
{
    return result.Success;
}

So that I can do

var result = GetResult();

if (!result)
{
    Print(result.Error);
}

2. Is this a silly idea?

Your result wrapper sounds like a good idea. I would modify it by allowing a collection of error strings, in case of multiple errors, warnings and the like, or perhaps consider holding an inner exception type (depending on how you raise these errors).

I am, on the other hand, wondering why you've hidden the constructor and opted for two static factory methods. As far as I can tell they offer nothing over simply offering two constructors instead, and they make for a less intuitive creation process for other developers (who will assume a constructor before trying other options):

public class Result
{
    public bool Success { get; private set; }
    public string Error { get; private set; }
    public MyClass MyClass { get; private set; }

    public Result(MyClass myClass)
    {
        Success = true;
        MyClass = myClass;
    }

    public Result(string error)
    {
        Error error;
    }
}

If you are concerned that other programmers could confuse the constructors, consider XML comments, or possibly keeping Result as a baseclass, and create SuccessResult and  FailedResult subclasses like so:

public abstract class Result
{
    public abstract bool Success {get;}
}

public class SuccessResult : Result
{
    public override bool Success { get {return true; }}
    public MyClass MyClass { get; private set; }

    public Result(MyClass myClass)
    {
        MyClass = myClass;
    }
}

public class FailedResult : Result
{
    public bool Success { get {return false};}

    public string Error { get; private set; }

    public Result(string error)
    {
        Error error;
    }
}

This successfully conforms to the Single Responsibility Principle, here a result only contains error information if an error actually occurs. From this point you could also extend Result (or FailedResult) to create an ExceptionResult as well, which would contain any Exceptions raised and providing more detailed information on the nature of the error.

As for your second part, I would leave it out. The implicit conversion gives me the impression I'm getting a boolean value of the result itself, not of its success/failure condition.


Converting an array into an object I have an array of this fashion:

var arr = ["akira",9248,"padilla",100];

I'm using filter to take the names and the points:

var names = arr.filter(function(e,i){return i%2 == 0});
var points = arr.filter(function(e,i){return i%2 !== 0});

Then I do this to obtain turn it into object:

console.log(JSON.stringify(_.object(names,points)));

What's best in terms of performance/JS quality?

Could you do it this way which would mean you're only traversing the array once?

var obj = {};

for (var i=0; i < arr.length - 1; i = i + 2) {
    obj[arr[i]] = arr[i+1];
}


How to speed up a treeview population in Access? I have the following function that causes a significant visual jittering and lag in my access database - This is a treeview selection based on data in a table (tblDATA) with the fields:
Object | Attribute | Property | Value
I have tried echo and me.painting but figured my actual for loops need to be looked at for optimisation - wondering if there is a better way to cycle through a Recordset and populate a Treeview ?
' [1] Update the treeview
'------------------------------------

Private Sub btnUpdateFilter_Click()

    'Me.Painting = False
    'Echo False
    'DoCmd.Hourglass True

    Dim sArea As String, N As Integer
    Dim oData_distinct As DAO.Recordset
    Dim oData As DAO.Recordset
    Dim oNone As DAO.Recordset
    Dim rs As DAO.Recordset
    Dim element As Variant
    Dim Arr_distinct() As Variant

    If IsNull(cbxAttributeFilter01) Or IsNull(cbxPropertyFilter01) Then Exit Sub

    Set oData_distinct = CurrentDb.OpenRecordset("SELECT DISTINCT [Value] FROM tblData WHERE Attribute = " & cbxAttributeFilter01 _
                                                & " AND Property = " & cbxPropertyFilter01, dbOpenSnapshot)

    Set oNone = CurrentDb.OpenRecordset("SELECT [Tagname] FROM tblData WHERE NOT Attribute = " & cbxAttributeFilter01 _
                                                & " AND NOT Property = " & cbxPropertyFilter01, dbOpenSnapshot)

    Set oData = CurrentDb.OpenRecordset("SELECT [Tagname], [Value] FROM tblData WHERE [Attribute] = " & cbxAttributeFilter01 _
                                                & " AND [Property] = " & cbxPropertyFilter01, dbOpenSnapshot)

    If oData.RecordCount = 0 Then TreeView.Nodes.Clear: Exit Sub

    Arr_distinct = oData_distinct.GetRows(oData_distinct.RecordCount)

    TreeView.Nodes.Clear

    ' Standard
    TreeView.Nodes.Add , , "MISC", "MISC"
    For Each element In Arr_distinct
        If Not element = "" Then
            If IsNumeric(element) Then element = "_" & element
            TreeView.Nodes.Add , , element, element    ' ADD Parent
        End If
    Next element

    Dim sTemp As String

    With oData
        .MoveFirst
        Do While Not .EOF
            sTemp = !Value
            If IsNumeric(sTemp) Then sTemp = "_" & sTemp
            If sTemp = "" Or sTemp = " " Then sTemp = "MISC"
            On Error Resume Next
            TreeView.Nodes.Add sTemp, tvwChild, DLookup("Tagname", "MASTER", "ID = " & !Tagname), DLookup("Tagname", "MASTER", "ID = " & !Tagname)
        .MoveNext
        Loop
    End With

    With oNone
        .MoveFirst
        Do While Not .EOF
            On Error Resume Next
            TreeView.Nodes.Add "MISC", tvwChild, DLookup("Tagname", "MASTER", "ID = " & !Tagname), DLookup("Tagname", "MASTER", "ID = " & !Tagname)
            .MoveNext
        Loop
    End With

   'DoCmd.Hourglass False
   'Echo True
   'Me.Painting = True

End Sub
```

Avoid the multiple DLookUps with eachTreeView.Nodes.Add call for every row of recordset! At the very least store your DLookUp value once and apply it to treeview. This cuts down the repetitive 4 calculations down to 2 for every row.
But since DLookUp essentially is a query anyway, simply integrate a JOIN to Master object in SQL queries of OpenRecordset calls. This cuts down the repetitive 4 calculations to 0 for every row.
sql = "SELECT d.[Tagname], m.[Tagname] AS [MasterTagname] " _
       & "FROM tblData d " _
       & "LEFT JOIN Master m " _
       & "    ON m.ID = d.[Tagname] " _
       & "WHERE NOT d.[Attribute] = " & cbxAttributeFilter01 _
       & "  AND NOT d.[Property] = " & cbxPropertyFilter01

Set oNone = CurrentDb.OpenRecordset(sql, dbOpenSnapshot)

sql = "SELECT d.[Tagname], m.[Tagname] AS [MasterTagname] " _
       & "FROM tblData d " _
       & "LEFT JOIN Master m " _
       & "    ON m.ID = d.[Tagname] " _
       & "WHERE d.[Attribute] = " & cbxAttributeFilter01 _
       & "  AND d.[Property] = " & cbxPropertyFilter01

Set oData = CurrentDb.OpenRecordset(sql, dbOpenSnapshot)

...

TreeView.Nodes.Add sTemp, tvwChild, !MasterTagname, !MasterTagname

...

TreeView.Nodes.Add "MISC", tvwChild, !MasterTagname, !MasterTagname

Also, best practice in VBA is to never use On Error Resume Next but to handle any error that raises which in this case may be a missing or Null DLookUp result. Therefore, enclose If logic.
...

If Not IsNull(!MasterTagName)
   TreeView.Nodes.Add sTemp, tvwChild, !MasterTagname, !MasterTagname
End If

...

If Not IsNull(!MasterTagName)
   TreeView.Nodes.Add "MISC", tvwChild, !MasterTagname, !MasterTagname
End If