Storing computed values in an array In several places I need to compute the next value, given a N:

double size = 1.0 / Math.Pow(2, N);

Since I won't be computing this value for N > 15 I have thought of the following performance optimization:

    public static readonly double[] RegionSizes =
{
    1, 0.5, 0.25, 0.125, 0.0625, 0.03125, 0.015625, 0.0078125,
    0.00390625, 0.001953125, 0.000976563, 0.000488281, 0.000244141, 0.00012207, 0.00006103515625, 0.000030517578125
};

//Usage
    double size = RegionSizes[N];

Is this a good solution? Why / why not?

It isn't a good solution and there are several reasons.

1.: Your values are less precise than what the double data type allows for. For example,

1.0 / Math.Pow(2, 12)

computes to 0.000244140625, while your lookup value is just 0.000244141. Depending on the following calculations, this can make quite a big difference.

2.: Your array is public and mutable. While the array reference is readonly (so you can't assign a new array to the reference), the array itself is not. For example

RegionSizes[2] = 5;

compiles and runs without any problems. You (or every other dev) can override values in the array as he desires. Better use an immutable collection, like ReadOnlyCollection<double>, to prevent your cached values from manipulation during runtime. (If this is a little fun-project with you as the only dev this might not become a problem; in bigger projects with >1 dev this type of oversight can lead to hard-to-track bugs.)

3.: Experience tells that assumptions like

  I won't be computing this value for N > 15

do not always hold true. To be sure, a more flexible approach (like calculating every value once it's first needed, then store it and use it when needed again) would allow for more flexibility. Of course, without any information about what you intend to do in your program it's hard to tell if this is relevant.

4.: I strongly suspect you are doing premature optimizations (because you mention neither actual performance problems, nor any measurements indicating that floting-point operations are the cause for such problems). Donald Knuth said

  The real problem is that programmers have spent far too much time worrying about efficiency in the wrong places and at the wrong times; premature optimization is the root of all evil (or at least most of it) in programming.

and I have nothing to add. From my experience, in >95% of all software projects you don't need to worry about the performance of >95% of your code. So when making an optimization, you should follow these steps:

Set a realistic measurable performance goal. This is important, you can optimize code forever, so you have to have some definition of 'good enough' and stop once your code is good enough.
Measure if there is an actual performance problem (by comparing actual performance to your definition of 'good enough').
Identify the parts of your code that cause the performance problem; that involves measuring again. For example measure how much time the program spends in each method, then measure how much time the operations in the slow methods take. Do this until you know for sure what makes your program run slow.
Optimize the code that you found out to be slow in step 3 and measure again, to make sure your optimizations are making your code run faster (it's suprising how many 'optimizations' actually make things worse).
Go back to 2.

Yes, I know, optimizing code is fun - but you can quickly get lost and spend hundreds of hours optimizing code that never needed any optimization.


Using a Function to emulate F# Match in C# So this was inspired by:
this question
But doesn't actually answer it:
What do you think of:

public class Case<TRes>
{
    public Case(bool condition, Func<TRes> result)
    {
        Condition = condition;
        Result = result;
    }
    public bool Condition {get; private set;}
    public Func<TRes> Result { get; private set; }

    public static Case<TRes> Default(Func<TRes> value)
    {
        return new Case<TRes>(true, value);
    }
}

public static class Logic
{
    public static T Match<T> (params Case<T>[] cases)
    {
        return cases.First(c => c.Condition).Result();
    }
}

Which you could invoke:

int i = Logic.Match
(
    new Case<int>(IsCloudy(skyImage), () =>
    {
        ChangeWeatherIcon(Icons.Clouds);
        return 13;
    }),
    new Case<int>(a < b, () => a),
    Case<int>.Default(() => {throw new Exception(); })
);

it's purpose isn't quiet to emulate F#'s Match.
(Because F# match doens't actually take conditions, it is a stuctural/pattern match)
So really its kind of like the trinary operator, meets the nonOrdinal Select statment.

Anyone got any idea's on how to make it work cleaner? Maybe getting a way to get rid of the new's and the brackets

To be honest, it's pretty much impossible to do elegant pattern matching in a language that doesn't support it (e.g., C#).  The problem with what you've written there is it's more complicated than the equivalent if-then-else chain:

if (IsCloudy(skyImage)) {
    ChangeWeatherIcon(Icons.Clouds);
    return 13;
}
if (a < b) {
    throw new Exception();
}

Now, I would dearly love to see some later version of C# adopt algebraic data types (a la F#) with pattern matching, but until then I very much doubt you'll be able to do better than if-then-else chains.


Show time in format yyyy-MM-ddThh:mm:ss.SSS My requirement is to convert a time (long) to a String with this date format:

  yyyy-MM-ddThh:mm:ss.SSS

For instance: 2017-03-07T03:52:31.298

My implementation:

private final static String DATE_FORMAT = "yyyy-MM-dd_hh:mm:ss.SSS";

private final static DateFormat DF =
        new SimpleDateFormat(DATE_FORMAT, Locale.ENGLISH);

public static String formatTime(long time) {
    return DF.format(time).replace('_', 'T');
}

The underscore trick is to avoid  the Illegal pattern character T error I was getting if I had a T character in DATE_FORMAT. I am sure there is a better way to do this?
Also, I am sure more explicit names can be found for DATE_FORMAT/DF?

Don't reinvent the wheel, this is java.time.format.DateTimeFormatter.ISO_LOCAL_DATE_TIME.

long time = System.currentTimeMillis();
ISO_LOCAL_DATE_TIME.format(Instant.ofEpochMilli(time).atZone(ZoneOffset.UTC))
// "2017-03-07T12:09:04.374"


Regex to first match, then replace found matches In my C# program I am using Regular expressions to:

Loop through a list of possible words in need of replacing.
For each word, to find out if a string I am given has any matches.
If it does, I perform some (slightly costly) logic to create the replacement.
I then perform the actual replacement.

My current code looks roughly as follows:

string toSearchInside; // The actual string I'm going to be replacing within
List<string> searchStrings; // The list of words to look for via regex

string pattern = @"([:@?]{0})";
string replacement;

foreach (string toMatch in searchStrings)
{
    var regex = new Regex(
                            string.Format(pattern, toMatch),
                            RegexOptions.IgnoreCase
                            );
    var matches = regex.Matches(toSearchInside);

    if (matches.Count == 0)
        continue;

    replacement = CreateReplacement(toMatch);

    toSearchInside = regex.Replace(toSearchInside, replacement);
}

And I can get this working, but it seems somewhat inefficient in that it is using the regex engine twice - Once to find the matches (regex.Matches()) and once for the replacing regex.Replace()). I was wondering if there was a way to simply say replace the matches you already found?

Also, if has been asked what is within the CreateReplacement() method since it could be possibly done via a Match Elevator, but it is actually a separate method that's fairly costly and not really what I'm asking in this case - My bigger question here is how to deal with this situation of having to use Regex twice - Once to find the matches and then a second time to replace them.

I hope that what I'm trying to find out how to do actually makes sense.

Regex.Matches returns a MatchCollection which contains Matches which captures the index and length of each match. So as such you won't have to fire up the regex engine again because you can do something like this:

string toSearchInside; // The actual string I'm going to be replacing within
List<string> searchStrings; // The list of words to look for via regex

string pattern = @"([:@?]{0})";
string replacement;

foreach (string toMatch in searchStrings)
{
    var regex = new Regex(
                            string.Format(pattern, toMatch),
                            RegexOptions.IgnoreCase
                            );
    var matches = regex.Matches(toSearchInside);

    if (matches.Count == 0)
        continue;

    replacement = CreateReplacement(toMatch);

    // in case the replacement is of a different length we replace from
    // from back to front to keep the match indices correct
    foreach (var match in matches.Cast<Match>().Reverse())
    {
        toSearchInside = toSearchInside.Replace(match.Index, match.Length, replacement);
    }
}

Unfortunately the .NET framework doesn't come with a positional Replace so we have to create one which I did as an extension method:

public static string Replace(this string s, int index, int length, string replacement)
{
    var builder = new StringBuilder();
    builder.Append(s.Substring(0,index));
    builder.Append(replacement);
    builder.Append(s.Substring(index + length));
    return builder.ToString();
}

If you do this often and the match patterns don't change you could consider two things:

Pre-compile the regular expressions
Pre-create the replacements


Comparing rows of a matrix with some row vectors I have run this program for five hours and it has not yet completed. I wonder if we can make changes to reduce the computation time.

I use several for loop and this greatly increases the computation time. my goal is to compare the contents of the rows of a matrix (4435 × 2000) with 6 row vectors (1 × 2000). The comparison is made on parts of 64 values.

for i=1:4435
    for j=70:2000
        L = A(i,j);
        j1=0;
        j2=0;
        j3=0;
        j4=0;
        j5=0;
        j6=0;
        for i1=70:2000
            if D1(B(v,1),i1)==L
                j1=i1;
                Break
            end
        end

        for i2=1:2000
            if A(B(v-1,1),i2)==L
                j2=i2;
                Break
            end
        end

        for i3=1:2000
            if A(B(v-2,1),i3)==L
                j3=i3;
                Break
            end
        end

        for i4=1:2000
            if A(C(mm,1),i4)==L
                j4=i4;
                Break
            end
        end

        for i5=1:2000
            if A(C(mm-1,1),i5)==L
                j5=i5;
                Break
            end
        end

        for i6=1:2000
            if A(C(mm-2,1),i6)==L
                j6=i6;
                Break
            end
        end

        if j1>64
           if A(i,j-63:j)==A(B(v,1),j1-63:j1)
              m_A1(i,j)=1;
           else
              m_A1(i,j)=0;
           end
        end

        if j2>64
           if A(i,j-63:j)==A(B(v-1,1),j2-63:j2)
              m_A2(i,j)=1;
           else
              m_A2(i,j)=0;
           end
        end

        if j3>64
           if A(i,j-63:j)==A(B(v-2,1),j3-63:j3)
              m_A3(i,j)=1;
           else
              m_A3(i,j)=0;
           end
        end

        if j4>64
           if A(i,j-63:j)==A(C(mm,1),j4-63:j4)
              m_A4(i,j)=1;
           else
              m_A4(i,j)=0;
           end
        end

         if j5>64
           if A(i,j-63:j)==A(C(mm-1,1),j5-63:j5)
              m_A5(i,j)=1;
           else
              m_A5(i,j)=0;
           end
         end

         if j6>64
           if A(i,j-63:j)==A(C(mm-2,1),j6-63:j6)
              m_A6(i,j)=1;
           else
              m_A6(i,j)=0;
           end
        end

    end
end

In Matlab, unlike C, you have to vectorize your code.
A first step would be to understand how to use find like this:

i1=70:2000;
j1=find(D1(B(v,1),i1)==L);

and:

if j1>64
       m_A1(i,j) = all( A(i,j-63:j)==A(B(v,1),j1-63:j1) );
end

You may also find any and all useful. Eventually you should be able to eliminate all the for loops and if statements.


Word/expression list frequency distribution I have another Python issue, creating a frequency distribution out of a text matching a predefined wordlist. In fact, I work with more than 100,000 text files (each consisting of about 15,000 words), which I want to inread and accordingly match against a word/expression list (vocabulary_dict) of about 6,000 entries. The result should be a dictionary of all that entries attached with their respective frequency.

Here is what I am currently doing:

sample text = "As the prices of U.S. homes started to falter, doubts arose throughout the global financial system. Banks became weaker, private credit markets stopped functioning, and by the end of the year it was clear that the world banks had sunk into a global recession."

vocabulary_dict = ['prices', 'banks', 'world banks', 'private credit marktes', 'recession', 'global recession']

def list_textfiles(directory):
    # Creates a list of all files stored in DIRECTORY ending on '.txt'
    textfiles = []
    for filename in listdir(directory):
        if filename.endswith(".txt"):
            textfiles.append(directory + "/" + filename)
    return textfiles

for filename in list_textfiles(directory):
    # inread each report as textfile, match tokenized text with predefined wordlist and count number of occurences of each element of that wordlist
    sample_text = read_textfile(filename).lower()
    splitted = nltk.word_tokenize(sample_text)
    c = Counter()
    c.update(splitted)
    outfile = open(filename[:-4] + '_output' + '.txt', mode = 'w')
    string = str(filename) # write certain part of filename to outfile
    string_print = string[string.rfind('/')+1:string.find('-')] + ':' + string[-6:-4] + '.' + string[-8:-6] + '.' + string[-12:-8]
    for k in sorted(vocabulary_dict):
    # recognize if wordlist element consist of one or more tokens, accordingly find matching token length in text file (report)
        spl = k.split()
        ln = len(spl)
        if ln > 1:
            if re.findall(r'\b{0}\b'.format(k),sample_text):
                vocabulary_dict[k.lower()] += 1
        elif k in sample_text.split():
            vocabulary_dict[k.lower()] += c[k]
    outfile.write(string_print + '\n')
    # line wise write each entry of the dictionary to the corresponding outputfile including comapany name, fiscal year end and tabulated frequency distribution
    for key, value in sorted( vocabulary_dict.items() ):
        outfile.write( str(key) + '\t' + str(value) + '\n' )
    outfile.close()

# Output accoring to the above stated example should be in the form:
"selected part of filename (=string1)"
'prices' 1
'banks' 2
'world banks' 1
'private credit marktes' 1
'recession' 1
'global recession' 1

The code works quite well, but I still think there is room for optimization, since the time to process a textfile is ca. 1 minute.

Is there a way to make the matching of text to the word/expression list faster`?

One obvious improvement: you call elif k in sample_text.split(): inside the loop over sorted(vocabulary_dict), so this operation happens multiple times. Instead, you could create a set (which has more efficient membership tests) outside the loop, so it only happens once:

samples = set(sample_text.split())
for k in sorted(vocabulary_dict):
    ...
    elif k in samples:
        ...

Also, does it really matter if you iterate over vocabulary_dict in sorted order?


C# Quicksort Assignment For my CS minor I was assigned to implement the Quicksort algorithm in C#. I gave it a go and found that I like the implementation of the algorithm more than Mergesort. Now that I've got my Quicksort implementation running, I'm very curious about what optimizations could be done.

using System.Collections.Generic;

namespace Assignment
{
    public class QuickSort
    {
        public void Sort(List<int> list)
        {
            if (list.Count <= 1)
            {
                return;
            }

            int pivot = MedianOfThree(list);
            int equalElementCount = 0;

            List<int> smaller = new List<int>();
            List<int> bigger = new List<int>();

            for (int i = 0; i < list.Count; i++)
            {
                if (list[i] == pivot)
                {
                    equalElementCount++;
                }
                else if (list[i] < pivot)
                {
                    smaller.Add(list[i]);
                }
                else
                {
                    bigger.Add(list[i]);
                }
            }

            Sort(smaller);
            Sort(bigger);

            int pointer = 0;
            for (int i = 0; i < smaller.Count; i++)
            {
                list[pointer++] = smaller[i];
            }
            for (int i = 0; i < equalElementCount; i++)
            {
                list[pointer++] = pivot;
            }
            for (int i = 0; i < bigger.Count; i++)
            {
                list[pointer++] = bigger[i];
            }
        }

        private int MedianOfThree(List<int> list)
        {
            int firstElement = list[0];
            int middleElement = list[list.Count / 2];
            int lastElement = list[list.Count - 1];

            if ((firstElement > middleElement) != (firstElement > lastElement))
            {
                return firstElement;
            }
            else if ((middleElement > firstElement) != (middleElement > lastElement))
            {
                return middleElement;
            }
            else
            {
                return lastElement;
            }
        }
    }
}

Over the years I have seen other students here and they are quite proud of the performance of their code.  In some of those past instances I have discovered that they had simply run their code and thought it was fast.  They did not compare it to other methods.

You don't mention any testing or provide any numbers.  If you were to do this, just be sure that (1) your code is a Release build and not a Debug, and (2) that you do have disabled "Prefer 32-bit" for Build.

The biggest thing that strikes me about your implementation versus others I have seen is that you make 2 lists (smaller and bigger) per invocation whereas other implementations perform an in-place sort on the list or array.

Does this matter?

YES.  Performance is significantly faster when performing in-place sorting using the one list.

I wrote a sample for you.  I have decide not to post my code here because you are a CS student and your namespace is Assignment which tells me this is classwork.  It would be unethical of me to write this code for you.  What I will do instead you point you to the resources I used, and show my performance timings.

My reference was Wikipedia Quicksort.  In particular I used the Hoare partition scheme.  I translated the psuedocode to C#.

I will share key method signatures.  Yes, plural.

public static void Sort2(List<int> list) => Sort2(list, 0, list.Count - 1);

private static void Sort2(List<int> list, int lowIndex, int highIndex)
{
    // Good luck on your assignment!
}

I also had a Partition and Swap method.  I leave it to you to decide whether these should be public or private and what they do.

PERFORMANCE

Size of list: 10_000
Number of trials: 10_000
Original QuickSort ElapsedTicks: 117_816_477
In-place QuickSort ElapsedTicks:  50_171_790

Which means your original is 2.3 times slower than my in-place sorting.


Kolmogorov-Smirnov function I'm trying to compare two lists of floating point data and see if they might have come from different underlying distributions. The two samples are a before and after, so by comparing the two I thought I could detect whether any change occurred between the two timeframes.

To do this I'm using the two-sample Kolmogorov-Smirnov test. I have the following function which calculates the core statistic used in the test:

def kolmogorov_smirnov(data1, data2):
    """
    Given two lists of data, finds the two-sample Kolmogorov–Smirnov statistic
    """
    data1 = sorted(data1)
    data2 = sorted(data2)

    index1 = 0
    index2 = 0

    ks_stat = 0

    while index1 < len(data1) and index2 < len(data2):
        if data1[index1] == data2[index2]:
            index1 += 1
            index2 += 1

        elif data1[index1] < data2[index2]:
            index1 += 1

        elif data1[index1] > data2[index2]:
            index2 += 1

        ks_stat = max(ks_stat, abs(index1/len(data1) - index2/len(data2)))

    return ks_stat

I realise that I can also shorten the while loop like so:

while index1 < len(data1) and index2 < len(data2):
    if data1[index1] <= data2[index2]:
        index1 += 1

    if data1[index1] >= data2[index2]:
        index2 += 1

    ks_stat = max(ks_stat, abs(index1/len(data1) - index2/len(data2)))

Which version should I use? Also, is there anything worth pointing out about the main code?

The shorter version seemed better to me at first sight (because it is simpler), but actually it is incorrect. Both if statements intend to compare the same two values, but incrementing index1 changes data1[index1] for the second statement. You could fix this by assigning the values to variables:

while index1 < len(data1) and index2 < len(data2):
    value1, value2 = data1[index1], data2[index2]
    if value1 <= value2:
        index1 += 1
    if value1 >= value2:
        index2 += 1

Updating ks_stat one value at a time feels a bit awkward to me. You could collect all the absolute differences in a list and take max() of it in the end. Or, extract the loop into a generator to avoid the list.


Find all row combinations which sum elements equal to any given number It works fine, but it is slow.

Could anybody help make this faster?

import itertools

from decorators import timer
from settings import MENU, CASH

class Cocktail(object):
    def __init__(self, group, name, sell, count=0):
        self.group = group
        self.name = name
        self.sell = sell
        self.count = count

class Check(object):
    def __init__(self, cash, menu=MENU):
        self.__cash = cash
        self.__cheapest_cocktail = 10000
        self.__menu = self.__read_menu(menu)
        self.__matrix = self.__create_matrix()
        self.correct = []

    def __read_menu(self, menu):
        result = []
        for group in menu:
            key = group.keys()[0]
            for cocktail in group[key]:
                if self.__cheapest_cocktail > cocktail['sell']:
                    self.__cheapest_cocktail = cocktail['sell']

                result.append(Cocktail(
                    key,
                    cocktail['name'],
                    cocktail['sell'],
                ))

        return result

    def __create_matrix(self):
        result = []
        max_count = self.__cash // self.__cheapest_cocktail
        for cocktail in self.__menu:
            row = []
            for i in range(0, max_count):
                row.append(Cocktail(
                    cocktail.group,
                    cocktail.name,
                    cocktail.sell,
                    i
                ))

            result.append(row)
        return result

    def find_combinations(self):
        for check in itertools.product(*self.__matrix):
            if sum([(c.sell * c.count) for c in check]) == self.__cash:
                self.correct.append(check)

check = Check(CASH)
check.find_combinations()
check.__matrix size 80x25

Take a look at Python Performace Tips and search for list comprehension. As far as I can understand your code you can use it.

An example would be this function.

def __create_matrix(self):
    max_count = self.__cash // self.__cheapest_cocktail
    return [ [Cocktail(cocktail.group,cocktail.name,cocktail.sell,i)
         for i in xrange(0, max_count)] for cocktail in self._menu]

You don't need to create a list if you are not using it elsewhere. In the find_combinations function you can avoid the overhead of list creation because you are only interested in the sum of the elements.

def find_combinations(self):
    for check in itertools.product(*self.__matrix):
        if sum((c.sell * c.count) for c in check) == self.__cash:
            self.correct.append(check)

I think there is a bug in your code. You are using a classCocktail but using cocktail in your code. I understand they are different but I think you have messed up C with c in your code in the __read_menu function. It is confusing whether or not you have messed it up.

That is all I can think of because I don't understand what the code is doing unless. Please edit your question to add sample input and output.


Handler for an Android form to calculate a cross product Button crossButton = (Button) findViewById(R.id.crossButton);
crossButton.setOnClickListener(new View.OnClickListener() {
    @Override
    public void onClick(View view) {
        if (!x1.getText().toString().equals("")) {
            if (!x2.getText().toString().equals("")) {
                if (!x3.getText().toString().equals("")) {
                    if (!y1.getText().toString().equals("")) {
                        if (!y2.getText().toString().equals("")) {
                            if (!y3.getText().toString().equals("")) {
                                double x1no = Double.parseDouble(x1.getText().toString());
                                double x2no = Double.parseDouble(x2.getText().toString());
                                double x3no = Double.parseDouble(x3.getText().toString());
                                double y1no = Double.parseDouble(y1.getText().toString());
                                double y2no = Double.parseDouble(y2.getText().toString());
                                double y3no = Double.parseDouble(y3.getText().toString());
                                double z1 = (x2no * y3no) - (x3no * y2no);
                                double z2 = (x3no * y1no) - (x1no * y3no);
                                double z3 = (x1no * y2no) - (x2no * y1no);
                                resultText.setText(getString(R.string.decimal_vector_output, z1, z2, z3));
                            } else { y3.setError("Needs value"); y3.requestFocus(); }
                        } else { y2.setError("Needs value"); y2.requestFocus(); }
                    } else { y1.setError("Needs value"); y1.requestFocus(); }
                } else { x3.setError("Needs value"); x3.requestFocus(); }
            } else { x2.setError("Needs value"); x2.requestFocus(); }
        } else { x1.setError("Needs value"); x1.requestFocus(); }
    }
});

How would I go about cleaning up these if statements and whatnot?

I've tried a few OR conditions and it seemed to break.

Help would be appeciated.

I expect x/y 1-3 are textfields, though not enough context to be entirely certain, going from that assumption:

Create a helper method to check for value:

private boolean hasValue(TextField field) {
    return !field.getText().toString().isEmpty();
}

Store all textfields in an array.
Loop through and validate each one.

for (TextField field : textFields) {
   if (!hasValue(field)) {
      field.setError("needs value");
      field.requestFocus();
   }
}

Depending on your needs, I would apply this to use break, and perhaps apply it as a boolean returning method that ensures a validated state before continuing application logic.

Once the validation is done and completed (why a boolean returning method for step 3 could be helpful), proceed with the application logic.


Using the Rotten Tomatoes API This is a Python module I made to search the Rotten Tomatoes movie API.  It caches the search results in a SQLite database.

What improvements can I make to the cache system?  Is there a better way to store the results of the API search?

Is there a better way to communicate with the API?

import urllib2
import json
import sqlite3
import time

# sqlite database file
cache_database = "movies.db"
# how many seconds before the entry expires
cache_expiration = 60*60  # one hour

class Cache:
    def get_conn(self):
        """
        connect with the sqlite database
        """
        conn = sqlite3.connect(cache_database)
        c = conn.cursor()
        c.execute(""" CREATE TABLE IF NOT EXISTS movies(
                        search_query TEXT PRIMARY KEY,
                        page_number INT,
                        timestamp INTEGER,
                        search_results BLOB); """)
        return conn

    def get(self, search_query, page_number):
        """
        get the search results from the database
        """
        with self.get_conn() as conn:
            c = conn.cursor()
            query = """SELECT search_results FROM movies WHERE
                       search_query = \"{search_query}\" AND
                       page_number = {page_number} AND
                      (strftime('%s', 'now') - timestamp) < {cache_expiration}"""

            query = query.format(search_query = search_query,
                                 page_number = page_number,
                                 cache_expiration = cache_expiration)
            results = ""
            for a in c.execute(query):
                results = a
            return results

    def put(self, search_query, page_number, search_results):
        """
        put the results into the database
        """
        timestamp = int( time.time() )
        with self.get_conn() as conn:
            c = conn.cursor()
            insert = """ INSERT OR REPLACE INTO movies
                         (search_query, page_number, timestamp, search_results) VALUES
                        (\"{search_query}\", {page_number}, {timestamp}, ?); """

            insert = insert.format(search_query = search_query,
                                   page_number = page_number,
                                   timestamp = timestamp)
            c.execute(insert, (search_results,))
            conn.commit()

class Movie:

    api_key = ""
    userAgent = "MovieInfoBot/1.0"

    def search(self, query, results_per_page=25, page_number=1):
        """
        searches for movies: movie name, result limit, page number
        """
        cache = Cache()
        result = cache.get(query, page_number)
        if result != "":
            print "using cache"
            movie_json = result[0]
        else:
            print "using web"
            #format the url
            base_url = "http://api.rottentomatoes.com/api/public/v1.0/movies.json"
            url = "{base_url}?apikey={api_key}&q={search_term}&page_limit={results_per_page}&page={page_number}"
            param = {}
            param["base_url"] = base_url
            param["api_key"] = self.api_key
            param["search_term"] = urllib2.quote(query.encode("utf8"))
            param["results_per_page"] = results_per_page
            param["page_number"] = page_number
            url = url.format(**param)
            req = urllib2.Request(url, headers={ 'User-Agent': self.userAgent })
            movie_json = urllib2.urlopen(req).read()

            # put the results into the movie cache
            cache.put(query, page_number, movie_json)

        movie_dict = json.loads(movie_json)
        return movie_dict

movie = Movie()
movie.api_key = "xua5v8mbvermd2t9v5gyeaua"
data = movie.search("the wizard of oz")

print json.dumps(data, indent=4)

Here is the module on GitHub https://github.com/kylelk/Rotten-Tomatoes

It's a pretty nice script! (You should open-source it, I would use it.)

Prepared statements

You should use prepared statements in your queries. It's not only easier to write, it's also more efficient. For example:

def get(self, search_query, page_number):
    with self.get_conn() as conn:
        c = conn.cursor()
        query = """SELECT search_results FROM movies
                   WHERE search_query = ?
                   AND page_number = ?
                   AND strftime('%s', 'now') - timestamp < ?; """

        c.execute(query, (search_query, page_number, cache_expiration))
        return c.fetchone()

Notice that you don't need to worry about quoting anymore.

You can do the same thing to your Cache.put method too:

def put(self, search_query, page_number, search_results):
    timestamp = int(time.time())
    with self.get_conn() as conn:
        c = conn.cursor()
        insert = """INSERT OR REPLACE INTO movies
                    (search_query, page_number, timestamp, search_results)
                    VALUES (?, ?, ?, ?); """

        c.execute(insert, (search_query, page_number, timestamp, search_results,))
        conn.commit()

Statements can be simplified

Instead of this:

results = ""
for a in c.execute(query):
    results = a
return results

It would be simpler and cleaner like this:

c.execute(query)
return c.fetchone()

Unless, you really want to iterate over multiple results and return only the last one, as it was in your original code.

Python classes

Modern classes should extend object, for example:

class Cache(object):
    ...

class Movie(object):
    ...

Coding style

You can simplify this:

if result != "":
    ...

like this:

if result:
    ...

And it's good to follow PEP8, the official Python style guide.


PHP upload to database I finally got my upload form and code that uploads to db. I used tutorial from this site.

As I am php newbie, I want to know how secure this code is and if it is not secure at all (which I am almost sure of...) could someone tell me how to make it secure. This upload should be only for pdf, jpeg, jpg, doc files. Also this upload will be available to public.

<form method="post" enctype="multipart/form-data">

please select a file

<input type="hidden" name="MAX_FILE_SIZE"
value="16000000">
<input name="userfile" type="file" id="userfile">

<input name="upload"
type="submit" class="box" id="upload" value=" Upload ">

</form>

<?php
if(isset($_POST['upload'])&&$_FILES['userfile']['size']>0)
{
$fileName = $_FILES['userfile']['name'];
$tmpName  = $_FILES['userfile']['tmp_name'];
$fileSize = $_FILES['userfile']['size'];
$fileType = $_FILES['userfile']['type'];
$fileType=(get_magic_quotes_gpc()==0 ? mysql_real_escape_string(
$_FILES['userfile']['type']) : mysql_real_escape_string(
stripslashes ($_FILES['userfile'])));
$fp      = fopen($tmpName, 'r');
$content = fread($fp, filesize($tmpName));
$content = addslashes($content);
fclose($fp);
if(!get_magic_quotes_gpc())
{
    $fileName = addslashes($fileName);
}
$con = mysql_connect('localhost', 'root', 'root') or die(mysql_error());
$db = mysql_select_db('olearyinternational', $con);
if($db){
$query = "INSERT INTO upload (name, size, type, content ) ".
"VALUES ('$fileName', '$fileSize', '$fileType', '$content')";
mysql_query($query) or die('Error, query failed');
mysql_close();
echo "
File $fileName uploaded
";
}else { echo "file upload failed"; }
}
?>

Restrict file types:
As your concern is security, you should make sure that the file type is one of those you allow.
For that you must implement a file type checker, that validates file type is allowed, then you let the user upload the file. Learn how to do that here or at stackOverflow.
Upload Size  You may like to have max upload size, so users do not upload too large files. Find how to do that at phpmaster.
Check the referrer: Check to make sure that the information being sent to your script is from your website and not an outside source. While this information can be faked, it's still a good idea to check.
Rename files: You can rename the files that are uploaded. In doing so, check for double-barreld extensions like yourfile.php.gif and eliminate extensions you don't allow, or remove the file completely.
Change permissions: Change the permissions on the upload folder so that files within it are not executable. Your FTP program probably allows you to chmod right from it.
Login and Moderate: Making your users login might deter some deviant behavior. You can also take the time to moderate all file uploads before allowing them to become live on the web.
Keep away from root: If your document root is /var/www/html, create a directory /var/www/uploads and use it to store uploaded files. That way, an attacker will not be able to retrieve the file directly. This will allow you to provide fine grained access control. The file will not be parsed by the server's application language module but the source of the file will be streamed.
Malware Scan:
The extension is right, and you checked that the file is actually a valid JPEG file per it's header. However, it could still be a malicious JPEG using one of the many image parser bugs to exploit clients downloading the file. There is no great defense against this as far as I am aware. One possible work around is to "rebuild" the file. Convert the JPEG to a GIF and back to a JPEG. This will likely strip out any malicious feature. But this technique could expose your servers to just the same image parser bugs.

Here are some links to help,

http://software-security.sans.org/blog/2009/12/28/8-basic-rules-to-implement-secure-file-uploads/

http://webcheatsheet.com/PHP/file_upload.php


Longest substring with 2 unique characters in a given string at linear time This is my java implementation based on the solution of Aasmund Eldhuset here.
Please give your opinion on my code (correctness, efficiency and coding conventions):

public static String longest2Str(String s){
    int len;
    int i = 1;
    int subStart = 0;
    int nextGroupStart = 0;
    int longestStart = 0;
    int longestEnd = 0;
    char firstUnique, secondUnique, c;

    if ( (s == null) || (s.length() < 2) ) {
        return null;
    }

    len = s.length();
    firstUnique = s.charAt(subStart);

    // find first letter in s that isn't equal to the firstUnique
    for (; (i < len) && (s.charAt(i) == firstUnique); i++);

    secondUnique = s.charAt(i);
    for (; i < len; i++) {
        c = s.charAt(i);
        if ( (c == firstUnique) || (c == secondUnique) ) {
            if (c != s.charAt(i - 1)) {
                nextGroupStart = i;
            }
        }
        else {
            if ( i - subStart > longestEnd - longestStart ){
                longestStart = subStart;
                longestEnd = i;
            }
            subStart = nextGroupStart;
            nextGroupStart = i;
            firstUnique = s.charAt(subStart);
            secondUnique = c;
        }
    }

    return s.substring(longestStart, longestEnd);
}

Corner cases

You correctly handle the corner cases of too short input and null input.
But you forgot to handle the case of all same characters, for example "aaa".
On such input the program throws StringIndexOutOfBoundsException on line secondUnique = s.charAt(i);.

Variable scope

The variable c is only needed within the main for loop,
so it would be best to declare it inside the loop.

Style

This is fine:

for (; (i < len) && (s.charAt(i) == firstUnique); i++);

But it looks a bit strange.
A for loop is easiest to read in its most classing form a counting loop:
it performs some initialization,
then on each iteration it does its main work in the body.
Here, the main work to do is the counting.
So if you transform this to a while loop with a body,
it becomes slightly easier to read.


Multiple values against same condition Is there any other way to optimize the below code? I feel the below code is huge for the operations it performs.

{
    if ((currentElement == null ||
        (firstGridRow["Low"].ToString() == string.Empty ||
        firstGridRow["High"].ToString() == string.Empty ||
        firstGridRow["Mean"].ToString() == string.Empty ||
        firstGridRow["StdDev"].ToString() == string.Empty)))
    {
        continue;
    }

    double currentLow = Convert.ToDouble(firstGridRow["Low"]);
    double currentHigh = Convert.ToDouble(firstGridRow["High"]);
    double currentMean = Convert.ToDouble(firstGridRow["Mean"]);
    double currentStdDev = Convert.ToDouble(firstGridRow["StdDev"]);
    if (newRow.Length != 0)
    {
        AddColorList(currentElement, opid, currentLow, "Low", newRow, listCollectionLow);
        AddColorList(currentElement, opid, currentHigh, "High", newRow, listCollectionHigh);
        AddColorList(currentElement, opid, currentMean, "Mean", newRow, listCollectionMean);
        AddColorList(currentElement, opid, currentStdDev, "StdDev", newRow, listCollectionStdDev);
    }
}

So I'll preface this with it's a little difficult without the actual context of the code, and that all suggestions need to be checked to ensure they don't break the existing flow of control.

A few options:

Breaking out repetitive operations into reusable methods
Wrapper methods (within reason) to apply some common operations and call some other method

With the assumption that these are inside of a loop, and you're not relying on an exception from the Convert.ToDouble() call to break out then you might consider some approaches like follows.

Simplified from provided example. Assuming additional trailing '}' is not significant.

if (currentElement != null
    && !RowHasEmptyCells(firstGridRow, "Low", "High", "Mean", "StdDev")
    && newRow.Lenth != 0)

{
    AddColourListWithSourceValue(firstGridRow, newRow, "Low", currentElement, opid, listCollectionLow);
    AddColourListWithSourceValue(firstGridRow, newRow, "High", currentElement, opid, listCollectionHigh);
    AddColourListWithSourceValue(firstGridRow, newRow, "Mean", currentElement, opid, listCollectionMean);
    AddColourListWithSourceValue(firstGridRow, newRow, "StdDev", currentElement, opid, listCollectionStdDev);
}

Functions attempting to contain some of the previously duplicated logic:

//return true if any of the named cells in the row are 'empty'
bool RowHasEmptyCells(RowType row, params string[] cellNames)
{
    return cellNames.Any(cellName => row[cellName].ToString() == string.Empty);
}

//RowType is the type of your row object (DataGridRow?)
void AddColourListWithSourceValue(RowType sourceRow, RowType newRow, string cellName, object currentElement, object opid, object listCollection)
{
    double currentValue = Convert.ToDouble(sourceRow[cellName]);
    AddColourList(currentElement, opid, currentValue, cellName, newRow, listCollection);
}

For the above double.Parse or .TryParse might be the way to go for a string > double conversion. If using Parse you could remove the empty cells check and just catch a FormatException/ArgumentNullException

This can likely be improved upon, but I've tried to make the flow of the main part a bit easier to follow whilst keeping roughly the same logic.

Edit 1: Sorry - fixed typo. Also AddColourListWithSourceValue() is just a guess at what's going on :)

Edit 2:  Quick edit with @svick's correction on logic and suggestion for LINQ to check for empty. Inverted to allow for the positive method prefix 'Has' instead of a 'CellsNotEmpty'. Would welcome thoughts on !IsSomething() vs IsNotSomething()...

Edit 3: != to == in RowHasEmptyCells() as pointed out by @svick


Counting pairs of numbers that differ by a given amount I am trying to do these online competitions where you get the given exercise and are supposed to upload the source file.
The site is not in English so I will try to summarize the exercise.

The first line of input states P, the number of problems ( 1 <= P <= 100 ). Subsequently, there are two lines for each problem. The first contains numbers X and Y ( 1 <= X, Y <= 100000) — X being number of elements and Y being the difference I am looking for. On the next line there are X numbers (0 <= N <= 100000) divided by a space. For every problem, I am supposed to print the number of pairs that differ by Y.

As an example input :

2
5 2
1 5 3 4 2
5 4
6 3 6 2 2

Example output:

3
4

I wrote a simple programme that did the thing but when I uploaded the source file I got "time limit exceeded" error. After that I discovered that my program is unable to handle big input. I tried to move on to the next exercise but I discovered almost all of them have big inputs. I tried to improve it myself, then I tried to google it, but sadly I still cannot get past the error. This is the code I ended up with :

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

#define SIZE 80000

int compare(const void *a, const void *b);

int main(int argc, char const *argv[]) {
  char buffer[SIZE];
  unsigned int numberOfGames, numberOfStudents, wantedDifference;
  unsigned int numberOfDifferentPairs;
  unsigned int counter;
  size_t i, j;

  scanf("%u", &numberOfGames);

  while (numberOfGames--) {
    scanf("%u %u", &numberOfStudents, &wantedDifference);
    scanf("\n");
    unsigned long long int *studentsHeights =
        (unsigned long long int *)malloc(sizeof(unsigned long long int) * numberOfStudents);
    counter = 0;
    numberOfDifferentPairs = 0;
    studentsHeights[counter] = 0;
    while (counter < numberOfStudents) {
      fgets(buffer, SIZE, stdin);
      size_t stringSize = strlen(buffer);
      for (i = 0; i < stringSize; i++) {
        if (buffer[i] == ' ' || buffer[i] == EOF || buffer[i] == '\n') {
          counter++;
          if (counter < numberOfStudents)
            studentsHeights[counter] = 0;
        } else {
          studentsHeights[counter] =
              studentsHeights[counter] * 10 + (buffer[i] - '0');
        }
      }
    }
    qsort(studentsHeights, numberOfStudents, sizeof(unsigned int), compare);
    for (i = 0; i < numberOfStudents - 1; i++) {
      unsigned int lastAddition;
      if (studentsHeights[i] == studentsHeights[i - 1]) {
        numberOfDifferentPairs += lastAddition;
        continue;
      } else {
        lastAddition = 0;
      }
      for (j = i + 1; j < numberOfStudents; j++) {
        if ((studentsHeights[j] - studentsHeights[i]) == wantedDifference) {
          numberOfDifferentPairs++;
          lastAddition++;
        }
      }
    }
    printf("%u\n", numberOfDifferentPairs);
  }
  return 0;
}

int compare(const void *a, const void *b) {
  if (*(unsigned int *)a < *(unsigned int *)b)
    return -1;
  else if (*(unsigned int *)a == *(unsigned int *)b)
    return 0;
  else
    return 1;
}

I know I am probably doing some trivial mistake but if you could point it out for me I would be really grateful.

There are the variables I used:

P = numberOfGames
X = numberOfStudents
Y = wantedDifference
N = studentsHeights

Your solution is \$O(X^2)\$ which is too slow for \$X\approx 10^5\$.

Instead of checking differences with two nested loops, you should just loop over the first element a and perform binary search to find the number of elements of the form a+Y. This will reduce the complexity to \$O(X \log X)\$ which will be fast enough.


Simple repository for a small/medium sized ASP.NET MVC site I am about to start a small/medium sized project. I am by no means a software architect. But I tend to question every move I make at times, since I want to do things correct.

I found a way to implement a simple repository, and I wanted to know if this is a "correct" way of doing it. I came to this solution, since I know what is going on, and not taking in something to complex before I have the knowledge.

Unit of work

Where I make sure I to keep all my repositories under the same dbcontext. In my UOW I can access all repos when calling it from the controller.

public class UnitOfWork : IDisposable
{
    private ContactRepository _contactRepo;
    private ApplicationDbContext _entities;

    public UnitOfWork(ApplicationDbContext entities)
    {
        _entities = entities;
    }

    public ContactRepository ContactRepo
    {
        get
        {
            if (_contactRepo == null)
            {
                _contactRepo = new ContactRepository(_entities);
            }
            return _contactRepo;
        }
    }

    public void Save()
    {
        _entities.SaveChanges();
    }

    private bool disposed = false;

    protected virtual void Dispose(bool disposing)
    {
        if (!this.disposed)
        {
            if (disposing)
            {
                _entities.Dispose();
            }
        }
        this.disposed = true;
    }

    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }
}

ContactRepository

This is a short example of a repository, where it receives the dbcontext and uses it to grab whatever data I want.

   public class ContactRepository
   {
    private ApplicationDbContext _entities;

    public ContactRepository(ApplicationDbContext entities)
    {
        _entities = entities;
    }

    public IEnumerable<Contact> GetAll()
    {
        return _entities.Contacts;
    }
}

Controller

Short example of controller

public class ContactController : Controller
{

    UnitOfWork uow = new UnitOfWork(new ApplicationDbContext());

    public ActionResult Index()
    {
        var contacts = uow.ContactRepo.GetAll();
        return View(contacts);
    }

    protected override void Dispose(bool disposing)
    {
        uow.Dispose();
        base.Dispose(disposing);
    }
}

In this way I will have access to all my repositories under the same dbcontext which I was aiming for.

I know things can be done smarter/different. With for example a extendable generic repo. But in this case I am aiming for something simple and understandable. But still don't want to make a huge mistake, if there is a major flaw.

Do you see any major flaws with this way of handling data through entity framework?

Keep your disposables short lived and you'll never have to implement IDisposable yourself.

In other words, avoid keeping disposable objects as members. Always try to wrap them in a using statement. If you do that, you'll find that your Unit of Work doesn't need to be disposable.

I can't ignore the obvious here though. There is no reason for any of this code to exist. It literally does nothing but pass through to the DbContext. Entity Framework is already a repository & unit of work.

That's not to say that you shouldn't hide the fact that you're using EF from your controller though. It's much better to create a domain specific API to decouple your controller from the persistence. In our projects we'll often have things like FooService.GetByName(string name) and FooService.GetMostRecent(int numberToRetrieve) kinds of methods. It makes for pleasant reading and easy implementation changes.


Timer with interval function I've been tasked with writing a function that executes a passed function every interval, passed to the function by the user. Below is the code, and how to call it:
import time
from typing import Callable

last_milliseconds: int = 0
def set_timer(interval: int, function: Callable) -> bool:
    global last_milliseconds
    milliseconds: int = int(round(time.time() * 1000))
    done: bool = (milliseconds - last_milliseconds) >= interval
    if done:
        function()
        last_milliseconds = milliseconds

Called by:
p: Callable = lambda: print("set_timer called.")

while True:
    set_timer(1000, p)

Is there a better way to go about this? I'm unsure with the use of the global variable, and the function must be called in an infinite loop structure in order to work. Any and all feedback is accepted and appreciated.

First of all, your function doesn't return anything and so you can remove -> bool since it only creates confusion. You can either use -> None or use nothing at all. Still related to type annotations, I find it kinda hard to read your code with annotations everywhere. This is subjective and it's just my personal preference but I usually tend to use type annotations only when dealing with function / class definitions and their arguments.
For example, milliseconds: int = int(round(time.time() * 1000)) it's obviously going to be an integer since we can see the cast to int().
More, round() already returns an int in your case so that can be removed as well. The done variable can also be omitted and added directly to the condition itself:
def set_timer(interval: int, function: Callable) -> None:
    global last_milliseconds
    milliseconds = round(time.time() * 1000)

    if milliseconds - last_milliseconds >= interval:
        function()
        last_milliseconds = milliseconds

You can also remove the parentheses from your condition since - has a higher precedence than >=.

Strictly speaking about the implementation, if you're not tied to using a function and want to get rid of the usage of global, I'd use a class:
from time import time
from typing import Callable

class Timer:
    def __init__(self):
        self.last_milliseconds = 0

    def set(self, interval: int, function: Callable) -> None:
        milliseconds = round(time() * 1000)

        if milliseconds - self.last_milliseconds >= interval:
            function()
            self.last_milliseconds = milliseconds

Which can be used like this:
def main():
    timer = Timer()
    while True:
        timer.set(1000, lambda: print("set_timer called."))

main()

Or, even better, you can have a static class if your workflow allows it:
class StaticTimer:
    last_milliseconds = 0

    @classmethod
    def set(cls, interval: int, function: Callable) -> None:
        milliseconds = round(time() * 1000)

        if milliseconds - cls.last_milliseconds >= interval:
            function()
            cls.last_milliseconds = milliseconds

Which you'd call like this:
def main():
    while True:
        StaticTimer.set(1000, lambda: print("set_timer called."))

main()

The difference between the two classes is that you can't have multiple of the latter because of the class attribute and @classmethod. So even if you made multiple instance of the class, they'd all share that value.


Go Back to Previous Item I have a Back function, and it is growing.  I also think this is terrible, but I don't seem to get how it can be improved:

public void GoBack()
{
    NavButtonUsed = true;

    Forward.Insert(0, Back[0]);
    Back.RemoveAt(0);

    if (Back[0].Title.StartsWith(" ") && CurrentItem.Menu != Back[0].Menu)
    {
        switch (Back[0].Menu)
        {
            case Menus.WSOneNote:
                CurrentItem = ItemList[0];
                break;

            case Menus.WSMainMenu:
                CurrentItem = ItemList[ItemList.IndexOf(MenuItem.CreateMenuItem(resourceFile.GetString("MainMenu"), typeof(WindowsData.MainMenu), Menus.WSMainMenu))];
                break;

            case Menus.WSTextMenu:
                CurrentItem = ItemList[ItemList.IndexOf(MenuItem.CreateMenuItem(resourceFile.GetString("TextMenu"), typeof(WindowsData.TextMenu), Menus.WSTextMenu))];
                break;

            case Menus.WSTextBlockMenu:
                CurrentItem = ItemList[ItemList.IndexOf(MenuItem.CreateMenuItem(resourceFile.GetString("TextBlockMenu"), typeof(WindowsData.TextBlockMenu), Menus.WSTextBlockMenu))];
                break;

            case Menus.WSTableMenu:
                CurrentItem = ItemList[ItemList.IndexOf(MenuItem.CreateMenuItem(resourceFile.GetString("TableMenu"), typeof(WindowsData.TableMenu), Menus.WSTableMenu))];
                break;

            case Menus.WSTableCellsMenu:
                CurrentItem = ItemList[ItemList.IndexOf(MenuItem.CreateMenuItem(resourceFile.GetString("TableCellsMenu"), typeof(WindowsData.TableCellsMenu), Menus.WSTableCellsMenu))];
                break;

            case Menus.WSDrawMenu:
                CurrentItem = ItemList[ItemList.IndexOf(MenuItem.CreateMenuItem(resourceFile.GetString("DrawMenu"), typeof(WindowsData.DrawMenu), Menus.WSDrawMenu))];
                break;

            case Menus.WSDrawnItemsMenu:
                CurrentItem = ItemList[ItemList.IndexOf(MenuItem.CreateMenuItem(resourceFile.GetString("DrawnItemsMenu"), typeof(WindowsData.DrawnItemsMenu), Menus.WSDrawnItemsMenu))];
                break;

            case Menus.WSPictureMenu:
                CurrentItem = ItemList[ItemList.IndexOf(MenuItem.CreateMenuItem(resourceFile.GetString("PictureMenu"), typeof(WindowsData.PictureMenu), Menus.WSPictureMenu))];
                break;

            case Menus.WSFileMenu:
                CurrentItem = ItemList[ItemList.IndexOf(MenuItem.CreateMenuItem(resourceFile.GetString("FileMenu"), typeof(WindowsData.File), Menus.WSFileMenu))];
                break;

            case Menus.WSAppBarsMenu:
                CurrentItem = ItemList[ItemList.IndexOf(MenuItem.CreateMenuItem(resourceFile.GetString("AppBarsMenu"), typeof(WindowsData.AppBar), Menus.WSAppBarsMenu))];
                break;

            case Menus.WSCharmsMenu:
                CurrentItem = ItemList[ItemList.IndexOf(MenuItem.CreateMenuItem(resourceFile.GetString("CharmsMenu"), typeof(WindowsData.Charms), Menus.WSCharmsMenu))];
                break;

            default:
                break;
        }
    }

    CurrentItem = ItemList[ItemList.IndexOf(Back[0])];
    NavButtonUsed = false;
}

I have more menus to account for, but I haven't implemented going back to them yet as I do not have them fully implemented.  What is really going on here is I need to programmatically select a menu to open it so I can select a menu item within it.  If I do not open the menu, ItemList will not have CurrentItem, so the app will crash.

This is CurrentItem:

private MenuItem _currentItem = new MenuItem(resourceFile.GetString("OneNote"), typeof(WindowsData.OneNote), Menus.WSOneNote);
public MenuItem CurrentItem
{
    get { return _currentItem; }
    set
    {
        if (value == _currentItem) { return; }
        _currentItem = value;
        OnPropertyChanged();
    }
}

ItemList, Back, and Forward are just ObservableCollections of MenuItems (they can't be Lists because they are bound to XAML objects that need to know when they are updated).

Your GoBack() method could be simplified in its current pattern to

public void GoBack()
{
    NavButtonUsed = true;

    Forward.Insert(0, Back[0]);
    Back.RemoveAt(0);

    CurrentItem = ItemList[ItemList.IndexOf(Back[0])];

    NavButtonUsed = false;
}

The whole switch..case is senseless, because the only thing it is doing is triggering the OnPropertyChanged() event twice.

Inside the switch..case you are assigning a MenuItem to the CurrentItem property (triggering an OnPropertyChanged() event) and you are breaking out of the switch.

Then you are assigning ItemList[ItemList.IndexOf(Back[0])] to the CurrentItem property (triggering an OnPropertyChanged() event), so you are just overwriting the property.

So, if this method works like intended, the switch..case can be removed.


Kattis Problem 'Mali' I wrote a solution to the Mali problem on Kattis:

  Given inputs a1, a2, a3, …, an and b1, b2, b3, …, bn, determine n pairings (ai, bj) such that each number in the A sequence is used in exactly one pairing, each number in the B sequence is used in exactly one pairing, and the maximum of all sums ai + bj is minimal.

  Input

  The first line of input contains a single integer N (1 ≤ N ≤ 100000), the number of rounds.

  The next N lines contain two integers A and B (1 ≤ A, B ≤ 100), the numbers given in that round.

  Output

  The output consists of N lines, one for each round.  Each line should contain the smallest maximal sum for that round.

I'm fairly sure I can get the right answer 100% of the time, but the code exceeds the time limit of one second. I was wondering if you could help me optimize my code to help it run in time, or if you could explain why my code is inefficient.

#include <iostream>
#include <vector>

int main() {
    int x, c = 0, big = 0;
    std::cin >> x;
    std::vector<int> as, bs;
    for (int i = 0; i < x; i++) {
        bool founda = false, foundb = false;
        int a, b;
        std::cin >> a >> b;
        c++;
        if (c == 1) {
            as.push_back(a);
            bs.push_back(b);
        }
        else {
            for (int i = 0; i < c; i++) {
                if (as[i] < a || i == c-1) {
                    as.insert(as.begin()+i, a);
                    founda = true;
                }
                if (bs[i] < b || i == c-1) {
                    bs.insert(bs.begin()+i, b);
                    foundb = true;
                }
                if (founda == true && foundb == true)
                    break;
            }
        }
        for (int i = 0; i < c; i++) {
            if (as[i] + bs[c-1-i] > big)
                big = as[i] + bs[c-1-i];
        }
        std::cout << big << std::endl;
    }
}

You are storing all of the A and B inputs in sorted vectors, basically doing an insertion sort.  A few observations (two minor, one major):

The if (c == 1) special case should be eliminated.
Flag variables (founda and foundb) suck.  You would be better off writing two separate for loops.
Each time you insert a value, all values beyond the insertion point need to be shifted over to make room for the insertion.  That means that constructing as and bs takes O(N2) time altogether — which is unacceptable, since N can be very large.

However, there are only 100 possible values of A and B.  So, do a counting sort instead, building a histogram with 100 buckets.  That would be a much more efficient way to represent the same information, using O(N) time and a fixed amount of space.


Paste from Excel to DataGridView I have this code that helps me to paste data from an Excel file to a Datagridview:

If e.Control AndAlso e.KeyCode = Keys.V Then

    DGVLookUP.Rows.Clear()
    DGVLookUP.Columns.Clear()

    Try
        Dim Generar As Boolean = False
        Dim N As Integer
        For Each line As String In Clipboard.GetText.Split(vbNewLine)
            If Not line.Trim.ToString = "" Then
                Dim item() As String = line.Trim.Split(vbTab)
                If Generar = False Then
                    For C = 0 To UBound(item)
                        DGVLookUP.Columns.Add("C" + CStr(N), CStr(N))
                        N += 1
                    Next
                    Generar = True
                End If
                Me.DGVLookUP.Rows.Add(item)
            End If
        Next

    Catch ex As Exception
        MessageBox.Show(ex.Message, My.Application.Info.Title, MessageBoxButtons.OK, MessageBoxIcon.Error)
    End Try

End If

I am using VB.Net and I am pasting over 3k rows and it takes me between 5-10 mins to paste the info. I would like to optimize my code or if there is another way better to paste the data.

Before we look at how to improve performance, lets start by polishing your code.

Polishing

Dim Generar As Boolean = False
Dim N As Integer

Despite the fact that the .NET naming guidelines doesn't apply for internal and private fields the general consensus is to use English, descriptive and lower Camel case names.

I've renamed the variable and changed the initial value to True as "generate" (IMO) is referring to "columns".

The variable N is useless and should be removed as you already got a variable named c which you could use.

Dim generate As Boolean = True

For Each line As String In Clipboard.GetText.Split(vbNewLine)
    If Not line.Trim.ToString = "" Then
        Dim item() As String = line.Trim.Split(vbTab)

The first line "breaks" my compiler with the error BC30512 and correctly state that "Option Strict On disallows implicit conversions from 'String' to 'Char'". You should always make sure that the strict compiler option is set to On. I'm tempted to say that this is the most important thing you'll learn by reading this review. Oh, wait! I just did! ;)

The constants vbNewLine and vbTab (defined as String) are remnants from the past and should be regarded as obsolete. Many of the methods and classes inside the Microsoft.VisualBasic assembly were added to make the transition from VB6 to VB.NET more convenient. If there's a .NET equivalent, use it! However, in this case, we need a Char array so we create a few Char constants.

The Split method has an overload which allows us to pass an option to remove empty entries. We'll use this and remove the second line. But please note that a better alternative to your boolean expression would be to use the static method String.IsNullOrWhiteSpace. I've also renamed the variable item to values.

Const tab As Char = ChrW(9)
Const lineFeed As Char = ChrW(10)
Const carriageReturn As Char = ChrW(13)

For Each line As String In Clipboard.GetText.Split({lineFeed, carriageReturn}, StringSplitOptions.RemoveEmptyEntries)
    Dim values() As String = line.Trim.Split({tab})

For C = 0 To UBound(item)
    DGVLookUP.Columns.Add("C" + CStr(N), CStr(N))

Again, you're using an obsolete method, UBound. Just read the Length property of the array. (If you need to read the length of a give dimension, invoke GetLength.)

It's advisable to use & for string concatenation. And you ought to take advantage of all of the static methods the string type has to offer. I've also renamed the variable C to  cell.

For cell As Integer = 0 To (values.Length - 1)
    DGVLookUP.Columns.Add(String.Format("C{0}", cell), cell.ToString())

Performance

The complete code at this point looks like below. The great disadvantage with this code is that you add a new row in each iteration. Each call to Add will queue a bunch of messages all of which needs to be processed by the window. Do we really need to repaint the control 3000 times? The answer is of course NO. So we need to fix this.

Const tab As Char = ChrW(9)
Const lineFeed As Char = ChrW(10)
Const carriageReturn As Char = ChrW(13)

Dim generate As Boolean = True

For Each line As String In Clipboard.GetText.Split({lineFeed, carriageReturn}, StringSplitOptions.RemoveEmptyEntries)

    Dim values() As String = line.Trim.Split({tab})

    If (generate) Then
        For cell As Integer = 0 To (values.Length - 1)
            Me.DGVLookUP.Columns.Add(String.Format("C{0}", cell), cell.ToString())
        Next
        generate = False
    End If

    Me.DGVLookUP.Rows.Add(values)

Next

A much better option would be to first read the data into a data structure (we'll be using a DataTable) and then update the grid.

Const tab As Char = ChrW(9)
Const lineFeed As Char = ChrW(10)
Const carriageReturn As Char = ChrW(13)

Dim table As New DataTable()
Dim generate As Boolean = True

table.BeginLoadData()

For Each line As String In Clipboard.GetText.Split({lineFeed, carriageReturn}, StringSplitOptions.RemoveEmptyEntries)

    Dim values() As String = line.Trim.Split({tab})

    If (generate) Then
        For cell As Integer = 0 To (values.Length - 1)
            table.Columns.Add(String.Format("C{0}", cell))
        Next
        generate = False
    End If

    table.Rows.Add(values)

Next

table.EndLoadData()

Me.DGVLookUP.DataSource = table

To further increase the performance we'll need to write our own parser and scrap the split method. You might need to fine tune this if the data inside the excel cell contains a line feed and/or a carriage return.

Const tab As Char = ChrW(9)
Const lineFeed As Char = ChrW(10)
Const carriageReturn As Char = ChrW(13)

Dim data As String = Clipboard.GetText()
Dim table As New DataTable()
Dim generate As Boolean = True
Dim cursor As Integer = 0
Dim position As Integer = 0
Dim cell As Integer = 0
Dim row As DataRow = Nothing

table.BeginLoadData()

While (cursor < data.Length)

    Select Case data.Chars(cursor)

        Case tab, carriageReturn

            If (generate) Then
                table.Columns.Add(String.Format("C{0}", cell), GetType(String)).Caption = cell.ToString()
            End If

            If (cell = 0) Then
                row = table.NewRow()
            End If

            row.Item(cell) = data.Substring(position, (cursor - position))

            cell += 1
            cursor += 1
            position = cursor
            Continue While

        Case lineFeed

            If (Not row Is Nothing) Then
                table.Rows.Add(row)
                row = Nothing
            End If

            cell = 0
            cursor += 1
            position = cursor
            generate = False
            Continue While

        Case Else

            cursor += 1
            Continue While

    End Select

End While

table.EndLoadData()

Me.DGVLookUP.DataSource = table

Test

I've tested this with 3000 rows x 10 columns in release build and as you can see there's a distinct performance boost.

  Paste #1: count= 10, average= 3181.4678 ms
  Paste #2: count= 10, average= 1483.8070 ms
  Paste #3: count= 10, average= 1058.3944 ms

Option Strict On

Imports System.Text

Public Class Form1

    'Copy and paste the text below into each cell of the range A1:J3000
    'Lorem ipsum dolor sit amet, consectetur adipiscing elit. Maecenas in convallis nunc, nec gravida sem. Praesent malesuada dignissim arcu, eu euismod enim. In non neque ut augue sodales dictum eu eget dui. Pellentesque dolor diam, vestibulum sit amet pulvinar et, sollicitudin nec mauris. Vestibulum ante ipsum primis in faucibus orci luctus et ultrices posuere cubilia Curae; Cras sollicitudin metus vitae arcu faucibus, at posuere mauris dictum. Pellentesque in varius sem. Pellentesque molestie varius metus, a dignissim purus vulputate a. Cras venenatis orci et urna efficitur, vel fermentum dui eleifend. Pellentesque et malesuada mauris. Nulla eget nunc et turpis dictum pretium. Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nullam porttitor diam nunc, ut dapibus felis porta at. Nunc non ante sit amet ipsum maximus imperdiet at vitae metus. Aliquam ut lacus in elit laoreet sodales et in eros.

    Private Class TestInfo
        Public Name As String
        Public Method As Action
        Public ReadOnly Results As New List(Of Double)
    End Class

    Private WithEvents input As New DataGridView With
    {
        .Dock = DockStyle.Fill
    }

    Private WithEvents output As New TextBox With
    {
        .Dock = DockStyle.Top,
        .Multiline = True,
        .Height = 100,
        .ReadOnly = True,
        .ScrollBars = ScrollBars.None,
        .Text = "Waiting for input..."
    }

    Private tests As TestInfo() = New TestInfo() _
    {
        New TestInfo With {.Name = "Paste #1", .Method = AddressOf Me.Paste1},
        New TestInfo With {.Name = "Paste #2", .Method = AddressOf Me.Paste2},
        New TestInfo With {.Name = "Paste #3", .Method = AddressOf Me.Paste3}
    }

    Private switch As Integer = 0

    Public Sub New()
        Me.Size = New Size(800, 600)
        Me.Text = "Test"
        Me.KeyPreview = True
        Me.Controls.AddRange({Me.input, Me.output})
    End Sub

    Private Sub HandleKeyDown(sender As Object, e As KeyEventArgs) Handles Me.KeyDown

        If (e.Control AndAlso (e.KeyCode = Keys.V)) Then

            If (Me.input.DataSource Is Nothing) Then
                Me.input.Rows.Clear()
                Me.input.Columns.Clear()
            Else
                Me.input.DataSource = Nothing
            End If

            Dim watch As New Stopwatch()
            Dim info As TestInfo = Me.tests(Me.switch)

            watch.Start()
            info.Method.Invoke()
            watch.Stop()
            info.Results.Add(watch.Elapsed.TotalMilliseconds)

            Dim builder As New StringBuilder()

            For Each info In Me.tests

                If (builder.Length > 0) Then
                    builder.AppendLine()
                End If

                Dim average As Double = 0

                If (info.Results.Count > 0) Then
                    average = info.Results.Average()
                End If

                builder.Append(String.Format("{0}: count= {1}, average= {2:F4} ms", info.Name, info.Results.Count, average))

            Next

            Me.output.Text = builder.ToString()
            Me.switch += 1

            If (Me.switch = Me.tests.Count) Then
                Me.switch = 0
            End If

        End If

    End Sub

    Private Sub Paste1()

        Const tab As Char = ChrW(9)
        Const lineFeed As Char = ChrW(10)
        Const carriageReturn As Char = ChrW(13)

        Dim generate As Boolean = True

        For Each line As String In Clipboard.GetText.Split({lineFeed, carriageReturn}, StringSplitOptions.RemoveEmptyEntries)

            Dim values() As String = line.Trim.Split({tab})

            If (generate) Then
                For cell As Integer = 0 To (values.Length - 1)
                    Me.input.Columns.Add(String.Format("C{0}", cell), cell.ToString())
                Next
                generate = False
            End If

            Me.input.Rows.Add(values)

        Next

    End Sub

    Private Sub Paste2()

        Const tab As Char = ChrW(9)
        Const lineFeed As Char = ChrW(10)
        Const carriageReturn As Char = ChrW(13)

        Dim table As New DataTable()
        Dim generate As Boolean = True

        table.BeginLoadData()

        For Each line As String In Clipboard.GetText.Split({lineFeed, carriageReturn}, StringSplitOptions.RemoveEmptyEntries)

            Dim values() As String = line.Trim.Split({tab})

            If (generate) Then
                For cell As Integer = 0 To (values.Length - 1)
                    table.Columns.Add(String.Format("C{0}", cell))
                Next
                generate = False
            End If

            table.Rows.Add(values)

        Next

        table.EndLoadData()

        Me.input.DataSource = table

    End Sub

    Private Sub Paste3()

        Const tab As Char = ChrW(9)
        Const lineFeed As Char = ChrW(10)
        Const carriageReturn As Char = ChrW(13)

        Dim data As String = Clipboard.GetText()
        Dim table As New DataTable()
        Dim generate As Boolean = True
        Dim cursor As Integer = 0
        Dim position As Integer = 0
        Dim cell As Integer = 0
        Dim row As DataRow = Nothing

        table.BeginLoadData()

        While (cursor < data.Length)

            Select Case data.Chars(cursor)

                Case tab, carriageReturn

                    If (generate) Then
                        table.Columns.Add(String.Format("C{0}", cell), GetType(String)).Caption = cell.ToString()
                    End If

                    If (cell = 0) Then
                        row = table.NewRow()
                    End If

                    row.Item(cell) = data.Substring(position, (cursor - position))

                    cell += 1
                    cursor += 1
                    position = cursor
                    Continue While

                Case lineFeed

                    If (Not row Is Nothing) Then
                        table.Rows.Add(row)
                        row = Nothing
                    End If

                    cell = 0
                    cursor += 1
                    position = cursor
                    generate = False
                    Continue While

                Case Else

                    cursor += 1
                    Continue While

            End Select

        End While

        table.EndLoadData()

        Me.input.DataSource = table

    End Sub

End Class


Chaining Applicatives I wrote this code to append Maybe's:

ghci> (:) <$> Just 5 <*> ((:) <$> Just 100 <*> Just [])
Just [5,100]

Similarly:

ghci> (:) <$> Just 5 <*> ( (:) <$> Just 10 <*> ((:) <$> Just 100 <*> Just []) )
Just [5,10,100]

Is this code idiomatic? Perhaps there's a more concise way to append a bunch of Maybe's together?

Well, while the code surely works, it does look ugly. I would suggest something like this:

sequence [Just 5, Just 10, Just 100]

or, if you really want to use Applicatives instead of Monads,

sequenceA [Just 5, Just 10, Just 100]

from Data.Traversable.


Reuse a base class [Activity] handler in all sub classes for background work We are using the Thread of a common base class but using its handler in all its sub classes. The code is working fine but is it the right way to go about?

Our base class looks like this:

public class BaseActivity extends AppCompatActivity {
    public Handler mHandler;

    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        // Try to reuse the activity handler. If null, create new.
        mHandler = getWindow().getDecorView().getHandler();
        if (mHandler == null) {
            mHandler = new Handler();
            Log.w(TAG, "failed to reuse activity handler");
        }
    }
}

And we are using this instance of handler in all the activities which extend from BaseActivity. We have not faced any issues. No task in the sub classes is very long. Could this be done any better?

You make some assumptions about your window in this line:

mHandler = getWindow().getDecorView().getHandler();

As mentioned by the Activity docs for getWindow()

  Returns
  Window The current window, or null if the activity is not visual

Okay. NullPointerException number 1 waiting to happen. Interestingly #getDecorView() seems to be guaranteed to not return null, which is good. One place less where this can fail.

It is irrelevant whether the returned handler is null or not, because luckily that's what this is about.

Other nitpicks:

Don't use Hungarian notation. mHandler should be just handler. Your IDE is perfectly capable of telling you that handler is a member of type Handler
Failing to reuse an activity handler is not something I'd categorize as Warning.
The comment you place there is partly restating the obvious. Drop the second sentence.


Using a mutex to read from a file I am new to C and I am trying to implement a mutex.

The idea of the program is:

main() will create three threads.
Each thread will read one character each from individual files in serial, i.e. thread 1 will read one character store in global and then wait for thread 2 & 3 to complete the same tasks, until the end of the file.
main() will then write all the data from single character global to output file.

#include <stdio.h>
#include <stdlib.h>
#include <pthread.h>
#include <stdlib.h>
#include <unistd.h>
#include <string.h>
#include <ctype.h>

pthread_t tid[3];
pthread_mutex_t lock;

char characters[1024] = "";

readFromFile(int fileNumber)
{
    pthread_mutex_lock(&lock);

    char* firstPart = "file";
    char *secondPart = ".in";

    char fileN[1];

    sprintf(fileN, "%d", fileNumber);

    char * fileName = malloc(1 + strlen(firstPart) + strlen(fileN) + strlen(secondPart));
    strcpy(fileName, firstPart);
    strcat(fileName, fileN);
    strcat(fileName, secondPart);

    FILE *fp;
    int c;

    if (!(fp = fopen(fileName, "rt")))
    {
         printf("\nError Opening File\n");
         exit(1);
    }
    while ((c = fgetc(fp)) != EOF)
    {
         strcat(characters, c);
    }
   fclose(fp);

   pthread_mutex_unlock(&lock);
}

int main(int argc, char** argv) {
    int i = 0;
    int err;

    if (pthread_mutex_init(&lock, NULL) != 0) {
        printf("\n Mutex Init Failed\n");
        return 1;
    }

    while (i < 3) {
        err = pthread_create(&(tid[i]), NULL, readFromFile(i), NULL);
        if (err != 0) {
            printf("\nCan't Create thread :[%s]", strerror(err));
        }
        i++;
    }

    pthread_join(tid[0], NULL);
    pthread_join(tid[1], NULL);
    pthread_join(tid[2], NULL);
    pthread_mutex_destroy(&lock);

    return 0;
}

Code broken

Your code doesn't work for several reasons:

You are not passing a function to pthread_create():

err = pthread_create(&(tid[i]), NULL, readFromFile(i), NULL);

Here you are calling the function readFromFile() instead of passing a function pointer to pthread_create().  So you aren't using a thread at all.  Worse than that, when readFromFile() returns, a random value will be passed to pthread_create() because readFromFile() doesn't actually return any particular value.  This will cause the newly created thread to crash.
Even had you done that part correctly, your readFromFile() function doesn't do what you wanted.  It looks like you wanted each thread to read one character and then let another thread read the next character.  But what your function actually does is read the whole file instead of one character.
You are calling strcat() with a character as the second argument instead of a string.  This will cause a crash if the program ever reaches that line.

Pay attention to warnings

If you had built with full warnings, you would have been alerted to mistakes #1 and #3 I listed above.

Implementing a mutex?

The title of the question is misleading.  You aren't implementing a mutex.  You are trying to use one.


Project Euler #5 in Ruby with Prime class Question:

  Smallest positive number that is evenly divisible by all of the numbers from 1 to 20?

I tackle the problem with reference to how I solved #3, which is using the prime_division method in the Prime class. I have created an array of prime_divisions for 2..20.

The array looks like this:

[[2, 1], [3, 1], [2, 2], [5, 1], [2, 1], [3, 1], [7, 1], [2, 3], [3, 2], [2, 1], [5, 1], [11, 1], [2, 2], [3, 1], [13, 1], [2, 1], [7, 1], [3, 1], [5, 1], [2, 4], [17, 1], [2, 1], [3, 2], [19, 1], [2, 2], [5, 1]]

And after using the .uniq method and sorting it, it shortens to the following:

[[2, 1], [2, 2], [2, 3], [2, 4], [3, 1], [3, 2], [5, 1], [7, 1], [11, 1], [13, 1], [17, 1], [19, 1]]

which is then easy, I just multiply the highest index for each factor:

\$2^4 \times 3^2  \times 5  \times 7  \times 11  \times 13  \times 17  \times 19 = 232792560\$

At this last stage, the right thing to do would be to multiply the highest power of each factor. I couldn't figure out how to do that in an elegant way.  However, I realised that coincidentally, the number of occurrences of each factor in array b always equals to the highest power. But I would welcome a more elegant solution to this.

require 'prime'

a=[]
c= 1

# Get all prime factors from 2 to 20
(2..20).each do |n|
    a = a.concat(Prime.prime_division(n))
end

# Select only the unique elements and extract the factor, multiply them all
b = a.uniq
b.each do |n|
    if n[0] != 0 || !n[0].nil?
        c = c * n[0]
    end
end

puts c

Shortening

Building arrays by concatenating results to an empty list is inelegant.  You should strive to define the result "all at once":

a = (2..20).flat_map { |n| Prime.prime_division(n) }

Usually, one-blocks should be written using { … }.  Use do … end if it spans multiple lines.

In the second half, n[0] should always be a prime number, so I don't see why it would ever be 0 or nil.  Therefore, you could finish up by writing

puts a.uniq.map { |prime, power| prime }.inject(:*)

Refining

To find the maximum power of each prime, use Enumerable#group_by.

require 'prime'

puts (2..20).flat_map { |n| Prime.prime_division(n) }
            .group_by { |prime, power| prime }.values
            .map { |factors| factors.max_by { |prime, power| power } }
            .map { |prime, power| prime ** power }
            .inject(:*)


Secure random number generator I just wrote this function and I was curious if anyone could find any flaws in it.

It look pretty secure to me but I just want to make sure since I'm no cryptography expert.

function urandom_rand($min, $max) {

    if ($max <= $min) {
        trigger_error('Minimum value must be greater than maximum value.');
    }

    $maxHex = dechex($max);
    $maxHexLength = strlen($maxHex);
    $ivSize = (int)($maxHexLength + ($maxHexLength % 2)) / 2;
    // Reads bytes from /dev/urandom (or its Windows equivalent) - byte size is based on max value
    $r = hexdec(bin2hex(mcrypt_create_iv($ivSize, MCRYPT_DEV_URANDOM)));
    // Min/max modulo conversion to avoid using floats/round which is imprecise and prone to attacks
    $r = (($r - $min) % ($max - $min + 1) + ($max - $min + 1)) % ($max - $min + 1) + $min;
    return $r;
}

function urandom_rand($min = 0, $max = 0x7FFFFFFF)
{
    $min = (int)$min;
    $max = (int)$max;
    if ($max <= $min)
        trigger_error('Minimum value must be greater than maximum value.');
    if ($min < 0 || $max > 0x7FFFFFFF)
        trigger_error('Values must be between 0 and 2147483647.');

    $M = bcadd(0x7FFFFFFF,1); // (up bound of iv)+1
    $N = bcadd($max-$min, 1); // how many different values this function can return
    $h = bcmod($M, $N); // the last h integers from unpack are "invalids"

    do
    {
        $bytes = mcrypt_create_iv(4, MCRYPT_DEV_URANDOM);
        $r = unpack("N", $bytes)[1] & 0x7FFFFFFF;
    } while ($r > ($M-$h));
    return (bcmod($r, $N) + $min);
}

Valid values for $bytes are

0x00000000...0xffffffff

After the unpack(), every value from $bytes is matching exactly one value of:

-0x80000000..0x00000000..0x7ffffffff

So, perfect-equiprobability. After bitwise &, every value from unpack() matches exactly two values of:

0x00000000..0x7ffffffff

Now, we cut that interval into S sub-intervals of [KN..(K+1)N[ (S>=1 because $max<=0x7fffffff) plus one remaining interval I. Then, we map each [KN..(K+1)N[ to [0..N-1].

So far, each value from [0..N-1] has exactly S ways to be picked.

Now, let's deal with the remaining interval I (might be an empty interval). If $r is in that interval, then we cannot do anything with it: we cannot easily match elements from I with elements from [0..N-1] because their size mismatch (I has less elements). We could group elements from [0..N-1] and then match these elements with I but that's far too complex for the earn.

So, if $r is in that remaining I, easy way is just to pick another $r.

Since I has h = (0x7fffffff+1)%N elements, that's why we keep picking random values as long as $r > (0x7fffffff+1)-h.

Only problem you can have is that the function urandom_rand() may take anytime to execute, since it loops until it luckily gets a $r outside I.

Here is one scenario of exploiting the inequiprobability problem:

Let's say you pick a random number out of {0,1,...14}, expecting a probability of p(0)=P(1)=...=p(14)=1/15=6.67%. People can bet on a number, and they will get 14.5x their bet if they win (the 0.5x remaining is your commission).

If you actually have p(0)=7.03% and p(1)=..=p(14)=6.64% then one can bet on 0 and expect 7.03% chances to win 14.5x their bet. So they averagely win 7.03%*14.5=1.01935. Since it's greater than 1, you can keep betting forever and be a winner, despite the casino's commission.

There are probably similar betting game applied to cryptography.

Last correction to my comment about ceil(log(N,k)) (I was tired yesterday night): correct formula is

floor(log(N,k)+1)

gives the number of digits of N in base k for N integer and N>=1

Without using bc function:

function urandom_rand($min = 0, $max = 0x7FFFFFFF)
{
    $min = (int)$min;
    $max = (int)$max;
    if ($max <= $min)
        trigger_error('Minimum value must be greater than maximum value.');
    if ($min < 0 || $max > 0x7FFFFFFF)
        trigger_error('Values must be between 0 and 2147483647.');

    // Decrease everything from 1st version by $N to avoid INT overflow
    // if min == 0 and max == 0x7fffffff then these M,N,h will overflow
    // BUT in such case, MNh won't be used (see loop below)
    // 1 <= $max - $min <= 0x7FFFFFFE
    $N = ($max - $min) + 1; // 2 <= $N <= 0x7FFFFFFF
    $M = (0x7FFFFFFF - $N) + 1; // 1 <= $M <= 0x7FFFFFFE
    $h = $M % $N; // 0 <= $h <= $N-1 <= 0x7FFFFFFE

    do
    {
        $bytes = mcrypt_create_iv(4, MCRYPT_DEV_URANDOM);
        $r = unpack("N", $bytes)[1] & 0x7FFFFFFF;
        if ($min == 0 && $max == 0x7FFFFFFF)
            return $r; // direct corresponding
    } while (($r - $N) > ($M - $h));
    return (($r%$N)+$min);
}


Write input bytes to a file directly in Python3 I am using Python 3.6.1. This program simply allows a user to enter some bytes as a "string" and it writes those bytes to a file as actual bytes in the order provided.

import sys

WELCOME_MSG = ("Welcome to Byte Writer. Enter some bytes separated by spaces "
            "like 49 A7 9F 4B when prompted. 'exit' or 'quit' to leave")
FILE_WRITE_ERROR = "Error writing to file... exiting."
INVALID_INPUT_ERROR = ("Improper byte format. Remember to enter "
    "2 chars at a time separated by spaces. Hex digits only")
INPUT_PROMPT = "Enter bytes: "
VALID_CHARS = ['0', '1', '2', '3', '4', '5', '6', '7', '8',
            '9', 'A', 'B', 'C', 'D', 'E', 'F', 'a', 'b', 'c', 'd', 'e', 'f']

def main(argv):
    if not validate_args(argv):
        return -1
    with open(argv[1], "ab") as f:
        print(WELCOME_MSG)
        while run_loop(argv, f) is True:
            pass

    print("Have a nice day.")

def run_loop(argv, f):
    output_list = []
    ui = input(INPUT_PROMPT)
    if ui == "exit" or ui == "quit":
        return False

    list_of_str_bytes = ui.split(' ')
    if validate_input_list(list_of_str_bytes) is False:
        print(INVALID_INPUT_ERROR)
        return False

    for b in list_of_str_bytes:
        output_list.append( int(b, 16) )

    try:
        f.write(bytes(output_list))
    except:
        print(FILE_WRITE_ERROR)
        return False
    return True

def validate_args(argv):
    if len(argv) != 2:
        print("USAGE: {} [filename to save to]".format(argv[0]))
        return False
    return True

def validate_input_list(input_list):
    for b in input_list:
        if len(b) != 2 or not valid_chars(b):
            return False

    return True

def valid_chars(chars):
    for c in chars:
        if c not in VALID_CHARS:
            return False

        return True
if __name__ == '__main__':
    main(sys.argv)

Consider using any() for implementing validate_input_list and valid_chars


Removing the word "NULL" from a single column in Excel using VBA I am using VBA in Excel to remove the word NULL from cells. I have the following code:

Sub RemoveNull()
    Dim c, count, r
    Application.ScreenUpdating = False

    count = 0
    r = ActiveCell.row
    c = ActiveCell.Column
    c = GetLetterFromNumber(c)

    For x = 1 To Range(c & Rows.count).End(xlUp).row Step 1
        count = count + 1
        If Range(c & x).Value = "NULL" Then
            Range(c & x).Clear
        End If
        'This section is for trouble shooting
        'If count = 1000 Then
        '    Range(c & x).Select
        '    count = 1
        'End If
    Next
    Application.ScreenUpdating = True
    MsgBox "Finished"
    Range(c & r).Select
End Sub

This works just fine and is designed to only check one column. I don't want to check the whole sheet with this I already have another sub that does this.

I want to make sure that this is the most efficient way to go about it. right now it checks each cell from top to bottom and then clears the contents if it contains NULL.

Static Code Analysis

I took your code and ran it through Rubberduck (an open-source VBIDE add-in project I manage) code inspections - here's what came out of it:

Implicit references to ActiveSheet

When they're not qualified with an actual Worksheet object instance, members Range and Rows (and Columns, Names and Cells) implicitly refer to the ActiveSheet. These implicit references make the code frail, and harder to debug. Consider making these references explicit when they're intended (e.g. ActiveSheet.Range instead of just Range; ActiveSheet.Rows instead of just Rows), and prefer working off object references (e.g. DataSheet.Range, DataSheet.Rows).
Procedure RemoveNull is implicitly Public

Module members are public by default, which can be counter-intuitive (especially if you're coming from other programming languages). Consider specifying explicit access modifiers to avoid ambiguity. e.g. Public Sub RemoveNull() instead of just Sub RemoveNull().
Instruction contains multiple declarations

Dim c, count, r

Declaring multiple variables in the same instruction is legal, but should be used sparingly. Consider declaring variables closer to their usage, in a single instruction per declaration.
Implicit Variant variables

Variables c, r, and count are all implicitly Variant, because they're declared without an As clause. Consider specifying an appropriate data type for all variables (e.g. String for strings, Long for row numbers, etc.), and use an explicit Variant where Variant is intended. Note that there's a common beginner trap here:

Dim c, count, r As Long ' only r is a Long, c and count are both Variant

Use meaningful names

c, r, and x are poor names. Prefer using names that convey usage and meaning, e.g. activeColumn, activeRow, and currentRow. Avoid disemvoweling, numeric suffixes, Systems Hungarian prefixes and 1-2 character identifiers.
Undeclared variables

Variable x isn't declared, which means it's an implicit Variant, declared on-the-fly by the VBA runtime. If your code can compile and run, it means Option Explicit isn't specified either.
Option Explicit not specified

VBA will happily compile a typo: use Option Explicit to prevent successfully compiling a program that doesn't explicitly declare every single variable it's using.

Future versions of Rubberduck will also highlight that Step 1 is the default step for a For loop, and therefore redundant.

Redundancies

Static code analysis is nice, but can't pick up everything. Like this:

c = ActiveCell.Column
c = GetLetterFromNumber(c)

You have the Column number already - there's absolutely no need to work out what column "letter" that number is referring to. Simply change how you're accessing the cell - instead of this:

[ActiveSheet.]Range(c & x).Clear

Use Cells:

[ActiveSheet.]Cells(x, c).Clear

Notice how much easier that is to parse, with meaningful names:

[ActiveSheet.]Cells(currentRow, activeColumn).Clear

So, remove GetLetterFromNumber, you don't need it at all. Ever.

Static code analysis also won't pick up (for now, anyway) that Range.Value has a non-zero possibility of containing an error value - in which case a comparison with any non-error type will raise run-time 13 / Type Mismatch:

If Range(c & x).Value = "NULL" Then ' possible error here

The loop body is pulling the same cell twice, and in order to validate whether it's an error or not, you'd need to pull it a third time: consider extracting a local variable.

Dim currentCell As Range
Set currentCell = [ActiveSheet.]Cells(x, c)

If Not IsError(currentCell.Value) Then
    If currentCell.Value = "NULL" Then ' comparison is now safe
        currentCell.Clear
    End If
End If

The last thing you want is the possibility of throwing a run-time error *while Application.ScreenUpdating is turned off. For this reason, whenever you need to toggle that setting, consider introducing some minimal error handling:

Public Sub DoSomething()
    On Error GoTo CleanFail
    Application.ScreenUpdating = False

    'code...

CleanExit:
    Application.ScreenUpdating = True
    Exit Sub
CleanFail:
    Debug.Print Err.Description
    Stop ' we don't know what happened; break here and debug
    Resume CleanExit
End Sub

Performance

Range is an object collection - the fastest way (by a lot) to iterate it is with a For Each loop. But, if all you need out of a Range its its Value, you can iterate the values even faster by dumping the range into an array, and iterating the array with a For...Next loop:

With ActiveSheet

    Dim lastRow As Long
    lastRow = .Cells(.Rows.count, c).End(xlUp).Row

    Dim values As Variant ' a variant array
    values = .Range(.Cells(1, 1), .Cells(.Rows.Count, lastRow)).Value

End With

There are plenty of Stack Overflow Q&A's about the best ways of turning that 2D array into a 1D array, I'll leave that bit of research up to you.

Once you have a one-dimensional array, you can iterate and modify it in-memory:

For x = LBound(values) To UBound(values)
    If Not IsError(values(x)) Then
        If values(x) = "NULL" Then values(x) = vbEmpty
    End If
Next

And then you can convert back the 1D array to a 2D array, and dump it onto the worksheet in one single write:

ActiveSheet.Cells(1, activeColumn).Resize(UBound(values)) = Application.Transpose(values)

And with one single read and one single write from the worksheet, and the iteration and actual work being done in-memory at lightning speed (as opposed to one cell at a time with Excel repainting itself every time, if not recalculating the whole worksheet too), there shouldn't even be a need to switch off Application.ScreenUpdating now - toggling it off won't make a difference anyway.

I suspect the result will be so fast, you won't even need that MsgBox call to wake the user up.