Credit card / IMEI check digit in Kotlin I recently had the need to implement Luhn's Algorithm in a Java/Kotlin application. I needed a function that would add the check digit to the string of a number like a credit card or IMEI. It could be easily modified to check if the last digit is valid.

At first, I had a code of around 20 lines in Java, but by using every Kotlin quirk I could think of, I managed to do it in one line.

fun String.addLastImeiDigit(): String = "$this${10 - this.withIndex().map { if (it.index.and(1) == 0) it.value.toInt() - 48 else (2 * (it.value.toInt() - 48)).run { (this % 10) + (this / 10) } }.sum() % 10}"

Here it is in a more understandable fashion

fun String.addLastImeiDigit(): String {
    return "$this${10 - this.withIndex().map {
        if (it.index.and(1) == 0) /* if even */
            it.value.toInt() - 48 /* ascii to Int */
        else
            (2 * (it.value.toInt() - 48)).run {
                (this % 10) + (this / 10) /* sum both digits */
            }
    }.sum() % 10}"
}

I understand that obfuscating code like this in a shared code environment would be very bad, but still I think it's great that we can get to do this and that Kotlin avoids so much boilerplate. What do you think about using one-line functions like this one?

I personally like one-line functions as long as they are short but I also prefer breaking things out into smaller functions and/or objects to improve readability, reuse, etc. e.g.:

fun String.plusLuhnCheckDigit() = this + luhnCheckDigit(this)
fun luhnCheckDigit(digits: CharSequence): Char {
    return digits.sumByIndexed { index, digit ->
        when {
            index.isEven -> digit.asInt()
            else -> sumDigits(2 * digit.asInt())
        }
    }.let { sum -> 10 - sum % 10 }.asChar()
}

With the following also defined:

Strings.kt

inline fun CharSequence.sumByIndexed(selector: (Int, Char) -> Int): Int {
    return foldIndexed(0) { index, sum, element -> sum + selector(index, element) }
}

Numbers.kt

val Int.isEven: Boolean
    get() = and(1) == 0

NumericalChars.kt

fun Char.asInt() = toInt() - 48
fun Int.asChar() = toChar() + 48

IntMath.kt

val Int.magnitude: Int
    get() = if (this < 0) -this else this

fun sumDigits(n: Int): Int = if (n.magnitude < 10) n else n % 10 + sumDigits(n / 10)


2-dimensional "wrapping" array I'm writing a simple JavaScript game, which uses a 2-dimensional array as the main data structure. I wrote this function-class-thing that allows me to give any two integers, positive or negative.  This class should "wrap" those integers back onto the game grid.

For example:

var g = new Grid(10, 10)
g.set(15, -2, "hello")
g.get(5, 8); //should be "hello"

Anyway, I'm striving to improve my JavaScript, so I'm wondering if there are any improvements (performance, "javascriptness", etc.) that I might be able to make.

var Grid = (function(w, h){
    this._width = w;
    this._height = h;
    this._grid = new Array((this._width * this._height) | 0);
    this.get = function(x, y){
            var coords = _normalize(x, y);
            return this._grid[(coords.y * coords.x) + coords.x];
    }
    this.set = function(x, y, item){
            var coords = _normalize(x, y);
            this._grid[(coords.y * coords.x) + coords.x] = item;
    }
    this._normalize = function(x, y){
            if(x < 0) x = (this._width + x) % this._width;
            else if(x >= this._width) x = x % this._width ;
            if(y < 0) y = (this._height + y) % this._height;
            else if(y >= this._height) y = y % this._height;
            return {'x': x, 'y': y};
    }
});

I can't say, that my variant of normalize function is easier to understand, but at least it is linear and it works correct (as i think) with x, that is less than -width

this._normalize = function(x, y) {
    x = x % this._width;
    y = y % this._height;

    x = (this._width + x) % this._width;
    y = (this._height + y) % this._height;

    return {'x': x, 'y': y};
}

Also, you missed this statement in get and set functions (looks like a typo):

this.get = function(x, y){
    var coords = this._normalize(x, y);
    return this._grid[(coords.y * coords.x) + coords.x];
}
this.set = function(x, y, item){
    var coords = this._normalize(x, y);
    this._grid[(coords.y * coords.x) + coords.x] = item;
}

It is better to move get, set and normalize functions to Grid prototype.
Putting these functions in prototype will increase execution speed and require less memory for creating an instance of Grid because these functions will not
be created every time Grid instance is created.
Like this:

var Grid = function(w, h){
    this._width = w;
    this._height = h;
    this._grid = new Array((this._width * this._height) | 0);
};
Grid.prototype.get = function(x, y) {
    // get code
};
Grid.prototype.set = function(x, y, item){
    // set code
};
Grid.prototype._normalize = function(x, y){
    // normalize code
};


List and filter files in Rust I'm learning Rust and looking for better/cleaner ways to list all file names that do not start with filter, given a pathname as a parameter.

    fn list_files(&self, pathname: &PathBuf, filter: &str) -> Result<Vec<PathBuf>, Error> {
        fs::read_dir(pathname).map(|read_dir| {
            read_dir
                .filter_map(|res| {
                    res.map(|entry| {
                        entry
                            .path()
                            .strip_prefix(pathname)
                            .ok()
                            .map(|path| {
                                if path.starts_with(filter) {
                                    None
                                } else {
                                    Some(path.to_path_buf())
                                }
                            })
                            .flatten()
                    })
                    .ok()
                    .flatten()
                })
                .collect()
        })
    }

As far as I can tell, you're returning error only in case of read_dir failure. Not sure if anyone ever really needs it, but you could change upmost Some() to Ok(), remove .ok() and fix the signature so that my version works just like yours. Note: the code below wasn't tested. I didn't like nested structure of your function, so I came up with this:

use std::fs;
use std::path::PathBuf;

fn list_files(pathname: &PathBuf, filter: &str) -> Option<Vec<PathBuf>> {
    Some(fs::read_dir(pathname).ok()?
        .filter_map(|entry| {
            Some(
                entry
                    .ok()?
                    .path()
                    .strip_prefix(pathname)
                    .ok()?
                    .to_path_buf(),
            )
        })
        .filter(|path| path.starts_with(filter))
        .collect())
}

Explanation: .ok() maps Result<T, K> to Option<T>, ? operator returns early in case if Option or Result aren't Some()/Ok()


Java concurrent Map of List I need concurrent HashMap of List as value with following behavior:

count of read/write ops approximately equals
support add/remove values in lists
thread safe iterations over lists

After some research I implemented ConcurrentMapOfList, but I'm not sure in his correctness.

public class ConcurrentMapOfList<Key, ListValue> {
    private final ConcurrentMap<Key, ConcurrentList<ListValue>> values
            = new ConcurrentHashMap<Key, ConcurrentList<ListValue>>();

    public void add(Key key, Value value) {
        List<Value> list = values.get(key);

        if (list == null) {
            final ConcurrentList<Value> newList = new ConcurrentList<Value>();
            final ConcurrentList<Value> oldList = values.putIfAbsent(key, newList);

            if (oldList == null) {
                list = newList;
            } else {
                list = oldList;
            }
        }

        list.add(value);
    }

    public List<ListValue> remove(Key key) {
        return values.remove(key);
    }

    public boolean remove(Key key, ListValue value) {
        final List<ListValue> list = values.get(key);

        if (list == null) {
            return false;
        }

        return list.remove(value);
    }

    public List<ListValue> obtainListCopy(Key key) {
        final ConcurrentList<ListValue> list = values.get(key);

        if (list == null) {
            return Collections.emptyList();
        }

        return list.clone();
    }

    public void removeAll() {
        values.clear();
    }

    private class ConcurrentList<V> extends ArrayList<V> {
        final ReentrantReadWriteLock lock = new ReentrantReadWriteLock();
        final Lock readLock = lock.readLock();
        final Lock writeLock = lock.writeLock();

        @Override
        public boolean add(V v) {
            writeLock.lock();
            try {
                return super.add(v);
            }finally {
                writeLock.unlock();
            }
        }

        @Override
        public V remove(int index) {
            writeLock.lock();
            try {
                return super.remove(index);
            }finally {
                writeLock.unlock();
            }
        }

        @Override
        public ConcurrentList<V> clone() {
            readLock.lock();
            try {
                return (ConcurrentList<V>)super.clone();
            }finally {
                readLock.unlock();
            }
        }
    }
}

Ok, after the hint from the comment it makes more sense for me. I felt a bit bad because of the wrong comment, so I have investigated this deeper.

As far as I see it, there is only one problem which I will explain in the next paragraph. But I have to admit, the implementation is rather complex because we have several layers of parallelization and locking with multiple data structures. It would need very carefully analysis to prove the correctness.

The problem I found is the remove method. You have implemented remove(int):

@Override
public V remove(int index) {
    writeLock.lock();
    try {
        return super.remove(index);
    }finally {
        writeLock.unlock();
    }
}

But you are using remove(Object):

public boolean remove(Key key, ListValue value) {
    final List<ListValue> list = values.get(key);

    if (list == null) {
        return false;
    }

    return list.remove(value);
}

Test case to show it:

    final int max = 500;
    final int maxThreads = 200;
    for (int i = 0; i < max; i++) {
        final int currentI = i;
        final List<Thread> threadList = new ArrayList<>();
        for (int j = 0; j < maxThreads; j++) {
            final int currentJ = j;
            threadList.add(new Thread(new Runnable() {
                @Override
                public void run() {
                    map.add(currentI, currentJ);
                    map.remove(currentI, currentJ);
                }
            }));
        }
        for (final Thread thread : threadList)
            thread.start();
        for (final Thread thread : threadList)
            thread.join();
    }

    if (map.getEntrySet().size() != max)
        System.err.println("entryset size should be: " + maxThreads + " is:" + map.getEntrySet().size());

    for (final Entry<Integer, ConcurrentList<Integer>> entry : map.getEntrySet()) {
        final ConcurrentList<Integer> value = entry.getValue();
        if (value.size() != 0)
            System.err.println("value size should be: " + 0 + " is:" + value.size() + ", value: " + value);
    }

This can easily be solved, if you just implement the remove(Object) method:

@Override
public boolean remove(final Object o) {
    writeLock.lock();
    try {
        return super.remove(o);
    } finally {
        writeLock.unlock();
    }
}

This errors could be avoided if you use one of the following patterns:

Use a synchronized list here, Vector (not recommended), CopyOnWriteArrayList or Collections.synchronizedList
Use encapsulation, rather then extension. Do not extend ArrayList, use implements List and use a member field with the backed up data structure. This will be in the end, nearly the same as using Collections.synchronizedList, but you could avoid the mutex and use the ReantrantLocks if you really need it

As already partly mentioned in my comment. To get the code running, I had to change the generic identifier Value to ListValue at some places, it looked like they were mixed up.


