EF GroupBy query optimization I have this method:

    public async Task<ActionResult> Index()
    {
        var orders = await db.Orders.AsNoTracking().GroupBy(x => x.Status).Select(g => new { Status = g.Key, Count = g.Count() }).ToDictionaryAsync(x => x.Status, x => x.Count);
        ViewBag.InWork = orders[WebApplication.Models.Order.OrderStatus.InWork];
        ViewBag.InProcess = orders[WebApplication.Models.Order.OrderStatus.InProcess];
        ViewBag.Accepted = orders[WebApplication.Models.Order.OrderStatus.Accepted];
        ViewBag.Deleted = orders[WebApplication.Models.Order.OrderStatus.Deleted];

        return View();
    }

and it's generated this SQL:

SELECT
    [GroupBy1].[K1] AS [Status],
    [GroupBy1].[A1] AS [C1]
    FROM ( SELECT
        [Extent1].[Status] AS [K1],
        COUNT(1) AS [A1]
        FROM [dbo].[Orders] AS [Extent1]
        GROUP BY [Extent1].[Status]
    )  AS [GroupBy1]

How can I escape from SELECT FROM SELECT and optimize this query?

I don't see anything wrong with the generated query. EF does generate verbose T-SQL, but that doesn't mean it's inefficient. If the generated query generates a decent execution plan there is no need to try to get EF to generate T-SQL the way you would have written it.

However the code would be easier to read like this:

var orders = await db.Orders.AsNoTracking()
                            .GroupBy(x => x.Status)
                            .Select(g => new { Status = g.Key, Count = g.Count() })
                            .ToDictionaryAsync(x => x.Status, x => x.Count);


Simplifying Macro-Generating Racket Macro I'm trying to write a small macro library in Racket that extends a few of the racket/match functions I use, by printing which clause was expanded.

While this seems to be decent practice for learning macros in Racket, I still find them incredibly daunting.  At some point, I wind up just trying everything with no real sense of direction until it works how I want, and that's partially how I built my macros up to their current state, as seen here:

#lang racket

(require
  (for-syntax
   racket/string
   racket/list
   racket/syntax
   syntax/parse
   syntax/parse/experimental/template
   syntax/parse/lib/function-header))

;; From answer to my question here:
;; http://stackoverflow.com/a/38577121/1017523
(begin-for-syntax
  (define (clauses->numbers stx)
    (range (length (syntax->list stx)))))

