Balance braces method This is a continuation of this question, at least the first part of that question.
I want to check if everything in the new code is right or if there are other way to make it better.

Exercise

The validBraces method takes a string of braces as a single parameter and determines if the order of the braces is valid.  All input strings will only consist of open curly braces {, closed curly braces }, open parentheses (, closed parentheses ), and open brackets [ and closed brackets ].

A string of braces is considered valid if all open braces are matched with their corresponding closed brace.  For example: (){}[] and ([{}]) would be considered valid, while (}, [(]), and [({})](] would be considered invalid.
An empty string is considered valid.
A non-string input is considered an invalid input.  The method should throw an InvalidArgumentException in this case.

class StringUtil {

  public function validBraces($braces) {
    if ($braces === "") {
      return true;
    } elseif (!is_string($braces)) {
      throw new InvalidArgumentException("Not a string");
    }

    $balance = array();

    for ($i = 0; $i < strlen($braces); $i++) {
      //allowed chars  (){}[]
      if (!($braces[$i] === "(" || $braces[$i] === ")"
           || $braces[$i] === "{" || $braces[$i] === "}"
           || $braces[$i] === "[" || $braces[$i] === "]")) {
        throw new InvalidArgumentException("Invalid string");
      } elseif ($braces[$i] === "(" || $braces[$i] === "{" || $braces[$i] === "[") {
        array_push($balance, $braces[$i]);
      } elseif ($braces[$i] === ")") {
        if (array_pop($balance) !== "(") {
          return false;
        }
      } elseif ($braces[$i] === "}") {
        if (array_pop($balance) !== "{") {
          return false;
        }
      } elseif ($braces[$i] === "]") {
        if (array_pop($balance) !== "[") {
          return false;
        }
      }
    }

    if (count($balance) == 0) {
      return true;
    }

    return false;
  }
}