Generating buttons efficiently I've written a Minesweeper program, but my code for generating the buttons is taking much longer than I'd like. Here is the GUI code:

        this.boardPanel.Controls.Clear();
        this.boardValues = new int[mode.size.Width, mode.size.Height];
        this.boardFound = new bool[mode.size.Width, mode.size.Height];
        this.boardButtons = new Button[mode.size.Width, mode.size.Height];
        EventHandler click = new EventHandler(btn_Click);
        for (int x = 0; x < boardButtons.GetLength(0); x++)
        {
            for (int y = 0; y < boardButtons.GetLength(1); y++)
            {
                boardButtons[x, y] = new Button();
                boardButtons[x, y].Size = new Size(Minesweeper.BUTTON_LENGTH, Minesweeper.BUTTON_LENGTH);
                boardButtons[x, y].Location = new Point(x * Minesweeper.BUTTON_LENGTH, y * Minesweeper.BUTTON_LENGTH);
                boardButtons[x, y].Tag = new Point(x, y);
                boardButtons[x, y].Click += click;
                this.boardPanel.Controls.Add(boardButtons[x, y]);
            }
        }
        this.boardPanel.Size = new Size(mode.size.Width * Minesweeper.BUTTON_LENGTH, mode.size.Height * Minesweeper.BUTTON_LENGTH);
        this.Size = new Size(5 + this.boardPanel.Width + 5 + 8, this.tstMain.Height + 5 + this.boardPanel.Height + 5 + this.stsStatus.Height + 30);

As you can see I am creating an EventHandler for a method called btn_Click, outside of the loop, and assigning it to each button. This method reads the Tag field of the clicked button to determine which button as pressed.

In an Intermediate game with a board size of 16x16, generating the buttons takes approximately 2-3 seconds. I know WinForm controls aren't known for the best performance, but is there a way I can generate a number of buttons, assign Event handlers and add them to a panel more efficiently?

You don't really need the EventHandler you can just replace
boardButtons[x, y].Click += click; withboardButtons[x, y].Click += btn_Click;. The += already works with delegates and kinda does that for you.
The only time you want to have this is when your method parameters have the same names as your class variables it appears that's not case for you, because I don't see any of the variables used without it.

Overall I really don't see what you can improve, you are simply assigning some values to some variables which really cant be speed up. Windows forms isn't really good for many controls at once that's it.


Wrapper for a standard STL-container In the codebase I support, I found the following code. I am interested in your opinion about its expediency and especially the expediency of redefining the operator().

  template <typename KEY, typename VALUE>
  class MyUnorderedMap {
    std::unordered_map<KEY, VALUE> map;

  public:
    template <typename T>
    auto insert(const T value) {
      return map.insert(value);
    }
    inline auto& operator()(const KEY& r) { return (*this)[r]; }
    inline auto& operator[](const KEY r) { return map[r]; }
    inline auto find(const KEY& key) const { return map.find(key); }
    inline auto size() const { return map.size(); }
    inline auto begin() { return map.begin(); }
    inline auto end() { return map.end(); }

    inline bool exists(const KEY& userId) const {
      return map.find(userId) != map.end();
    }
    bool remove(const KEY& userId) {
      auto it = map.find(userId);
      bool retval = (it != map.end());
      if (retval)
        map.erase(it);
      return retval;
    }
  };

I question the value of this template class.  We hide many important members of std::unordered_map (including crbegin(), empty(), reserve(), at(), try_emplace()) for no good reason.  The additional functions that are provided have no business being members: they are perfectly implementable as free functions without privileged access.

Considering the members in turn:

insert(): only instantiable if T is convertible to the map's value_type.  Unnecessarily copies the argument.
operator(): unnecessary, given that we have operator[].
operator[]: unnecessarily copies its argument.
find(), size() begin(), end(): required only because the map is private.
exists(): should be a free function.  If this is generic, why is the parameter called userId?
remove() - why not just use the erase() that accepts a key?  Or do we have to build with pre-C++11 library?

When I've wanted to provide something like the exists() here, I've made it a standalone template function that can work with many kinds of container:

/*
 * contains()
 * A convenience overload that searches from beginning to end of a container.
 *
 * If an iterator is supplied, it will be written with the found position or
 * container.end() as appropriate.
 */
template<typename T, typename U>
inline bool contains(const T& container, const U& value)
{
    using std::begin;
    using std::end;
    return find(begin(container), end(container), value) != end;
}

template<typename T, typename U, typename Iter>
    /* Iter should be a T::iterator or T::const_iterator */
inline bool contains(T& container, const U& value, Iter& it)
{
    using std::begin;
    using std::end;
    return (it = find(begin(container), end(container), value)) != end;
}

/*
 * Overloads for pointer-to-const values within containers of pointer-to-mutable.
 */
template<typename T, typename U>
inline bool contains(const T& container, const U* value)
{
    using std::begin;
    using std::end;
    return find(begin(container), end(container), value) != end;
}

template<typename T, typename U, typename Iter>
inline bool contains(const T& container, const U* value, Iter& it)
{
    using std::begin;
    using std::end;
    return (it = find(begin(container), end(container), value)) != end;
}

/*
 * Overloads for maps (by key)
 */
template<typename T>
inline bool contains(const T& container, const typename T::key_type& key)
{
    return container.find(key) != container.end();
}

template<typename T, typename Iter>
inline bool contains(T& container, const typename T::key_type& key, Iter& it)
{
    return (it = container.find(key)) != container.end();
}

template<typename T>
inline bool contains(const T& container, const typename T::key_type& key,
                     typename T::mapped_type& result)
{
    typename T::const_iterator it;
    return contains(container, key, it) && (result = it->second, true);
}


Playing with digits in JavaScript I want to achieve this:

digPow(89, 1) should return 1 since \$8^1 + 9^2 = 89 = 89 * 1\$
digPow(92, 1) should return -1 since there is no \$k\$ such that \$9^1 + 2^2\$ equals \$92 * k\$
digPow(695, 2) should return 2 since \$6^2 + 9^3 + 5^4 = 1390 = 695 * 2\$
digPow(46288, 3) should return 51 since \$4^3 + 6^4 + 2^5 + 8^6 + 8^7 = 2360688 = 46288 * 51\$

I have implemented it like this:

function digPow(n, p){
  var digits = n.toString().split('');
  var result = 0;
  for(var i=0; i<digits.length; i++){
    result = result + Math.pow(digits[i], p);
    p++;
  }
  var data = result/n;
  if(result % n === 0){
  return data;
  }else{
  return -1;
  }
}

Some benchmarks

I am guessing a function like this is written, apart from an exercise, to search for power numbers. Thus execution time is important.

String manipulation and type conversion is not something that javascript likes to do, so running the two given answers in a profiler I measured the time to run the following loop.

var i;
for (i = 0; i < 1000; i += 1) {
    digPow(Math.floor(Math.random() * 10000000),i%10);
}

I ran the tests for many minutes the results were

ashraf aaref's  function ran twice as fast as Oleander's on Firefox, running on an old Win10 32Bit laptop. The times; 2ms and 4ms respectively for the above loop. Which equates to 500,000 and 250,000 calls to the function digPow (plus overhead of loop and call) per sec,

The array method reduce is the bottleneck in the slowest version. For performance always avoid any of the array iteration functions as each interaction involves a function call that needs to save the current context.

Type conversion is the bottleneck in the second function.

Avoiding both bottlenecks

But there is a much faster method that avoids the type conversion

 function digPowC(n, p){
    var digits, result, i, num;
    digits = Math.floor(Math.log10(n)) + p;  // get the number of digits
                                             // add the power p
    result = 0;
    num = n;                                 // running least significant digit
    while(digits >= p){                      // Stop at p
        result += Math.pow((num % 10),digits--);  // pow least significant digit
        num = Math.floor(num / 10);
    }
    if(result % n === 0){  // vet result
        return result / n;
    }
    return -1;
}

Which is twice as fast again at 1ms or 1,000,000 calls per second.

Which is the best, that could be debated forever as all three work except for one problem that is often forgotten when manipulating numbers.

digPow(1e100,1) is not dealt with. The two string methods fail as they try to evaluate the "e" and the pure number version fails trying to maintain the least significant digit that is way outside the 64bit Double's precision. But then we assume that arguments are vetted.

Results of a benchmark

Not runnable just hidden to avoid clutter.

// Each function is duplicated and randomly called the mean is the mean of all 4 tests
/* Raw output from tester

Test : 'Oleander'
4.155microsec 3155 samples
4.161microsec 3057 samples
4.169microsec 3128 samples
4.124microsec 3142 samples
Test : 'ashraf aaref'
2.207microsec 3186 samples
2.200microsec 3150 samples
2.203microsec 3011 samples
2.200microsec 3214 samples
Test : 'Blindman67'
1.068microsec 3158 samples
1.060microsec 3146 samples
1.058microsec 3110 samples
1.068microsec 3143 samples
----------------------------------------------------------
Test : 'Oleander'
Mean : 4.152ms 12482 samples
Min : 4.124ms Max : 4.169ms
----------------------------------------------------------
Test : 'ashraf aaref'
Mean : 2.203ms 12561 samples
Min : 2.200ms Max : 2.207ms
----------------------------------------------------------
Test : 'Blindman67'
Mean : 1.064ms 12557 samples
Min : 1.058ms Max : 1.068ms
----------------------------------------------------------
Mean : 2.469ms Totals time : 92849.685ms 37600 samples

end of output */

// the sample function. Running this function constitutes one sample.
func : function(){
    var i;
    for (i = 0; i < 1000; i += 1) {
        digPow(Math.floor(Math.random() * 10000000),i%10);
    }
},

// Functions all run in strict mode.

// Test : 'Oleander'
function digPowA(n, p){
    const digits = n.toString().split('');
    const result = digits.reduce(function(acc, value, index) {
        return acc + Math.pow(value, index + p);
    }, 0);

    if (result % n === 0){
        return result / n;
    } else {
        return -1;
    }
}
// Test : 'ashraf aaref'
function digPowB(n, p){
    var digits = n.toString(); //** without split should be faster**
    var result = 0;
    for(var i=0; i<digits.length; i++){
        result = result + Math.pow(digits[i], p);
        p++;
    }
    if(result % n === 0){
        return result/n;
    }else{
        return -1;
    }
}
// Test : 'Blindman67'
function digPowC(n, p){
    var digits, result,i;
    digits = Math.floor(Math.log10(n)) + p;
    result = 0;
    var num = n;
    while(digits >= p){
        result += Math.pow((num % 10),digits--);
        num = Math.floor(num / 10);
    }
    if(result % n === 0){
        return result/n;
    }
    return -1;
}


Grouping sales transactions by person I have a csv file with sales transactions. Each transaction includes person identifiers (which are sometimes/often missing) and transaction data. Person identifiers are fname, lname, phone, email and social security number. I want to link each transaction to a unique person. As a business rule, I have set that two transactions belong to the same person if fname and lname are identical AND at least one of the 3 other person identifiers are identical. As an outcome, I need to have two dataframes (and ultimately two csv files): one with the unique persons and one copy of the initial data with an additional column for the person id.