(define-syntax (generate-debug-version stx)
  (syntax-case stx ()
    [(_ function-name)
     (with-syntax ([new-name (format-id stx "~a/debug" #'function-name)])
       #'(define-syntax (new-name stx)
           (...
            (syntax-parse stx
              [(_ id [pattern value] ...)
               (with-syntax ([(n ... )
                              (clauses->numbers #'([pattern value] ... ))])
                 (syntax/loc stx
                   (function-name
                    id
                    [pattern
                     (begin
                       (displayln (format "Matched case ~a" (add1 n)))
                       value)]
                    ...)))]))))]))

(generate-debug-version match)
(generate-debug-version match*)

The small first macro, clauses->numbers, determines which clause is expanded in racket/match functions
The main macro, generate-debug-version, is what I'm trying to simplify.

This macro currently takes one function name (for example, match or match*), and generates a macro that defines a new function with a /debug suffix that prints the expanded clause number.

Before asking this question, I was trying to add the ability to supply multiple function names to this macro using ellipses (making the last two lines (generate-debug-versions match match*)), but I can't figure out how in my macro's current state.

Can someone give me advice on how to simplify/clarify my generate-debug-version macro, so I can ultimately rewrite it to accept multiple arguments?

I'm aware of the simpler define-syntax-rule macro tool, but it didn't work in the circumstances I attempted.

Starting at the top, you have a lot of unused imports. Fortunately, DrRacket can tell you which imports are unused, since it will highlight them in red when you hover your mouse over them. Additionally, if you click the Check Syntax button, it will color all the unused imports red. Using that, we can trim the import list down to the following set:

(require (for-syntax racket/list
                     racket/syntax
                     syntax/parse))

Next, let’s take a look at the bulk of your code, generate-debug-version. First of all, it’s a bit odd that you’re using syntax-case for the outer macro, but syntax-parse for the inner one. Just use syntax-parse everywhere; there’s really no reason to ever use syntax-case in Racket.

Furthermore, the syntax/parse/define module provides a nice define-syntax-parser abbreviated form, which helps to simplify things slightly and remove some redundancy:

(require (for-syntax racket/list
                     racket/syntax)
         syntax/parse/define)

(define-syntax-parser generate-debug-version
  [(_ function-name)
   (with-syntax ([new-name (format-id this-syntax "~a/debug" #'function-name)])
     #'(define-syntax-parser new-name
         (...
          [(_ id [pattern value] ...)
           (with-syntax ([(n ... )
                          (clauses->numbers #'([pattern value] ... ))])
             (syntax/loc this-syntax
               (function-name
                id
                [pattern
                 (begin
                   (displayln (format "Matched case ~a" (add1 n)))
                   value)]
                ...)))])))])

Note the replacement of explicit uses of stx with uses of this-syntax, which always refers to the current piece of syntax being matched within a syntax-parse form.

Now with this in mind, we can start to refactor the bulk of the macro itself. There are a few things that could be improved, staring with the uses of with-syntax. Conveniently, syntax/parse permits #:with clauses after patterns themselves, which act sort of like wrapping the body with with-syntax, but they can use syntax-parse patterns, and the parser will backtrack if they fail. This lets us simplify the code even further:

(define-syntax-parser generate-debug-version
  [(_ function-name)
   #:with new-name (format-id this-syntax "~a/debug" #'function-name)
   #'(define-syntax-parser new-name
       (...
        [(_ id [pattern value] ...)
         #:with (n ...) (clauses->numbers #'([pattern value] ...))
         (syntax/loc this-syntax
           (function-name
            id
            [pattern
             (begin
               (displayln (format "Matched case ~a" (add1 'n)))
               value)]
            ...))]))])

However, there’s actually an issue here, which is that the use of format-id really shouldn’t use this-syntax at all. Instead, it should pull lexical context from #'function-name itself, since the provided identifier should control where the generated identifier’s lexical context comes from. (For an example of where this can be important, try using the define-tracing-match* form from the end of this answer without this change, and you’ll see what the issue is.)

(define-syntax-parser generate-debug-version
  [(_ function-name)
   #:with new-name (format-id #'function-name "~a/debug" #'function-name)
   ; ...
   ])

Another thing we can improve is that we can use the id syntax class for the function-name pattern, which will ensure that the generate-debug-version macro is actually provided an identifier, and will raise a syntax error if it isn’t. Additionally, function-name really isn’t a great name for that pattern, since match is not a function, it is a form. Let’s fix that:

(define-syntax-parser generate-debug-version
  [(_ form-id:id)
   #:with debug-id (format-id #'form-id "~a/debug" #'form-id)
   #'(define-syntax-parser debug-id
       ; ...
       )])

While we’re discussing names, generate-debug-version isn’t a very good one. For one thing, it doesn’t just generate a debug form, it defines one. For another, it specifically generates debug versions of match-like forms, nothing else, so that should probably be included in the name, too. I picked the name define-tracing-match, but you could pick a similar name if you wished.

Okay, what now? Well, while clauses->numbers works, it could honestly be better. It’s really nice that we can use syntax patterns to write such a declarative style of macro, but clauses->numbers isn’t declarative at all, it’s completely procedural. To help fix that, we can write a splicing syntax class which will number the clauses for us, lifting out the procedural component into a separate piece. That looks like this:

(begin-for-syntax
  (define-splicing-syntax-class numbered-clauses
    #:attributes [[pattern 1] [value 1] [n 1]]
    #:description #f
    [pattern {~seq [pattern value] ...}
             #:with [n ...] (range (length (attribute pattern)))]))

You can read more about what syntax classes do and how they work in the extensive documentation, but the basic idea here is to extract out some procedural logic into a reusable pattern than syntax-parse can understand. The #:attributes and #:description options are optional here, but the former helps readability, and the latter helps with error messages.

With that syntax class in place, we can further simplify the main macro by using it:

(define-syntax-parser define-tracing-match
  [(_ form-id:id)
   #:with debug-id (format-id #'form-id "~a/debug" #'form-id)
   #'(define-syntax-parser debug-id
       (...
        [(_ id clause:numbered-clauses)
         (syntax/loc this-syntax
           (form-id id
             [clause.pattern
              (begin
                (displayln (format "Matched case ~a" (add1 'clause.n)))
                clause.value)]
            ...))]))])

This makes our main macro completely declarative (save for the small use of format-id, which is pretty harmless), and it’s pretty much entirely built out of patterns and templates. This is what the Racket macro system is so good at: it lets you pretty much just write what you mean, and you still get great error reporting for when things go wrong.

With all these changes in place, here’s what the final code looks like:

#lang racket

(require (for-syntax racket/list
                     racket/syntax)
         syntax/parse/define)

(begin-for-syntax
  (define-splicing-syntax-class numbered-clauses
    #:attributes [[pattern 1] [value 1] [n 1]]
    #:description #f
    [pattern {~seq [pattern value] ...}
             #:with [n ...] (range (length (attribute pattern)))]))

(define-syntax-parser define-tracing-match
  [(_ form-id:id)
   #:with debug-id (format-id #'form-id "~a/debug" #'form-id)
   #'(define-syntax-parser debug-id
       (...
        [(_ id clause:numbered-clauses)
         (syntax/loc this-syntax
           (form-id id
             [clause.pattern
              (begin
                (displayln (format "Matched case ~a" (add1 'clause.n)))
                clause.value)]
            ...))]))])

A bit more concise, and hopefully a bit clearer, too.

Now for a couple of extras. You mentioned that you wanted to write a generate-debug-versions macro. This wouldn’t be too hard to do with the new version of generate-debug-version by using some well-placed ellipses, actually, but it would probably be even easier to just define a new generate-debug-versions macro (or, since we changed the name, a define-tracing-match* macro) that defers to the existing one.

The easiest way to do this is probably to use the define-simple-macro form, also from syntax/parse/define, which is basically syntax-rules mixed with all the syntax-parse enhancements. The macro itself is trivial:

(define-simple-macro (define-tracing-match* form-id:id ...)
  (begin (define-tracing-match form-id) ...))

Now, it’s extremely easy to define both match/debug and match*/debug at the same time:

(define-tracing-match* match match*)

One final additional thing you can do is adjust the format-id call to copy source location information and syntax properties from the provided form-id, like this:

#:with debug-id (format-id #'form-id "~a/debug" #'form-id
                           #:source #'form-id #:props #'form-id)

By doing this, when you hover your cursor over a use of match/debug in DrRacket, it will draw an arrow to the identifier used with define-tracing-match to generate the debug version in the first place, which is useful.

However, if you wanted to get even fancier, you could skip that step and use the 'sub-range-binders syntax property to convey even more fine-grained information to DrRacket. Specifically, you can adjust the outer macro for define-tracing-match to attach the property as follows:

(define-syntax-parser define-tracing-match
  [(_ form-id:id)
   #:with debug-id (format-id #'form-id "~a/debug" #'form-id)
   (syntax-property
    #'(define-syntax-parser debug-id
        (...
         [(use-id id clause:numbered-clauses)
          (syntax/loc this-syntax
            (form-id id
                     [clause.pattern
                      (begin
                        (displayln (format "Matched case ~a" (add1 'clause.n)))
                        clause.value)]
                     ...))]))
    'sub-range-binders
    (let ([id-len (string-length (symbol->string (syntax-e #'form-id)))])
      (vector (syntax-local-introduce #'debug-id) 0 id-len 0.5 0.5
              (syntax-local-introduce #'form-id) 0 id-len 0.5 0.5)))])

This will make DrRacket draw an arrow for part of the match/debug identifier, the part that shares the same name as the provided one. The best way to illustrate what this does is with a screenshot:

(This is the technique the built-in struct macro uses to get the special binding arrows for its field accessors.)

Admittedly, however, this is pretty advanced macrology, and I probably wouldn’t do something this fancy unless I was distributing it as part of a library, so no worries if you leave that part out.


Debouncing function, ES6 version Please, review the code:

const callback = (...args) => {
  console.count('callback throttled with arguments:', args);
}

const debounce = (callback, delay) => {
  let timeoutHandler = null
  return (...args) => {
    if (timeoutHandler) {
      clearTimeout(timeoutHandler)
    }
    timeoutHandler = setTimeout(() => {
      callback(...args)
      timeoutHandler = null
    }, delay)
  }
}

window.addEventListener('oninput', debounce(callback, 100))

P.S. As @Anshul explained: Debouncing enforces that a function not be called again until a certain amount of time has passed without it being called. As in "execute this function only if 100 milliseconds have passed without it being called."

DELAY V DEBOUNCE
What you have implemented is a delay not a debounce. You delay the firing of the event until after a set time. Events during this time further delay the final event.
Design
Debouncing should respond to the first event immediately and then hold any response during the debouncing period after the listener has been called.
It would be better to clearly identify the event argument passed to the listener.
You do not provide a way of passing additional arguments to the event. By adding them to the outer function, you get better control. (see example)
General points

Inconsistent use of semicolon. Either use it (recommended) or not. Don't mix styles.
var is function scoped, let os block scope. It is better to use var when a variable is declared in function scope.
timeoutHandler is a poor name. The timeout function returns and Id (AKA a handle). The name you used implies that the variable is a function. A better name could be timeoutHandle, or in context just id or handle will do.
The is no need to assign null to the variable timeoutHandler. undefined variable are undefined and making it null is just noise and semantically incorrect
The handle returned by setTimeout (and setInterval) are unique to the context and the call. It is never reused. clearTimeout ignores invalid handles so there is no need check if the handle is valid, nor change its value inside the timeout function.
You can reduce the call stack depth by using the trailing arguments of setTimeout to pass the arguments.
window is the default object. Inconsistent use is poor style. You do not use it for clearTimeout, setTimeout, and console so why for addEventListener?
addEventListener('oninput'... is incorrect, there is no event called oninput. Should be addEventListener('input'...

Examples
Covering the general points and comparing debounce, delay, and immediate event responses. The delay is also used to clear the displays 2sec after the last event.
It also shows when to use window. In the example emoticons can not be used as direct references to elements, thus I use window bracket notation to access the names.

const DELAY_MS = 250;
const CLEAR_DELAY_MS = 2000;
const CLEAR_SPEED_MS = 17;
const DISPLAYS = ["","","⚡"];

const logger = (event, ...args) =>  log(args[0]);

const delay = (callback, delay, ...args) => {
    var handle;
    return event => {
        clearTimeout(handle);
        handle = setTimeout(callback, delay, event, ...args);
    }
}

const debounce = (callback, delay, ...args) => {
    const timerClear = () => clear = true;
    var clear = true;
    return event => {
        if (clear) {
            clear = false;
            setTimeout(timerClear, delay);
            callback(event, ...args);
        }
    }
}

addEventListener('input', debounce(logger, DELAY_MS, DISPLAYS[0]));
addEventListener('input', delay(logger, DELAY_MS, DISPLAYS[1]));
addEventListener('input', event => logger(event, DISPLAYS[2]));
addEventListener('input', delay(clear, CLEAR_DELAY_MS));
gun.focus();

/* Helper functions unrelated to question */
function clear() {
    var allClear = "";
    DISPLAYS.forEach(name => {
        var text = window["log" + name].textContent;
        allClear += window["log" + name].textContent = text.substring(1);
    })
    if (allClear !== "") {
       setTimeout(clear, CLEAR_SPEED_MS);
    } else {
       gun.value = "";
    }
}

function log(which) { window["log" + which].textContent += which }
b { font-size : large }

Rumble!! Debounce V Delay V Immediate. With a 2 second delayed clear.

Debounced 250ms:

Delayed.. 250ms:

Immediate.. 0ms:

<input type="text" placeholder="Rapid fire key strokes" size="50" id="gun">


Expand compressed string range Say you have two strings: exp->aaccbb comp-> aa2bb

This example will return true as there are 2 chars between aa and bb. I have written this code snippet and am wondering if there is a better way do solve this problem and if I am missing any edge cases.

Also, what would be the fastest way to solve this problem with respect to time/place complexity? The solution is a 2 pointer approach with linear complexity. Follow up -> either side can have compressions or both at the same time.

const isNumber = (ch)=>{
  return Number(ch) == ch
}

const isExp = (exp, comp)=>{
  let i=j=0;

  while(i<exp.length && j<comp.length){
    if(comp[j] !== exp[i] && !isNumber(comp[j])){
      return false
    }
    else if(isNumber(comp[j])){

     let currentCompIdx = j;

     /*Find consexutive numbers*/
      while(isNumber(comp[currentCompIdx+1]))currentCompIdx++;

      /*Get Number from the compressed string*/
      const moveIndexBy = Number(comp.slice(j,currentCompIdx+1));
            /*set j pointer to index after the number*/
            j=currentCompIdx+1;

      /*Check if the letters after number is same*/
      if(comp[j] !== exp[i+moveIndexBy]){
        return false
      }
     i++;
     j++;

    }
    else{
      i++;
      j++;
    }
  }
  return true
}

/*Returns true as it has 2 chars between aa and cc*/
alert(isExp("aaccbb", "aa2bb"))

/*Returns true as it has 2 chars between aa and cc and 4 chars between bb and cc*/
alert(isExp("aaccbbqqqqcc", "aa2bb4cc"))

/*Returns true as it has 10 chars between aa and cc */
alert(isExp("aaccbbqqqqsacc", "aa10cc"))

/*Returns false as the exp string as only 1 char between aa and bb */
alert(isExp("aacbb", "aa2bb"))
```

Notice how close the syntax of your compressed strings resemble that of a regular expression:

   aa2bb => aa.{2}bb
aa2bb4cc => aa.{2}bb.{4}cc
  aa10cc => aa.{10}cc

Therefore you can easily create a regexp from your compressed string and test the expanded string against it:

function isExp(exp, comp) {
  const regexp = RegExp(`^${comp.replace(/\d+/g, '.{$&}')}$`)
  return regexp.test(exp);
}


Get subsets of a set given as a list This code is meant to create a list of subsets of a given set which is represented as a list.

I'm doing this to improve my style and to improve my knowledge of fundamental algorithms/data structures for an upcoming coding interview.

def subsets(s):
    if s == []:
        return [s]
    sets = [s]
    for i in range(0, len(s)):
        tmp_subsets = subsets(s[:i] + s[i+1:])
        for subset in tmp_subsets:
            if subset not in sets:
                sets.append(subset)
    return sets

I'm also not sure if using the set() data structure would be considered cheating in an interview context.

If there are any ideas about how to improve its performance without significantly compromising on readability I would also be open to that.

This is unnecessary:

if s == []:
    return [s]

You can safely remove it, the program will still work.

This step is not so great:

if subset not in sets:
    sets.append(subset)

For two reasons:

Checking if a list contains some items is an \$O(n)\$ operation
Comparing sets is not free either

A more efficient solution is to count from 0 until 1 << n, and use the bitmap of the count to decide the elements that are part of a subset.

def subsets(s):
    sets = []
    for i in range(1 << len(s)):
        subset = [s[bit] for bit in range(len(s)) if is_bit_set(i, bit)]
        sets.append(subset)
    return sets

def is_bit_set(num, bit):
    return num & (1 << bit) > 0

That is, in a subset, each element may be either present, or not. So each element has only 2 possible states: in or out. 1 or 0. If we look at the binary representation of numbers from 0 to 2^n -1, where n is the number of elements, for example when n=3, we have:

    cba
0 = 000
1 = 001
2 = 010
3 = 011
4 = 100
5 = 101
6 = 110
7 = 111

There are 8 possible subsets, and the bits represent whether an element is in the subset or not.
This is the idea used by the program:

The outer loop goes from 0 until 2^n - 1.
The inner loop goes from 0 until n - 1.
1 << bit is 1 shifted to the left bit times.

For example, when i = 3, that corresponds to bits 011.
We loop from 0 until 2, comparing i against 001, 010, and 100.
For these values, the expression i & (1 << bit) will be evaluated as
011 & 001 = 001, 011 & 010 = 010, and 011 & 100 = 000, respectively.
The first two are greater than 0, the last one is not.


Log Visitor Count in Asp.net MVC I have a table to Log visitor Count Like :

public class VisitorLog:BaseEntity
{
    public int Id { get; set; }
    public DateTime VisitedOn { get; set; }
    public string LocationIP { get; set; }
    public string BrowserName { get; set; }

    public VisitorLog()
    {
        VisitedOn = DateTime.Now;
    }
}

and in Global.asax I have this :

protected void Application_BeginRequest(object sender,EventArgs e)
{
    if (new HttpRequestWrapper(Request).IsAjaxRequest()) return;

    foreach (var task in ApplicationObjectFactory.Container.GetAllInstances<IRunOnEachRequest>())
    {
        task.Execute();
    }
}

and Implementation in Service Layer is like below :

public bool Add(string locationId, string browserName)
{
    if (string.IsNullOrEmpty(locationId) || string.IsNullOrEmpty(browserName)) throw new ArgumentNullException("Visitor Log");
  return  Add(new AddVisitLogViewModel { BrowserName=browserName,LocationIP=locationId.ToLower()});
}

public bool Add(AddVisitLogViewModel visitorLogViewModel)
{

    if (visitorLogViewModel == null) throw new  ArgumentNullException("visitorLog");
    if (Any(row => row.LocationIP == visitorLogViewModel.LocationIP && DbFunctions.TruncateTime(row.VisitedOn) ==DbFunctions.TruncateTime(DateTime.Now))) return false;
    var model = Mapper.Map<VisitorLog>(visitorLogViewModel);
    AddItem(model);
    return true;
}

In Application_BeginRequest I have to check request is Ajax or Action Call and others , is there a better way to check this or Improve these code ?

First of all I'd make code in Application_BeginRequest little bit more self-explicative:

if (!IsAjaxRequest())
    ExecuteAllRunOnEachRequestHandlers();

You have to introduce two small utility functions but it will make the intent clear when reading Application_BeginRequest (which may grow over time) without going in the details.

private bool IsAjaxRequest()
    => new HttpRequestWrapper(Request).IsAjaxRequest();

private void ExecuteAllRunOnEachRequestHandlers()
{
    foreach (var task in GetAllHandlers<IRunOnEachRequest>())
        task.Execute();

    IEnumerable<T> GetAllHandlers<T>()
        => ApplicationObjectFactory.Container.GetAllInstances<T>();
}

It may happen that you need to ignore other types of requests, in this case simply change IsAjaxRequest() to IsPageVisit() (or whatever else expresses the concept you want to capture) and remove the negation.

In Add(string, string) you always throw ArgumentNullException but which parameter is wrong is an important information that is lost. Also to be String.Empty and to be null are different things:

if (locationId == null)
    throw new ArgumentNullException(nameof(locationId));

if (String.IsNullOrWhiteSpace(locationId))
    throw new ArgumentException("...", nameof(locationId));

if (browserName == null)
    throw new ArgumentNullException(nameof(browserName));

if (String.IsNullOrWhiteSpace(browserName))
    throw new ArgumentException("...", nameof(browserName));

Vertical space is free, do not pack your code in one line, it hurts readability.

return Add(new AddVisitLogViewModel
{
     BrowserName = browserName,
     LocationIP = locationId.ToLower()
});

In Add(AddVisitLogViewModel) you may apply same principles:

public bool Add(AddVisitLogViewModel visitorLogViewModel)
{
    if (visitorLogViewModel == null)
        throw new  ArgumentNullException(nameof(visitorLog));

    if (HasVisistedToday(visitorLogViewModel)
        return false;

    AddItem(Mapper.Map<VisitorLog>(visitorLogViewModel));

    return true;
}

Where HasVisitedToday() is your actual code with a given name to quickly understand what it does (and probably to match an entry in your specifications).

Note the use of nameof() to avoid string constants.

Side note: if Add(AddVisitLogViewModel) isn't invoked outside your assembly (its container class is internal) then you might want to replace parameters validation with assertions:

Debug.Assert(visitorLogViewModel != null);


Implementing a custom iterator by inheriting from an existing iterator class and overriding operator* My goal is to write very little code and I want a custom iterator that only changes the behavior of the dereference operator* but copies the rest of the behavior from the underlying container iterator.

The following code works but is it correct?

#include <iostream>
#include <unordered_map>

using namespace std;

template <typename K, typename V>
class KeyValueStore
{
    using base_iterator = typename unordered_map<K, V>::iterator;

public:
    struct iterator : public base_iterator
    {
        iterator(base_iterator it) : base_iterator(it)
        { }

        // The custom behavior for the dereference operator.
        V& operator*()
        {
            return base_iterator::operator*().second;
        }
    };

    V& get(const K& key)
    {
        return store.at(key);
    }

    void put(K key, V value)
    {
        store[key] = value;
    }

    iterator begin() { return iterator {store.begin()}; }

    iterator end() { return iterator {store.end()}; }
private:
    unordered_map<K, V> store;
};

int main()
{
    KeyValueStore<string, int> store;
    store.put("aa", 5);
    store.put("vf", 4);

    for (auto item : store)
        cout << item << endl;

    return 0;
}

Use of public inheritance in this case strikes me as problematic.

Public inheritance means that a pointer/reference to an object of the derived class can be converted implicitly to a pointer/reference to the base class. Worse, it also means that an object of the derived class can be implicitly converted ("sliced") to an instance of the base class.

If that happens, you quickly run into a problematic situation: since operator * isn't defined as virtual in the base class, invoking it via a pointer/reference to base will invoke the base version of the operator. Your operator overload isn't really overriding the base class'--it's just hiding it.

This isn't saving a whole lot of work either--you're using an unordered associative container, which only provides forward iterators, so all your inheritance is saving is implementing operator++.

I suppose to be fair, I should add that people rarely pass iterators by reference, so it's not particularly likely that you'll trigger the problems outlined above. On the other hand, history teaches us that idiots can be surprising crafty in figuring out ways to shoot themselves in the feet.


Implementing cache for server I've just implemented a cache for my server, and I'd like your thoughts about performance improvements and maybe some possible bug fixes:

public class Cache<CacheKey, CacheValue>
{
    private Dictionary<CacheKey, KeyValuePair<CacheValue, DateTime>> _map;

    /// <summary>
    /// The first element is the least recently used.
    /// </summary>
    private List<CacheKey> _lru;

    private int _maxSize;

    private ReaderWriterLock _lock;

    private int _minutes;

    public Cache(int maxNumberOfRecords, int minutes)
    {
        _maxSize = Math.Max(maxNumberOfRecords, 1);
        _lru = new List<CacheKey>(_maxSize + 1);
        _map = new Dictionary<CacheKey, KeyValuePair<CacheValue, DateTime>>();
        _lock = new ReaderWriterLock();
        _minutes = minutes;
    }

    public CacheValue find(CacheKey key)
    {
        _lock.AcquireReaderLock(-1);

        CacheValue ans;

        if (_map.ContainsKey(key) && (DateTime.Now - _map[key].Value).Minutes < _minutes)
        {
            ThreadPool.QueueUserWorkItem(o =>            //don't wait for lru to update
            {
                _lock.AcquireWriterLock(-1);
                _lru.Remove(key);
                _lru.Add(key);
                _lock.ReleaseWriterLock();
            });

            ans = _map[key].Key;
        }
        else
        {
            ans = default(CacheValue);
        }

        _lock.ReleaseReaderLock();
        return ans;
    }

    public void insert(CacheKey key, CacheValue value)
    {
        _lock.AcquireWriterLock(-1);

        if (_map.ContainsKey(key))      //if exists
        {
            _map.Remove(key);
            _map.Add(key, new KeyValuePair<CacheValue, DateTime>(value, DateTime.Now));       //update value
        }
        else
        {                               //if not
            _lru.Add(key);
            _map.Add(key, new KeyValuePair<CacheValue, DateTime>(value, DateTime.Now));       //add to cache

            if (_lru.Count > _maxSize)
            {
                _map.Remove(_lru[0]);   //and delete least recently used

                _lru.RemoveAt(0);
            }
        }

        _lock.ReleaseWriterLock();
    }
}

I've used ThreadPool because the List operations are expensive, but I think that this hurts the LRU integrity.  I'm also thinking to implement a queue with \$O(1)\$ Remove to replace the List class.

If you are using .NET 4 or above drop your code, and use MemoryCache.


ES6 class that generates an area where you can draw on a page I have the following javascript es6 class (along with some functions) that generates an area where you can draw in a page.

class Painter {
    constructor(selector, cols, rows, style) {
        this.looper = null;
        if (!selector)
            throw new TypeError("Provide a valid table selector");
        this.cols = cols || 20;
        this.rows = rows || 20;
        this.selector = selector;
        this.style = style;
        let markup = "";
        for (let height = 0; height < this.rows; height++) {
            markup += "
";
            for (let width = 0; width < this.cols; width++)
                markup += "	";
            markup += "
";
        }
        $(function() {
            $(selector).html(markup);
            $(selector).on("click", "td", function() {
                $(this).toggleClass("tile-active");
            });
        });
    }
    setStyleSheet(stylesheet) {
        this.style = stylesheet;
    }
    changeTileColor(stylesheet, newColor) {
        if (!newColor)
            return;
        var css = ".tile-active{\n\tbackground:" + newColor + ";\n}";
        try {
            changeCss((stylesheet || this.style), css);
        } catch (e) {
            console.log("Suppressed error");
        }
    }

    createChessBoard() {
        var id = this.selector;
        $(function() {
            $(id + " tr:nth-child(even) td:nth-child(even)").addClass("black-tile");
            $(id + " tr:nth-child(odd) td:nth-child(odd)").addClass("grey-tile");
        });
    }
    toggleBorder() {
        var bname = $(this.selector).css("border");
        $(this.selector).css("border", bname.indexOf("none") != -1 ? "2px solid" : "none");
    }
    clearBoard() {
        $(this.selector + " .tile-active").removeClass("tile-active");
    }
    createRandowDraw() {
        this.clearBoard();
        let mat = $(this.selector + " td");
        let max = mat.length;
        for (let i = 0; i < max; i++) {
            let num = Math.floor(Math.random(2) * 2);
            if (num && num <= 1)
                $(mat[i]).addClass("tile-active");
        }
    }
}

As I am new to classes in ECMAScript 6 I was wondering if there is anything I can do to improve the performance/readability of this code.

Here is a link to the following code:
Painter

The first weird thing I saw in your code is the use of the $(fun).

This jQuery function is used to execute code when the DOM is ready.

I think it should be better to remove all this calls and instead instantiate the class itself in a function executed after the DOM is ready.

That helps to follow the code properly.

// here you declare the class
class MyClassThatUseDOM {
    constructor(selector) {
        this.myElement = $(selector);
    }
}

$(function(){

    // here you instantiate the class
    // when the DOM is ready and all selector
    // will properly match.
    const myObj = new MyClassThatUseDOM("#theselector");

});

As the element $(selector) or $(this.selector) are used may times it is best to save in an instance variable:

this.element = $(selector);

And then access always from this variable, instead of continue to search it with jQuery.

The following method have some issue for me:

createChessBoard() {
    var id = this.selector;
    $(function() {
        $(id + " tr:nth-child(even) td:nth-child(even)").addClass("black-tile");
        $(id + " tr:nth-child(odd) td:nth-child(odd)").addClass("grey-tile");
    });
}

Maybe if you remove the $(func) part as I suggested in my first point the code will be more clear.

But let's discuss what we have now.

You use a variable id to save the selector, but id is more specific term than selector. It could confuse some developer.

A selector for jQuery is anything is a valid css selector, like a tag name, a css class name or even and id.

Please pay attention to the names you use for your variables.

In this case you can do something like:

let _selector = this.selector

As you can see you understand that you just assign a private variable, here.

Use of var is not wrong, but we have let.

Similar considerations for:

var bname = $(this.selector).css("border");

About the method:

createRandowDraw() {
    this.clearBoard();
    let mat = $(this.selector + " td");
    let max = mat.length;
    for (let i = 0; i < max; i++) {
        let num = Math.floor(Math.random(2) * 2);
        if (num && num <= 1)
            $(mat[i]).addClass("tile-active");
    }
}

Here you could use jQuery API to simplify your code:

createRandowDraw() {
    this.clearBoard();
    $(this.selector + " td").each(function(i, mat){
        let num = Math.floor(Math.random(2) * 2);
        if (num && num <= 1) {
           $(mat).addClass("tile-active");
        }
    });
}

Or in a plain javascript fashion by using filter and forEach.


Function for removing forbidden characters void removeForbiddenChar(string* s)
{
    string::iterator it;

    for (it = s->begin() ; it < s->end() ; ++it){
        switch(*it){
        case '/':case '\\':case ':':case '?':case '"':case '<':case '>':case '|':
            *it = ' ';
        }
    }
}

I used this function to remove a string that has any of the following character: \, /, :, ?, ", <, >, |. This is for a file's name. This program runs fine. It simply change a character of the string to a blank when the respective character is the forbidden character. However, I have a feeling against this use of switch statement. I simply exploit the case syntax here, but this, somehow nags me. I just don't like it. Anybody else got a better suggestion of a better implementation in this case?

Declare a string containing the illegal characters: "\\/:?"<>|". All you need to do is check if the char is in the array, so use a native function for that, or write a method CharInString(char* needle, string* haystack) which loops through the contents of the provided haystack to check if the needle is inside it.

Your loop should end up looking like this:

string illegalChars = "\\/:?\"<>|"
for (it = s->begin() ; it < s->end() ; ++it){
    bool found = illegalChars.find(*it) != string::npos;
    if(found){
        *it = ' ';
    }
}

It's more maintainable and readable. You can tell if you've duplicated a character quite easily and since you can do it with any target string and any string of illegalChars you've just created for yourself a generic RemoveIllegalChars(string* targetString, string* illegalChars) method usable anywhere in your program.

I may be using those pointers wrong. My C++fu is weak... for now.


Locate substrings within a file Here, I am reading in a newline delimited file that can be millions of repeating lines that looks like this:

TGATAGTTAGTCATATGAAAGCATCATTAGTAAACCACATTGCTTATTATATTGAACAGT
TACATCTGGCTTATTATACAAAGAGAAAACCATACTATTCATACTATTCTCTTTTTGATC
TTCTCAATCTTCTGTTGTTAGATATTCTATTCCTTGCTCACCATATATACTACTTATGTC
etc.

The goal is to count the number substring instances within this file, including those substrings that span two lines.

The challenge I have is that the input files can be large, so speed is very desirable. So, I am looking for any good suggestions to improve speed here.

My header:

package main

import ("fmt";"os";"bufio";"compress/gzip";"strings";"time")

This function reads the input file into memory as an array. This takes about 5s on my machine. The input file is newline delimited, so this function iterates through each line of the file and adds each line as a value in the genome array.

func input_file(loc string) []string {
    var genome []string

    // my file is zipped because it can be pretty large
    file, err := os.Open(loc)
    f, err := gzip.NewReader(file)
    check(err)

    // a scanner is a convenient interface to read newline-delimited files
    scanner := bufio.NewScanner(f)
    for scanner.Scan() {
        genome = append(genome, scanner.Text())
    }

    if err := scanner.Err(); err != nil {
        fmt.Fprintln(os.Stderr, "reading standard input:", err)
    }

    f.Close()
    return genome
}

The above function uses this check for read errors.

func check(e error) {
    if e != nil {
        panic(e)
    }
}

Here I iterate through the array that contains all of the strings read in from the file. This takes about 4s on my machine. It is a bit unfortunate that I am iterating through the file to read it, then iterating through the data a second time to search for substrings. The reason I did this is so that I could reuse the file reading code so, I didn't want it to do anything other than read in the data. I'm sure there is a better way of handling this though.

func search (genome []string, kmers map[string]int64) map[string]int64 {
    left := ""
    right := ""
    full_seq := ""

    // iterate through data pulled from file
    for _, seq := range genome {

        /* maybe not really necessary to understand why i'm doing this but
        the input file is newline delimited and so i am merging lines
        together in case a substring overlaps a newline. */
        left = right
        right = seq
        full_seq = left + right

        kmers = contain_kmer(full_seq, kmers) // check for substring
    }
    return kmers
}

This function is called by the search function and looks for all substrings and if they are found, are counted.

func contain_kmer (seq string, kmers map[string]int64) map[string]int64 {
    for k := range kmers {
        if strings.Contains(seq, k) {
            kmers[k] += 1 // if found, add to total count
        }
    }
    return kmers
}

Here is the main function which points to the input file location, reads the file into memory using the input_file function and then searches for substrings using the search function. On my machine, the code takes about 7s to run.

func main() {
    start := time.Now()
    loc := "./myFile.gz" // input file location

    genome := input_file(loc) // read file into memory

    // counts of substrings to be located within input file
    kmers := map[string]int64 {
        "TATTC":0,
        "GTAAACCACATTGCTTATTA":0,
    }

    // locate the substrings
    kmers = search(genome, kmers)
    end := time.Now()

    fmt.Println(kmers) // display counts
    fmt.Println(end.Sub(start))
}

You're reading the file into memory inefficiently. I can't write the full code currently, but here's the gist:

Instead, use the ioutil package: func ReadFile(filename string) ([]byte, error).

Once you've read the file into a byte array, you can remove extraneous characters (newlines, invalid characters, etc). One way is to use bytes.Replace with n of -1.

Then, you can search the byte array using the bytes package: func Count(s, sep []byte) int.

Also, if you want to correctly time a function, use the testing package.


Loading a combobox with an enum and binding to it I have the following code to load an enumeration into a bound combo box, but am not quite happy with it.

I would prefer to store the actual enum value in the combo box and bind directly to it.
However, I can't quite figure out how to do so.

 public enum HemEnum
    {
        HemNone = -1,
        Hemsew = 0,
        HemWeld = 1,
        Hemdoublefold = 2
    }

 public static void LoadHemCombo(ComboBox cbo)
    {
    var values = from Enum e in Enum.GetValues(typeof(HemEnum))
                     select new { ID = e, Name = e.ToString() };

          foreach (var value in values)
    {
        var s = GetHemTypeDescription((HemEnum)value.ID );
        cbo.Items.Add(s);
    }
    }

  public static string GetHemTypeDescription(HemEnum hemType)
    {
        string s = null;
        switch (hemType)
        {
            case HemEnum.HemNone:
                s = "none";
                break;
            case HemEnum.Hemsew:
                s = "sewn";
                break;
            case HemEnum.HemWeld:
                s = "welded";
                break;
            case HemEnum.hemdoublefold:
                s = "double folded";
                break;
            default:
                s = "not known";
                break;
        }
        return s;
    }

Inside the form load event I load the combo and bind to it.

 LoadHemCombo(cboHem)
 cboHem.DataBindings.Add("Text", myBindingSource, "HemTypeDescription");

 class myObject
 {
    public string HemTypeDescription
    {
        get
        {
            return GetHemTypeDescription(this.HemType);

        }
        set
        {
            this.HemType = GetHemTypeFromDescription(value);
        }
  }

public static vHemEnum GetHemTypeFromDescription(string description)
    {
        int r =0;
        for (var i = (int)HemEnum.HemNone; i <= (int)HemEnum.Hemdoublefold; i++)
        {
            var s = GetHemTypeDescription((HemEnum)i);
            if (description == s)
            {
                r  = i;
                break;
            }
        }
        return (HemEnum)r;
    }
 }

in the designer I set

myBindingSource.DataSource = myObject

and before loading the form I create an instance of myObject and add it to myBindingSource using

myBindingSource.Add(myObject)

Naming
HemEnum, or any enum name that ends with the word Enum, is a bad name for an enum. Enum types should not contain the word "enum" in their names.
Similarly, HemEnum values should not contain "Hem" in their names either. Your HemEnum should therefore read something like this:
public enum HemType
{
    None = -1,
    Sewn = 0,
    Weld = 1,
    DoubleFold = 2
}

myObject is a bad name for a class - should be MyObject. Kidding (although not really - types should be named following a PascalCasing convention). Anything that ends with "Object" should be banned from being a class name. Note that myObject doesn't compile as provided. (where's this.HemType?)
The pseudo-Hungarian notation is ok in winforms for naming controls (i.e. the "cbo" prefix for ComboBoxes, "txt" for TextBoxes, etc.) - had you been using current technology (WPF) I would have strongly advised against such naming though.
Nitpicks
You seem to make static anything that can be made static. Don't. Just because a method doesn't use instance members now doesn't mean it never will, and changing a public static void method to be public void, is a breaking change.
vHemEnum doesn't seem to be a type defined anywhere. Typo?
I'll assume the out-of-whack indentation is a Copy+Paste glitch.
Binding?
You say you want databinding, and yet you're doing this:
foreach (var value in values)
{
    var s = GetHemTypeDescription((HemEnum)value.ID );
    cbo.Items.Add(s);
}

Create a type that exposes DisplayValue and EnumValue properties, and use @DanLyons' or @JesseCSlicer's answer to bind your combobox items.
Captions
A more extensible way (vs. attributes) of providing captions for your enums, would be to use a resource file (.resx) - call the strings per the enum's names, and then retrieve the caption from the resource strings:
var description = resx.ResourceManager.GetString(hemType.ToString());

Bottom line, I've seen worse WinForms code (and WPF for that matter), but it could be improved.


Prefix search by trie tree I'm working on the prefix search problem:

  Given a set of words, for example words = ['a', 'apple', 'angle', 'angel', 'bat', 'bats'], for any given prefix, find all matched words. For example, if input is ang, return 'angle', 'angel', if no match, return an empty list [].

Any advice on performance improvement in terms of algorithm time complexity (not sure if Trie Tree is the best solution), code bugs or general advice on code style is highly appreciated.

from collections import defaultdict
class TrieNode:
    def __init__(self):
        self.children = defaultdict(TrieNode)
        self.isEnd = False
    def insert(self, word):
        node = self
        for w in word:
            node = node.children[w]
        node.isEnd = True
    def search(self, word):
        node = self
        for w in word:
            if w in node.children:
                node = node.children[w]
            else:
                return []
        # prefix match
        # traverse currnt node to all leaf nodes
        result = []
        self.traverse(node, list(word), result)
        return [''.join(r) for r in result]

    def traverse(self, root, prefix, result):
        if root.isEnd:
            result.append(prefix[:])
        for c,n in root.children.items():
            prefix.append(c)
            self.traverse(n, prefix, result)
            prefix.pop(-1)
if __name__ == "__main__":
    words = ['a', 'apple', 'angle', 'angel', 'bat', 'bats']
    root = TrieNode()
    for w in words:
        root.insert(w)
    print root.search('a') # 'a', 'apple', 'angle', 'angel'
    print root.search('ang') # 'angle', 'angel'
    print root.search('angl') # 'angle'
    print root.search('z') # []

What comes to the data structure, I don't think it gets any more efficient, not at least as long as you confine yourself to search by prefix. For that reason, I will "review" your actual code and not the algorithm.

self.isEnd = False

I believe more canonic name would be self.is_leaf. Note that isEnd is not idiomatic Python, is_end is.

You should have two empty lines before any class declaration.

import ...

class FooBar:
    pass

Also, in

for c,n in ...

PEP 8 requires a space after (each) comma:

for c, n in ...
      ^
    space!

What comes to renaming self:

def insert(self, word):
    node = self
    for w in word:
        node = node.children[w]
    node.isEnd = True

I would not do it, yet I have to admit that it is just an opinion.


Text message "reverser" I decided to step out of my comfort zone (Java) and try out Python. I wrote a very simple reverser.py and want it to be reviewed.

"""
This python script basically takes a string and reverses it.

This python script can be used as a command line script as well as a separate
module.
"""
import sys

def reverse(message):
    """
    This function takes a string and reverses it, so as to somewhat obfuscate
     it.

    :param message: The unencrypted text message to be reversed.
    """
    text = ''
    for character in message:
        text = character + text
    return text

if __name__ == '__main__':
    numberOfArgs = len(sys.argv)

    message = ''
    if numberOfArgs == 1:  # i.e. the program name only
        message = input('Please enter the text to be encrypted : ')
    elif numberOfArgs == 2:
        message = sys.argv[1]
    else:
        print('Incorrect number of arguments : %d\n' % numberOfArgs)
        print('Usage: python3 reverseCipher [unencrypted string]')
        exit(2)  # '2' stands for command-line syntax errors in unix

    print(reverse(message))

Examples in the terminal:

$ python3 reverser.py

Please enter the text to be encrypted : I understand that text manipulation in python is quite easy.
.ysae etiuq si nohtyp ni noitalupinam txet taht dnatsrednu I

$ python3 reverser.py "I am just starting out with python"

nohtyp htiw tuo gnitrats tsuj ma I

It would be great if I would be advised about:

Code style and idioms
The required mindset (for I have one for Java)
Anything that can help me learn the language

This is going to be a short answer, but did you know Python has such a feature in-built?

text = "I'm a stringy!"
print(text[::-1])

Above would replace your reverse method and is the most Pythonic way to solve this.

This is called extended slicing and has been added since Python 2.3. Slicing is a very easy to use method when handling array-like data.

The following illustrates how slicing and extended slicing can be used, executed in the interactive Python IDE:

In [1]: text = "I'm a stringy!"

In [2]: print(text)
I'm a stringy!

In [3]: print(text[:-1])
I'm a stringy

In [4]: print(text[::-1])
!ygnirts a m'I

In [5]: print(text[:4:-1])

In [6]: print((text:4:-1] + text[:3:5])[::-1])
I stringy!

In [7]: print(text[::-1][::-1][::-1])
!ygnirts a m'I

As picture.


SQL query to obtain totals and subtotals The query works fine but I'm just intrigued to know if there is a cleaner solution. The data are results from a survey and I'm obtaining the counts per question within a zone and an overall zone count.

With Cte1 (ZoneId, QuestionId, Count1)
AS
(
    Select
        tz.ZoneId,
        qn.QuestionId,
        Count(1) as Count1
    From
        Customers cust
        Inner Join Shops td on td.ShopCode = cust.ShopCode
        Inner Join Zones tz on tz.Id = td.SlsZone
        Inner Join Responses res on res.SampleId = cust.SampleId
        Inner Join QstNodes qn on qn.QuestionId = res.QstNodeId
    Where
        Event >= 201201 And Event <= 201212
    Group BY
        tz.ZoneId, qn.QuestionId
)
SELECT
    ZoneId,
    QuestionId
    Count1,
    (
        SELECT SUM(Count1)
        FROm Cte1 as innercte
        Where innercte.ZoneId = outercte.ZoneId
    )
    as ZoneTotal
FROM
    Cte1 outercte

how about using CASE

SELECT  tz.ZoneId,
        qn.QuestionId,
        SUM(CASE WHEN Event >= 201201 AND Event <= 201212
                THEN 1
                ELSE 0
            END) as Count1,
        COUNT(*) AS ZoneTotal
FROM    Customers cust
        INNER JOIN Shops td
            ON td.ShopCode = cust.ShopCode
        INNER JOIN Zones tz
            ON tz.Id = td.SlsZone
        INNER JOIN Responses res
            ON res.SampleId = cust.SampleId
        INNER JOIN QstNodes qn
            ON qn.QuestionId = res.QstNodeId
GROUP   BY tz.ZoneId, qn.QuestionId

UPDATE 1

The following illustrated on the demo has different records but that thought is the same.

SQLFiddle Demo Link


SQL GROUPING SETS in Python using Pandas The code below is intended to provide SQL's GROUPING SETS functionality in Python with the aid of Pandas.

Background on SQL GROUPING SETS

There are at least two advantages to doing this in Python:

it is less verbose than SQL, and
it provides a non-redundant set of aggregations without the need to manually specify them.

There is at least one disadvantage to doing this in Python:

memory usage is quite likely higher than would be in SQL.

I am posting this question in the hopes that the code can be cleaned up. Secondarily I am posting to determine if anyone would find this useful. The example below has been tested with Python 2.7.6 and Pandas 0.15.1.

from __future__ import division, print_function

import itertools as it
import pandas as pd

from pandas.util.testing import assert_frame_equal

def powerset(iterable):
    "powerset([1,2,3]) --> () (1,) (2,) (3,) (1,2) (1,3) (2,3) (1,2,3)"
    s = list(iterable)
    return it.chain.from_iterable(it.combinations(s,r) for r in range(len(s)+1))

def grouper(df,grpby=None,aggfunc=None):
    ''' produces aggregate DataFrame from DataFrames for non-redundant groupings
       `workingdf` is used to avoid modifying original DataFrame
    '''
    uniqcols = (i for i in grpby if len(df[i].unique()) == 1)
    pwrset = (i for i in powerset(grpby))
    s = set()
    for uniqcol in uniqcols:
        for i in pwrset:
            if uniqcol in i:
                s.add(i) # add a level of aggregation only when non-redundant

    workingdf = df.copy()
    #for idx,i in enumerate(powerset(grpby)):
    for idx,i in enumerate(s):
        print('  grouping by: {}'.format(i))
        if i != ():
            tmp = aggfunc( workingdf.groupby(i) )
        else:
            # hack to get output to be a DataFrameGroupBy object:
            #   insert dummy column on which to group by
            # old, naive code:
            #   tmp = aggfunc( workingdf )
            dummycolname = hash(tuple(workingdf.columns.tolist()))
            workingdf[dummycolname] = ''
            tmp = aggfunc( workingdf.groupby(dummycolname) )

        # drop the index and add it back
        if i == (): tmp.reset_index(drop=True,inplace=True)
        else: tmp.reset_index(inplace=True)

        for j in grpby:
            if j not in tmp: # if column is not in DataFrame add it
                tmp[j] = '(All)'

        # new list with all columns including aggregate ones; do this only once
        if idx == 0:
            finalcols = grpby[:]
            addlcols = [k for k in tmp if k not in grpby] # aggregate columns
            finalcols.extend(addlcols)

        # reorder columns
        tmp = tmp[finalcols]

        # creation of final DataFrame
        if idx == 0:
            final = tmp; del tmp
        else:
            final = pd.concat( [final,tmp] ); del tmp

    del workingdf

    final = final.sort(columns=finalcols)
    final.reset_index(drop=True,inplace=True)

    return final

def agg(grpbyobj):
    ''' the purpose of this function is to:
          specify aggregate operation(s) you wish to perform,
          name the resulting column(s) in the final DataFrame.
    '''
    tmp = pd.DataFrame()
    tmp['Total (n)'] = grpbyobj['Total'].sum()
    return tmp

if __name__ == '__main__':
    df = pd.DataFrame({'Area':['a','a','b',],
                       'Year':[2014,2014,2014,],
                       'Month':[1,2,3,],
                       'Total':[4,5,6,],})
    final = grouper(df,grpby=['Area','Year'],aggfunc=agg)
    print(final)

    # test against expected result
    expected = '''{"Area":{"0":"(All)","1":"a","2":"b"},
                   "Year":{"0":2014,"1":2014,"2":2014},
                   "Total (n)":{"0":15,"1":9,"2":6}}'''
    testfinal = pd.read_json(expected)
    testfinal = testfinal[final.columns.tolist()] # reorder columns
    try:
        # check_names kwarg True: compare indexes and columns
        assert_frame_equal(final,testfinal,check_names=True)
    except AssertionError as e:
        print(e)

About this part:

def grouper(df,grpby=None,aggfunc=None):
    uniqcols = (i for i in grpby if len(df[i].unique()) == 1)
    pwrset = (i for i in powerset(grpby))
    s = set()
    for uniqcol in uniqcols:
        for i in pwrset:
            if uniqcol in i:
                s.add(i) # add a level of aggregation only when non-redundant

The two optional arguments are in fact required by the function.
The convention is that i is a loop variable of type int.
Bug: pwrset is a generator expression. As such it can only be iterated through once, just like the iterator returned from powerset(grpby). Using square brackets [] instead of () would turn it into a list comprehension and fix the bug.
Changing the other generator expression into a list comprehension would allow you to swap the order of the nested for loops. This would be more natural since the nested loop is filtering pwrset, and more efficient because you could break out of the inner loop after first s.add(i). Then s does not even need to be a set (because the rest of the code merely iterates over s). If you make uniqcols a set instead, the inner loop could be avoided altogether.

Revised code:

def grouper(df, grpby, aggfunc):
    uniqcols = set(col for col in grpby if len(df[col].unique()) == 1)
    s = [cols for cols in powerset(grpby) if not uniqcols.isdisjoint(cols)]


Threading and queue experiment I'm just beginning to get the idea of queue and threading.

I would welcome input on this exercise script:

from threading import Thread as T
from Queue import Queue as Q
from random import randint as R
import time

'''
Silly script exploring simple use of threadding.Thread and Queue.Queue
'''

q = Q()

now = time.time()

def one_up(queue):
        while time.time() - now <= 20:
                other_num = queue.get()
                big_num = 5*other_num
                print("       {}!?!?".format(other_num))
                queue.put(big_num)
                print("Oh yea, well I put {} in the queue!".format(big_num))
                time.sleep(1)
        print("{} is a BIG number, man. Wow. Look. We made a pyramid.".format(big_num))

a_num = R(1,10)
q.put(a_num)
print("I put {} in the queue.".format(a_num))
t = T(target=one_up, args=(q,))
t.start()

Which outputs something along the lines of:

    I put 5 in the queue.
       5!?!?
Oh yea, well I put 25 in the queue!
       25!?!?
Oh yea, well I put 125 in the queue!
       125!?!?
Oh yea, well I put 625 in the queue!
       625!?!?
Oh yea, well I put 3125 in the queue!
       3125!?!?
Oh yea, well I put 15625 in the queue!
       15625!?!?
Oh yea, well I put 78125 in the queue!
       78125!?!?
Oh yea, well I put 390625 in the queue!
       390625!?!?
Oh yea, well I put 1953125 in the queue!
       1953125!?!?
Oh yea, well I put 9765625 in the queue!
       9765625!?!?
Oh yea, well I put 48828125 in the queue!
       48828125!?!?
Oh yea, well I put 244140625 in the queue!
       244140625!?!?
Oh yea, well I put 1220703125 in the queue!
       1220703125!?!?
Oh yea, well I put 6103515625 in the queue!
       6103515625!?!?
Oh yea, well I put 30517578125 in the queue!
       30517578125!?!?
Oh yea, well I put 152587890625 in the queue!
       152587890625!?!?
Oh yea, well I put 762939453125 in the queue!
       762939453125!?!?
Oh yea, well I put 3814697265625 in the queue!
       3814697265625!?!?
Oh yea, well I put 19073486328125 in the queue!
       19073486328125!?!?
Oh yea, well I put 95367431640625 in the queue!
       95367431640625!?!?
Oh yea, well I put 476837158203125 in the queue!
476837158203125 is a BIG number, man. Wow. Look. We made a pyramid.

Single-letter variable names are usually a bad practice, with few exceptions (loop counters, caught exception objects). Importing modules with single-letter names is very silly indeed. So don't do this:

from threading import Thread as T
from Queue import Queue as Q
from random import randint as R

Just stick to good old-fashioned:

from threading import Thread
from Queue import Queue
from random import randint

This is not a very interesting experiment with threading and a Queue. Check out the documentation, it's quite basic but much more interesting, and really using threading and a Queue.


Countdown program in x86 NASM I am fairly new to Assembly language programming and, for practice, I gave myself a problem: count down from 10 and right after 1, say "Blast off!".

section .text
    global _start
_start:
    mov eax, '10' ; asci 10
    mov [num], eax ; num now equals ascii 10

    mov edx, 2
    mov ecx, num
    mov ebx, 1
    mov eax, 4
    int 80h ; prints out '10' in the console

    mov eax, 9 ; numerical 9
    call subtract
subtract:
    call log ; logs the current number

    sub eax, 1 ; subract 1

    cmp eax, 0
    jne subtract ; if it doesn't equal 0, jump to subtract
    je blast_off ; if it does equal 0, jump to blast off
log:
    call to_ascii ; converts eax's value to ascii
    mov [num], eax ; num = eax
    mov edx, 1
    mov ecx, num
    mov ebx, 1
    mov eax, 4
    int 80h ; output the current number
    call to_number ; converts eax back to numerical
    ret
to_number:
    mov eax, [num] ; is this necessary? Moves num back into eax
    sub eax, '0' ; converts to numerical
    ret
to_ascii:
    add eax, '0' ; converts to ascii
    ret
blast_off:
    mov edx, len
    mov ecx, msg
    mov ebx, 1
    mov eax, 4
    int 80h ; outputs "Blast off!"

    mov eax, 1
    int 80h ; exits program
section .data
    msg db "Blast off!", 0xA ; message
    len equ $-msg ; length of message
section .bss
    num resb 1 ; this is for outputting the current number

Output:

  1098654321Blast off!

Assembled: nasm -f elf blast_off.s

Linked: ld -m elf_i386 -s -o blast_off blast_off.o

My main concerns are:

My code seems functional - is this bad, or does it not really matter, being assembly language?
Is there repetition? This was a very difficult challenge for me, and at times I lost my head - sometime during those times I might have typed something completely "off topic" in regards to the subject of the code
Aside from the first concern, does the organization need re-organizing? As in, should I reorder the different sections(.test, .data, .bss)? Or, does it not make a difference?

Section sequence

That's rather a matter of taste. Programmers used to 68k usually use the sequence .text, .data, .bss, programmers used to Java usually use the sequence .data, .bss, .text. In the assemblers known to me it doesn't make any difference, so it's rather a matter of taste or corporate policy.

Actually, strictly speaking it's not the assembler but the linker who might have a word to say about the sequence, and the linkers that I know don't mind the sequence (even the Keil Linker for 8051, L51, which is really weird stuff).

Functional code

Breaking up bigger chunks of code into smaller parts very often makes sense because very often it increases maintainability.

Basically in Assembler you do a lot of trade-off between maintainability, speed and size.

Your functions are not universally reusable, for example your function to_ascii performs no sanity checks.
In that case, it doesn't always make sense, especially when the function body is just on instruction.

However, as far as I'm aware, on a modern member of the 80x86 family, instructions like call and ret basically come free of cost, thanks to the pipeline and branch prediction, so maybe there is no loss of speed, just of space.

Redundancy

The sequence

mov ecx, num
mov ebx, 1
mov eax, 4
int 80h ; output the current number

is redundant.
I don't think it's entirely bad.
Compared to having a single-instruction function to_ascii it looks, however, inconsistent.

I'd extract this into something like a print_number function.

Exit status

(You fixed this meanwhile.)

Your exit code looks like this:

mov ebx, 1
mov eax, 4
int 80h ; outputs "Blast off!"

mov eax, 1
int 80h ; exits program

The contents of register ebx are still 1 when you call exit.
This is probably wrong.
I do not see any indication why you would want to indicate that your program exits unsuccessfully.
You probably want to insert a mov ebx, 0 like this:

mov ebx, 1
mov eax, 4
int 80h ; outputs "Blast off!"

mov ebx, 0
mov eax, 1
int 80h ; exits program

Asymmetry in to_number vs. to_ascii

to_number accesses [num] itself, whereas in the case of to_ascii, [num] is accessed by the caller.
This is asymmetric, a form of inconsistent.
In bigger programs this might lead to confusion with respect to how APIs work.

Name log

Nowadays when you say log people think of generic logging along the lines of Logging APIs like log4j or sclog4c.

I'd rather call this function something else, in order to prevent confusion.

Formatting

I'm not sure whether this is a convention, but I'd do this:

blank line before every new function (i.e. after a ret that is not followed by code which could be reached by a conditional jmp before the ret).
Use local labels for loop and branch labels inside a function.
Write a small function header on top of each function which describes input, output and access to globals (especially side-effects).

Function header

Here's a sample function header for to_ascii

; Returns the ASCII value of the input digit.
; @param eax digit (value in the range of [0..9]) which is converted into an ASCII digit.
; @return eax ASCII value (in the range of ['0'..'9']).
; @warning If eax is out of the supported range, the behavior is undefined.
to_ascii:
    add eax, '0'
    ret

Local labels vs Global labels

AFAIK this is also true for NASM:

Local labels start with '.' and are valid until the next global label.
Global labels do not start with '.'.

Looking at your code, subtract should be a local label and thus renamed to .subtract.

This is not mandatory, and there is a valid interpretation of your code which includes tail recursion and last call optimization in which your code and label structure are valid.

It's just a bit surprising (maybe even positively) to see such constructs in assembler.

Conditional jumps

At the end of subtract you have two conditional jumps:

jne subtract
je blast_off

While this code is correct, I think the following would be more clear:

je blast_off
jmp subtract

Or in case you want to optimize for speed

jne subtract
jmp blast_off

The point is maintainability.
I found it less error-prone to have the last conditional jmp, which semantically is identical to an unconditional jmp as unconditional jmp indeed.
When you change the conditions, there is less necessity of changing the last jmp, and therefore less risk of getting code which would just go haywire for running unintentionally into the next function, in case the current function doesn't end with ret itself.

Structured Programming regarding _start / subtract?

I personally would almost always follow structured programming even in Assembly language.

At _start / subtract, this is not done because you call subtract.
Maybe that call even is unintentional, as I cannot see where subtract would ever return.

Independently of that, a construct like

    call foo
foo:
    more code

looks confusing, at least to me.
First, foo is called, and everything that is called is expected to return, and then when foo returned, we would run it once more.

I guess in this case, best is to simply remove the call subtract statement.

Structured Programming regarding _start / blast_off?

A similar problem is there with _start and blast_off.
blast_off is not called, so it is not a function - or to be seen as last-call optimization.
I'd actually avoid putting the good-case exit anywhere outside start.

In general, I'd avoid jumping over functions.
I feel jumps should be more local than calls, not the other way round.
The reason is that jumps are where we program conditions and loops, which is transfer of control inside a function, whereas calls is where we call subroutines, which is the transfer of control to another function.
I find it confusing if the transfer of control inside a function is "bigger" in distance than the transfer of control to another function.

Output

The output looks like this:

10987654321Blast off!

You might want to put space in between.

Makefile

You might want to use a Makefile for this.

I wrote the following Makefile for building and running this myself:

AS:=nasm
ASFLAGS:=-f elf
LINK.o:=ld
LOADLIBES:=-m elf_i386 -s

.PHONY: all
all: blast_off

blast_off: blast_off.o

.PHONY: run
run: blast_off
        ./$^

Disclaimer

I am new to NASM / x86 / x64 assembler myself.
I'm very experienced in 680x0, ARM and somewhat experienced in 6502, Z80, 8051, 80251 and CalmRISC but, as said, not (yet) in x86 / x64.


Convert an in-ordered and pre-ordered array to a BST I am working on a Daily Coding Challenge to print the BST if given an in-ordered and pre-ordered array.

Below is my code in JavaScript ES6.

Please can you review it. This is my first fully-featured JS program so I am very unaware of standard syntax or shortcuts in JS

function Tree(){
    this.value = null
    this.right = null
    this.left = null
    this.reconstruct = (pre_order, in_order)=>{
        if (pre_order){
            if (this.value == null){
                this.value = pre_order[0]
            }
            let node = pre_order[0]
            let split_index = in_order.indexOf(node)
            let left_tree = in_order.slice(0, split_index)
            let right_tree = in_order.slice(split_index+1)

            this.left = new Tree()
            this.right = new Tree()

            if (left_tree.length>1){
                this.left.reconstruct(pre_order.slice(1, split_index), left_tree)
            }else{
                if (left_tree){
                    this.left.value = left_tree[0]
                }
            }
            if (right_tree.length>1){
                this.right.reconstruct(pre_order.slice(split_index+1), right_tree)
            }else{
                if (right_tree){
                    this.right.value = right_tree[0]
                }
            }
        }
    }
    this.print_tree = (level) =>{
        if (this.value){
            console.log(" ".repeat(level), this.value)
            if(this.left){
                this.left.print_tree(level+1)
            }
            if(this.right){
                this.right.print_tree(level+1)
            }
        }
    }
}
in_order = [4, 3, 5, 2, 6, 1, 12, 8, 13, 7, 10, 9, 11]
pre_order = [1, 2, 3, 4, 5, 6, 7, 8, 12, 13, 9, 10, 11]
t = new Tree()
t.reconstruct(pre_order, in_order)
t.print_tree(0)

A few key points:

Make sure to put in semicolons!
You don't need to export properties (i.e do this.myVar) if you're not going to access them - this can lead to potentially slowing down recurrence, or in some cases, security vulnerabilities.
If you're going to use arrow functions, keep it consistent - do const Tree = () => { on the first line

Some more specific things about the actual functionality/logic:

By tracking this.level, you could simplify all required actions into an array by simply pushing their position in the tree. If this.level === 0, you know you are at the top of the tree, and you know to end recurrence.
Using a position-based tracking lets you make it asyncronous, and print the result at the end. Remember that in JS, console.logs take a lot of time, so you want to avoid it as much as possible.
This would let you essentially JSONify the tree into a bunch of function references, which, when called, would populate themselves. Giving the option to return this would make the class more integratable into other things, where a console output is not necessarily the most useful thing. Further, it would let you cut out the repetition of if (left_tree.length>1){, which is not DRY.


Improve this numeric solver for properties of geometric objects I'm converting some C# code to F#. Basically it is some geometry classes that can have properties tuned by a numerical solver. I start with a discriminated union of type parameter that can be fixed or free ( tunable ) and then build tunable geometry objects out of them.

I create a discriminated union called shape with Line, Circle, Point as sub types. The code is below.

module SketchSolveFS.Solver

type parameter =
    | Fixed of double
    | Free of double ref

let free v = Free (ref v)
let fix v = Fixed v

type point  = {x: parameter; y:parameter}
type line   = {p0: point; p1:point}
type circle = {center: point; radius:parameter}

type shape =
    | Parameter of parameter
    | Point of point
    | Line of line
    | Circle of circle

let rec freeVars v = seq {
    match v with
    | Parameter p ->
        match p with
        | Free v -> yield v
        | _ -> ()
    | Point p ->
        yield! Parameter p.x |> freeVars
        yield! Parameter p.y |> freeVars
    | Line line ->
        yield! Point line.p0 |> freeVars
        yield! Point line.p1 |> freeVars
    | Circle c ->
        yield! Point c.center |> freeVars
        yield! Parameter c.radius |> freeVars
    }

I have a function called freeVars that return me a sequence of
tunable parameters in a list of geometry objects. The code works
and passes my tests but the implementation leaves me uncomfortable.

Here is why. I keep feeling the need to write my discriminated
union this way

type shape =
    Parameter of parameter
    Point of Parameter * Parameter
    Line of Point * Point

instead of having to create concrete types before hand. Then my
function freeVars would look like this.

let rec freeVars v = seq {
    match v with
    | Parameter p ->
        match p with
        | Free v -> yield v
        | _ -> ()
    | Point p ->
        yield! p.x |> freeVars
        yield! p.y |> freeVars
    | Line line ->
        yield! line.p0 |> freeVars
        yield! line.p1 |> freeVars
    | Circle c ->
        yield! c.center |> freeVars
        yield! c.radius |> freeVars
    }

where I don't have to wrap the concrete classes back in the discriminated
union wrappers each time I extract and recursively process. Now obviously
the F# compiler doesn't like this or I wouldn't be asking the question.

Is there a better way to frame the working code. Are discriminated unions
not appropriate for this code perhaps?

A few suggestions on the first code fragment:

Change type names to upper case
Change record field names to upper case
Change fields of line to Begin and End to be descriptive
Use pattern matching on records instead of . access

The updated code fragment is as follows:

type Parameter =
    | Fixed of double
    | Free of double ref

let free v = Free (ref v)
let fix v = Fixed v

type Point  = { X: Parameter; Y: Parameter }
type Line   = { Begin: Point; End: Point }
type Circle = { Center: Point; Radius: Parameter }

type Shape =
    | Parameter of Parameter
    | Point of Point
    | Line of Line
    | Circle of Circle

let rec freeVars v = seq {
    match v with
    | Parameter p ->
        match p with
        | Free v -> yield v
        | _ -> ()
    | Point {X = x; Y = y} ->
        yield! Parameter x |> freeVars
        yield! Parameter y |> freeVars
    | Line {Begin = p0; End = p1} ->
        yield! Point p0 |> freeVars
        yield! Point p1 |> freeVars
    | Circle {Center = c; Radius = r} ->
        yield! Point c |> freeVars
        yield! Parameter r |> freeVars
    }

And here are comments on the second code fragment.

You mistakenly assume that Line, Circle, Point, etc as sub types of shape. They are just union cases, so your type definition doesn't make sense.

You can change shape to be recursive:

type shape =
    | Parameter of parameter
    | Point of shape * shape
    | Line of shape * shape

but it allows more cases than it should.

You can use tuples instead of records:

type shape =
    | Parameter of parameter
    | Point of parameter * parameter
    | Line of (parameter * parameter) * (parameter * parameter)
    | Circle of (parameter * parameter) * parameter

but it is less desirable since Point, Line and Circle deserve to be types on their own rights.

  Are discriminated unions not appropriate for this code perhaps?

It depends. If you have fixed number of cases, then DUs and pattern matching are great to use. Otherwise, you have to fall back to class hierarchy which is much more painful to deal with.


Better way of setting input scope on text box in WP7 This works but there must be a better way:

txtBox.InputScope = new InputScope { Names = { new InputScopeName { NameValue = InputScopeNameValue.Text } } };

I'm not really sure what you want from this, if all you want is to make it more readable, then a few well placed new-lines will make a world of difference. If you want to reduce the amount of code, I don't think there is much room for that other than switching constructors for InputScopeName.

txtBox.InputScope = new InputScope
{
    Names =
    {
        new InputScopeName(InputScopeNameValue.Text)
    }
};

Note: Text does not appear to be a valid member of InputScopeNameValue, but ill keep it for the sake of consistency with the question.


Using bisect to flip coins I am trying to solve this problem on Codechef, which has a list of \$N\$ coins, all initially tails-up, and a list of \$Q\$ commands. There are two possible commands, 0 A B flips all coins in the interval [A, B] and 1 A B prints the number of heads-up coins in the interval [A, B] (both intervals include B).

After reviewing somebody else's answer that exceeded the time-limit, I came up with a fast numpy answer, which was still not fast enough for the online judge (3s instead of 0.3s).

However, one other reviewer suggested the following algorithm:

  The key observation is that a 0 a b operation is in fact a pair of operations: flip coins from a to the end, and flip coins from b + 1 to the end. This leads to the solution:

  Decouple the 0 a b operation into a and b+1
  Maintain the list of flip points, and keep it ordered
  To perform a 1 operation, find the largest flip point before A. If its index in flip list is even, the A coin is tails
  up, otherwise it is heads up. Then, traverse the flip list until B
  adding up lengths of every other interval.

I tried to implement this, using bisect to keep the list of flips sorted, but it is a lot slower (20s for the same worst-case input as the 3s quoted above).

from bisect import bisect_left, insort

n, q = map(int, input().split())

flips = []

for _ in range(q):
    command, start, end = map(int, input().split())
    if command == 0:
        insort(flips, start)
        insort(flips, end + 1)
    elif command == 1:
        flip_start = bisect_left(flips, start)
        flip_end = bisect_left(flips, end + 1)
        flips_temp = flips[flip_start:flip_end]
        insort(flips_temp, end + 1)
        if flip_start % 2:
            insort(flips_temp, start)
        print(sum(flips_temp[1::2]) - sum(flips_temp[:-1:2]))

However, this is probably not the best implementation of that algorithm, maybe there is a way to use heapq to keep the list sorted (but then slicing and traversing it become harder). So, any comments, especially on how to speed this up, are welcome.

I am aware that one should normally put this into a function and make it re-usable, but since this is time-critical code I think it is fine to leave it all in the global namespace. It is a one-shot script, anyway.

The worst-case input file I used (which I generated using the code in my linked answer from above) can be found here (1.4 MB). It assumes the maximum allowed values for \$N\$ and \$Q\$ and contains a random sequence of the two allowed commands.

I feed it to this code with the following command (and time it at the same time):

time cat flipping_coins.dat | python3 flipping_coins.py

Running only the cat command already takes 0.17s.

As Ev. Kounis noted in the comments, there is currently no solution in pure Python that successfully beats this challenge. There are, however, a couple of PyPy solutions, which use segment trees.

if command == 0:
        insort(flips, start)
        insort(flips, end + 1)

Well, the main reason it's slow is insort. As the doc says,

  Keep in mind that the O(log n) search is dominated by the slow O(n) insertion step.

That said, there's a significant performance improvement possible here without changing data structure. Flipping coins from a to the end and then flipping coins from a to the end is equivalent to doing absolutely nothing. So rather than always insert, toggle whether or not the value is in flips. I see an execution time reduction from about 30s to about 16s with

def toggle(flips, val):
    idx = bisect_left(flips, val)
    if idx < len(flips) and flips[idx] == val:
        del flips[idx]
    else:
        flips.insert(idx, val)

and

    if command == 0:
        toggle(flips, start)
        toggle(flips, end + 1)

Obviously if you want speed then you need to switch to using a better data structure than a list, but within the constraints of list I think this is fairly efficient.

    elif command == 1:
        flip_start = bisect_left(flips, start)
        flip_end = bisect_left(flips, end + 1)
        flips_temp = flips[flip_start:flip_end]
        insort(flips_temp, end + 1)
        if flip_start % 2:
            insort(flips_temp, start)
        print(sum(flips_temp[1::2]) - sum(flips_temp[:-1:2]))

The first thing that's suspicious here is insort. However, to my surprise, using flips_temp.append(end + 1) and flips_temp.insert(0, start) made things slightly slower. The real speedup comes from eliminating the copying. Careful analysis shows that the logic is equivalent to

    elif command == 1:
        flip_start = bisect_left(flips, start)
        flip_end = bisect_left(flips, end + 1)

        accum = sum(flips[flip_start+1:flip_end:2]) - sum(flips[flip_start:flip_end:2])
        if flip_start % 2:
            accum = -start - accum
        if flip_end % 2:
            accum = accum + end + 1
        print(accum)

This gave me a further speedup from 16 seconds to 12 seconds.

IMO it would make sense to implement the difference of sums using functools.reduce(operator.sub, ..., 0), but that's much much slower (~40 seconds).


Reversing a linked list by iteration and recursion From my original question on Stack Overflow: Is my implementation of reversing a linked list correct?

I'm a beginner at C and I'd like to know about style, and correctness of the reverse algorithms. What about memory management?

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

typedef struct Node {
    int val;
    struct Node *next;
} Node;

Node * build_list(int num);
Node * reverse_list(Node * head);
static Node * r_reverseItem(Node * last, Node * head);
Node * r_reverse(Node * head);

/*
 * Function which iterates to 100 creating nodes
 * and linking them to head
 *
 */
Node * build_list(int num) {

    Node * head = NULL;
    Node * curr;

    int i;
    for(i=0;i<num;i++) {
    curr = (Node *)malloc(sizeof(Node));
    curr->val = i;
    curr->next = head;
    head = curr;
    }
    return head;
}

/*
 * Function which reverses linked list
 * by traversing list and flipping ptr of curr to last
 * @param head
 *
 */
Node * reverse_list(Node * head) {

    if (head == NULL) //only one elem
    return NULL;

    Node * last = NULL;
    do {
    Node * next = head->next;
    head->next = last;
    last = head;
    head = next;
    }
    while (head != NULL);
    return last;
}

/*
 * Function which recursively reverses linked list
 * @param last
 * @param head
 *
 */
static Node * r_reverseItem(Node * last, Node * head) {

    if (head == NULL)
    return last;

    Node * next = head->next;
    head->next = last;
    return r_reverseItem(head,next);
}

Node * r_reverse(Node * head) {
    return r_reverseItem(NULL, head);
}

int main (void) {
    Node * head = build_list(5);
    Node * curr;
    head = r_reverse(head);
    //    head = reverse_list(head);
    while(head) {
      printf("%d\n",head->val);
      curr = head;
      head = head->next;
      free(curr);
    }
    return 0;
}

Code Comments:
curr = malloc(sizeof(Node));

malloc() returns void* thus you need to cast it before assignment.
curr = (Node*) malloc(sizeof(Node));

Most (all I have seen) coding standards mention not to use this:
Node * head, * curr;
head = NULL;

I agree with them as it just makes things hard to read (thus maintain). So one variable per line. And initialize on declaration (where appropriate).
Node  *head = NULL;
Node  *curr;

OK. The next one is arguable (so feel free to use best judgement).
Personally I find that hard to read:
 if (!head->next)

I think it would be easier to read as:
if (head->next == NULL)

But surprisingly I normally I prefer using the ! when the object is a pointer (or bool). I think in this case it is because it is combined with testing a nested member of a structure it just does not look correct (but as I said it is an arguable case).
Edge Case
What happens if the list is empty?
Node * reverse_list(Node * head) {

  if (!head->next) //only one elem
    return head;

You have chosen the wrong edge case.
The one you should be checking is checking for NULL. A list of one node is no different than a list of many. The edge case with pointers is nearly always the NULL case.
Algorithm
Iterative
The code before your while loop looks like the code inside the while loop. This sort of suggests you can replace it with a do-while loop. Thus your iterative code should look more like this:
Node* reverse(Node* list)
{
    if (list == NULL)
    {
        return NULL;
    }

    Node*   last    = NULL;
    do
    {
        Node*   next    = list->next;
        list->next      = last;
        last            = list;
        list            = next;
    }
    while(list != NULL);

    return last;
}

Recursive
Converting a loop into recursion requires an extra function to cope with the extra variable used for last. So the main recursion looks like this (it just calls the actually recursive function setting up the last parameter). You should have this wrapper function to help users of the code from calling the recursive part of the function incorrectly.
Node* r_reverse(Node* list)
{
    return r_reverseItem(NULL, list);
}

Now we can do the recursion. Again the edge case is still NULL.
Node* r_reverseItem(Node* last, Node* list)
{
    if (list == NULL)
    {
        return last;
    }

    Node*   next    = list->next;
    list->next      = last;
    return r_reverseItem(list,next);
}


HTML5 Local Storage toggle and save multiple classes I put together a code will allow me to toggle a class and save it in local storage.

Now I need to add 2 more classes to be toggled and be saved in the same manner.
I don't know how to iterate properly with the code and the DOM to achieve this.

This is my "semi-working" example:
http://jsbin.com/nimekebura/1/edit?html,css,js,output

HTML:

    toggle and save state

JS:

 //retrieve current state
 $('.container').toggleClass(localStorage.toggled);

    /* Toggle */
    $('.bar-toggle').on('click',function(){

     // set 1 localstorage values are always strings (no booleans)
       if (localStorage.toggled != "with_toggle" ) {
          $(".container").toggleClass("with_toggle", true );
          localStorage.toggled = "with_toggle";

       } else {
          $(".container").toggleClass("with_toggle", false );
          localStorage.toggled = "";
       }

    });

I need to add and toggle another 2 classes to each .box class already in the code.

so the code would toggle like this:

what would be the best code practice to achieve the above?

Also  Iwas told, the localStorage object is not meant to be extended with new keys like toggled and If wanted to add a new item, I would need to you use localStorage.setItem('key', 'value') https://developer.mozilla.org/en-US/docs/Web/API/Storage

Needless to say I have no idea how to do this the best code practice way. Could someone help me refactor the code properly and also add the two new classes?

The following are all acceptable ways to set/get values to/from localStorage (or sessionStorage for that matter):

// set
localStorage.key = 'value';
localStorage['key'] = 'value';
localStorage.setItem('key', 'value');

// get
value = localStorage.key;
value = localStorage['key'];
value = localStorage.getItem('key');

I think from a practical standpoint though, you might see explicit setItem() or getItem() method calls used most frequently, as oftentimes one might find themselves working with dynamic key names, which would eliminate the first usage example from each of the the sets above.  Oftentimes developers choose to simply be consistent and use the getter/setter methods rather than mix direct property access when key names are known vs. getter/setter or "array-like" access when the key names are dynamic.

More complex applications may also abstract out direct access to localStorage and sessionStorage altogether, especially when dealing with browsers that do not support web storage (and therefore may need cookie-based fallback).  In such cases, you would absolutely not be able to have hard-coded key names when interacting with the underlying web storage, because you might have a usage pattern like:

myStorage.set(key, value, inSession) {
    if (typeof Storage !== undefined) {
        // we have local and session storage available
        if(inSession === true) {
            sessionStorage.setItem(key, value);
        } else {
            localStorage.setItem(key, value);
        }
    } else {
        // fallback
    }
}


Calculate prime numbers This is a program to calculate the largest prime number.  Are there any possible improvements in style/speed/accuracy?

public static int? LargestPrime(int max)
{
    if (max < 0)
        return null;
    int largestPrime = 0;
    for (int i = 0; i <= max; i++)
    {
        bool? isPrime = IsPrime(i);
        if ((bool)isPrime)
            largestPrime = i;
        if (i % 100000 == 0)
            Console.WriteLine(largestPrime.ToString("N0"));
    }
    return largestPrime;
}
private static List<int> listPrimes = new List<int>() { 2, 3 };
private static int listPrimeMax;
private static int listPrimesCount = 100000;
private static bool firstA = true;
private static bool firstB = true;
public static bool? IsPrime(int n)
{
    if (listPrimes.Count < listPrimesCount)
    {
        // from wiki
        // function is_prime(n : integer)
        // if n ≤ 1
        //     return false
        // else if n ≤ 3
        //     return true
        // else if n mod 2 = 0 or n mod 3 = 0
        //     return false
        // let i ← 5
        // while i×i ≤ n
        //     if n mod i = 0 or n mod(i + 2) = 0
        //         return false
        //     i ← i + 6
        //return true
        if (n < 0)
            return null;
        if (n <= 1)
            return false;
        if (n <= 3)
            return true;
        if (n % 2 == 0 || n % 3 == 0)
            return false;
        for (int i = 5; i*i <= n; i += 6)
        {
            if (n % i == 0 || n % (i + 2) == 0)
                return false;
        }
        if (listPrimes.Count < listPrimesCount)
        {
            listPrimes.Add(n);
            listPrimeMax = n;
        }
        return true;
    }
    else
    {
        if(firstA)
        {
            Console.WriteLine("in listPrimes");
            firstA = false;
        }
        foreach (int i in listPrimes)
        {
            if (n % i == 0)
                return false;
            if (i * i > n)
                break;
        }
        for (int i = listPrimeMax + 2; i*i <= n; i += 2)
        {
            if (n % i == 0)
                return false;
        }
        if (listPrimes.Count < 100 * listPrimesCount)
        {
            listPrimes.Add(n);
            listPrimeMax = n;
        }
        else if(firstB)
        {
            Console.WriteLine("in 100*listPrimes");
            firstB = false;
        }
        return true;
    }
}

public static int? LargestPrime(int max)
{
    if (max < 0)
        return null;
    int largestPrime = 0;
    for (int i = 0; i <= max; i++)
    {
        bool? isPrime = IsPrime(i);
        if ((bool)isPrime)
            largestPrime = i;
        if (i % 100000 == 0)
            Console.WriteLine(largestPrime.ToString("N0"));
    }
    return largestPrime;
}

Omitting braces {}, although they might be optional, won't do you good in the long run because it makes your code error-prone.

If an argument of a method isn't correct like max < 0 you should throw either an ArgumentException or better an ArgumentOutOfRangeException.

You are iterating from 0 to max to check if the value is a prime, but I wonder what else could the number be? Either it is a prime or it isn't a prime, there is no third way so it doesn't make sense that IsPrime() returns a nullable bool.

bool? IsPrime(int n)
Here you are iterating from 5 to i*i<n for each number which is passed to the method.
So let us assume that we pass max = 10.000.000 (the dots are there for clearity) this

    if (n % i == 0 || n % (i + 2) == 0)
        return false;

in the for loop in the IsPrime() method is executed 16.194.513 times.
By using a sieve you could pretty much speed this thing up like so
static int CalculateLargestPrime(int maxPrime)
{
    bool[] isComposite = new bool[maxPrime + 1];
    for (int x = 2; x * x <= maxPrime; x++)
    {
        if (!isComposite[x])
        {
            for (int y = x * x; y <= maxPrime; y = y + x)
            {
                isComposite[y] = true;
            }
        }
    }
    for (int i = maxPrime; i >= 0; i--)
    {
        if (!isComposite[i])
        {
            return i;
        }
    }

    return -1;
}

taken and adjusted from https://codereview.stackexchange.com/a/62158/29371


Checking for JSON or YAML file with promises I'm using bluebird promises. I'm converting fs.exists to existsAsync (here's how) and fs.readFile (here's how) to promises.

existsAsync(o.inputFilePathJSON).then(function(exists) {
  debug("%s exists %s", o.inputFilePathJSON, exists);
  if (exists) return fs.readFile(o.inputFilePathJSON).then(JSON.parse);
  return existsAsync(o.inputFilePathYML).then(function(exists) {
    debug("%s exists %s", o.inputFilePathYML, exists);
    if (exists) return fs.readFile(o.inputFilePathYML).then(yaml.load);
  });
})

This will check if the JSON version exists, and if it doesn't check for the YML version.

I was wondering how good this code is as promise code.

In this case I'm switching to YML from JSON. I'd love to build my YML files and check if both the JSON and YML are the same. I can't do that with this code because both of the promises don't run independently.

Here's another version of the code:

var json = existsAsync(o.inputFilePathJSON).then(function(exists) {
  debug("%s exists %s", o.inputFilePathJSON, exists);
  if (exists) return fs.readFile(o.inputFilePathJSON).then(JSON.parse);
  return false;
})

var yml = existsAsync(o.inputFilePathYML).then(function(exists) {
  debug("%s exists %s", o.inputFilePathYML, exists);
  if (exists) return fs.readFile(o.inputFilePathYML).then(yaml.load);
  return false;
});

json.then(function(json) {
  yml.then(function(yml) {
    if (yml) return yml;
    if (json) return json;
    return false;
  });
});

What about error reporting when it comes to building these small promises? Should the errors be caught separately for each, or can they be caught in the conjoined version?

My two questions are:

How should this promise code be written?
Where should the errors be caught?

Interesting question,

since you need to evaluate both Promises, I would use Collections.
Something like this:

Promise.all([yml(),json()]).then(function( results ) {
  // Return the results of yml() if truthy, or json() if truthy, or false
  return results[0] || results[1] || false;
});

Other than that, for the love of readable code:

Use newlines in your if statements
Use curly braces in your if statements
Dont stretch your code too much horizontally

I like your 2nd approach far more than the 1st approach, the 1st approach is simply head ache inducing.