public function validBraces($braces) {

I'd call this isValidBraces instead to show that it returns a Boolean value.  This also makes it read a little better in normal usage.

if ($braces === "") {

The difference between "" and '' is that the double quoted string allows variable interpolation.  Since you are not using variable interpolation, consider

if ( '' === $braces ) {

The single quoted string will also be slightly quicker to interpret, although I don't know that the difference would matter in this case.

I also changed the order.  It's not uncommon to see the constant on the left in C-based languages.  This is because '' = $braces would give a compiler error while $braces = '' will happily evaluate as false and continue.

Incidentally, you don't need this statement, as the later for loop and count check will handle this case for you.  There's an argument that the code is a little cleaner without this check.  It certainly would be a little shorter.

$balance = array();

I might call this $open_braces.  It doesn't balance anything.  It's a stack of all the currently open braces, so call it that.

for ($i = 0; $i < strlen($braces); $i++) {

As a general rule, you want to avoid calling a function on every loop iteration which always returns the same value.

for ($i = 0, $n = strlen($braces); $i < $n; $i++) {

That way you only do the strlen call once at the initialization of the loop.  Each iteration can do the simpler check instead.

  if (!($braces[$i] === "(" || $braces[$i] === ")"
       || $braces[$i] === "{" || $braces[$i] === "}"
       || $braces[$i] === "[" || $braces[$i] === "]")) {

It's possible to write the if more simply by first defining a constant:

private $closing_match_of = array( '(' => ')', '{' => '}', '[' => ']' );

Then set a variable in the function before the loop starts:

$opening_match_of = array_flip($this->closing_match_of);

The array_flip will reverse the arrows from the way that they are in CLOSING_MATCH_OF.  So where you had '{' => '}', you will now have '}' => '{'.  Using the array_flip ensures that you will have parallel matches in both.  You could declare a second constant, but then that would be fragile in the face of changes.  This way you can make changes in a single place.

Then replace the if as follows:

  if ( isset($this->closing_match_of[$braces[$i]]) ) {
    $open_braces[] = $braces[$i];
  } elseif ( isset($opening_match_of[$braces[$i]]) ) {
    if ( array_pop($open_braces) !== $opening_match_of[$braces[$i]] ) {
      return false;
    }
  } else {
    throw new InvalidArgumentException('Invalid string');
  }

This checks if the character at position $i in the $braces string has a closing match (i.e. is a key in the $this->closing_match_of array).  If so, then you know that it is an opening brace and add it to the list of things that you need to match.  If not, it checks if $braces[$i] has an opening match (i.e. if it is a key in the $opening_match_of array).  If so, then it checks to see if it matches the most recent unmatched open brace.  If not, then it returns false to indicate that the string does not consist of matching braces.

The final case is if the character has no closing match nor opening match defined in the arrays.  In that case, it's not a brace.  Since the string is supposed to consist entirely of braces, it throws the exception.  We don't have to check anything in this case because we already did the checks earlier in the if and elseif clauses.

If you are only adding one element to the end of an array, the []= operator is more efficient than an array_push.

I changed the order of the checks to avoid repeating them.  The other way you did two or three checks on a valid case and two on an invalid case.  This way, you do one or two on a valid case and two on an invalid case.  Hopefully the valid cases are more common.

Note that I haven't tried to run or test this code.


Chained hash table implementation While reading a data structure book, I have implemented a chained hash table. I want to share the code with you and get your opinion about it.

chained_hash_table.h

#ifndef CHAINED_HASH_TABLE_H_INCLUDED
#define CHAINED_HASH_TABLE_H_INCLUDED

#include <stdlib.h>
#include "../linked_list/linked_list.h"

typedef size_t hkey_t;

typedef struct ChainedHashTable_ {
    size_t buckets;
    List **table;
    hkey_t (*hash)(const void *key);
    void (*destroy)(void *data);
    int (*compare)(const void *fdata, const void *sdata);
}ChainedHashTable;

ChainedHashTable *chtb_init(size_t buckets,
        hkey_t (*hash)(const void *key),
        void (*destroy)(void *data),
        int (*compare)(const void *fdata, const void *sdata));
void chtb_destroy(ChainedHashTable *chtb);
int chtb_insert(ChainedHashTable *chtb, void *data);
int chtb_remove(ChainedHashTable *chtb, void **data);
int chtb_lookup(const ChainedHashTable *chtb, const void *data);

#endif

chained_hash_table.c

#include <string.h>

#include "chained_hash_table.h"

#define chtb_hash_key(chtb, data)   chtb->hash(data) % chtb->buckets

ChainedHashTable *chtb_init(size_t buckets,
        hkey_t (*hash)(const void *key),
        void (*destroy)(void *data),
        int (*compare)(const void *fdata, const void *sdata)) {

    ChainedHashTable *chtb;

    if( (chtb = malloc(sizeof *chtb)) != NULL ) {
        chtb->buckets = buckets;
        chtb->hash = hash;
        chtb->destroy = destroy;
        chtb->compare = compare;

        if( (chtb->table = malloc(buckets * sizeof *chtb->table)) == NULL) {
            free(chtb);
            return NULL;
        }

        int i;

        for(i = 0; i < buckets; ++i) {
            if( (chtb->table[i] = list_init(destroy, compare)) == NULL) {
                for(--i; i >= 0; --i)
                    free(chtb->table[i]);

                free(chtb);
                return NULL;
            }
        }
    }

    return chtb;
}

void chtb_destroy(ChainedHashTable *chtb) {
    size_t i;

    for(i = 0; i < chtb->buckets; ++i)
        if(chtb->table[i] != NULL)
            list_destroy(chtb->table[i]);

    free(chtb->table);
    free(chtb);
}

int chtb_insert(ChainedHashTable *chtb, void *data) {
    size_t bucket = chtb_hash_key(chtb, data);

    if(list_find(chtb->table[bucket], data) != NULL)
        return 1;

    if(list_prepend(chtb->table[bucket], data) != 0)
        return -1;

    return 0;
}

int chtb_remove(ChainedHashTable *chtb, void **data) {
    size_t bucket = chtb_hash_key(chtb, data);

    if(chtb->table[bucket] == NULL)
        return -1;
    return list_remove(chtb->table[bucket], data);
}

int chtb_lookup(const ChainedHashTable *chtb, const void *data) {
    size_t bucket = chtb_hash_key(chtb, data);

    return list_find(chtb->table[bucket], data) == NULL ? 0 : 1;
}

typedef size_t hkey_t; doesn't seem right. Since hash has to be implemented by the client, the client must know exactly what hkey_t is. It would however make certain sense to use hkey_t throughout your code.
It is very suspicious that data argument to remove has an extra level of indirection, compared to insert and lookup. Is it a typo?
I'd recommend to allocate buckets with calloc instead of malloc. Then a cleanup loop of chtb_init can be replaced with a call to chtb_destroy, that is stating your intention explicitly.
Since the linked_list.h code is not posted, it is impossible to reason about the rest of the functions (specifically, return codes). In general, it make sense to return more than just failure code (a pointer to an inserted node, maybe? - cf STL inserts).


Custom function extending jQuery.Deferred I couldn't find an example on how to have a custom function inherit jQuery's functions while leaving the original jQuery function I'm inheriting from unchanged (I wanted my function to have a read but didn't want it to get added to $.Deferred)

This works, but I really suspect it isn't the right way to do it.

var DeferredFileEntry = function(){
    var def = $.Deferred();
    var promise = def.promise;

    def.read = function(textFnCallback){ // <--this is the method I'm adding
        return $.Deferred(function(newDefer){
            var newDefer = $.Deferred();
            def.done(function(fileEntry){
                fileEntry.getText()
                    .done(function(text){
                        textFnCallback(text);
                        newDefer.resolve(fileEntry);
                    });
            });
        }).promise();
    };
    def.promise = function(){
        //expose the custom method to the returned promise
        promise.read = def.read;
        return promise();
    };
    return def;
};

An example usage:

var getFileEntry = function(){
    var def = DeferredFileEntry();
    libraryCall() // that promises to resolve with a FileEntry object
        .done(function(fileEntry){
            def.resolve(fileEntry);
        });
    return def.promise();
}

getFileEntry()
    .read(function(text){
        console.log("reading content:" + text);
    })
    .read(function(text){
        //chained,
        //doesn't run until the previous one has resolved
        console.log("reading content again: "+text);
    });

Tips:

1)

Returning this from a function of an object instance allows for the method to become chainable.

Example:

var File = function(){
    this.readCallCount = 0;
    return this;
};
File.prototype.readLine = function(){
    this.readCallCount++;
    console.log( "read line: " + this.readCallCount );
    return this;
};
var fileA = new File();
fileA.readLine().readLine().readLine();
/*
output:
read line: 1
read line: 2
read line: 3
*/

2)

The passed arguments from deferred.resolve() are saved until deferred.resolve() is invoked again.
Therefore the callbacks stored from deferred.done() will be passed the same values from the last call from deferred.resolve().

Example:

var arr = [];
function log(str){
    return arr.push( str );
}
function fn1(time) {
  return log( "fn1 called at " + time );
}
function fn2(time) {
  return log( "fn2 called at " + time );
}
var dfd = $.Deferred();
dfd.done(fn1);
dfd.resolve( +(new Date()) );

setTimeout(function(){
    dfd.done(fn2);
    console.log( arr.join( ", " ) );
}, 2000);
/*
 outputs after 2 seconds
 fn1 called at 1348177948625, fn2 called at 1348177948625
*/

Different results can be produced if a function or object is passed to deferred.resolve().

Example:

var arr = [];
function log(str){
    return arr.push( str );
}
function fn1(getTimeFunc) {
  return log( "fn1 called at " + getTimeFunc() );
}
function fn2( getTimeFunc ) {
  return log( "fn2 called at " +  getTimeFunc() );
}
var dfd = $.Deferred();
dfd.done(fn1);
dfd.resolve(function(){
    return +(new Date())
});

setTimeout(function(){
    dfd.done(fn2);
    console.log( arr.join( ", " ) );
}, 2000);
/*
output after 2 seconds
fn1 called at 1348178107859, fn2 called at 1348178109860
*/

3)

The callbacks passed to deferred.done() are saved in a callback queue called $.Callbacks().
The queue is set like so, jQuery.Callbacks("once memory"). This means that all the callbacks stored in the queue will only fired once and that every callback receieves the same arguments.
Also notes that the callbacks are fired in order of First in First out.

Final Code:

This might work. Instead of extending the deferred object to include a read method, I opted to return an plain object.

Note: My code is assuming that libraryCall() and fileEntry.getText() return a deferred.promise() or deferred object.

Code:

var FileEntryObj = function(){
    this.libraryCallObj = libraryCall();
};
FileEntryObj.prototype.read = function(fn){
    this.libraryCallObj.done( function( fileEntry ){
        return fileEntry.getText().done( fn );
    });
    return this;
};
var getFileEntry = function(){
    return new FileEntryObj();
};

Usage:

getFileEntry().read(function (text) {
    console.log("reading content:" + text);
}).read(function (text) {
    console.log("reading content again: " + text);
});


Async/await in Parallel.ForEach I have a Timer:

var QueryReportTimer = new Timer(QueryReportTimerCallback, null, TimeSpan.Zero, TimeSpan.FromSeconds(15));

The important part here is the method QueryReportTimerCallback.

I have a few doubts here:

Is it ok to have an async void method? In general it is not, but how can I avoid this when the timer callback is delegate public delegate void TimerCallback(object state);?
Is it ok to have await inside Parallel.ForEach?

Method QueryReportTimerCallback:

private async void QueryReportTimerCallback(object state)
{
    if (await semaphoreQueryReportSlim.WaitAsync(10))
    {
        try
        {
            if (machineConfigurations != null)
            {
                await Task.Factory.StartNew(() =>
                    Parallel.ForEach(machineConfigurations.Where(x => x.QueryReport), async (configuration) =>
                    {
                        if (configuration.IsConnectionValid)
                        {
                            var queryReport = new QueryReport(configuration, ReportConfigurations, fileContainer, applicationConfiguration, logger);
                            await QueryAReport(configuration, queryReport);
                        }
                    })
                );
            }
        }
        catch (Exception e)
        {
            logger.LogError(e, e.Message);
        }
        finally
        {
            semaphoreQueryReportSlim.Release();
        }
    }
}

Is it ok to have an async void method?

Referencing Async/Await - Best Practices in Asynchronous Programming

As already stated in the OP async void should be avoided as much as possible.

The one exception to that rule being for event handlers, which can be the loophole to achieving the desired behavior while still having the ability to catch and handle any thrown exceptions.

Create an event

event EventHandler QueryReportCallbackEvent = delegate { };

to be raised by the timer callback

private void QueryReportTimerCallback(object state) {
    QueryReportCallbackEvent(this, EventArgs.Empty);
}

The event handler allows async void, so now you can safely do

private async void QueryReportCallbackEventHandler(object sender, EventArgs e) {
    if (await semaphoreQueryReportSlim.WaitAsync(10))
        await queryReportsCore();
}

  Is it ok to have await inside Parallel.ForEach?

NO!!!!

The async lambda

async (configuration) => ...

will be converted to async void which takes us right back to what was said in the beginning. async void BAD!!!

Instead refactor that lambda out into its own method that returns a Task

private async Task HandleReport(MachineConfiguration configuration) {
    if (configuration.IsConnectionValid) {
        var queryReport = new QueryReport(configuration, ReportConfigurations, fileContainer, applicationConfiguration, logger);
        await QueryAReport(configuration, queryReport);
    }
}

and this will now allow for the use of Task.WhenAll with all the machine configurations retrieved from the query.

var tasks = machineConfigurations
                .Where(x => x.QueryReport)
                .Select(configuration => HandleReport(configuration));

await Task.WhenAll(tasks);

This actually removes the need for the Paralell.ForEach.

Here is the complete code for what was described above.

//CTOR
public MyClass() {
    QueryReportCallbackEvent += QueryReportCallbackEventHandler;
    var QueryReportTimer = new Timer(QueryReportTimerCallback, null, TimeSpan.Zero, TimeSpan.FromSeconds(15));
}

event EventHandler QueryReportCallbackEvent = delegate { };

private void QueryReportTimerCallback(object state) {
    QueryReportCallbackEvent(this, EventArgs.Empty);
}

private async void QueryReportCallbackEventHandler(object sender, EventArgs e) {
    if (await semaphoreQueryReportSlim.WaitAsync(10))
        await queryReportsCore();
}

private async Task queryReportsCore() {
    try {
        if (machineConfigurations != null) {
            var tasks = machineConfigurations
                            .Where(x => x.QueryReport)
                            .Select(configuration => HandleReport(configuration));

            await Task.WhenAll(tasks);
        }
    } catch (Exception e) {
        logger.LogError(e, e.Message);
    } finally {
        semaphoreQueryReportSlim.Release();
    }
}

private async Task HandleReport(MachineConfiguration configuration) {
    if (configuration.IsConnectionValid) {
        var queryReport = new QueryReport(configuration, ReportConfigurations, fileContainer, applicationConfiguration, logger);
        await QueryAReport(configuration, queryReport);
    }
}

Lastly take note of how the functions were broken down into smaller chunks that allowed for cleaner, more easy to read code.


Counting Bouncy Numbers: Project Euler #113 Problem statement:

  Working from left-to-right if no digit is exceeded by the digit to its
  left it is called an increasing number; for example, 134468.

  Similarly if no digit is exceeded by the digit to its right it is
  called a decreasing number; for example, 66420.

  We shall call a positive integer that is neither increasing nor
  decreasing a "bouncy" number; for example, 155349.

  As n increases, the proportion of bouncy numbers below n increases
  such that there are only 12951 numbers below one-million that are not
  bouncy and only 277032 non-bouncy numbers below \$10^{10}\$.

  How many numbers below a googol (\$10^{100}\$) are not bouncy?

Code:

import time
from scipy import special

def count_non_bouncy_numbers(n):
    '''n - number of digits'''
    include_zeroes = scipy.special.comb(10,n,exact=True,repetition=True) - 1
    exclude_zeroes = scipy.special.comb(9,n,exact=True,repetition=True)
    only_zeroes = include_zeroes - exclude_zeroes
    total = only_zeroes + exclude_zeroes*2 - 9
    return total

if __name__ == '__main__':
    total = 0
    for n in range(1,101) : total+= count_non_bouncy_numbers(n)
    print(total)

The reasoning behind the code:

The formula I should note here is the combinations with replacement, which is \$ {\frac{(q+r-1)!}{r!(q-1)!} }\$. I will denote it as \$C(q,r)\$ where \$q\$ represents number of items that can be selected and \$r\$ numbers of items that are selected.

My function takes an argument \$n\$, which represents number of digits, e.g:

If \$n =  2\$, then we consider all the numbers in range [10,99]

If \$n = 3 \$, then we consider all the numbers in range [100,999]

Step 1. We find all the non-bouncy numbers that contain zeroes. Specifically, for all numbers with \$n\$ digits, there will be \$C(10,n) - C(9,n)\$ non-bouncy numbers that contain zeroes in them

Step 2. We find number of strictly increasing non-bouncy  numbers with \$n\$ digits, there are \$C(9,n)\$ of them.

Step 3. We find number of strictly decreasing non-bouncy numbers with \$n\$ digits. Since decreasing numbers are just increasing numbers in reverse, we can see that there are \$C(9,n)\$ of them.

Step 4. We note that numbers such as 111, 33, 555 etc can be considered as strictly decreasing/increasing at the same time, hence we need to remove the overcount. To do so, we subtract \$9\$.

Step 5. Add up the results, i.e: \$C(10,n) - C(9,n) + C(9,n) - C(9,n) - 9\$ , which represents total number of non-bouncy numbers with \$n\$ digits.

What can be improved?

Project Euler problems generally can be computed with a calculator or manually.
So, take a different approach:

For ascending numbers, choose the transitions (digits 0-9; 9 transitions).
For descending numbers, choose the transitions (initial zeros, digits 9-0; 10 transitions).
Subtract those where initial zeros are followed only by a non-empty string of a single repeated digit (0-9) (1 transition, but the last place cannot be chosen).
Subtract the case of only the initial zeros.

In the end you have to calculate:

$$\binom{100+9}{9} + \binom{100 + 10}{10} - 10 * \binom{100}{1} - 1 = \binom{109}{9} + \binom{110}{10} - 1001$$


Rounding JavaScript decimals JavaScript has some inaccurate rounding behavior so I've been looking for a reliable solution without success. There are many answers in this SO post but none cover all the edge cases as far as I can tell. I wrote the following which handles all the edge cases presented. Will it be reliable with edge cases I haven't tested?

If this is a viable solution, any enhancements to make it more efficient would be appreciated. It's not fast (time to run function 1000000 times: 778ms) but doesn't seem to be  terrible either. If there is a better solution, please post.

The edge cases that seemed to give the most problem were the first two:

console.log(round(1.005, 2)); // 1.01
console.log(round(1234.00000254495, 10)); //1234.000002545
console.log(round(1835.665, 2)); // 1835.67))
console.log(round(-1835.665, 2)); // -1835.67))
console.log(round(10.8034, 2)); // 10.8
console.log(round(1.275, 2)); // 1.28
console.log(round(1.27499, 2)); // 1.27
console.log(round(1.2345678e+2, 2)); // 123.46
console.log(round(1234.5678, -1)); // 1230
console.log(round(1235.5678, -1)); // 1240
console.log(round(1234.5678, -2)); // 1200
console.log(round(1254.5678, -2)); // 1300
console.log(round(1254, 2)); // 1254
console.log(round("123.45")); // 123
console.log(round("123.55")); // 124

function round(number, precision) {
  precision = precision ? precision : 0;

  var sNumber = "" + number;
  var a = sNumber.split(".");
  if (a.length == 1 || precision < 0) {
    // from MDN https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math/round
    var factor = Math.pow(10, precision);
    var tempNumber = number * factor;
    var roundedTempNumber = Math.round(tempNumber);
    return roundedTempNumber / factor;
  }
  // use one decimal place beyond the precision
  var factor = Math.pow(10, precision + 1);
  // separate out decimals and trim or pad as necessary
  var sDec = a[1].substr(0, precision + 1);
  if (sDec.length < precision + 1) {
    for (var i = 0; i < (precision - sDec.length); i++)
      sDec = sDec.concat("0");
  }
  // put the number back together
  var sNumber = a[0] + "." + sDec;
  var number = parseFloat(sNumber);
  // test the last digit
  var last = sDec.substr(sDec.length - 1);
  if (last >= 5) {
    // round up by correcting float error
    // UPDATED - for negative numbers will round away from 0
    // e.g. round(-2.5, 0) == -3
    number += 1/(factor) * (+number < 0 ? -1 : 1);
  }
  number = +number.toFixed(precision);
  return number;
};

Here is a solution which is quick and handles all cases - negative number and negative precision. It was adapted from MDN - the main differences are

I pass positive numbers to specify decimal places where it is the opposite on MDN.
I took it out of the prototype and limited it to just the round function (removed ceil and floor)
it is significantly faster

function round(number, precision) {
  number = +number;
  precision = precision ? +precision : 0;
  if (precision == 0) {
    return Math.round(number);
  }
  var sign = 1;
  if (number < 0) {
    sign = -1;
    number = Math.abs(number);
  }

  // Shift
  number = number.toString().split('e');
  number = Math.round(+(number[0] + 'e' + (number[1] ? (+number[1] + precision) : precision)));
  // Shift back
  number = number.toString().split('e');
  return +(number[0] + 'e' + (number[1] ? (+number[1] - precision) : -precision)) * sign;
}

console.log(round(1.0e-5, 5)); // 0.00001
console.log(round(1.0e-20, 20)); // 1e-20
console.log(round(1.0e20, 2)); // 100000000000000000000
console.log(round(1.005, 2)); // 1.01
console.log(round(1234.00000254495, 10)); //1234.000002545
console.log(round(1835.665, 2)); // 1835.67))
console.log(round(-1835.665, 2)); // -1835.67))
console.log(round(1.27499, 2)); // 1.27
console.log(round(1.2345678e+2, 2)); // 123.46
console.log(round(1234.5678, -1)); // 1230
console.log(round(1234.5678, -2)); // 1200
console.log(round(1254.5678, -2)); // 1300
console.log(round(1254, 2)); // 1254
console.log(round("123.55")); // 124


Enforce what is passed to an activity in an intent similar to the newInstance pattern for fragments For fragments I have this code to create a new instance:

public static FoodDetails newInstance(Food food) {
    FoodDetails foodDetails = new FoodDetails();
    Bundle args = new Bundle();
    args.putParcelable(FoodConstants.BundleConstants.FOOD_OBJECT, food);
    foodDetails .setArguments(args);
    return foodDetails ;
}

What I like about this is that I can see what arguments are passed to the fragment.

I want a similar way for Activities so this is my proposed solution.

public class FullScreenImageActivity extends BaseActivity {

public static Intent createIntent(String[] images, String googleAnalyticsCategory,int positionToStartFrom){
    Intent intent = new Intent(MyApplication.getInstance(),FullScreenImageActivity.class);
    intent.putExtra(FoodConstants.BundleConstants.LIST_OF_IMAGES_TO_DISPLAY,images);
    intent.putExtra(FoodConstants.BundleConstants.GOOGLE_ANALYTICS_CATEGORY,googleAnalyticsCategory);
    intent.putExtra(FoodConstants.BundleConstants.POSITION_TO_START_FROM, positionToStartFrom);
    return intent;
}

The method returns a built intent and adds the parameters to the bundle
Note that MyApplication is a subclass of Application so I am using the application context in the intent argument.

And then you can start the activity like this:

startActivity(FullScreenImageActivity.createIntent(mImages, AnalyticsUtils.CATEGORY_FOOD,position));

The benefits are:

Do not need type the bundle keys all the time
You can see what arguments are passed to the activity

Your solution looks great to me, apply Fragment's instantation to a Class seems to be a good pattern and a nice idea. However, I have just a little tip for this static method: pass the current Context instead of Application Context:

public class SampleActivity extends BaseActivity {
    public static Intent createIntent(Context context, String[] a, String b) {
        Intent intent = new Intent(context, SampleActivity.class);
        intent.putExtra(AppConstants.KEY_ARG_ARRAY,  a);
        intent.putExtra(AppConstants.KEY_ARG_STRING, b);
        return intent;
    }
}

This allows you to get the right Activity calling the new Intent instead of whole application context. So, the called syntax could be:

startActivity( SampleActivity.createIntent(this, a, b) );
// 'this' being the calling class Context (like 'MainActivity.this' or 'getActivity()')


AJAX request authentication to PHP script Recently I started looking at my AJAX call security and found some code that uses tokens, request and referer in an attempt to authenticate the call and stop XSS and CSRF.

The AJAX calls are for the article list / article table in the admin section.

Note: The PHP will not run unless you are logged in — it uses a cookie and sessions to authenticate user.

EDIT: Implemented schisms function name suggestion.

The following code works as it should but is it a good method?

Create our Token:

$token = md5(rand(1000,9999));
$_SESSION['token'] = $token;

The button to deactivate / activate an article:

if($article_active === 1){
echo '<input type="button" class="active_button" title="Deactivate" value="YES" name="'.$article_id.'"/>';
}
else {
echo '<input type="button" class="active_button" title="Activate" value="NO" name="'.$article_id.'" />';
}
echo '

The AJAX call:

// ACTIVATE / DEACTIVATE WITH LINK
$(document).on("click",".active_button",function() {

    var form_data = {
        articleID: $(this).attr("name"),
        active: $(this).val(),
        selector: $("#selector").attr("name"),
        data: $("#data").val(),
        token:'<?php echo $token; ?>',
        is_ajax: 1
    };

    $.ajax({
    type: "POST",
    url: "controllers/articlecontrol.php",
    data: form_data,
    success: function(data){
        document.location.reload(true);
    }
    });
});

The PHP that validates the AJAX request and sends the data to a PHP function:

// ACTIVATE / DEACTIVATE w/BUTTON
if(isset($_POST['active']) && $selector === 'article') {

    if($_SERVER['HTTP_X_REQUESTED_WITH'] == 'XMLHttpRequest' && isset($_POST['token']) && $_POST['token'] === $_SESSION['token']){

                $articleid = $_POST['articleID'];
                $active = ($_POST['active'] === 'YES') ? 0 : 1;
                $backend->setArticleStatus($articleid, $active);
            }
            else {
                $_SESSION['status'] = '
There was a Problem Activating / Deactivating that Article
';
            }
}

The PHP activate / deactivate function (part of a class):

/**
 * Activate / Deactivate Article w/Button
 */

 public function setArticleStatus($articleid, $active) {
    $query = 'UPDATE wcx_articles SET article_active = :active WHERE article_id = :articleid';
    $stmt = $this->queryIt($query);
    $stmt = $this->bind(':active', $active);
    $stmt = $this->bind(':articleid', $articleid);
    if($this->execute()) {
        if ($active === 0) {
            $_SESSION['status'] = '
Article Deactivated Successfully
';
        }
        else {
            $_SESSION['status'] = '
Article Activated Successfully
';
        }
    }
    else {
        $_SESSION['status'] = '
There was a Problem Activating / Deactivating that Article
';
    }
 }

Prefer mt_rand() to rand().
Just a comment: your usage of tokens is similar to using nonces. There are established nonce generation libraries for PHP, and I suggest you take a look into them.
HTTP_X_REQUESTED_WITH is in general not a 100% reliable way to detect AJAX-ness. However, jQuery sends this header with its AJAX calls, so you should be okay. I'd suggest getting rid of the is_ajax in your request data though.
Your input element is at best confusing, and at worst messy. Why is Deactivate paired with YES, and Activate with NO? I'd prefer just using deactivate and activate as the button values, and templating inline:

<input type="button" class="active_button" name="<?= $article_id ?>" value="<?= $article_active ? "de" : "" ?>activate" />

activateDeactivate isn't a very good name. Maybe setActiveStatus or something?


Terrain Generator - KhanAcademy I'm new to JavaScript, and I've been learning off Khanacademy. I wrote the following basic terrain generator and I'm wondering if it could use any improvements. Here's the code:

var res = 7;

var area = [];

var camP = [random(500,1000), random(500,1000)];

var setArea = function()
{
    area = [];
    for (var y = -200; y < 200; y+=res)
    {
        var row = [];
        for (var x = -200; x < 200; x+=res)
        {
            row.push(noise((x+camP[0])/200, (y+camP[1])/200));
        }
        area.push(row);
        row = [];
    }
};

var drawPix = function(v, x, y){
    if (v < 0.49)
    {
        fill(random(30,40), random(100,128), random(20,30));
    }
    else
    {
        fill(random(0,10), random(40,50), random(245,255));
    }
    rect(x*res,y*res,res,res);
};

var drawArea = function()
{
    background(255, 255, 255);
    noStroke();
    for (var y in area)
    {
        for (var x in area[y])
        {
            drawPix(area[y][x],x,y);
        }
    }
};

setArea();
drawArea();

Here's an example of output:

It's nicely done! One thing I'd pick on though is that your code is full of magic numbers:

var camP = [random(500,1000), random(500,1000)];
// ...
for (var y = -200; y < 200; y+=res)
// ...
for (var x = -200; x < 200; x+=res)
// ...
row.push(noise((x+camP[0])/200, (y+camP[1])/200));
// ...
if (v < 0.49)
// ...
fill(random(30,40), random(100,128), random(20,30));
// ...
fill(random(0,10), random(40,50), random(245,255));

Put these numbers into variables, defined at the top.
By doing so you will effectively give them names,
which could explain what they are.

This is especially recommended for values that you use repeatedly, for example 200.

And when a value depends on another (derived from another),
that relationship can be expressed in the definition.
For example, the difference in the parameter pairs in random(0,10), random(40,50), random(245,255) is suspiciously 10, probably for a reason, which would become clear in the variable definition.
I've no idea how fill works, so these names are probably completely inappropriate, but to illustrate what I mean:

var FILL_RGB_RANGE = 10;
var FILL_R = 0;
var FILL_G = 40;
var FILL_B = 245;

fill(
  random(FILL_R, FILL_R + FILL_RGB_RANGE),
  random(FILL_G, FILL_G + FILL_RGB_RANGE),
  random(FILL_B, FILL_B + FILL_RGB_RANGE)
);


CRUD (database layer) for F# with TypeProviders I am a newbie in F# and I am thinking about use F# in my next project.  The project will work the database and I need to store and retrieve instances of objects in the Db.

Could you please check my code and say if there is something that I can improve?

Here is a simple object:

namespace Entities

type Item(id : string, name : string) =
    member val ItemId = id with get, set
    member val ItemName = name with get, set

    new() = new Item()

Here is Database Layer:

namespace SqlController

open System
open System.Data
open System.Data.Linq
open Microsoft.FSharp.Data.TypeProviders
open Microsoft.FSharp.Linq

open Entities

type dbSchema = SqlDataConnection<"Data Source=VM-WIN8\SQLEXPRESS;Initial Catalog=mobile;Integrated Security=SSPI;">

type SqlConnector() =

    // universal sql functions
    member private this.DeleteRowsFrom (table : Table<_>) rows =
        table.DeleteAllOnSubmit(rows)

    // map functions
    member private this.Item2Record (item : Item) =
        new dbSchema.ServiceTypes.ItemsTable(
            ItemId = item.ItemId,
            ItemName = item.ItemName)

    member private this.Record2Item (e : dbSchema.ServiceTypes.ItemsTable) =
        new Item(e.ItemId, e.ItemName)

    // ### Items

    // return all items
    member this.GetItems() =
        use db = dbSchema.GetDataContext()

        query {
            for row in db.ItemsTable do
            select row
        } |> Seq.map this.Record2Item |> Seq.toArray

    // return one item by item id
    member this.GetItemByItemId(itemId : string) =
        use db = dbSchema.GetDataContext()

        query {
            for rows in db.ItemsTable do
            where (rows.ItemId = itemId)
            select rows
        }
        |> (fun s ->
                if Seq.isEmpty s then
                    None
                else
                    s |> Seq.head |> this.Record2Item |> Some)

    // insert new item
    member this.InsertItem (item : Item) =
        use db = dbSchema.GetDataContext()
        item |> this.Item2Record |> db.ItemsTable.InsertOnSubmit

        try
            db.DataContext.SubmitChanges()
        with
            | exn -> printfn "Exception: \n%s" exn.Message

    // update item
    member this.UpdateItem (item : Item) =
        use db = dbSchema.GetDataContext()

        query {
            for rows in db.ItemsTable do
            where (rows.ItemId = item.ItemId)
            select rows
        }
        |> Seq.nth 0
        |> (fun e -> e.ItemName <- item.ItemName)

        try
            db.DataContext.SubmitChanges()
        with
            | exn -> printfn "Exception: \n%s" exn.Message

    // delete item by itemId
    member this.DeleteItemByItemId(itemId : string) =
        use db = dbSchema.GetDataContext()

        query {
            for rows in db.ItemsTable do
            where (rows.ItemId = itemId)
            select rows
        } |> this.DeleteRowsFrom db.ItemsTable

        try
            db.DataContext.SubmitChanges()
        with
            | exn -> printfn "Exception: \n%s" exn.Message

Here is some tests:

module Tests

open NUnit.Framework
open FsUnit

open Entities
open SqlController

[<TestFixture>]
type SqlControllerTest() =
    let sqlConnector = new SqlConnector();

    [<Test>]
    member x.SelectionAllTest() =
        let items = sqlConnector.GetItems()
        items.Length |> should equal 3

    [<Test>]
    member x.SelectById() =
        let item = sqlConnector.GetItemByItemId("20")
        if Option.isSome item then
            (Option.get item).ItemName |> should equal "two"

    [<Test>]
    member x.SelectNotExistingItem() =
        let item = sqlConnector.GetItemByItemId("xx")
        Option.isNone item |> should equal true

new() = new Item()

This is infinite recursion, in order to call the parameterless constructor, you call the parameterless constructor. It's not quite clear to me what the default values of id and name should be. In C#, it might be okay to use null, but that's not the common approach in F#.

query {
    for row in db.ItemsTable do
    select row
}

Wouldn't just db.ItemsTable work here too?

query {
    for rows in db.ItemsTable do
    where (rows.ItemId = itemId)
    select rows
}
|> (fun s ->
        if Seq.isEmpty s then
            None
        else
            s |> Seq.head |> this.Record2Item |> Some)

That second part could be simplified to:

|> Seq.tryPick Some
|> Option.map this.Record2Item

try
    db.DataContext.SubmitChanges()
with
    | exn -> printfn "Exception: \n%s" exn.Message

Are you sure that it's okay for your DB layer to just print any errors to the console and then continue as if everything was fine? That doesn't sound like a very robust approach to me.

If you're sure you want to do it this way, then consider extracting this code into a separate function, since you're repeating it at several places.

|> (fun e -> e.ItemName <- item.ItemName)

I personally don't like this style of code. Pipelining using |> should be used for code that doesn't mutate anything.

if Option.isSome item then
    (Option.get item).ItemName |> should equal "two"

I would probably use pattern matching here.


"Invert" a JavaScript object hash whose values are arrays to produce a new object hash with keys as the elements of those original value vectors Background

lodash and underscore have an invert function that takes an object hash and converts it to a new one, which has keys as the input object's values and values as the input object's keys. As such, if the values in the input object aren't unique, this non-unique input value will, when it's a key in the output object, have as its value only one of the input object's keys.

An example:

_.invert({a: 1, b: 2, c: 2})
// { 1 : "a", 2 : "c" }

The twist

I frequently work with object hashes whose values are one-dimensional arrays (vectors). I've written a function using lodash/underscore (NB. only tested with lodash) that performs an array-aware invert, whose output object has keys that are the unique elements of the input object's value vectors, and values that are the input object's keys.

function arrayAwareInvert(obj) {
    return _.object(_.flatten(_.map(obj, function(valVec, key) {
        return valVec.map(function(val) { return [ val, key ]; });
    })));
}

I'd appreciate feedback on this function.

(NB. I can't use lodash/underscore's built-in invert with an object hash with array-valued keys because the resulting object has keys which simply stringify the arrays—an output object key might be [1, 2, 3].toString(), and entirely useless.)

Example use

Just to confirm that it works:

arrayAwareInvert({a: [1, 2], b: [3, 4]})
// { 1: "a", 2: "a", 3: "b", 4: "b" }

Per the original invert's behavior, if the elements of the input object's value vectors aren't unique (that is, if multiple value vectors contain the same element), the output object's value for those non-unique elements will be one of the input object keys:

arrayAwareInvert({a: [1, 2], b: [3, 4, 2]})
// { 1: "a", 2: "b", 3: "b", 4: "b" } // NB: 2: "b" here

Implementation notes

My implementation seems as brute-force as possible: it's effectively a doubly-nested loop, with the outer loop going over all input object keys and the inner loop going over the contents of each input object value (which is a vector).

A [key, value] tuple is built for each iteration of the inner-most loop, where "key"/"value" refer to the output object.

flatten is used to remove one level of nesting, i.e., transform [[[k1, v1]], [[k2, v2], [k3, v3]]] to [[k1, v1], [k2, v2], [k3, v3]].

Finally object is called to convert this list of 2-tuple key-value pairs into an object hash.

Summary

Are there implementations that occupy more advantageous positions in the clarity-speed-elegance phase space?

Does this operation have a more general name?

Analyses of solutions

reduce

I can simplify? @Flambino's solution a bit with _.merge:

function arrayAwareInvert(obj) {
  return _.reduce(obj, function (result, values, key) {
    return _.merge(result,
                   _.mapValues(_.object(values), function(v) { return key; }));
  }, {});
}

I like using merge here because it describes what's happening very well. But I don't really like how much code is needed to combine a vector and a string into an object hash with keys as elements of the vector and values as the string:

_.mapValues(_.object(values), function(v) { return key; })

As one (of many) alternatives, you could do this instead:

_.object(values, values.map(function(v) { return key; }))

but both of these seem obfuscated, compared with how clear the reduce and merge steps are.

lodash and underscore both have a reduce (aka "fold") function that works on objects, meaning you could also do this:

function arrayAwareInvert(obj) {
  return _.reduce(obj, function (result, values, key) {
    _.forEach(values, function (value) { result[value] = key; });
    return result;
  }, {});
}

It's pretty much the functional-style equivalent of Bergi's answer.

Point is, reduce is probably what you want for this.

Edit: As Bergi points out in the comments the inner iteration could also be a reduce operation

function arrayAwareInvert(obj) {
  return _.reduce(obj, function (result, values, key) {
    return _.reduce(values, function (result, value) {
      result[value] = key;
      return result;
    }, result);
  }, {});
}


Find height of a tree without using recursion I want you to pick my code apart and give me some feedback on how I could make it better or simpler.

public int heightHelper(TreeNode node) {
    int height = -1;

    if (node == null) return height;

    final Queue<TreeNode> queue = new LinkedList<TreeNode>();
    queue.add(node);

    int currentLevelNodeCount = 1;
    int nextLevelNodeCount = 0;

    while (!queue.isEmpty()) {
        TreeNode current = queue.poll();
        currentLevelNodeCount--;

        if (current.left != null) { queue.add(current.left); nextLevelNodeCount++; }

        if (current.right != null)  { queue.add(current.right);  nextLevelNodeCount++; }

        // current level is done.
        if (currentLevelNodeCount == 0) {
            height++;
            currentLevelNodeCount = nextLevelNodeCount;
            nextLevelNodeCount = 0;
        }
    }
    return height;
}

The code is basically sound.

It's not a helper function, so don't name it as such.  (Helper functions would usually be private.)

Comments would be good.  In particular, a JavaDoc explaining how to interpret the height of a degenerate tree would be helpful.

Your currentLevelNodeCount and nextLevelNodeCount are basically segmenting the queue into two queues the hard way.  Why not reduce the bookkeeping by using two queues?  (I've named them thisLevel and nextLevel because variable names that have the same length make the code look pretty.)

A few tweaks here and there make the code slightly more compact.

/**
 * Returns the maximum depth of a tree.
 *
 * @param node The root of the tree
 *
 * @return -1 if node is null, 0 if node has no children,
 *         a positive number otherwise.
 */
public int height(TreeNode node) {
    int height = -1;
    if (node != null) {
        // Breadth-first traversal, keeping track of the number of levels
        Queue<TreeNode> thisLevel = new LinkedList<TreeNode>(),
                        nextLevel = new LinkedList<TreeNode>();

        thisLevel.add(node);
        while (null != (node = thisLevel.poll())) {
            if (node.left  != null) nextLevel.add(node.left);
            if (node.right != null) nextLevel.add(node.right);

            if (thisLevel.isEmpty()) {
                height++;

                Queue<TreeNode> swapTemp = thisLevel;
                thisLevel = nextLevel;
                // We could create a new nextLevel queue, but reusing the
                // newly emptied thisLevel queue is more efficient.
                nextLevel = swapTemp;
            }
        }
    }
    return height;
}


Hours and Minutes math in a work week calculator I have written a small bit of code to tell me how many hours I need to work to finish up my 40 hours a week each day.

I can add hours per day and it will calculate total hours left in the work week.

My main issue with it is the way my math is written. It just seems so complicated and unnecessary but I was unable to find a better solution and was hoping someone could point me in the right direct for a better formula.

If you check the hours_left_update(self) method you will see that I am using floats, the Decimal method with str and quantize as well as split. All to try and get an output to a label that shows my results in hours and minutes.

Keep in mind when you put in hours worked the time clock system I get my information from uses a decimal system to display work hours. So if I worked 7 and 1/2 hours one day it would show me 7.50 hours worked. This is the kind of format I am putting into the program and it automatically convert that number to display as 7 hours 30 min.

from decimal import Decimal, ROUND_HALF_UP
from time import strftime
import tkinter as tk
import datetime

class TimeClockApp(tk.Frame):
    def __init__(self, master, *args, **kwargs):
        tk.Frame.__init__(self, master, *args, **kwargs)
        self.master = master
        self.hours_worked = 0.00
        self.dtlbl = tk.Label(self.master, text="Current Date-Time: {} ~ {}".format(datetime.date.today(),strftime("%H:%M:%S")))
        self.dtlbl.grid(row=0, column=0, columnspan=2)

        self.lbl1 = tk.Label(self.master, text="Hours worked").grid(row=1, column=0, columnspan=2)

        weekdays = ["Monday: ", "Tuesday: ", "Wednesday: ", "Thursday: ", "Friday: "]
        for i in range(5):
            tk.Label(self.master, text=weekdays[i], anchor = "e").grid(row=i+2, column=0, sticky="e")

        self.lbl7 = tk.Label(self.master, text="Total Hours worked:").grid(row=7, column=0, sticky="e")
        self.lbl8 = tk.Label(self.master, text="00h 0m")
        self.lbl8.grid(row=7, column=1, sticky="w")

        self.lbl9 = tk.Label(self.master, text="Hours left to 40:").grid(row=8, column=0, sticky="e")
        self.lbl10 = tk.Label(self.master, text="40h 0m")
        self.lbl10.grid(row=8, column=1, sticky="w")

        self.entry_list = []

        for i in range(5):
            self.entry_list.append(tk.Entry(self.master,width=7))
            self.entry_list[i].bind("<Key>", lambda x: self.master.after(500, self.hours_left_update))
            self.entry_list[i].grid(row = i+2, column = 1, sticky = "w")

        self.status_clock()

    def status_clock(self):
        self.dtlbl.config(text ="Current Date-Time: {} ~ {}".format(datetime.date.today(),strftime("%H:%M:%S")))
        self.dtlbl.after(1000, lambda: self.status_clock())

    def hours_left_update(self):
        for entry in self.entry_list:
            if entry.get() != "":
                try:
                    #print(Decimal(entry.get()).quantize(Decimal("0.01"), ROUND_HALF_UP))
                    self.hours_worked += float(entry.get())
                except:
                    pass

        x = str(Decimal(str(self.hours_worked)).quantize(Decimal("0.01"), ROUND_HALF_UP)).split('.')
        m60 = "00"

        if int(x[1]) != 0:
            m60 = Decimal(str(60 * float("0.{}".format(x[1])))).quantize(Decimal("1"))

        self.lbl8.config(text="{}h {}m".format(x[0], m60))

        var = 40 - int(x[0])
        if int(x[1]) != 0:
            var = 39 - int(x[0])

        x2 = "00"

        if Decimal(str(60 * float("0.{}".format(x[1])))).quantize(Decimal("1")) != 0:
            x2 = (60 - Decimal(str(60 * float("0.{}".format(x[1])))).quantize(Decimal("1")))

        self.lbl10.config(text="{}h {}m".format(var,x2))

        self.hours_worked = 0.00

if __name__ == "__main__":
    root = tk.Tk()
    app = TimeClockApp(root)
    root.mainloop()

It's a nice little program. Intuitively I'd prefer to input the times as 1:30 instead of 1.5, but that's just a personal preference.

The self.hours_worked field is redundant. It should simply be a local variable in the hours_left_update method. It's kind of weird that you reset it at the end of the method instead of initializing it at the beginning of the method. Anyway, just make it a local variable.

I played around a bit and came up with this code:

def hours_left_update(self):
    minutes_worked = 0
    for entry in self.entry_list:
        if entry.get() != "":
            try:
                minutes_worked += 60 * float(entry.get())
            except:
                pass

    minutes_worked = round(minutes_worked)   # just in case
    minutes_left = 40 * 60 - minutes_worked

    self.lbl8.config(text="{:02d}h {:02d}m".format(minutes_worked // 60, minutes_worked % 60))
    self.lbl10.config(text="{:02d}h {:02d}m".format(minutes_left // 60, minutes_left % 60))

Is there any reason for waiting 500 ms before updating the total times after each key press? I generally don't expect programs to have unnecessary delays.

I changed the output format for the two computed fields. As a result, the format changes after I press the first key because the labels are initialized with the old format. To make things consistent, I'd create the labels without any text and call self.hours_left_update() at the end of the constructor.

After cleaning up the rest of the code, it looks like this:

from time import strftime
import tkinter as tk
import datetime

class TimeClockApp(tk.Frame):
    def __init__(self, master, *args, **kwargs):
        tk.Frame.__init__(self, master, *args, **kwargs)

        self.now_lbl = tk.Label(master)
        self.now_lbl.grid(row=0, column=0, columnspan=2)

        tk.Label(master, text="Hours worked").grid(row=1, column=0, columnspan=2)

        self.entries = []
        weekdays = ["Monday: ", "Tuesday: ", "Wednesday: ", "Thursday: ", "Friday: "]
        for i in range(5):
            tk.Label(master, text=weekdays[i], anchor="e").grid(row=i+2, column=0, sticky="e")
            entry = tk.Entry(master, width=7)
            entry.grid(row=i+2, column=1, sticky="w")
            entry.bind("<Key>", lambda event: master.after(0, self.update_totals))
            self.entries.append(entry)

        tk.Label(master, text="Total Hours worked:").grid(row=7, column=0, sticky="e")
        self.time_worked_lbl = tk.Label(master)
        self.time_worked_lbl.grid(row=7, column=1, sticky="w")

        tk.Label(master, text="Hours left to 40:").grid(row=8, column=0, sticky="e")
        self.time_left_lbl = tk.Label(master)
        self.time_left_lbl.grid(row=8, column=1, sticky="w")

        self.update_clock()
        self.update_totals()

    def update_clock(self):
        now = "Current Date-Time: {} ~ {}".format(datetime.date.today(), strftime("%H:%M:%S"))
        self.now_lbl.config(text=now)
        self.now_lbl.after(200, self.update_clock)

    def update_totals(self):
        minutes_worked = 0
        for entry in self.entries:
            try:
                minutes_worked += 60 * float(entry.get())
            except:
                pass

        minutes_worked = round(minutes_worked)
        minutes_left = 40 * 60 - minutes_worked

        self.time_worked_lbl.config(text="{:02d}h {:02d}m".format(minutes_worked // 60, minutes_worked % 60))
        self.time_left_lbl.config(text="{:02d}h {:02d}m".format(minutes_left // 60, minutes_left % 60))

if __name__ == "__main__":
    root = tk.Tk()
    app = TimeClockApp(root)
    root.mainloop()

I did at least the following:

Start the method names with a verb.
Rename lbl8 to time_worked_lbl, and similar ones.
Only store those labels in self that are needed by the other methods.
Replace self.master with a simple master.
Made the formatting consisten, especially the spacing around named arguments.
Removed unnecessary lambdas.
Merge the two for loops into one.
Build the UI elements strictly from top to bottom.

Too bad that the grid method doesn't return self. That would have saved some more lines. :) By the way, this means that your code did something unexpected. It stored None into self.lbl1.


Iterating thousands of times over large collections: advices This is a pet project :)

I have hundreds of playlist files .m3u whose many entries have a wrong path. I'd like to fix them programmatically. Suppose one of my .m3U contains the following entry:

z:\wrong-folder\Michael Jackson - Thriller.mp3

The file is stored in:

G:\my-collection-of-mp3\any-folder\Michael Jackson - Thriller.mp3

The goal is to update the wrong path with the right one :)

I have to create a collection of all my mp3 (around 6000 files) and their path. As I said, I have hundreds of .mu3 ; each .m3u has 15-20 entries. I have to check each entry against my 6000-entries-collection until the name matches. Well it is a plethora of loops :)

To ease the process I would create an ordered dictionary of dictionary whose the key would be the first letter of the name i.e. "M" (for Michael Jackson - Thriller.mp3) and the sub dictionary would be:

- key: Michael Jackson - Thriller.mp3
- value: G:\my-collection-of-mp3\any-folder\Michael Jackson - Thriller.mp3

Doing so I would only to iterate over a subset of the collection (looking up for the first letter of the name) instead of iterating over the whole non ordered collection.

A / Albert Jones - Song 1.mp3 -> G:\my-collection-of-mp3\any-folder\Albert Jones - Song 1.mp3

M / Madonna - Song 1.mp3 -> D:\somewhere-else\Madonna - Song 1.mp3
M / Michael Jackson - Thriller -> G:\my-collection-of-mp3\any-folder\Michael Jackson - Thriller.mp3

Do you think it is ok? What else would you recommend?

If you are particularly interested in doing substring look-ups, you could look into the Trie data structure.  You will be stuck writing your own, however, as there is no implementation I know of in the BCL.

Alternatively, if you just want to put something together quick, you could just use a normal, single-level-deep dictionary keyed by file name.  If you build your dictionary for existing MP3s first using an actual System.Collections.Hashtable or (preferably) System.Collections.Generic.Dictionary<TKey,TValue>, the key lookups will be constant-time.

The only loops you will need are the initial loop to build the correct file path dictionary, then the loops over .m3u files with sub-loops over their referenced files.  Retrieving the correct mp3 reference for each file in a m3u is just value = dictionary [key] or dictionary.TryGetValue (key, out value).  The lookups themselves are not loops - rather, they generate hash codes for the keys which ultimately translate to array indexes in the underlying data structure.


How to access a servlet in Tomcat Contanier using Java I have a servlet deployed in a Tomcat Servlet Container in this link, http://localhost:9764/example/servlets/servlet/HelloWorldExample. I want to send a HTTP request to that servlet using Java. Just I want to get the return as a String. Currently I am using this code to do it, Is it OK to use this code?

import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.net.HttpURLConnection;
import java.net.MalformedURLException;
import java.net.URL;
import java.net.URLConnection;

public class Client {

    public static void main(String[] args) {
        try {
            String webPage = "http://10.100.3.83:9764/example/servlets/servlet/HelloWorldExample";
            URL url = new URL(webPage);
            HttpURLConnection urlConnection = (HttpURLConnection)url.openConnection();
            urlConnection.addRequestProperty("Name","andunslg");
            urlConnection.addRequestProperty("PW","admin");

            InputStream is = urlConnection.getInputStream();
            InputStreamReader isr = new InputStreamReader(is);

            int numCharsRead;
            char[] charArray = new char[1024];
            StringBuffer sb = new StringBuffer();
            while ((numCharsRead = isr.read(charArray)) > 0) {
                sb.append(charArray, 0, numCharsRead);
            }
            String result = sb.toString();

            System.out.println("*** BEGIN ***");
            System.out.println(result);
            System.out.println("*** END ***");
        } catch (MalformedURLException e) {
            e.printStackTrace();
        } catch (IOException e) {
            e.printStackTrace();
        }
    }
}

when you are sending any data to the servlet make sure it you send it via POST/GET depending on how sensitive the information is. In your case the user and password is sent through the get which is unsafe since it can be viewed by anyone.

you can add.urlConnection.setRequestMethod("POST"); Also to get the response you have to send the request first , which in your case is missing.

dataInput= new DataOutputStream(urlConnection.getOutputStream());
String content = "param1=" + URLEncoder.encode("first parameter")
         + "&param2="
         + URLEncoder.encode("the second one...");

dataInput.writeBytes(content);
dataInput.flush();
dataInput.close();


Return character frequency in the order of occurrence Problem:
Given a string aaabbcccaad print the no. of occurrence of the character in the same order in which the character occurred.
Expected output:
(a, 3), (b, 2), (c, 3), (a, 2), (d, 1)
The approach:

Loop through each character.
When the next character is not same as the current character, record the next character and its position.
count of occurence = Iterate the pos_list (position list ), take the difference
Return character and its count in in the same order

Solution:
def seq_count(word):
    wlen = len(word)
    pos = 0
    pos_list=[pos]
    alph = [word[0]]
    result = []

    for i in range(wlen):
        np = i+1
        if np < wlen:
            if word[i] != word[i+1]:
                pos = i+1
                pos_list.append(pos)
                alph.append(word[i+1])
    pos_list.append(wlen)

    for i in range(len(pos_list)-1):
        result.append((alph[i], pos_list[i+1] - pos_list[i]))
    return result

print seq_count("aaabbcccaad")

Note:
I know this may not be the conventional solution, i was finding difficult to count the character and maintain its count as we iterate through the string, since I'm a beginner in python. Pls let me know how can i do it better.

Problems like this tend to be annoying: so close to a simple solution, except for the need to be adjusting state during the loop. In this case, itertools.groupby() offers a shortcut, because its grouping function defaults to an identity, which is exactly what you need.
from itertools import groupby

def seq_count(word):
    return [
        (char, len(tuple(g)))
        for char, g in groupby(word)
    ]

If you don't want to use another library, you can reduce the bookkeeping a bit by yielding rather than returning and by iterating directly over the characters rather than messing around with indexes. But even with such simplifications, tedious annoyances remain. Anyway, here's one way to reduce the pain a little. The small code comments are intended to draw attention to the general features of this pattern, which occurs in many types of problems.
def seq_count(word):
    if word:
        # Initialize.
        prev, n = (word[0], 0)
        # Iterate.
        for char in word:
            if char == prev:
                # Adjust state.
                n += 1
            else:
                # Yield and reset.
                yield (prev, n)
                prev, n = (char, 1)
        # Don't forget the last one.
        if n:
            yield (prev, n)

exp = [('a', 3), ('b', 2), ('c', 3), ('a', 2), ('d', 1)]
got = list(seq_count("aaabbcccaad"))
print(got == exp)    # True


Working C code dealing with lists: problem with memory leak I have written a working code that generates a simple linked list of dates and associated holidays. I realize that I have a problem with memory leaking as I copy lists repeatedly, so I wrote a freeList function. I am just having issues determining where to place this function.

//Creates a linked list of dates/holidays

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

typedef struct hashElem{
    char* date;
    char* holiday;
    struct hashElem* next;

}hashElem;

// Initialize list with input value
hashElem* createNode(char* input_date, char* input_holiday)
{
    hashElem* ptr = malloc(sizeof(*ptr));

    if (ptr == NULL)
    {
        perror("Error");
        return NULL;
    }

    ptr -> date = input_date;
    ptr -> holiday = input_holiday;
    ptr -> next = NULL;

    return ptr;
}

void freeList(hashElem* head)
{
    hashElem* ptr = head;
    while (head != NULL)
    {
        ptr = ptr->next;
        free(head);
        head = ptr;
    }
}

hashElem* addToTail(hashElem* head, char* input_date, char* input_holiday)
{
    hashElem* ptr = malloc(sizeof(*ptr));
    hashElem* temp = head;

    // If there is not yet a list, initialize the list with the input value
    if (head == NULL)
        return createNode(input_date, input_holiday);

    if (ptr == NULL)
        return NULL;
    else
    {
        ptr->date = input_date;
        ptr->holiday = input_holiday;
        ptr->next = NULL;

        // Find the last element of the list
        while (temp->next != NULL)
            temp = temp->next;

        // Add new node to the end of the list
        temp->next = ptr;
        ptr = head;

        //free(head);
        return ptr;
    }
}

hashElem* holidayList(void)
{
    hashElem* List = NULL;
    List = addToTail(List, "01/01/13", "New Years Day");
    List = addToTail(List, "14/02/13", "Valentine's Day");
    List = addToTail(List, "26/03/13", "Start of Passover");
    List = addToTail(List, "16/06/13", "Father's Day");
    List = addToTail(List, "31/10/13", "Halloween");
    List = addToTail(List, "28/11/13", "Thanksgiving");
    List = addToTail(List, "25/12/13", "Christmas");

    return List;
}

void printList(hashElem* head)
{
    hashElem* ptr = head;

    printf("------------------------------\n");
    printf("PRINTING LINKED LIST\n");
    while (ptr != NULL)
    {
        printf("Date: %s,  Holiday: %s\n", ptr->date, ptr->holiday);
        ptr = ptr->next;
    }

    printf("------------------------------\n");
}

In your current code, I can't see the copying of lists repeatedly as you stated. You are using pointers here so you're not actually duplicating the list.

The freeList function should be called once you're done using your list (the one returned by holidayList()).

Side Note:
You can refactor the code by utilizing createNode function when creating nodes and adding it to the tail.


Printing primes between user-given numbers I want to display all the prime numbers between two integers taken from the user as input. Also I have to take a user input to specify the number of times the loop should run. Here is the Java code that I have written. I have to now reduce the run-time.

 import java.util.Scanner;

    public class Main{
        private static Scanner x;

        public static void main(String[] args) {
            Scanner x = new Scanner(System.in);
            int num = x.nextInt();
            for (int t = 1; t <= num; t++) {
                int m = x.nextInt();
                int n = x.nextInt();

                if (m >= 1 && n <= 1000000000 && n - m <= 100000) {
                    for (int current = m; current <= n; current++) {
                        int sqr_root = (int) Math.sqrt(current);
                        boolean is_prime = true;
                        for (int i = 2; i <= sqr_root; i++) {
                            if (current % i == 0) {
                                is_prime = false;
                            }
                        }
                        if (is_prime) {
                            System.out.println(current);
                        }
                    }
                }
            }
        }
    }

Others have already said to change the algorithm.
There are other general coding practice problems that are worth highlighting too.

Naming

Most of the variable names are awful. If I see x, I'll never guess it's a Scanner.
m and n for the lower and upper limits are poor names too,
as it's easy to mix up which is which.
sqr_root and is_prime don't follow the Java convention of using camelCase.

Testing

How do you test that your solution actually works?
You rerun the program and enter some inputs and verify that the result is correct?
And if you make a change? You recompile, rerun, reverify?
That's an awful lot of work.
It's a lot less work to write some unit tests instead,
which you can easily re-run as simply as the click of a button,
and the result is as easy to verify as seeing all green lights or not.

However, to make the implementation unit testable, it needs to change a bit:

Make the method to take the inputs as parameters instead of standard input
Make the method return the list of primes found

Something like this:

private static final int MAX_UPPER = 1000000000;
private static final int MAX_DIFF = 100000;

public static void main(String[] args) {
    Scanner scanner = new Scanner(System.in);
    int num = scanner.nextInt();
    for (int t = 1; t <= num; t++) {
        int lower = scanner.nextInt();
        int upper = scanner.nextInt();

        for (Integer prime : getPrimesBetween(lower, upper)) {
            System.out.println(prime);
        }
    }
}

static List<Integer> getPrimesBetween(int lower, int upper) {
    List<Integer> primes = new ArrayList<Integer>();
    if (lower >= 1 && upper <= MAX_UPPER && upper - lower <= MAX_DIFF) {
        for (int current = lower; current <= upper; current++) {
            int squareRoot = (int) Math.sqrt(current);
            boolean isPrime = true;
            for (int i = 2; i <= squareRoot; i++) {
                if (current % i == 0) {
                    isPrime = false;
                    break;
                }
            }
            if (isPrime) {
                primes.add(current);
            }
        }
    }
    return primes;
}

And some unit tests, to get you started:

@Test
public void testBetween_4_and_5() {
    assertEquals(Arrays.asList(5), getPrimesBetween(4, 5));
}

@Test
public void testBetween_4_and_9() {
    assertEquals(Arrays.asList(5, 7), getPrimesBetween(4, 9));
}

Extract constants

Notice in the example earlier,
I converted your original hard-coded constants to proper constants.

private static final int MAX_UPPER = 1000000000;
private static final int MAX_DIFF = 100000;

It's cleaner and better this way.

Correctness

It's not so good that the method returns an empty list if you use a too high input number or too big difference, without any indication that something went wrong.
The user might expect a huge list of numbers between 1 and 99999999, or an error, but definitely not an empty list.

Here's a unit test that fails now, until you fix the method to throw an exception in the case of unsupported input values.

@Test(expected = IllegalArgumentException.class)
public void testTooBigDiff() {
    int lower = 11;
    int upper = lower + MAX_DIFF + 1;
    getPrimesBetween(lower, upper);
}


Extracting original values from cumulative sum values I was writing a function in python to extract original values given a list containing cumulative sum values. For eg -

Given : cumulative_sum = [1, 3, 6, 10, 15, 21, 28, 36, 45]
Output : [1, 2, 3, 4, 5, 6, 7, 8, 9]

I came up with two functions A & B and am looking for feedback on them, especially using xrange() vs enumerate().

import numpy as np
cumulative = np.cumsum(range(1,10))

def A(cumulative):
    output = [0] * len(cumulative)
    for i in xrange(len(cumulative)-1):
        output[-(i+1)] = cumulative[-(i+1)] - cumulative[-(i+2)]
    output[0] = cumulative[0]
return output

def B(cumulative):
    output = [0] * len(cumulative)
    for i,e in reversed(list(enumerate(cumulative))):
        output[i]=cumulative[i] - cumulative[i-1]
    output[0]=cumulative[0]
return output

Both aren't great. B is really complicated, which is unnecessary. A has this extra negation:

output[-(i+1)] = cumulative[-(i+1)] - cumulative[-(i+2)]

You could have just used positive indices (why iterate backwards anyway?):

output[i+1] = cumulative[i+1] - cumulative[i]

I think sort of thing is best accomplished with itertools. In particular, there's a pairwise recipe:

def pairwise(iterable):
    "s -> (s0,s1), (s1,s2), (s2, s3), ..."
    a, b = tee(iterable)
    next(b, None)
    return izip(a, b)

Which translates directly into what we want to do:

def C(cumulative):
    yield cumulative[0]
    for a, b in pairwise(cumulative):
        yield b-a

This returns a generator, instead of a list, but generators are typically far more useful anyway. I feel that this approach makes it much more explicit what it is you're actually doing: you're yielding the first element, and then the pairwise difference between the rest.


Writing a hyperlink to a cell with Apache POI This code embeds a hyperlink into a cell of your choice of an Excel file.

/**
 * @param pathName  the path for the Excel file which you will update
 * @param string    the string you want to put in the cell, and if you put an empty string, it will default to
 *                  "Empty String"
 * @param hyperlink the hyperlink which will be embedded into the cell
 * @param row       the row number of the cell
 * @param column    the column number of the cell
 * @param firstTime this is used in case you want the file to be wiped and start from the beginning
 * @throws RuntimeException if the path can't be found or if an {@link IOException} is thrown for whatever reason
 */
public static void writeToCell(String pathName, String string, String hyperlink, int row, int column, boolean
        firstTime) {
    if (string.equals(""))
        string = "Empty String";

    if (!firstTime) {
        try {
            fileInputStream = new FileInputStream(new File(pathName));
        } catch (FileNotFoundException e) {
            throw new RuntimeException("File not found!", e);
        }
        try {
            workbook = new XSSFWorkbook(fileInputStream);
            sheet = workbook.getSheetAt(0);
        } catch (IOException e) {
            throw new RuntimeException("IOException thrown!", e);
        }
    } else {
        workbook = new XSSFWorkbook();
        sheet = workbook.createSheet();
    }

    CreationHelper createHelper = workbook.getCreationHelper();
    XSSFCell cell;
    try {
        cell = sheet.getRow(row).getCell(column, XSSFRow.CREATE_NULL_AS_BLANK);
    } catch (NullPointerException e) {
        try {
            cell = sheet.getRow(row).createCell(column);
        } catch (NullPointerException npe) {
            cell = sheet.createRow(row).createCell(column);
        }
    }
    XSSFHyperlink url = (XSSFHyperlink) createHelper.createHyperlink(XSSFHyperlink.LINK_URL);

    XSSFCellStyle hyperlinkStyle = workbook.createCellStyle();
    XSSFFont hlink_font = workbook.createFont();
    hlink_font.setColor(IndexedColors.BLUE.getIndex());
    hyperlinkStyle.setFont(hlink_font);

    url.setAddress(hyperlink);
    try {
        cell.setHyperlink(url);
    } catch (NullPointerException e) {
        cell.setCellType(XSSFCell.CELL_TYPE_STRING);
        cell.setHyperlink(url);
    }
    cell.setCellValue(string);
    cell.setCellStyle(hyperlinkStyle);

    try {
        if (fileInputStream != null)
            fileInputStream.close();
    } catch (IOException e) {
        throw new RuntimeException("IOException thrown!", e);
    }

    FileOutputStream fileOutputStream;
    try {
        fileOutputStream = new FileOutputStream(new File(pathName));
    } catch (FileNotFoundException e) {
        throw new RuntimeException("File not found!", e);
    }

    try {
        workbook.write(fileOutputStream);
        fileOutputStream.close();
    } catch (IOException e) {
        throw new RuntimeException("IOException thrown!", e);
    }
}

public static HyperlinkWriter newInstance(String pathName) {
    HyperlinkWriter writer = new HyperlinkWriter();
    try {
        fileInputStream = new FileInputStream(new File(pathName));
    } catch (FileNotFoundException e) {
        throw new RuntimeException("File not found!", e);
    }
    try {
        workbook = new XSSFWorkbook(fileInputStream);
        sheet = workbook.getSheetAt(0);
    } catch (IOException e) {
        throw new RuntimeException("IOException thrown!", e);
    }
    return writer;
}

I was wondering if there was a way to optimize this method in a way because I use this with a different class and I call upon this method multiple times but since I'm creating a FileInputStream and an XSSFWorkbook almost each time, I was wondering if there was a way to get around that and have the FileInputStream just add the bytes that it's missing, instead of reading the file from top to bottom almost each time the method is called. I should also mention that fileInputStream is static, so it won't be released from memory when the method ends (I think...)

Some thoughts:

Use methods! Break out complexities where you can to make it easier on readers of your code.
Avoid statics. There's no reason for it. Make a reusable class instead.
It also helps to rearrange the member variables so they're declared right before they're used.
You really shouldn't have your three static instance variables as static instance variables.
To my knowledge, no, POI does not support anything other than reading in the whole file, making your change, and then writing the whole file.
If you're going to make lots of these calls, create an object that holds the four relevant pieces of information (label, link, row, column), then create them and stick them in a queue. When you've got all of them, open the workbook once and add them all. Might not be worth the complexity - are you sure you have a real performance bottleneck, or are you guessing?

The exception handling needs work, and it only probably compiles, but this might get you pointed in a good direction. Note that it is not thread-safe! An interesting addition might be to make it implement AutoCloseable so that clients could use it in a try-with-resources block, and have it write automatically when they were done with it. Just add a close() method that calls flush, and a check in all public methods to make sure close() hasn't been called yet.

public final class ExcelFileEditor {

    private final File file;
    private final List<HyperlinkAddition> hyperlinkAdditions = new LinkedList<>();

    public ExcelFileEditor(final File file) {
        this.file = file;
    }

    public void setHyperlink(
            final String label,
            final String hyperlink,
            final int row,
            final int column)
    throws IOException {
        this.hyperlinkAdditions.add(new HyperlinkAddition(label, hyperlink, row, column));
    }

    public void flush()
    throws IOException {
        final XSSFWorkbook workbook = this.loadWorkbook();
        for (final HyperlinkAddition hyperlinkAddition : this.hyperlinkAdditions) {
            this.setHyperlink(
                    workbook,
                    hyperlinkAddition.label,
                    hyperlinkAddition.hyperlink,
                    hyperlinkAddition.rowIndex,
                    hyperlinkAddition.columnIndex);
        }
        this.hyperlinkAdditions.clear();
        this.writeWorkbook(workbook);
    }

    private void setHyperlink(
            final XSSFWorkbook workbook,
            final String label,
            final String hyperlink,
            final int rowIndex,
            final int columnIndex) {

        final CreationHelper createHelper = workbook.getCreationHelper();
        final XSSFHyperlink url = createHelper.createHyperlink(XSSFHyperlink.LINK_URL);
        url.setAddress(hyperlink);

        final XSSFFont hyperlinkFont = workbook.createFont();
        hyperlinkFont.setColor(IndexedColors.BLUE.getIndex());

        final XSSFCellStyle hyperlinkStyle = workbook.createCellStyle();
        hyperlinkStyle.setFont(hyperlinkFont);

        final XSSFSheet sheet = this.getSheet(workbook);
        final XSSFCell cell = this.getCell(sheet, rowIndex, columnIndex);
        cell.setCellType(XSSFCell.CELL_TYPE_STRING);
        cell.setHyperlink(url);
        cell.setCellValue(this.buildCellLabel(label));
        cell.setCellStyle(hyperlinkStyle);
    }

    private XSSFWorkbook loadWorkbook()
    throws IOException {

        if (!this.file.exists()) {
            return new XSSFWorkbook();
        }

        try (FileInputStream fis = new FileInputStream(this.file)) {
            return new XSSFWorkbook(fis);
        }
    }

    private XSSFSheet getSheet(final XSSFWorkbook workbook) {
        final XSSFSheet sheet = workbook.getSheetAt(0);
        if (sheet == null) {
            return workbook.createSheet();
        }
        return sheet;
    }

    private XSSFCell getCell(final XSSFSheet sheet, final int rowIndex, final int columnIndex) {
        final XSSFRow row = this.getRow(sheet, rowIndex);
        final XSSFCell cell = row.getCell(columnIndex, XSSFRow.CREATE_NULL_AS_BLANK);
        if (cell == null) {
            return row.createCell(columnIndex);
        }
        return cell;
    }

    private XSSFRow getRow(final XSSFSheet sheet, final int rowIndex) {
        final XSSFRow row = sheet.getRow(rowIndex);
        if (row == null) {
            return sheet.createRow(rowIndex);
        }
        return row;
    }

    private String buildCellLabel(final String label) {
        if (label.isEmpty()) {
            return "Empty String";
        }
        return label;
    }

    private void writeWorkbook(final XSSFWorkbook workbook)
    throws IOException {
        final FileOutputStream fileOutputStream = null;
        try (FileOutputStream fos = new FileOutputStream(this.file)) {
            workbook.write(fos);
        }
    }

    private static final class HyperlinkAddition {
        public final String label;
        public final String hyperlink;
        public final int rowIndex;
        public final int columnIndex;

        public HyperlinkAddition(
                final String label,
                final String hyperlink,
                final int rowIndex,
                final int columnIndex) {
            this.label = label;
            this.hyperlink = hyperlink;
            this.rowIndex = rowIndex;
            this.columnIndex = columnIndex;
        }
    }
}


Finding tiered income c-primer My thoughts are to create a method that contains all of the variables declared. Is it better to say initialized?

I would like it to be more efficient but still as clear as possible for future maintenance of the code. Any suggestions for the main bulk of information, such as methods?

if (incomeDec <= 300)
{
    taxRateDec1 = 0.15m;
    taxDec = incomeDec * taxRateDec1;
}
else if (incomeDec <= 450)
{
    tierAmtInt = 300;
    taxRateDec1 = 0.15m;
    taxRateDec2 = 0.2m;

    tempValueDec = tierAmtInt * taxRateDec1;
    taxDec = (incomeDec - tierAmtInt) * taxRateDec2;
    taxDec = taxDec + tempValueDec;
}
else
{
    tierAmtInt = 300;
    tierAmtInt2 = 150;
    taxRateDec1 = 0.15m;
    taxRateDec2 = 0.2m;
    taxRateDec3 = 0.25m;

    tempValueDec = tierAmtInt * taxRateDec1;
    tempValueDec = tempValueDec + tierAmtInt2 * taxRateDec2;
    taxDec = (incomeDec - tempValueDec) * taxRateDec3;
    taxDec = taxDec + tierAmtInt;
}

incomeDec = incomeDec - taxDec;

I don't like c-style of declaring all the variables at the start of a function (which is a necessity in c, not in c#). It's better to define them closer to where they are used.

I suggest not to define your types explicitly, use var instead such as:

var tierAmtInt = 300;

You'll see the benefits when you want to modify or refactor stuff. This one looks like a good read on that: https://softwareengineering.stackexchange.com/questions/42863/explicitly-defining-variable-data-types-vs-using-the-keyword-var

For taxRateDec1/2/3, you can perhaps consider using array or sth similar.

If you really want to be perfectionist, you can consider passing all these variables as parameters or read from somewhere else. Just consider one day these rates changing, would you like to modify the code and recompile?

Last but not least, these variables are actually constants, consider putting 'const' keyword whenever necessary..


XOR encryption program in C I've written a program that encrypts files (or stdin) with a user-specified key from 1 to 255. This "encryption" is very insecure, but still a bit better than ROT13 (at least for people trying to crack it using pencils and paper). How can I improve this program?

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

/*
 * A simple program for encrypting files (or stdin) using the XOR operation.
 * Such encryption is very insecure as there are only 254 valid key values,
 * but is still much more secure than ROT13, where there is only 1 valid key.
 *
 * Using a key value of 0 or 256 is equivalent to no encryption, and is
 * therefore not allowed. Other values are too large to fit into one byte
 * and therefore cannot be properly XORed with a byte.
 */

static void usage(const char *const);

int main(int argc, char *argv[])
{
    FILE *in = NULL;
    FILE *out = NULL;
    int k = 0;
    int c;

    while ((c = getopt(argc, argv, "i:o:k:h")) != -1) {
        switch (c) {
        case 'i':
            in = fopen(optarg, "r");
            break;
        case 'o':
            out = fopen(optarg, "w");
            break;
        case 'k':
            k = atoi(optarg);
            break;
        case 'h':
            usage(argv[0]);
            return EXIT_FAILURE;
        }
    }

    in = in ? in : stdin;
    out = out ? out : stdout;

    if (k <= 0 || k > 255) {
        fprintf(stderr, "%s: must specify a valid key via -k\n",
            argv[0]);
        fprintf(stderr, "%s: valid key values are from 1 to 255\n",
            argv[0]);
        return EXIT_FAILURE;
    }

    while ((c = fgetc(in)) != EOF) {
        fputc(c ^ k, out);
    }

    return EXIT_SUCCESS;
}

void usage(const char *const s)
{
    fprintf(stderr, "usage: %s -k key [-i input] [-o output]\n", s);
}

To compile:

$ clang -O2 -Weverything -Werror -o xor xor.c

Use binary mode
Currently, you are opening your files like this:

    in = fopen(optarg, "r");
    out = fopen(optarg, "w");

On a UNIX system that would be fine, but on a Windows system, that could result in CRLF pairs being converted into a single LF when reading, and single LF characters converted to CRLF pairs when writing.  This is particularly important because once you do the xor operation, your output may contain random LF characters.
You should open your files in binary mode instead:
    in = fopen(optarg, "rb");
    out = fopen(optarg, "wb");


Creating a Dog Shelter Background: I have created a program that allows a user to input information about their dog. The output is the bill they will receive commensurate their dog's weight.

The Problem: I believe I am doing this inefficiently. I don't like the way I have structured the current code and would like to improve upon my code. I would like critique from other programmers on how to improve this code

The Code:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace TheBarkingLotHasslerChasenB
{
    class Program
    {
        static bool isInt = false;
        static bool isDouble = false;

        static void Main(string[] args)
        {
            int id = 0;
            int age = 0;
            double weight = 0.0;

            id = HandleInput("Please enter your Owner ID", id);
            String name = HandleInput("Please enter the name of your dog");
            String breed = HandleInput("Please enter the breed of your dog");
            age = HandleInput("Please enter the age of your dog", age);
            weight = HandleInput("Please enter the weight of your dog", weight);

            DogOwner dogOwner = new DogOwner(id);

            Dog fido = new Dog(age, name, breed, weight);

            DetermineCost(fido.GetDogWeight());

            Console.Read();
        }

        private static int HandleInput(String input, int output)
        {
            do
            {
                Console.WriteLine(input);
                isInt = int.TryParse(Console.ReadLine(), out output);
                if (isInt)
                {
                    Console.WriteLine("");
                    return output;
                }

                Console.WriteLine("Please enter an integer value\n");

            } while (!isInt);

            return 0;

        }

        private static String HandleInput(String input)
        {
            String output;

            Console.WriteLine(input);

            output = Console.ReadLine();

            Console.WriteLine("");

            return output;
        }

        private static double HandleInput(String input, double output)
        {
            do
            {
                Console.WriteLine(input);
                isDouble = double.TryParse(Console.ReadLine(), out output);
                if (!isDouble)
                {
                    Console.WriteLine("Please enter a decimal value\n");
                }
                // If the user didn't enter a decimal, they are asked for input once again
                else
                {
                    Console.WriteLine("");
                    return output;
                }
            } while (!isDouble);

            return 0.0;
        }

        private static void DetermineCost(double weight)
        {
            if (weight < 15.0)
            {
                Console.WriteLine("Your bill for weekly day care is going to be: $55.00");
            }
            else if (weight > 15.0 && weight <= 30.0)
            {
                Console.WriteLine("Your bill for weekly day care is going to be: $75.00");
            }
            else if (weight > 30.0 && weight <= 80.0)
            {
                Console.WriteLine("Your bill for weekly day care is going to be: $105.00");
            }
            else
            {
                Console.WriteLine("Your bill for weekly day care is going to be: $125.00");
            }
        }
    }
}

1.- You should try to declare and instance variables at the same time. Instead of int age = 0; age = getUserInput(); this is better: int age = getUserInput();

2.- One method should do one thing, DetermineCost should 'determine cost', if you want to inform how much the cost is, then you should create another method (in this case I created PrintWeeklyCost())

3.- You should know where the variables are going to live, I mean, the scope of the variables. In you case isInt and isDouble should go only where they are going to be used.

4.- The if else if to see numeric ranges; it's really easy to make little mistakes. What if the dog is exactly 15 kg/pounds/whatever?. Will the cost be 125? An easy way to discover little errors is being consistent and, for me, that style helps a lot.

5.- What if the user doesn't enter text at all in HandleStringInput? (here I changed the name of the methods, I think that overloading methods can add more complexity to your program when it is not needed, now if you are learning go ahead!) Check for errors in input is a good practice !

class Program
{

    static void Main(string[] args)
    {
        int id = HandleIntegerInput("Please enter your Owner ID");
        String name = HandleStringInput("Please enter the name of your dog");
        String breed = HandleStringInput("Please enter the breed of your dog");
        int age = HandleIntInput("Please enter the age of your dog");
        double weight = HandleDoubleInput("Please enter the weight of your dog");

        DogOwner dogOwner = new DogOwner(id);
        Dog newDog = new Dog(age, name, breed, weight);

        double cost = DetermineCost(newDog.GetDogWeight());
        PrintWeeklyBill(cost);

        Console.Read();
    }

    private static int HandleIntInput(String input)
    {
        Console.WriteLine(input);

        int output;
        bool validInput = false;
        while(!validInput)
        {
            Console.WriteLine("Please enter an integer value\n");
            validInput = int.TryParse(Console.ReadLine(), out output);
        }

        Console.WriteLine("");
        return output;
    }

    private static String HandleStringInput(String input)
    {
        Console.WriteLine(input);

        String output;
        bool validInput = false;
        while(String.IsNullOrEmpty(output))
        {
            Console.WriteLine("Please enter a valid text value\n");
            output = Console.ReadLine();
        }

        Console.WriteLine("");
        return output;
    }

    private static double HandleDoubleInput(String input)
    {
        Console.WriteLine(input);

        double output;
        bool validInput = false;
        while(!validInput)
        {
            Console.WriteLine("Please enter an decimal value\n");
            validInput = double.TryParse(Console.ReadLine(), out output);
        }

        Console.WriteLine("");
        return output;
    }

    private static double DetermineCost(double weight)
    {
        double cost;
        if (weight <= 15.0)
        {
            cost = 55.00;
        }
        else if (15.0 < weight && weight <= 30.0)
        {
            cost = 75.00;
        }
        else if (30.0 < weight && weight <= 80.0)
        {
            cost = 105.00;
        }
        else
        {
            cost = 125.00;
        }

        return cost;
    }

    private static void PrintWeeklyBill(double costPerWeek)
    {
        String costMsge = String.Format("Your bill for weekly day care is going to be: $", costPerWeek);
        Console.WriteLine(costMsge);
    }
}


Resolving promises on resolver using ui-router We're rewriting some parts of our application. We're trying to avoid the promise anti-pattern and do things properly, but I'm not certain that we did so.

More precisely, we are rewriting the resolver of a state using ui-router.

.state( 'catalogue', {
  // skipped on purpose not relevant content
  resolve :
    boxCreateData : function( $stateParams, $q, Basket, Customer ) {

      var defer = $q.defer(),
          basket = null,
          customer = null;

      Basket.fromApi( $stateParams.id )
        .then( function( basket ) {
          Customer.fromApi( basket.getCustomer() )
            .then( function( customer ) {
              defer.resolve( { basket : basket, customer : customer } );
            });
        });

      return defer.promise;

    }
});

Basket.fromApi and Customer.fromApi are static methods of their corresponding "class"es, Basket and Customer. Both implementations are like this (more or less; it's not the whole code, but it shows the concept. Customer.fromApi is the same):

Basket.fromApi = function( id ) {
  var defer = $q.defer();
  boxesResource.get( { id : id }, function( response ) {
    // various manipulation here with the response...
   defer.resolve( new Basket( ... ) );
  });
  return defer.promise;
};

Note we are using the get method of $resource.

The major issue here is that until we have the basket, we can't query the customer (because customer ID comes in the basket object).

We are querying first the basket with the basket ID. Once we have it, we can get the customer ID from the basket object and then, retrieve the customer object.

I just don't like the then pyramid. I'm not sure if we are missing something or in this case, there is no other way to do it.

Is there a cleaner way to accomplish the same thing?

You shouldn't use $q.defer unless you have to.

Here you can just return the promise:

Basket.fromApi = function (id) {
    return boxesResource.get({
        id: id
    }, function (response) {
        // various manipulation here with the response...
        return new Basket(...));
    });
};

You could really clean up the resolve by creating a new service:

State

.state('catalogue', {
    resolve: {
        boxCreateData: function($stateParams, boxCreateService) {
             return boxCreateService.getBoxCreateData($stateParams.id);
        }
    }
})

Service

app.service('boxCreateService', function(Basket, Customer) {
    this.getBoxCreateData = function(id) {
        return Basket.fromApi(id)
                .then(function (basket) {
                    return Customer.fromApi(basket.getCustomer())
                        .then(function (customer) {
                            return {
                                basket: basket,
                                customer: customer
                            };
                    });
                });
    };
});

There's not much you can do to avoid two nested promises here. You need the customer ID before you can get the customer.

More on the deferred antipattern.