I have written code that works very well for solving the problem for small files. Except that when the file gets really long (hundreds of thousands of lines), it gets stuck. I am almost sure that my code is not optimized and I think I can find a better way using agreggate functions like groupby() or unique(), which I think are much faster. But I can't figure out how.

import pandas as pd
workDir=r"D:\fichiers\perso\perso\python\unicity\\"

sourceFile='rawdata.csv'
inFrame=pd.read_csv(workDir+sourceFile, sep=";",encoding='ISO-8859-1')
personFrame=pd.DataFrame(columns=('id','fname','lname','email', 'phone','social security number'))
outFrame=pd.DataFrame(columns=inFrame.columns)
idPerson=0
#print(inFrame)

def samePerson(p1, p2):
  response=0
  if p1['fname']==p2['fname'] and p1['lname']==p2['lname']:
      if p1['email']==p2['email'] or p1['phone']==p2['phone'] or p1['social security number']==p2['social security number']:
        response=1
  return(response)

def completePerson(old, new):
    #complete with new line missing data in ols version of the person
    for theColumn in ('fname','lname','email', 'phone','social security number'):
        if pd.isnull(old[theColumn]) :
            old [theColumn]=new[theColumn]
    return(old)

def processLine(theLine):
  global personFrame
  global idPerson
  global outFrame
  theFlag=0
  for indexPerson, thePerson in personFrame.iterrows():
      if theFlag==0:
          if samePerson(theLine,thePerson):
              theLine['idPerson']=thePerson.idPerson
              personFrame.loc[indexPerson]=completePerson(thePerson, theLine)
              theFlag=1
  if theFlag==0:
      theLine['idPerson']=idPerson
      idPerson=idPerson+1
      personFrame=personFrame.append(theLine)
  outFrame=outFrame.append(theLine)

def processdf():
    inFrame.apply(processLine, axis=1)
    with open(workDir+'persons.csv','w', encoding='ISO-8859-1') as f:
        personFrame.to_csv(f, index='false')
    with open(workDir+'transactionss.csv','w', encoding='ISO-8859-1') as f:
        outFrame.to_csv(f, index='false')

processdf()

Here's a much faster algorithm:

Create a Person object containing fields like fname, lname, etc and a list of associated Transaction objects.

Then, create a dictionary with "fname,lname" as the key and a list of Person objects as the value. Iterate through your CSV line-by-line, and if that "fname,lname" key isn't in the dictionary, add it along with a Person object that defines the person's details.

However, if the key is in the dictionary then check the other details to make sure it's an actual match. If it is an actual match, add the transaction to that Person's Transaction array. If not, add a new person item to the end of the array.


File read/write methods I'm using some simple files for caching and some basic user data. I was first just using file_put_contens and file_get_contents, but realized this could quickly go wrong when traffic starts increasing.

I've been trying to figure out how to do it properly with flock, correct file modes, etc, and come up with the following functions. Have I understood things correctly? Will this be safe in most normal use-cases, with small to medium web site usage?

class File
{

    public static function read($path, $default = NULL)
    {
        $fp = @fopen($path, 'r');
        if( ! $fp)
            return $default;

        flock($fp, LOCK_SH);
        $data = fread($fp, filesize($path));
        flock($fp, LOCK_UN);
        fclose($fp);

        return $data;
    }

    public static function write($path, $data)
    {
        self::check(dirname($path));

        $fp = fopen($path, 'c');
        flock($fp, LOCK_EX);
        ftruncate($fp, 0);
        fwrite($fp, $data);
        fflush($fp);
        flock($fp, LOCK_UN);
        fclose($fp);

        return $data;
    }

    public static function check($dir)
    {
        if( ! is_dir($dir))
        {
            // https://en.wikipedia.org/wiki/Chmod#System_call
            @mkdir($dir, 06750, true);
            @chmod($dir, 06750);
        }
        return $dir;
    }
}

Some particular things:

Should using LOCK_SH in read allow for multiple simultaneous reads?
(which will be the bulk of what's happening since writes will happen quite seldom)
Will using LOCK_EX in write make sure nobody else is reading or writing while the file is changed?
Will using the directory permission mask 06750 makes sure that

any files created in that directory will get the same permissions as the directory, and
the directory/file will only be writable by the web server user and readable by its group?

Suppressing errors

  Edit

Error suppression should be avoided if possible as it doesn't just suppress the error that you are trying to stop, but will also suppress errors that you didn't predict would ever occur. This will make debugging a nightmare.

Other then that, looks nice.


Code optimization of recursive function I have a mathematical function as shown below

  $$ (h+1) * U(k,h) = \sum_{r=0}^k \sum_{s=0}^h (r + 1) * (k - r + 1) *
 U(r + 1, h - s) * U(k - r + 1, s)\ + \sum_{r=0}^k \sum_{s=0}^h (k - r
  + 1) * (k - r + 2) * U(r, h - s) * U(k - r + 2, s)\,. $$

Base condition for U(k,h) is

  $$ U(k,0) = (-1)^k/k! $$

I am using the following code to calculate the function

public double U(int k, int h)

        {
            h--;
            Int16 r, s;

            if (h == -1)
            {
                return (double)(Power(-1, k) / fact[k]);
            }

            else if (arr1[k, h] != 0 && arr2[k, h] != 0)
                return (double)((arr1[k, h] + arr2[k, h]) / (h + 1));

            else
            {
                for (r = 0; r <= k; r++)
                {
                    for (s = 0; s <= h; s++)
                    {
                        arr1[k,h] += (r + 1) * (k - r + 1) * U(r + 1, h - s) * U(k - r + 1, s);

                    }

                }
                for (r = 0; r <= k; r++)
                {
                    for (s = 0; s <= h; s++)
                    {
                        arr2[k,h] += (k - r + 1) * (k - r + 2) * U(r, h - s) * U(k - r + 2, s);
                    }
                }
                return (double)((arr1[k,h] + arr2[k,h]) / (h + 1));
            }
        }

The function works fine for values of k and h up to 40. After that it prints

  at ndp.functions.U(Int32 k, Int32 h)

Base case first

It really helps to get a clear idea of a recursive function when the base case is the first thing you see. In your code it is even worse, as h is mutated before the base case, so it looks like the base case is -1 and not 0.

Smallest scope possible

Declare loop counters inside the for loop statement. It is more efficient and reduces mental load when reading the code.


Is there a better way to walk a directory tree? I just started to learn Haskell, and i wrote a function that walks the directory tree recursively and pass the content of each directory to a callback function:

The content of the directory is a tuple, containing a list of all the sub-directories, and a list of file names.
If there is an error, the error is passed to the callback function instead of the directory content (it use Either).

So here is the type of the callback function:

callback :: FileName -> Either IOError ([FileName], [FileName]) -> IO ()

And here is the type of my walk function:

walk :: FilePath -> (FilePath -> Either IOError ([FilePath], [FilePath]) -> IO ()) -> IO ()

And here is my complete code:

module Test where
import System.FilePath ((</>))
import Control.Monad (filterM, forM_, return)
import System.IO.Error (tryIOError, IOError)
import System.Directory (getDirectoryContents, doesFileExist, doesDirectoryExist)

myVisitor :: FilePath -> Either IOError ([FilePath], [FilePath]) -> IO ()
myVisitor path result = do
    case result of
        Left error -> do
            putStrLn $ "I've tryed to look in " ++ path ++ "."
            putStrLn $ "\tThere was an error: "
            putStrLn $ "\t\t" ++ (show error)

        Right (dirs, files) -> do
            putStrLn $ "I've looked in " ++ path ++ "."
            putStrLn $ "\tThere was " ++ (show $ length dirs) ++ " directorie(s) and " ++ (show $ length files) ++ " file(s):"
            forM_ (dirs ++ files) (\x -> putStrLn $ "\t\t- " ++ x)
    putStrLn ""

walk :: FilePath -> (FilePath -> Either IOError ([FilePath], [FilePath]) -> IO ()) -> IO ()
walk path visitor = do
    result <- tryIOError listdir

    case result of
        Left error -> do
            visitor path result

        Right (dirs, files) -> do
            visitor path result
            forM_
                (map (\x -> path </> x) dirs)
                (\x -> walk x visitor)

    where
        listdir = do
            entries <-  (getDirectoryContents path) >>= filterHidden
            subdirs <- filterM isDir entries
            files <- filterM isFile entries
            return (subdirs, files)
            where
                isFile entry = doesFileExist (path </> entry)
                isDir entry = doesDirectoryExist (path </> entry)
                filterHidden paths = do
                    return $ filter (\path -> head path /= '.') paths

main :: IO ()
main = do
    walk "/tmp" myVisitor
    walk "foo bar" myVisitor
    putStrLn "Done :-/"

As you can see, there is a lot of code and i import a lot of things...
I'm sure there is a better way to do that, and i'm interested in any
{idea,hint,tip} to improve this piece of code.

Thanks in advance :-)

You might be interested in the concept of iteratees/pipes that can be used to solve this problem. It allows you to separate producing the tree and consuming it somewhere else without direct callback functions: You create a producer that enumerates the directory tree, perhaps some filters that modify the data and a separate consumer that works on the data. And then compose and run the whole pipeline. See also Streaming recursive descent of a directory in Haskell.

There are also specialized Haskell packages for that such as directory-tree, which you can use or study.

Edit: As an example I reworked your code using conduit. This library (and others based on the same principle) has several advantages, namely:

It separates the producer of the data with its consumer.
In a source you just call yield when you want to send a piece of data to the pipe.
In a sink you call await whenever you want to receive a piece of data (here we used more specialized awaitForever).
You can have conduits that sit in the middle and consume and produce values at the same time. They can do whatever processing on the stream, mixing calls to yield and await as they wish.
This allows you to create complex computation where the behavior of your components depends on data sent/deceived earlier. We use this in our source (traversing a directory tree), and I also added it to the sink (visitor) - it keeps track of how many directories it has been passed so far.
Both source and sink (and intermediate conduits, if any) can have finalizers.

Some suggestions:

Create your own data types instead of using combinations of Either and (,). It makes your code shorter and easier to understand.
Sometimes it's worth declaring new functions instead of using complex case ... of expressions. It can make code easier to read.
hlint can suggest how to (syntactically) improve a piece of code.

import System.FilePath ((</>))
import Control.Monad (filterM, forM_, return)
import System.IO.Error (tryIOError, IOError)
import System.Directory (getDirectoryContents, doesFileExist, doesDirectoryExist)
import Control.Monad.Trans.Class (lift)
import Data.Conduit

data DirContent = DirList [FilePath] [FilePath]
                | DirError IOError
data DirData = DirData FilePath DirContent

-- Produces directory data
walk :: FilePath -> Source IO DirData
walk path = do
    result <- lift $ tryIOError listdir
    case result of
        Right dl@(DirList subdirs files)
            -> do
                yield (DirData path dl)
                forM_ subdirs (walk . (path </>))
        Left error
            -> yield (DirData path (DirError error))

  where
    listdir = do
        entries <- getDirectoryContents path >>= filterHidden
        subdirs <- filterM isDir entries
        files <- filterM isFile entries
        return $ DirList subdirs files
        where
            isFile entry = doesFileExist (path </> entry)
            isDir entry = doesDirectoryExist (path </> entry)
            filterHidden paths = return $ filter (\path -> head path /= '.') paths

-- Consume directories
myVisitor :: Sink DirData IO ()
myVisitor = addCleanup (\_ -> putStrLn "Finished.") $ loop 1
  where
    loop n = do
        lift $ putStrLn $ ">> " ++ show n ++ ". directory visited:"
        r <- await
        case r of
            Nothing     -> return ()
            Just r      -> lift (process r) >> loop (n + 1)
    process (DirData path (DirError error)) = do
        putStrLn $ "I've tried to look in " ++ path ++ "."
        putStrLn $ "\tThere was an error: "
        putStrLn $ "\t\t" ++ show error

    process (DirData path (DirList dirs files)) = do
        putStrLn $ "I've looked in " ++ path ++ "."
        putStrLn $ "\tThere was " ++ show (length dirs) ++ " directorie(s) and " ++ show (length files) ++ " file(s):"
        forM_ (dirs ++ files) (putStrLn . ("\t\t- " ++))

main :: IO ()
main = do
    walk "/tmp" $$ myVisitor


Classifying lexemes in a given C program Please review my code and tell where I can improve my program.

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

#define BUFFER_SIZE 1024
#define TOKEN_SIZE 32
#define TRUE 1
#define FALSE 0

int isKeyword(const char* token)
{
    const char keywords[32][20]={"auto","break","case","char",
                                "const","continue","default","do","double",
                                "else","enum","extern","float",
                                "for","goto","if","int","long",
                                "register","return","short","signed",
                                "sizeof","static","struct","switch",
                                "typedef","union","unsigned",
                                "void","volatile","while"};
    for(int i=0;i<32;i++)
    {
        if(strcmp(token,keywords[i])==0)
            return TRUE;
    }
    return FALSE;
}

int isIntLiteral(const char* token)
{
    for(int i=0;token[i]!=(char)NULL;i++)
    {
        if(!isdigit(token[i]))
            return FALSE;
    }
    return TRUE;
}

int isFloatLiteral(const char* token)
{
    if(isdigit(token[0]))
    {
        for(int i=1;token[i]!=(char)NULL;i++)
        {
            if(!isdigit(token[i]) && (token[i])!='.')
                return FALSE;
        }
        return TRUE;
    }
    return FALSE;
}

int isCharLiteral(const char* token)
{
    if(token[0]=='\'' && strlen(token)==3 && token[2]=='\'')
        return TRUE;
    else
        return FALSE;
}

int isLiteral(const char* token)
{
    if(isCharLiteral(token)||isIntLiteral(token)||isFloatLiteral(token))
        return TRUE;
    else
        return FALSE;
}

int isIdentifier(const char* token)
{
    if(isalpha(token[0]) || token[0]=='_')
    {
        for(int i=1;token[i]!=(char)NULL;i++)
        {
            if(!isalnum(token[i]))
                return FALSE;
        }
        return TRUE;
    }
    return FALSE;
}

int isOperator(const char* token)
{
    const char operators[41][10] = {".","->","++","--","!","~","(type)","*","&",
                                 "sizeof","*","/","%","+","-","<<",">>","<",
                                 "<=",">",">=","==","!=","&","^","|","&&","||",
                                 "?:","=","+=","-=","*=","/=","%=","&=","^=",
                                 "|=","<<=",">>=",","};

    for(int i=0;i<41;i++)
    {
        if(strcmp(token,operators[i])==0)
            return TRUE;
    }
    return FALSE;
}

FILE* readFile(int param1,char* param2[])
{
    if(param1<2)
    {
        fprintf(stderr,"No file specifed to read. \n");
        exit(1);
    }

    if(param1>2)
    {
        fprintf(stderr,"Too many arguments for read to perform. \n");
        exit(1);
    }

    FILE *fp = fopen(param2[1],"r");

    if(!fp)
    {
        fprintf(stderr,"File access denied on read. \n");
        exit(1);
    }
    return fp;
}

int main(int argc,char* argv[])
{

    FILE *fp = readFile(argc,argv);

    char *token;
    char string[BUFFER_SIZE];
    const char delimiters[]=" .,;!=\n";

    fgets(string,sizeof string, fp);
    while(string!=(char *)NULL || (strcmp(string,"\n"))==0)
    {
        token = strtok(string,delimiters);

        while(token!=NULL)
        {
            if(isKeyword(token))
                printf("%s is a keyword. \n",token);
            else if(isOperator(token))
                printf("%s is an operator. \n",token);
            else if(isIdentifier(token))
                printf("%s is an identifier. \n",token);
            else if(isLiteral(token))
                printf("%s is a literal. \n",token);
            else
                printf("Unrecognized token : %s \n",token);

            token = strtok(NULL,delimiters);
        }
        fgets(string,sizeof string, fp);
    }

    fclose(fp);
    return 0;
}`

Remove all these literals from the stack and place them in the .rodata/.const section of the executable image:

const char keywords[32][20]={"auto","break","case","char",
                            "const","continue","default","do","double",
                            "else","enum","extern","float",
                            "for","goto","if","int","long",
                            "register","return","short","signed",
                            "sizeof","static","struct","switch",
                            "typedef","union","unsigned",
                            "void","volatile","while"};

One very good way to do that would be to make it an array of pointers:

const char *keywords[]=
{
    "auto","break","case","char",
    "const","continue","default","do","double",
    "else","enum","extern","float",
    "for","goto","if","int","long",
    "register","return","short","signed",
    "sizeof","static","struct","switch",
    "typedef","union","unsigned",
    "void","volatile","while"
};

Similar considerations should be applied to the literal array in the isOperator() function.

Regarding this line:

for(int i=0;i<32;i++)

Do not use 'magic' numbers (other than 0 and 1) in the code as they are a maintenance nightmare. I suggest:

for(size_t i=0; i<(sizeof(keywords)/sizeof(char*)); i++)

Regarding this line:

for(int i=0;token[i]!=(char)NULL;i++)

This will not work quite correctly, as NULL is the size of a pointer and I think the code is looking for '\0', not NULL.

I suggest:

Note, no need to make any comparison as the contents of token[i] is either in the range 0x01...0xFF or 0x00 - and 0x00 will be seen as false.

for(int i=0; token[i];i++)

When #defineing a numeric value, always wrap the value in parens to avoid any possibility of 'text replacement' errors.

Regarding these lines:

fgets(string,sizeof string, fp);
while(string!=(char *)NULL || (strcmp(string,"\n"))==0)

string will NEVER be NULL because string is not a pointer
there is never a need to cast NULL

I suggest:

while( fgets(string,sizeof string, fp) )

To get the actual reason that a call to fopen() fails, change this:

FILE *fp = fopen(param2[1],"r");

if(!fp)
{
    fprintf(stderr,"File access denied on read. \n");
    exit(1);
}

to: (perror() will output the system message for the current value of 'errno')

FILE *fp = NULL;
if( NULL ==( fp = fopen(param2[1],"r") ) )
{ // then fopen failed:
    perror( "fopen for input file failed" );
    exit( EXIT_FAILURE );
}

I suggest that main() check the number of parameters and if not correct, then output a 'USAGE' statement and exit.  This would greatly simplify the readfile() function, which is not actually reading a file, but rather just opening a file.

There are plenty of other details that could be improved, but the above should get you started.


Beginning C BST implementation I'd really appreciate if I could get some feedback on the following code with regard to security, efficiency and possible uncaught errors. Personally I feel the printAndCleanString and friendlyLookup functions combined are very messy but I don't know how to fix them.

In particular I would love feedack on how to more cleanly and efficiently return strings.

#include <stdio.h>
#include <malloc.h>
#include <mem.h>

struct bstNode {
    int val;
    struct bstNode *left;
    struct bstNode *right;
};

void insert(struct bstNode **head, int val);

int lookup(struct bstNode **node, int val);

void printDFS(struct bstNode *head);

char *friendlyLookup(struct bstNode **node, int val);

void printAndCleanString(char *string);

int main() {
    struct bstNode *bstTree = NULL;
    insert(&bstTree, 8);
    insert(&bstTree, 5);
    insert(&bstTree, 98);
    insert(&bstTree, 2);
    insert(&bstTree, 15);
    insert(&bstTree, 65);
    insert(&bstTree, 15);
    printDFS(bstTree);
    printf("\n");
    printAndCleanString(friendlyLookup(&bstTree, 1));
    printAndCleanString(friendlyLookup(&bstTree, 65));
}

void insert(struct bstNode **head, int val) {
    if (*head == NULL) {
        *head = malloc(sizeof(struct bstNode));
        if (*head == NULL) {
            printf("malloc failed!");
            return;
        }
        (*head)->val = val;
        (*head)->left = NULL;
        (*head)->right = NULL;
        return;
    }

    if (val < (*head)->val) {
        return insert(&(*head)->left, val);
    } else {
        return insert(&(*head)->right, val);
    }
}

void printDFS(struct bstNode *head) {
    if (head->left != NULL) printDFS(head->left);
    printf("%d ", head->val);
    if (head->right != NULL) printDFS(head->right);
}

int lookup(struct bstNode **node, int val) {
    if (*node == NULL) {
        return -1;
    }
    if ((*node)->val == val) {
        return val;
    }
    if (val < (*node)->val) {
        return lookup(&(*node)->left, val);
    } else {
        return lookup(&(*node)->right, val);
    }
}

char *friendlyLookup(struct bstNode **node, int val) {
    int result = lookup(node, val);
    char resultString[256];
    char numberString[256];
    sprintf(numberString, "%d", val);
    if (result > -1) {
        sprintf(resultString, " is present in the bst\n");
    } else {
        sprintf(resultString, " is not present in the bst\n");
    }
    char *resultPointer = malloc(strlen(resultString) + strlen(numberString) + 1 * sizeof(char));
    if (resultPointer == NULL) {
        printf("Malloc error!");
        return NULL;
    }
    sprintf(resultPointer, "%s%s", numberString, resultString);
    return resultPointer;
}

void printAndCleanString(char *string) {
    printf(string);
    free(string);
}

#include <mem.h> should be properly #include <string.h>. The former is non-standard.
I prefer avoiding the duplication inherent in forward-declarations, so would put the definition before first use.
Unless, of course, I'm writing a header-file.
Remember that your friendlyLookup() can fail. And test for that.
It's a bad idea to name the type in a sizeof like in *head = malloc(sizeof(struct bstNode));. Changing it to *head = malloc(sizeof **head); is more resilient to change, obviously correct, and even shorter.
There are a few viable strategies for handling failure:

Ignoring it and hoping the program will crash. Even for toy-programs, that's extremely bad form.
Deteting it and aborting the program with an error-message. Often acceptable when resources are exceeded.
Fixing the failure. Often not possible, or at least not at that place.
Reporting the failure to the caller and letting them deal with it. The preferred method, especially for library-code.

You are instead doing a mixture: Printing an error-message (part of option 2) and then pretending you succeeded. That's a really bad combo.
Learn to love the ternary operator condition ? true-case : false-case. It would often simplify your code.

Applying some of the above:

void die(char* msg) {
    fprintf(stderr, "%s", msg);
    abort();
}

void insert(struct bstNode **head, int val) {
    if(!*head) {
        *head = malloc(sizeof **head);
        if(!*head)
            die("malloc failed!");
        **head = (struct bstNode){.val = val};
        return;
    }
    insert(val < (*head)->val ? &(*head)->left : &(*head)->right, val);
}

if (head->left != NULL) is exactly the same as if (head->left).
In friendlyLookup() you are copying one of two string-literals into a (far too big) array, just to copy it again. Why? Use a pointer.

const char* resultString = result > -1 ? " is present in the bst\n" : " is not present in the bst\n";

Also, numberString is far too big, but whatever.
printAndCleanString is a curious name. If such a utility-function makes sense, it should be "free" instead of "clean".
If you want to output a string, don't give it as format-string to printf(), but as an argument. Make it:

printf("%s", string);


Optimizing dynamic table creation: JavaScript vs. jQuery I'm on the road to prepare for the 70-480 Microsoft exam and I'm learning the basics of jQuery.

Now I wrote the following JavaScript function to create a table dynamically:

function MakeTableJavascript() {
    var rows = document.getElementById("rows").value;
    var cols = document.getElementById("cols").value;
    var placeholder = document.getElementById("placeholder");
    var table = document.createElement("table");
    table.setAttribute("border", "1");

    for (var r = 0; r <= rows; r++) {
        var row = document.createElement("tr");
        for (var c = 0; c < cols; c++) {
            if (r == 0) {
                var head = document.createElement("th");
                var texthead = document.createTextNode("Column " + c);

                head.appendChild(texthead);
                row.appendChild(head);
            }
            else {
                var col = document.createElement("td");
                var textrow = document.createTextNode(c.toString() + r.toString());

                col.appendChild(textrow);
                row.appendChild(col);
            }
        }
        table.appendChild(row);
    }
    placeholder.appendChild(table);
}

Then I re-wrote the same function with jQuery:

function MakeTablejQuery() {
    var rows = $('#rows').val();
    var cols = $('#cols').val();
    var placeholder = $('#placeholder').val();
    var table = document.createElement("table");

    $('table').attr("border", 1);
    for (r = 0; r <= rows; rows++) {
        var row = document.createElement("tr");
        for (var c = 0; c < cols; c++) {
            if (r == 0) {
                var head = document.createElement("th");
                var texthead = document.createTextNode("Column " + c);

                head.appendChild(texthead);
                row.appendChild(head);
            }
            else {
                var col = document.createElement("td");
                var textrow = document.createTextNode(c.toString() + r.toString());

                col.appendChild(textrow);
                row.appendChild(col);
            }
        }
        table.appendChild(row);
    }
    placeholder.appendChild(table);
}

The result in term of lines of written code is practically the same.

Surely I can use jQuery in a better way to write less code, but at the actual state I have no idea on how to do so.

Can anyone more experienced in jQuery give me a suggestion to narrow the above code?

Well, how about

function MakeTablejQuery(rows, cols) {
    var table = $("<table/>").attr("border", 1);
    for (r = 0; r < rows; r++) {
        var row = $("<tr/>");
        for (var c = 0; c < cols; c++) {
            if (r == 0) {
                row.append($("<th/>").text("Column " + c))
            } else {
                row.append($("<td/>").text(c.toString() + r.toString()))
            }
        }
        table.append(row);
    }
    $("#placeholder").append(table);
}

MakeTablejQuery(5, 3);

And here's a more elegant version in a functional style:

Number.prototype.times = function(fn) {
    for(var r = [], i = 0; i < this; i++)
        r.push(fn(i));
    return r;
}

function MakeTablejQuery(numRows, numCols) {

    var header = numCols.times(function(c) {
        return $("<th/>").text("Column " + c);
    });

    var row = function(r) {
        return $("<tr/>").append(numCols.times(function(c) {
            return $("<td/>").text([c, r].join(""));
        }));
    };

    return $("<table/>")
            .append(header)
            .append(numRows.times(row))
            .attr("border", 1);
}

$("#placeholder").append(MakeTablejQuery(5, 3));

Also, jQuery is not about "less" code, it's about more readable code - note how the last "return" statement reads almost like an english sentence.


GUID to hex-string and back I'm new to Python and I'm trying to convert between Microsoft GUIDs and plain hex strings:

def hex_to_guid(hex):
    h = binascii.unhexlify(hex)
    return '-'.join(map(bytes.decode, map(
        binascii.hexlify, (h[0:4][::-1], h[4:6][::-1], h[6:8][::-1], h[8:10], h[10:]))))

def guid_to_hex(guid):
    g = binascii.unhexlify(guid.translate(str.maketrans('', '', '-')))
    return ''.join(map(bytes.decode, map(
        binascii.hexlify, (g[3::-1], g[5:3:-1], g[7:5:-1], g[8:]))))

I suspect this may not be very readable. Am I being too clever?

This could definitely be easier to follow.

Here are some suggestions:

Add some comments and docstrings.

The single English sentence in your questions is a big help to understanding the purpose of these functions, and where I might find more information, but it’s not attached to the code.

Something as simple as:

def hex_to_guid(hex):
    """Convert a hex string to a Microsoft GUID"""
    ...

would be a big improvement. If you ever look back over this code, it’ll be easier if you have this reference alongside the function.
Label the parts of the GUID.

In hex_to_guid, you’re packing too much into that final line, and it’s not obvious what any of the components mean. Working from Microsoft’s specification, I think I was able to work out what all the slices were, and then I put them in named variables:

# The Microsoft GUID format has four named fields.  Split the
# bytes into the four fields.  Since the fields are big-endian,
# we need to reverse the stream returned by unhexlify().
# https://msdn.microsoft.com/en-us/library/aa373931(VS.85).aspx
data1 = reversed(h[0:4])
data2 = reversed(h[4:6])
data3 = reversed(h[6:8])
data4a = h[8:10]
data4b = h[10:]

return '-'.join(map(bytes.decode, map(
    binascii.hexlify, (data1, data2, data3, data4a, data4b))))

Now it’s much easier to check whether the implementation matches the spec.

(I’m not confident I’ve got this right, but it’s much easier to check when the parts are named and referenced than before.)
Prefer list comprehensions to map().

They’re much more Pythonic, and generally much easier to read. You should also split your code into intermediate steps – it makes it easier to disentangle and debug the individual parts. Something like this seems more natural to me:

components = [data1, data2, data3, data4a, data4b]

hex_bytes = [binascii.hexlify(d) for d in components]
decoded = [bytes.decode(b) for b in hex_bytes]

return '-'.join(decoded)

There’s a single logical operation per line of code. You don’t have to unpack complicated map() expressions to read it.

(My variable names are terrible; you can probably think of something better.)
Don’t do too much on a single line.

You should never do too much on a single line of code. Readability is much improved by splitting operations over multiple lines, and assigning your intermediate results to variables with meaningful names.

The first line of guid_to_hex could easily be split across two or three lines (possibly with interspersed comments to explain what’s going on). Here’s how I might do it:

# Remove all dashes from the guid string
# @@AWLC: I don't know why you went via translate() and maketrans()
# for something that it seems like replace() would handle.
guid = guid.replace('-', '')

g = binascii.unhexlify(guid)

You can then name and unpack the components as before.

For what it’s worth, this is what I was left with when I finished writing this answer. It’s substantially longer (although a lot of it is whitespace and prose), but I also claim that a new reader will find my version substantially easier to read, understand and debug:

def hex_to_guid(hex):
    """Convert a hex string to a Microsoft GUID"""
    h = binascii.unhexlify(hex)

    # The Microsoft GUID format has four named fields.  Split the
    # bytes into the four fields.  Since the fields are big-endian,
    # we need to reverse the stream returned by unhexlify().
    # https://msdn.microsoft.com/en-us/library/aa373931(VS.85).aspx
    data1 = reversed(h[0:4])
    data2 = reversed(h[4:6])
    data3 = reversed(h[6:8])
    data4a = h[8:10]
    data4b = h[10:]

    components = [data1, data2, data3, data4a, data4b]

    hex_bytes = [binascii.hexlify(d) for d in components]
    decoded = [bytes.decode(b) for b in hex_bytes]

    return '-'.join(decoded)

def guid_to_hex(guid):
    """Convert a Microsoft GUID to a hex string"""
    guid = guid.replace('-', '')
    g = binascii.unhexlify(guid)

    # As in hex_to_guid, we split the GUID into four named fields.
    data1 = reversed(g[0:3])
    data2 = reversed(g[3:5])
    data3 = reversed(g[5:7])
    data4 = g[8:]

    components = [data1, data2, data3, data4]

    hex_bytes = [binascii.hexlify(d) for d in components]
    decoded = [bytes.decode(b) for b in hex_bytes]

    return '-'.join(decoded)

This is still by no means perfect: there’s still a lot of code repetition in these two functions. Brevity and cleanliness are good, but they’re on a sliding scale with readability: you should find a balance, not an extreme.


Uncompressing a list with some run-length-encoded elements Regarding the following code

;; ANSI CL - p37
;; Run length encoding: expansion

;; > (uncompress '((3 1) 0 1 (4 0) 1))
;; (1 1 1 0 1 0 0 0 0 1)
(defun uncompress (lst)
  (if (null lst)
      nil
      (let ((elt (car lst))                     ; [1]
            (rest (uncompress (cdr lst))))      ; [2]
        (if (consp elt)
            (append (apply #'list-of elt) rest) ; [3]
            (cons elt rest)))))                 ; [4]

(defun list-of (n elt)
  (if (zerop n)
      nil
      (cons elt (list-of (- n 1) elt))))

;; [1] the compressed form, whether atom or list.
;; [2] uncompress the rest. recursion happens here. curious.
;; [3] compressed form, so expand it, & append to uncompressed rest.
;; [4] not a compressed form, so just cons it.

Left to my own devices, I would have written uncompress like so

;; How my intuition would have said to write it, with the recursive
;; calls made where their results are used:
(defun uncompress (lst)
  (if (null lst)
      nil
      (let ((elt (car lst)))
        (if (consp elt)
            (append (apply #'list-of elt) (uncompress (cdr lst)))
            (cons elt (uncompress (cdr lst)))))))

Question

Computing the value of rest in the let in the first example appears to alter the path of the recursion.  Is that correct? Or are let forms merely substituted into later code where they are used?

There is a slight difference in the order of evaluation, in that the book's solution recurses before calling consp, whereas your solution recurses after calling consp. Either way, the recursion happens before concatenation, because the innermost expressions in (append (apply #'list-of elt) (uncompress (cdr lst))) and (cons elt (uncompress (cdr lst))))))) have to be evaluated first.

I think that your solution is a bit worse than the book's, in that the common expression (uncompress (cdr lst)) is written twice.


Converting coins to USD I'm using 2 objects inside my class. My actions are almost identical, however I want to return 2 different data types and both are slightly different from each other.

import requests

class coins:

    def coins_arr(self):
        params = {'limit': 10, 'convert': 'USD'}
        data = requests.get('https://api.coinmarketcap.com/v1/ticker/?%s' % params).json()
        coinsData = []
        for objects in data:
            for k in objects.keys():
                if k == 'id':
                    coinsData += objects[k].split()
        #returns array of coin names
        return sorted(coinsData)

    def coins_dict(self):
        params = {'limit': 10, 'convert': 'USD'}
        data = requests.get('https://api.coinmarketcap.com/v1/ticker/?%s' % params).json()
        coinsDict = {}
        for objects in data:
            for k,v in objects.items():
                if k == 'id':
                    coinsDict.update({objects[k]:objects['price_usd']})
        #returns dictionary with name:price
        return coinsDict

d = coins()

print (d.coins_arr())

I thought of returning the coins_dict.keys(), but it won't return me it in a sorted order. I am not sure if I can implement a lambda function when I call the class object.

First things first, the convention is to use uppercase-initial for class names.

All modules, functions, classes and methods should have docstrings.

According to their API documentation, if limit is not specified, the default is 100 and by default convert is 'USD'. Your coins_arr() method is returning 100 coins, not 10, which I think is due to an incorrect formatting of the placeholder in the URL you are passing to requests.get(). You are splitting each coin and concatenating it to coinsData, which is not necessary since you just need to append it to the list.

The coins_dict() method is also returning 100 coins, not 10.

There is also some repetition in the two methods, which you could avoid.

Here I would write the class, like so:

import requests

class Coins:
    """This is a class for cryptocurrencies."""

    def __init__(self, url, limit=10):
        self.url = url
        self.limit = limit
        self.data = requests.get(self.url.format(self.limit)).json()

    def get_coins(self):
        """Return a sorted list of cryptocurrencies."""

        coins = sorted(v for obj in self.data
                       for k, v in obj.items() if k == 'id') # sorted() returns a list

        return coins

    def get_coins_prices(self):
        """Return a dictionary of cryptocurrencies and their prices."""

        coins_prices = {obj[k]: obj['price_usd'] for obj in self.data
                        for k, v in obj.items() if k == 'id'}

        return coins_prices

if __name__ == '__main__':
    url = 'https://api.coinmarketcap.com/v1/ticker/?limit={}'

    obj_a = Coins(url)
    print(obj_a.get_coins())

    obj_b = Coins(url)
    print(obj_b.get_coins_prices())

    obj_c = Coins(url, limit=5) # Setting limit to 5 results
    print(obj_c.get_coins_prices())

The Python version used to test the above code is 3.6.

Hope that helps.


Programming a simple clock Here's the start of some code for a clock that can display time in 24 and 12 hours.

What could be added to it? How could I can apply notions such as OOP, model and Enums here?

public Clock {
    private int time = 0;

    public int get24hrTime(){
        return time;
    }

    public int set24hrTime( int newTime ){
        this.time = newTime;
    }

    public int get12hrTime(){
        if( time - 12 < 0 ){
            return time;
        } else {
            return time - 12;
        }
    }

    public String get12hrPostfix(){
        if( hours - 12 < 0 ){
            return "AM";
        } else {
            return "PM";
        }
    }
}

Why do you check the time with (hours - 12 < 0). This involves two operations, one subtraction and one compare. You could simply use (hours < 12).

Also you rely that in the method set24hrTime(int newTime) the time is never > 23 for your code to work. Why don't you check this?

Also what about the value 0 for your hours? This must be displayed as 12 AM. At the moment you return 0 in this case where it must be 12.

Also for the value 12 of the hours you return 0 as value for the 12hrTime which is wrong.

To sum up I would suggest the following code:

public Clock {
    private int time = 0;
    public int get24hrTime() {
        return time;
    }
    public int set24hrTime(int newTime) {
        if (newTime > 23 || newTime < 0) {
            throw new IllegalArgumentException();
        }
        this.time = newTime;
    }
    public int get12hrTime(){
        if (time == 0) {
            return 12; // Special case 0
        } else if (time <= 12) {
            return time;
        } else {
            return time - 12;
        }
    }
    public String get12hrPostfix() {
        if (time < 12) {
            return "AM";
        } else {
            return "PM";
        }
    }
}


Python script to touch files of a particular pattern in all folders of a given name The intent of this script is to hunt down all "*/testResults/*.xml" files and touch them, so that our build system doesn't issue errors that the files are too old (it's entirely intentional that tests are only re-run when the libraries they change have been modified)

import fnmatch
import os
import time

matches = []

# Find all xml files  (1)
for root, dirnames, filenames in os.walk('.'):
  for filename in fnmatch.filter(filenames, '*.xml'):
      matches.append(os.path.join(root, filename))

# filter to only the ones in a "/testResults/" folder  (2)
tests = [k for k in matches if "\\testResults\\" in k]

t = time.time()

# touch them
for test in tests:
    os.utime(test,(t,t))

Is there a simpler way to achieve this?  Specifically to perform steps (1) and (2) in a single process, rather than having to filter the matches array?  An alternative solution would be to just recursively find all folders called "/testResults/" and list the files within them.

Notes:

This script would also find "blah/testResults/blah2/result.xml" - I'm not worried about that, as the testResults folders will only contain test result xml files (but not all xml files are test results!)
This runs on Windows.

For combining (1) and (2) you can reverse the sort by only trying if your are under a test directory

Also this is a great use for a generator / map combo to avoid extra loops

import os
import re

is_xml = re.compile('xml', re.I)
is_test = re.compile('testResults', re.I)

def find_xml_tests(root):
    for current, dirnames, filenames in os.walk(root):
        if is_test.search(current):
            for filename in filter(lambda p: is_xml.search(p),  filenames):
                yield  os.path.normpath(os.path.join(current, filename))

def touch(filename ):
    os.utime(test,(time.time(),time.time()))

map(touch, find_xml_tests('path/to/files'))


Proper way of mapping in C# I have a WCF service that itself uses another third party WCF service. It's basically a proxy. So I'm getting a request to my own one and I have just to "forward" it to the third party one.

I would like then to map from my flattened request (that's the way it's been decided to be coded) to the slightly more complex request the other service needs.

Although quite a few of the properties in each request are a naming match my issues are the following

There are properties flattened in my request in relation to theirs
There's a huge load of my properties that should go into theirs "generic" Data[] ApplicationDetails property being both as follows:

public class ApplicationDetails
{
    public Data[] Data { get; set; }
}

public class Data
{
    public string category { get; set; }

    public string attribute { get; set; }

    public string Value { get; set; }
}

And those will come from my request as a property named as the attribute should be, i.e. my request will have a property named AIM and I should put that as an the attribute of one of the elements of the Data array, it's value as the element's value and hardcode the category (there are 3 of them).

I couldn't see many advantages (apart for the equally named properties) for using Automapper (or any other it could be) and ended up with a massive static Mapper class that looks this way

public static class Mapper
{
    public static Request FromDecisionRequestToZRequest(DecisionRequest request)
    {
        var applicationDetails = new Data[]
        {
            new Data {category = "ID", attribute = "PubID", Value = request.PubID},
            new Data {category = "ID", attribute = "AID", Value = request.AID},
            new Data {category = "APP", attribute = "NID", Value = request.NID},
            new Data {category = "BOOK", attribute = "DupeApps90", Value = request.DupeApps90},
            new Data {category = "BOOK", attribute = "DupeApps30", Value = request.DupeApps30},

            //And many others of each of the three categories

        };

        var address = new Address[]
        {   new Address{
                City = request.City,
                Country = request.Country,
                HouseNumber = request.HouseNumber,
                HouseNumberExtension = request.HouseNumberExtension,
                Street = request.Street,
                ZIP = request.ZIP,
                kind = "MAIN"
            }
        };

        var ret = new Request()
        {
            UserName = request.UserName,
            RequestDateTime = Convert.ToDateTime(request.RequestDateTime),
            CompanyRegistrationID = request.CompanyRegistrationID,

            //And many others "direct" mappings

            Addresses = new Addresses { Address = address },
            Phone1 = new Phone1 { type = "1", Value = request.Phone1 },
            Phone2 = new Phone2 { type = "2", Value = request.Phone2 },
            Phone3 = new Phone3 { type = "3", Value = request.Phone3 },
            Email = request.Email,
            Amount = new Amount { Value = request.Amount },

            ApplicationDetails = new ApplicationDetails { Data = applicationDetails }
        };

        return ret;
    }
}

How would you tackle this?  To me this static class looks horrible (taking into account that the full one has more than 150 lines but not sure how I could improve it).

I would create two extension method , ToApplicationData and ToAddress and will define mapping over there. it will sorten you code and much better readablity. you can break this methods too if you want.

 public static class Mapping
    {
        public static Data[] ToApplicationData (this DecisionRequest request)
        {
            return new[]
            {
                new Data {category = "ID", attribute = "PubID", Value = request.PubID},
                new Data {category = "ID", attribute = "AID", Value = request.AID},
                new Data {category = "APP", attribute = "NID", Value = request.NID},
                new Data {category = "BOOK", attribute = "DupeApps90", Value = request.DupeApps90},
                new Data {category = "BOOK", attribute = "DupeApps30", Value = request.DupeApps30}
            };
        }

        public static Address[] ToAddresses(this DecisionRequest request)
        {
            return new Address[]
            {
                new Address
                {
                    City = request.City,
                    Country = request.Country,
                    HouseNumber = request.HouseNumber,
                    HouseNumberExtension = request.HouseNumberExtension,
                    Street = request.Street,
                    ZIP = request.ZIP,
                    kind = "MAIN"
                }
            };
        }
    }

This is how I will use this code

    public static class Mapper
    {
        public static Request FromDecisionRequestToZRequest(DecisionRequest request)
        {
            var applicationDetails = request.ToData();

            var address = request.ToAddresses();

            var mappedRequest = new Request
            {
                UserName = request.UserName,
                RequestDateTime = Convert.ToDateTime(request.RequestDateTime),
                CompanyRegistrationID = request.CompanyRegistrationID,
                Addresses = new Addresses {Address = address},
                Phone1 = new Phone1 {type = "1", Value = request.Phone1},
                Phone2 = new Phone2 {type = "2", Value = request.Phone2},
                Phone3 = new Phone3 {type = "3", Value = request.Phone3},
                Email = request.Email,
                Amount = new Amount {Value = request.Amount},
                ApplicationDetails = new ApplicationDetails {Data = applicationDetails}
            };

            return mappedRequest;
        }
    }


Flatten iterator for nested list in python3 flatten.py

from collections import Iterable, Mapping
from operator import methodcaller

from stack import Stack

class flatten(object):
    """
    Iterator that walk through nested iterable.
    If the initial item is not iterable, only itself will be yielded.
    Args:
        item         (Object): Any object.
        depth (Optional[int]): Maximum nesting layer.
        mapping_iter    (str): the method to be called when handling mapping

    >>> from collections import OrderedDict
    >>> kwargs = OrderedDict()
    >>> kwargs['file'] = 'test.txt'
    >>> kwargs['data'] = [[0, 1], [-1, 0]]
    >>> tuple(flatten(kwargs))
    ('test.txt', 0, 1, -1, 0)
    >>> tuple(flatten('apple'))
    ('apple',)
    """

    def __init__(self, item, depth=128, map_iter='values'):
        self.map_iter = methodcaller(map_iter)

        done, item = self.expand(item)
        if done:
            item = iter((item,))
        self.stack = Stack([item], depth)

    def __iter__(self):
        return self

    def __next__(self):
        while self.stack:
            for item in self.stack[-1]:
                done, item = self.expand(item)
                if done:
                    return item
                else:
                    self.stack.append(item)
                    break
            else:
                self.stack.pop()
        raise StopIteration

    def expand(self, item):
        if isinstance(item, str):
            return True, item
        elif isinstance(item, Mapping):
            return False, iter(self.map_iter(item))
        elif isinstance(item, Iterable):
            return False, iter(item)
        else:
            return True, item

class named_flatten(flatten):
    """
    Iterator that walk through nested iterable and create tuple key.
    For non Mapping iterable, the key are generated by enumerate.
    If the initial item is not iterable, only itself will be yielded.
    Args:
        item         (Object): Any object.
        depth (Optional[int]): Maximum nesting layer.

    >>> from collections import OrderedDict
    >>> kwargs = OrderedDict()
    >>> kwargs['file'] = 'test.txt'
    >>> kwargs['data'] = [[0, 1], [-1, 0]]
    >>> result = tuple(named_flatten(kwargs))
    >>> result[0]
    (('file',), 'test.txt')
    >>> result[1]
    (('data', 0, 0), 0)
    >>> tuple(named_flatten('apple'))
    (((), 'apple'),)
    """

    def __init__(self, item, depth=128):
        super().__init__(((), item), depth, map_iter)

    def expand(self, item):
        key, value = item
        if isinstance(value, str):
            return True, item
        elif isinstance(value, Mapping):
            it = value.items()
        elif isinstance(value, Iterable):
            it = enumerate(value)
        else:
            return True, item
        return False, ((key+(k,), v) for k, v in it)

if __name__ == '__main__':
    import doctest
    doctest.testmod()

stack.py

from collections import deque

class FullStackError(MemoryError):
    pass

class Stack(deque):

    @property
    def full(self):
        return 0 == (self.maxlen - len(self) if self.maxlen else self.maxlen)

    def append(self, x):
        if self.full:
            raise FullStackError
        super().append(x)

    def appendleft(self, x):
        if self.full:
            raise FullStackError
        super().appendleft(x)

    def insert(self, i, x):
        if self.full:
            raise FullStackError
        super().insert(i, x)

    def extend(self, iterable):
        for x in iterable:
            self.append(x)

    def extendleft(self, iterable):
        for x in iterable:
            self.appendleft(x)

    def __str__(self):
        return 'Stack' + super().__str__()[5:]

    def __repr__(self):
        return 'Stack' + super().__repr__()[5:]

Conceptually, this is a recursion problem.  You've unrolled the recursion into iteration to support the __iter__() method.  However, this is precisely the kind of problem that generators can solve trivially!  (Here, I've used yield from, which is a feature introduced in Python 3.3.)

from collections import Iterable, Mapping
from operator import methodcaller

def flatten(it, map_iter='values', max_depth=128):
    if max_depth < 0:
        raise RecursionError('maximum recursion depth exceded in flatten')
    elif isinstance(it, str):
        yield it
    elif isinstance(it, Mapping):
        for item in methodcaller(map_iter)(it):
            yield from flatten(item, map_iter=map_iter, max_depth=max_depth-1)
    elif isinstance(it, Iterable):
        for item in it:
            yield from flatten(item, map_iter=map_iter, max_depth=max_depth-1)
    else:
        yield it

I've named the parameter it, which is a triple entendre that can be interpreted as "iterable", "item", or just the third-person singular impersonal pronoun.

Instead of FullStackError, which leaks information about how your iterator is implemented, just use the built-in RecursionError.  Personally, I'd choose to scrap the depth limit altogether, since the Python interpreter's stack will have its own natural recursion limit.


Pounds-to-tons converter Finished my first lone project and I'm wondering if the execution is passable. This is a basic lbs to tons converter. The customer drives on the scale loaded, then dumps the load  and returns unloaded to the scale again. We need to convert the leftover lbs to tons. I am wondering if this is a concise way to perform this task.

loaded weight - unloaded weight = Total lbs in scrap

total lbs * 0.0005  = weight in tons

function calc() {
    let loadedAmount = document.getElementById("loaded").value;
    let unloadedAmount = document.getElementById("unloaded").value;
    let final = document.getElementById("result").value =
Math.round(`${(loadedAmount - unloadedAmount) * 0.0005}` * 100)/100;
    let resultInPounds = document.getElementById("result").value =
Math.round(`${(loadedAmount - unloadedAmount)}`);

    if (!loadedAmount) {
       document.getElementById("loaded").value=alert("waiting for an initial value for loaded");
    } else {
        loadedAmount = parseInt(loadedAmount);
    }
    if (!unloadedAmount) {
        document.getElementById("unloaded").value = alert("waiting for an initial value for unloaded");

    } else {
        unloadedAmount = parseInt(unloadedAmount);
    }
    if (loadedAmount < unloadedAmount) {
        alert("loaded amount must be greater than unloaded amount");
    }
    console.log(`loadedAmount: ${loadedAmount}`);
    console.log(`unloadedAmount: ${unloadedAmount}`);

    document.getElementById("result").innerHTML=final + " Tons or " +
resultInPounds + " lbs";

    console.log(result);

}
<!DOCTYPE html>

    <meta charset="utf-8">
<title></title>

    Negaunee Iron and Metal

input loaded pounds
            <input type="number"  id="loaded" placeholder="Loaded...">

 input unloaded pounds
            <input type="number" id="unloaded" placeholder="Unloaded" >

        <button type="submit" id="button" onclick="calc()">Click To
Convert</button>

The label element can be used for the labels of the input fields.

The placeholder attribute shouldn’t be used for labeling.

The output element can be used for the result. (You are using a label, which is not appropriate; and note that label can’t have a type attribute.)

Maybe a button with the button type is more appropriate here. (Button type “button” vs. “submit”)

This results in:

Loaded pounds
<input type="number" id="loaded">

Unloaded pounds
<input type="number" id="unloaded">

<button type="button" id="button" onclick="calc()">Convert</button>

<output for="loaded unloaded" id="result"></output>


Euler #4: Refined Palindrome in Haskell This is my attempt at the Problem Euler #4 in Haskell ("Find the largest palindrome made from the product of two 3-digit numbers.")

import Data.List

isPalindrome :: Show a => a -> Bool
isPalindrome n = l == reverse l
  where l = show n

maxPalindrome :: (Integral a, Show a) => a
maxPalindrome = maximum $ head . transpose $ allPalindrome <$> [999, 998 .. 1]
  where allPalindrome x = filter (isPalindrome) $ (x *) <$> [999, 998 .. x]

To my surprise I didn't see any such optimisation in the snippets I found (the head . transpose is there to only consider the highest of each pairs), which surprised me. However this is still running about 0.5 seconds which I find still slow?

Is there a way to make it run faster? I am aware of Project Euler #4 in Haskell however, my question is not about the algorithm I use but about its implementation.

Do you have any other recommendation about my code?

Thank you very much in advance

Your code is fine, but I would suggest some small changes. Instead of head . transpose, I would use concatMap (take 1). This captures your intend to take the first (and therefore largest) number from each allPalindrome.

Next, I would use Int instead of (Integral a, Show a), since 999 * 999 is smaller than maxBound :: Int. Why? Because by default, Integer will be used for Integral types, if they were note specified. Therefore, you end up with maxPalindrome handled as a Integer, which is slower than Int.

And last, but not least, I would stop at 111, since 111 * 111 is a palindrome.

We end up with:

isPalindrome :: Show a => a -> Bool
isPalindrome n = l == reverse l
  where l = show n

maxPalindrome :: Int
maxPalindrome = maximum $ concatMap (take 1) $ allPalindrome <$> [999, 998 .. 111]
  where allPalindrome x = filter isPalindrome $ (x *) <$> [999, 998 .. x]

main :: IO ()
main = print maxPalindrome

Note that you should compile your code if you want to check its performance.

Alternatively, if you want to keep maxPalindrome's type, use :: Int:

maxPalindrome :: (Integral n, Show n) => n
maxPalindrome = maximum $ concatMap (take 1) $ allPalindrome <$> [999, 998 .. 111]
  where allPalindrome x = filter isPalindrome $ (x *) <$> [999, 998 .. x]

main :: IO ()
main = print (maxPalindrome :: Int)


Merge Sort C++11 C++17 Question

Any way I can optimize this further using new C++11 or C++17 features? Would also like feedback on my variable naming, memory management, and style (notice my placement of varible_name++ in some areas), and how I've abstracted the sort into a recursive_merge_sort function and a merge function.

Code

#include <iostream>

void merge(int* data, int low, int mid, int high)
{
    int low_copy = low;
    int mid_copy = mid;

    int size = high - low;
    int* sorted = new int[size];
    int i = 0;

    while(low_copy < mid && mid_copy < high) {
        if(data[low_copy] < data[mid_copy]) {
            sorted[i] = data[low_copy++];
        }
        else {
            sorted[i] = data[mid_copy++];
        }
        ++i;
    }

    while(low_copy < mid) {
        sorted[i++] = data[low_copy++];
    }
    while(mid_copy < high) {
        sorted[i++] = data[mid_copy++];
    }

    for(i = 0; i < size; ++i) {
        data[low + i] = sorted[i];
    }
}

void recursive_merge_sort(int* data, int low, int high) {
    if(low >= high - 1) { return; }

    int mid = (high + low) / 2;
    recursive_merge_sort(data, low, mid);
    recursive_merge_sort(data, mid, high);

    merge(data, low, mid, high);
}

void merge_sort(int* data, int num_elements) {
    recursive_merge_sort(data, 0, num_elements);
}

int main()
{
    int data[] = { 5, 1, 4, 3, 65, 6, 128, 9, 0 };
    int num_elements = 9;

    std::cout << "unsorted\n";
    for(int i=0; i < num_elements; ++i) {
        std::cout << data[i] << " ";
    }

    merge_sort(data, num_elements);

    std::cout << "\nsorted\n";
    for(int i=0; i < num_elements; ++i) {
        std::cout << data[i] << " ";
    }

    return 0;
}

Design
Passing data with high and low works. But is equivalent to data+low and data+high. And just passing two values.
This is what is referred to as an Iterator range. If you look at the standard library you will see that a lot of the concepts hinge around iterators you should look that up and understand how it works.
Code Review
For every call to new there should be a call to delete.
    int* sorted = new int[size];

Or so the old text books tell us. In modern C++ you probably should never call new or delete (unless you know its a good idea).
But here you are leaking memory because you don't release it. You should add a delete [] sorted; to the end of the function.
BUT leaving the new/delete in your code is not a good idea. It's not exception safe to start with. So simply declare a vector<int> here and let it manage the memory for you.
We should also note that memory management is very expensive. So rather than doing this every loop. Why not do it once at the beginning and reuse the memory.
Sure this works:
    while(low_copy < mid && mid_copy < high) {
        if(data[low_copy] < data[mid_copy]) {
            sorted[i] = data[low_copy++];
        }
        else {
            sorted[i] = data[mid_copy++];
        }
        ++i;
    }

But we could simplify it:
     for(;low_copy < mid && mid_copy < high; ++i) {
        sorted[i] = (data[low_copy] < data[mid_copy])
                       ? data[low_copy++];
                       : data[mid_copy++];
    }

Also when you see loops like this you should look at the standard algorithms to see if there is one that will do all that work for you.
Another thing to note is that you are copying the value from data to sorted. This is fine for integers, but you should be able to sort nearly any C++ type and copying a C++ type is not usually the most efficient way to get the value from one location to another. You should look up the concept of moving. When the type being sorted is a bit more complex moving it (rather than copying is usually a better idea).
These loops can definitely be done by standard algorthims:
    while(low_copy < mid) {
        sorted[i++] = data[low_copy++];
    }
    // Or
    std::copy(data + low_copy, data + mid, sorted + i);
    // Or (when you move objects)
    std::move(data + low_copy, data + mid, sorted + i);

This is subject to overflow.
    int mid = (high + low) / 2;

Common mistake. If high and low are both large then this could easily overflow. Do the division fist (on the difference). Then add to the low.
    int mid = low + (high - low) / 2;

Are you sure there are 9 elements?
    int data[] = { 5, 1, 4, 3, 65, 6, 128, 9, 0 };
    int num_elements = 9;

Common source of errors. Don't manually do stuff the compiler can work out correctly. This is especially true if data is subsequently modified (and human person can make mistakes) does not change num_elements.
    int data[] = { 5, 1, 4, 3, 65, 6, 128, 9, 0 };
    int num_elements = sizeof(data)/sizeof(data[0]);

In more modern versions of C++ we even have a standard way of doing it:
    int num_elements = std::size(data);

No need for this in main:
    return 0;


Click and/or focus outside & -webkit-tap-highlight-color I'm working code sample here.

Note: Above only tested in Firefox 13.0.1, iPad (non-retina) and Chrome 20.0.x.

Here's the relevant code:

var $outside = $(document)
    .add($('html'))
    .add($(':not[' + data.target + ']'));

$outside.on('focusin.goober click.goober', function(e) {

    if ($(e.target).closest(data.target).length === 0) {

        $outside.off('.goober');

        data.divs.hide(); // Collection of sub menu divs.

    } else {

        return true;

    }

}).css('-webkit-tap-highlight-color', 'rgba(0, 0, 0, 0)'); // Hide tap highlight color via Mobile Safari.

I'm looking for feedback on two issues:

Clicking and/or focusing outside: Where you see

var $outside = $(document).add($('html')).add($(':not[' + data.target + ']'));

that's where I check for focusin/click events outside the target menu. I have my reservations about applying event handlers to everything just to detect an outside focus/click. Without using a plugin, is there a better way to detect focus/click outside of a target element? Could this code be improved?
I'm also concerned about how -webkit-tap-highlight-color behaves when tapping outside via iPad. As a quick monkey patch, I've added

.css('-webkit-tap-highlight-color', 'rgba(0, 0, 0, 0)')

but I don't yet see that as the perfect solution.

In regards to #2 above:

I wonder if I should add a CSS class to the <html> element, something like "disable-highlight", and have in my CSS

.disable-highlight * { -webkit-tap-highlight-color: rgba(0, 0, 0, 0); }

and have JS add the class when the menu is being interacted with (and remove it when its not)? I rarely use * selector in my CSS these days, but as a temporary way to hide the tap/click highlight, maybe that's not a bad idea?

After several days of hard work, I think that I've found the answers to my questions.

The best element I've found to check for outside click/focus is $(document).

Caveat: When on iPad, the event needs to be touchstart, not click.

I've got a question going here, but here's the "trick" I'm using currently:

var foo = (('ontouchstart' in window) || (window.DocumentTouch && document instanceof DocumentTouch)) ? 'touchstart' : 'click';

I'm using this technique here to handle focus/click. Essentially, when there's keyboard tab focus, I call click and within that event handler, I call my code to check for outside focus/click/touch.
The idea I had to add a CSS class to <html> works pretty good. When the event handlers are setup to listen for focus/click/touch outside, I add the class. Once there's been focus/click/touch on the outside, I remove the class.

My CSS looks like so:

html.mw-outside * { -webkit-tap-highlight-color: rgba(0, 0, 0, 0); }

It kinda feels a little hacky, though.


Function that uses an exception to check if user exists I have a User class that will throw an exception if the userid provided in the parameter does not correspond with a user.

I've written a helper function that will return true or false depending on whether a userid belongs to a user or not, but was wondering if my use of a try {} catch {} to do so is correct:

    private function userExists($userid)
    {
       $user_does_not_exist = false;
       try
       {
          $user = new User($userid);
       }
       catch (Exception $e)
       {
          $user_does_not_exist = true;
       }

       return $user_does_not_exist;
   }

Any and all critique is greatly appreciated.

I would suggest working the other way around. Expect that a user does not exist and only return true if a user is found.

private function UserExists($userId)
{
    $userExists = false;

    try
    {
        $user = new User($userId);
        if($user !== null)
            $userExists = true;
    }
    catch (Exception $e)
    {
        //handle exception
    }

    return $userExists;
}

You can drive this further and ease things by not using a variable to return, like this:

private function UserExists($userId)
{
    try
    {
        return new User($userId) !== null;
    }
    catch (Exception $e)
    {
        return false;
    }
}


Program that handles program options I'm using boost::program_options to parse input from the user. However, while the standard library makes this an easy task to do, it's causing an (in my opinion) excessive amount of allocations. Of course, for such a small program on modern systems, it probably isn't that big of a deal, but I would like to nip the problem in the bud before it spirals out of control.

How can I reduce the number of allocations in this program?

Here's an example of what valgrind reports:

 $ test
Compression level was not set.
 $ test --help
Allowed options:
  --help                produce help message
  --compression arg     set compression level

 $ test --compression 10
Compression level was set to 10.
 $ test --compression 5
Compression level was set to 5.
 $ test --compressio

error: the required argument for option '--compression' is missing
total heap usage: 266 allocs, 266 frees, 11,430 bytes allocated

And the code dump:

#include <algorithm>
#include <iostream>
#include <iterator>
#include <string>
#include <vector>
#include <memory>
#include <sstream>

#include <cstring>

#include <boost/program_options.hpp>
#include <boost/iterator/transform_iterator.hpp>
namespace po = boost::program_options;

// "std::vector<std::string> to char* array"
// http://stackoverflow.com/a/7048972/3920237
const char *convert_to_cstr(const std::string & s)
{
   return s.c_str();
}

int main(int argc, char* argv[])
{
    po::options_description desc("Allowed options");
    desc.add_options()
        ("help", "produce help message")
        ("compression", po::value<int>(), "set compression level")
    ;

    std::string input = "";

    const std::string prompt = " $ ";

    std::vector<std::string> args;
    std::istringstream iss;

    std::string token;

    po::variables_map vm;
    while (true)
    {
        try {
            std::cout << prompt;
            std::getline(std::cin, input);
            if (input == "quit")
                break;

            iss.clear();
            iss.str(input);

            args.clear();
            while (iss >> token)
            {
                args.push_back(token);
            }

            auto beg = boost::make_transform_iterator(args.begin(),
                convert_to_cstr);
            auto end = boost::make_transform_iterator(args.end(),
                convert_to_cstr);

            const std::vector<const char*> vc { beg, end };

            /* std::transform(args.begin(), args.end(), std::back_inserter(vc), convert); */
            vm.clear();
            po::store(po::parse_command_line(vc.size(), vc.data(), desc), vm);
            po::notify(vm);

            if (vm.count("help")) {
                std::cout << desc << "\n";
                continue;
            }

            if (vm.count("compression")) {
                std::cout << "Compression level was set to "
                     << vm["compression"].as<int>() << ".\n";
            } else {
                std::cout << "Compression level was not set.\n";
            }
        }
        catch(std::exception& e) {
            std::cerr << "error: " << e.what() << "\n";
            continue;
        }
        catch(...) {
            std::cerr << "Exception of unknown type!\n";
        }
    }

    return 0;
}

I think you should not really need to worry about that. This is for sure a non-performance critical part of your application and all the objects you create are almost immediately freed. My advice is to avoid premature optimisation and to focus on code readability instead.

I'd use some meaningful names. I'd replace iss, beg, vm with inputStream, beginning, variables_map. It improves readability a lot.

You should also consider splitting your code in different methods and make sure that everything has a single concern. Your main is doing everything and that's not very good in my opinion.
What about moving your prompt declaration to a constant defined at class level instead that inside the main?
I'd also introduce a create_options_descriptions() and a bool execute_next_command(...) methods.

Your main should look like:

int main(int argc, char* argv[])
{
    po::options_description options_description = create_options_description();
    po::variables_map vm;

    /*
     * It's better to pass std::cin and std::cout as dependencies so it
     * will be trivial to change it with references to other streams
     * if you ever need to.
     */
    while(execute_next_command(std::cin, std::cout, vm));
}

Execute next should follow the same idea and be split in a few methods like: read_input(...) that produces a list of arguments and bool process_args(...) that will take care of processing them and do the appropriate thing.