Adding thousand separator to various number types Am making a pure .net library with helper functions (GitHub).

However I wanted to have a thousand separator for all number types and here is what I am currently doing

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

namespace Avo
{
    public static class Extension_Number
    {
        #region ThousandSeparator
        public static string ToThousandSeparator(this decimal value, int numberOfDecimalPlaces)
        {
            if (numberOfDecimalPlaces < 0) numberOfDecimalPlaces = 0;
            return string.Format("{0:N" + numberOfDecimalPlaces + "}", value).ToString();
        }
        public static string ToThousandSeparator(this int value, int numberOfDecimalPlaces)
        {
            if (numberOfDecimalPlaces < 0) numberOfDecimalPlaces = 0;
            return string.Format("{0:N" + numberOfDecimalPlaces + "}", value).ToString();
        }
        public static string ToThousandSeparator(this double value, int numberOfDecimalPlaces)
        {
            if (numberOfDecimalPlaces < 0) numberOfDecimalPlaces = 0;
            return string.Format("{0:N" + numberOfDecimalPlaces + "}", value).ToString();
        }
        public static string ToThousandSeparator(this long value, int numberOfDecimalPlaces)
        {
            if (numberOfDecimalPlaces < 0) numberOfDecimalPlaces = 0;
            return string.Format("{0:N" + numberOfDecimalPlaces + "}", value).ToString();
        }
        #endregion
    }
}

As you can see above ,

l have to repeat the same for
int , decimal , long

l can achieve the results l want but is it best

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

namespace Avo
{
    class Program
    {
        static void Main(string[] args)
        {
            decimal tes = 294944.8484827M;
            int iint = 34;
            Console.WriteLine(tes.ToThousandSeparator(3));
            Console.WriteLine(iint.ToThousandSeparator(2));
            Console.ReadKey();
        }
    }
}

but is there an eloquent way of doing it , but allowing number types only.

There is no common base type for numeric types, unfortunately.

You could still reduce the repetitiveness of your code though, by encapsulating the common structure in a single function:

    private static string AddThousandsSeparator(Object numeric, int numberOfDecimalPlaces)
    {
        // note this would crash when passed a non-numeric object.
        // that's why it's private, and it's the class's responsibility
        // to limit the entry points to this function to numeric types only
        return String.Format("{0:N" + Math.Max(0, numberOfDecimalPlaces) + "}", numeric);
    }

    public static string WithThousandsSeparator(this decimal value, int numberOfDecimalPlaces)
    {
        return AddThousandsSeparator(value, numberOfDecimalPlaces);
    }

    public static string WithThousandsSeparator(this int value, int numberOfDecimalPlaces)
    {
        return AddThousandsSeparator(value, numberOfDecimalPlaces);
    }

    public static string WithThousandsSeparator(this double value, int numberOfDecimalPlaces)
    {
        return AddThousandsSeparator(value, numberOfDecimalPlaces);
    }

    public static string WithThousandsSeparator(this long value, int numberOfDecimalPlaces)
    {
        return AddThousandsSeparator(value, numberOfDecimalPlaces);
    }

A few side remarks:

There is no point in calling ToString() on the result of String.Format. String.Format already returns a string.
Math.Max is a more concise alternative for the conditional reassignment. (For what it's worth, Kotlin provides readable extension functions as syntax sugar for this sort of thing, eg.  someNumber.coerceAtLeast(0) - trivially easy to reimplement them in C# for readability).
I also corrected the naming slightly. It's "thousands" separator (plural), plus I think With... makes it clearer what the function does. The ToSomething phrase conventionally means a value is getting converted to Something, as if you were converting the number to the thousands separator itself - obviously nonsensical - rather than just adding it to the formatting.


Implementing an "Add Company" section in a project I'm working on avoiding JavaScript that either directly modifies the DOM a ton, contains a ton of anonymous functions, or is encapsulated in giant $(document).ready... blocks.

This is one of my first attempts at writing a section of my project in a better way.  The code is commented and organized fairly well, so it should be easy to understand how it functions.  My main question is: Am I going about this the right way?

// Object literal for handling all aspects of adding a company
var addCompany = {

  // Initialize object
  init: function() {

    // Save elements references
    addCompany.elements = {
      openForm: $('#add-company-btn'),
      form: $('#add-company-form'),
      usernameField: $('#username'),
      usernameErrorField: $('#username-error div'),
      passwordField: $('#password'),
      passwordErrorField: $('#password-error div'),
      password2Field: $('#password2'),
      password2ErrorField: $('#password2-error div'),
    };

    // Run on click
    addCompany.elements.openForm.click(
      addCompany.focusFirst
    );

    // Run on focus
    addCompany.elements.form.submit(
      addCompany.validate
    );
  },

  // Focus the first input field
  focusFirst: function() {
    setTimeout(function(){
      addCompany.elements.usernameField.focus();
    }, 0);
  },

  // Validate the form
  validate: function() {

    // Store the error tests
    var noUsername = (addCompany.elements.usernameField.val().trim() === ''),
        noPassword = (addCompany.elements.passwordField.val().trim() === ''),
        differentPasswords = (addCompany.elements.passwordField.val() !== addCompany.elements.password2Field.val());

    // Return the results of running the tests
    return addCompany.handleError(addCompany.elements.usernameErrorField, noUsername) &&
           addCompany.handleError(addCompany.elements.passwordErrorField, noPassword) &&
           addCompany.handleError(addCompany.elements.password2ErrorField, differentPasswords);
  },

  // Given an error, handle the display of error fields
  handleError: function(error_field, error) {
    if(error) {
      error_field.show();
      return false;
    } else {
      error_field.hide();
      return true;
    }
  }
};

You are going very well. There is nothing wrong with it, except there are some things that can be further improved. To improve it you can further do:

addCompany should be in pascal case i.e. AddCompany
Try to use this i.e. use this.something instead of using addCompany.something everytime.
Module name should be noun instead of word. You may use Company instead of addCompany
You can make the private properties using the following version of Modular Pattern:

var Company = function () {

// ------- Start #PrivateSection ------

// Save elements references
var elements = {
  openForm: $('#add-company-btn'),
  form: $('#add-company-form'),
  usernameField: $('#username'),
  usernameErrorField: $('#username-error div'),
  passwordField: $('#password'),
  passwordErrorField: $('#password-error div'),
  password2Field: $('#password2'),
  password2ErrorField: $('#password2-error div'),
};

// Given an error, handle the display of error fields
var handleError = function(error_field, error) {
  if(error) {
    error_field.show();
    return false;
  } else {
    error_field.hide();
    return true;
  }
}

// ------- End #PrivateSection --------

// Public Properties and methods
return {
  // Initialize the object
  init : function () {
    bindUI();
  },

  // All the event bindings to go here.
  bindUI : function () {
    // Run on click
    addCompany.elements.openForm.click(
      addCompany.focusFirst();
    );

    // Run on focus
    addCompany.elements.form.submit(
      addCompany.validate
    );
  },

  // Focus some element
  focusElement: function( el ) {

    var toFocus;

    // If no value provided
    if ( typeof el == "undefined" ) {
      // Focus the first element
      toFocus = elements.usernameField;
    } else {
      // Focus the provided element element
      toFocus = el;
    }

    toFocus.focus();
  },

  // Validate the form
  validate: function() {

    // Store the error tests
    var noUsername = (addCompany.elements.usernameField.val().trim() === ''),
        noPassword = (addCompany.elements.passwordField.val().trim() === ''),
        differentPasswords = (addCompany.elements.passwordField.val() !== addCompany.elements.password2Field.val());

    // Return the results of running the tests
    return handleError(elements.usernameErrorField, noUsername) &&
           handleError(elements.passwordErrorField, noPassword) &&
           handleError(elements.password2ErrorField, differentPasswords);
  }

};
};

And then to use it,

var c = new Company();
c.init();


3-level deep if-else tree for constructing a neural network The following code fragment constructs different types of neural networks outputs based on the options supplied. Currently, my code just has a huge note that describes what all the options are supposed to do. I wanted to learn how I can make this code less complex or perhaps break its complexity into simpler pieces. Any suggestions?

#------------------------------------------------------------#
# NOTE: Meaning of all the options.                          #
# stagger_schedule=extended: We copy input vec to output.    #
# stagger_schedule=external: We dont copy input to output.   #
# -----------------------------------------------------------#
# do_backward_pass: We use the output of the backward LSTM   #
#   Default:True.                                            #
# -----------------------------------------------------------#
# chop_bilstm: Should we chop the first and last vectors from#
# the sequence. Default:False                                #
#------------------------------------------------------------#
# extended_multiplicative: Multiply the forward and back LSTM#
#    and concatenate the input embedding.                    #
# external_multiplicative: Multiply the forward and back LSTM#
#    and but dont concatenate the input embedding.           #
#------------------------------------------------------------#
if (self.prm('stagger_schedule') == 'extended'):
    if self.prm('chop_bilstm'):
        if self.prm('do_backward_pass'):
            self.output_tv = T.concatenate(
                [forward, backward, input_tv], axis=1)[1:-1]
            pass
        else:
            self.output_tv = T.concatenate(
                [forward, input_tv], axis=1)[1:-1]
            pass
        pass
    else:
        if self.prm('do_backward_pass'):
            self.output_tv = T.concatenate(
                [forward, backward, input_tv], axis=1)
            pass
        else:
            self.output_tv = T.concatenate(
                [forward, input_tv], axis=1)
            pass
        pass
    pass
elif self.prm('stagger_schedule') == 'external':
    if self.prm('chop_bilstm'):
        if self.prm('do_backward_pass'):
            self.output_tv = T.concatenate(
                [forward[1:-1], backward[2:]], axis=1)
            pass
        else:
            self.output_tv = forward[1:-1]
            pass
        pass
    else:
        if self.prm('do_backward_pass'):
            self.output_tv = T.concatenate(
                [forward, backward], axis=1)
            pass
        else:
            self.output_tv = forward
            pass
        pass
    pass
elif self.prm('stagger_schedule') == 'extended_multiplicative':
    if self.prm('chop_bilstm') or (not self.prm('do_backward_pass')):
        raise NotImplementedError()
    self.output_tv = T.concatenate(
        [forward * backward, input_tv], axis=1)
    pass
elif self.prm('stagger_schedule') == 'external_multiplicative':
    if self.prm('chop_bilstm') or (not self.prm('do_backward_pass')):
        raise NotImplementedError()
    self.output_tv = forward * backward
    pass
else:
    raise NotImplementedError()

Here are some ways you can tidy up this code:

Add some comments. It’s quite hard for me to tell whether this code is correct, or whether I‘ve introduced a bug by refactoring, because I don’t know what this code is supposed to do.
Get rid of the unnecessary pass statements. The pass statement literally does nothing except provide a placeholder for unwritten code. If you delete them all, you’ll save a lot of lines and be able to fit more code on screen.
Under the extended branch, the code is almost the same except we remove the first and last character of self.output_tv. If we defer that until the end, we can have one set of branches as follows:

if (self.prm('stagger_schedule') == 'extended'):
    if self.prm('do_backward_pass'):
        self.output_tv = T.concatenate([forward, backward, input_tv], axis=1)
    else:
        self.output_tv = T.concatenate([forward, input_tv], axis=1)

    if self.prm('chop_bilstm'):
        self.output_tv = self.output_tv[1:-1]

22 lines cut down to 7, and this is only 2 levels deep.
The code in the external branch can be similarly consolidated: you do the same effect in both subbranches, but with slightly truncated forward and backward variables. Here’s an alternative version:

elif self.prm('stagger_schedule') == 'external':
    if self.prm('chop_bilstm'):
        forward = forward[1:-1]
        backward = backward[2:]

    if self.prm('do_backward_pass'):
        self.output_tv = T.concatenate([forward, backward, axis=1)
    else:
        self.output_tv = forward

20 lines cut down to 8, and this is only 2 levels deep.

You could consider wrapping each branch into its own method, and calling into that. For example, something like:

if (self.prm('stagger_schedule') == 'extended'):
    self._set_output_tv_extended(forward, backward, input_tv)
elif self.prm('stagger_schedule') == 'external':
    self._set_output_tv_external(forward, backward, input_tv)
elif self.prm('stagger_schedule') == 'extended_multiplicative':
    self._set_output_tv_extended_multiplicative(forward, backward, input_tv)
elif self.prm('stagger_schedule') == 'external_multiplicative':
    self._set_output_tv_external_multiplicative(forward, backward, input_tv)
else:
    raise NotImplementedError()

That saves you an immediate level of nesting and pushes the specific logic of each branch out of this method.


Counting K-mers (words) in a sequence string I recently found out how to use joblib for parallelization.  I want to iterate through this string seq in steps of 1 and count the number that word is seen.  I thought this would be really fast but for larger files it still takes long.

How could I make this word counter faster?

def iterSlice(seq, start_idx, end_idx):
    """
    Slice sequence
    """
    return seq[start_idx:end_idx]

def countKmers(seq, K=5, num_cores=multiprocessing.cpu_count()):
    """
    Count kmers in sequence
    """
    start_indices = np.arange(0, len(seq) - K + 1, step=1)
    end_indices = start_indices + K
    list_of_kmers = Parallel(n_jobs=num_cores)\
                        (delayed(iterSlice)\
                             (seq=seq, start_idx=start_idx, end_idx=end_idx)\
                                 for start_idx,end_idx in zip(start_indices,end_indices))
    return Counter(list_of_kmers)

seq = "ABCDBBBDBABCBDBABCBCBCABCDBACBDCBCABBCABDCBCABCABCABDCBABABABCD"
countKmers(seq=seq, K=3, num_cores=8)

Counter({'ABA': 2,
         'ABB': 1,
         'ABC': 7,
         'ABD': 2,
         'ACB': 1,
         'BAB': 5,
         'BAC': 1,
         'BBB': 1,
         'BBC': 1,
         'BBD': 1,
         'BCA': 6,
         'BCB': 3,
         'BCD': 3,
         'BDB': 2,
         'BDC': 3,
         'CAB': 6,
         'CBA': 1,
         'CBC': 4,
         'CBD': 2,
         'CDB': 2,
         'DBA': 3,
         'DBB': 1,
         'DCB': 3})

10 loops, best of 3: 186 ms per loop

Obligatory PEP8 reference: It is recommended to use lower_case names for variables and functions. camelCase is alright if you are consistent (you seem to be using lower_case for variables and camelCase for functions).

The hardest to read part is the calling of the Parallels part. I would make a few changes there to improve readability:

The way delayed is used I would think it is a decorator and you could do:

@delayed
def iterSlice(seq, start_idx, end_idx):
    """
    Slice sequence
    """
    return seq[start_idx:end_idx]

Unfortunately this does not work, I asked a stackoverflow question why here.

You can however do this:

def iterSlice(seq, start_idx, end_idx):
    """
    Slice sequence
    """
    return seq[start_idx:end_idx]
iterSlice = delayed(iterSlice)

This seems a bit too verbose to me:

iterSlice(seq=seq, start_idx=start_idx, end_idx=end_idx)

This is just as explicit:

itereSlice(seq, start_idx, end_idx)

Parallels has an implemented contextlib so you can do:

with Parallel(n_jobs=num_cores) as parallel:
    list_of_kmers = parallel(iterSlice(seq, start_idx, end_idx)
                             for start_idx,end_idx in zip(start_indices,end_indices))

It would also allow you to reuse the pool of workers if you had en explicit loop in there, which you don't.

You can probably move the definition of list_of_kmers directly into Counter(...) to avoid generating an intermediate list.

Regarding the performance:

The actually parallelized taks you are performing is getting the slice of the string. This is not really CPU intensive, so the overhead of setting up the parallelization might ruin the benefits.

For large files you will be limited by the reading of the file itself, which will be done linearly with the file size and with one CPU.

Have you tried comparing your code to a naive implementation like this:

from collections import Counter

def count_kmers(seq, K=5):
    """Count kmers in sequence"""
    return Counter(seq[start:start+K] for start in xrange(len(seq) - K))

(In python 3.x use range instead of xrange)


Pseudo Promise.all() polyfill A few years back I interviewed with a company for a Javascript position. After a couple of warm-up challenges I was presented with this:

  Please write a function that calls back with true if all
  promises have resolved successfully, or false if at least one
  promise has rejected.

  Given 'Promise' API:

promise.then(
  function resolve() { /* called when some async thing was successful */},
  function reject() { /* the async thing failed */ }
);

and also was given the following function structure and mocking code:

function all (promises, callback) {
    // TODO call back with `true` if all promises resolve(), or `false` if a promise has reject()ed
    promises.forEach(function (promise) {
        promise.then(
          function resolve() {},
          function reject() {}
        );
    });
}

// Some mocking code (NO NEED TO READ THIS):
function P() { return { then: function (resolve, reject) {
  setTimeout(function() { (5*Math.random()|0) ? resolve() : reject() }, Math.random()*1000);
}}}
var promises = [1,2,3,4,5].map(P);

all(promises, function (success) {
  console.log('The promises have ' + ( success ? '' : 'not ' ) + 'all resolved!' )
});

The gist of it: I had to write an almost Promise.all() method that would check if all async functions finished and how they finished (resolve/reject).

I didn't finish the challenge in the allocated time frame, so I failed (and years later, looking at the code I've written to finally solve it, if was the interviewer, I would failed me even if I was done in time...)

My current implementation

A few days ago I found the challenged buried on the hard drive and decided to give it a go (I timed myself to finish it in time):

Does not re-include the mocking code from above, but it is included in the jsbin below

function all (promises, callback) {
    const promisesStatus = [];
    const allPromisesChecked = (promisesArray = promises, promisesStatusArray = promisesStatus) => promisesStatusArray.length === promisesArray.length;
    const allPromisesPassed = (promisesArray = promisesStatus) => {
        if (promisesArray.filter(value => !value).length === 0) { return true; }
        return false;
    };
    promises.forEach(function (promise) {
        promise.then(
          function resolve() {
              promisesStatus.push(true);
              if (allPromisesChecked() && allPromisesPassed()) { callback(true); }
          },
          function reject() {
              promisesStatus.push(false);
              if (allPromisesChecked() && !allPromisesPassed()) { callback(false); }
          }
        );
    });
};

JSBin: https://jsbin.com/dajini/edit?js,console

Questions

Keeping in mind that this is to be done under the clock and under the interviewer's eyes (pair programming), hence under stress...

Implementation - Leaving aside minor performance optimizations, could I have done it better? Another way that I am unaware of ?
Time - How long does it take you ? (ballpark it) - Originally, I had to code two functions that dealt with string manipulations + this one in under 1 hour

implementation

The biggest thing that jumped out at me is your all function takes a callback instead of returning a Promise like Promise.all would do. (EDIT: it looks like the interview asked that of you, so that makes sense then. A section has been added below.)
When your handling the reject branch of each promise (in the .then call), there's no reason to do anything fancy. As soon as you encounter an error, you can immediately reject the outer promise (or in your case, callback with an Error
allPromisesChecked and allPromisesPassed gets a little verbose but as long as it works it kinda doesn't matter

This interested me so I took a shot at implementing it. Here's my code -

// Promise.all polyfill
function all(promises) {
  return new Promise(function(resolve,reject) {
    var count = promises.length
    var result = []
    var checkDone = function() { if (--count === 0) resolve(result) }
    promises.forEach(function(p, i) {
      p.then(function(x) { result[i] = x }, reject).then(checkDone)
    })
  })
}

// delay helper for creating promises that resolve after ms milliseconds
function delay(ms, value) {
  return new Promise(function(pass) {
    setTimeout(pass, ms, value)
  })
}

// resolved promises wait for one another but ensure order is kept
all([
  delay(100, 'a'),
  delay(200, 'b'),
  delay(50, 'c'),
  delay(1000, 'd')
])
.then(console.log, console.error) // [ a, b, c, d ]

// check that error rejects asap
all([
  delay(100, 'a'),
  delay(200, 'b'),
  Promise.reject(Error('bad things happened')),
  delay(50, 'c'),
  delay(1000, 'd')
])
.then(console.log, console.error) // Error: bad things happened

time

This took me about 10 minutes. If someone already has experience with Promises, I would expect someone could come up with a working solution in less than 30 minutes. If you've never seen Promises before, maybe 60 minutes?

using a callback

Re-reading the question, I see that my original answer is an actual polyfill of Promise.all, not what the interview asked of you.

Here's a dramatically simplified function that is essentially useless except for answering the interview question.

// Promise.all wannabe
// ([Promise], (bool-> void)) -> void
function all(promises, callback) {
  var count = promises.length
  promises.forEach(function(p, i) {
    p.then(
      function() { if (--count === 0) callback(true) },
      function() { callback(false) }
    )
  })
}

// delay helper for creating promises that resolve after ms milliseconds
function delay(ms, value) {
  return new Promise(function(pass) {
    setTimeout(pass, ms, value)
  })
}

// basic boilerplate to check an answer
function checkAnswer(label, promises) {
  all(promises, function(result) {
    console.log(label, result)
  })
}

// resolved promises wait for one another but ensure order is kept
checkAnswer('example1', [
  delay(100, 'a'),
  delay(200, 'b'),
  delay(50, 'c'),
  delay(1000, 'd')
]) // [ a, b, c , d ]

// check that error rejects asap
checkAnswer('example2', [
  delay(100, 'a'),
  delay(200, 'b'),
  Promise.reject(Error('bad things happened')),
  delay(50, 'c'),
  delay(1000, 'd')
]) // Error: bad things happened

remarks

So in hindsight, I do have some more critique to offer. Considering the function only has to return true or false, there's no reason to make it complex. Basically you just have to count the resolve branches until it reaches the count of promises provided as input. If a reject happens, you can immediately return false. There's no need for any other code.

back from the future (4 years later)

Functions have taught me a lot over the recent years. Small functions that do one thing are always better than big functions that do many. The design for this implementation starts with the simple idea of combining two promises into an array of two values. We'll call it and -

const and = (px, py) => // <-- two promises: "p of x" and "p of y"
  px.then(x => py.then(y => [ x, y ])) // two values: "x" and "y"

const all = (promises = []) =>
  promises.reduce
    ( (pr, px) =>
        and(pr, px) // <-- two promises: "p of r" and "p of x"
          .then(([ r, x ]) => // <-- two values: "r" and "x"
            [ ...r, x ]) // <-- result: append "x" to "r"
    , Promise.resolve([]) // <-- initial result, "p of empty"
    )

Already we're done implementing all and behaviour is the same -

all([
  delay(100, 'a'),
  delay(200, 'b'),
  delay(50, 'c'),
  delay(1000, 'd'),
])
.then(console.log, console.error)
// [ a, b, c, d ]

all([
  delay(100, 'a'),
  delay(200, 'b'),
  Promise.reject(Error('bad things happened')),
  delay(50, 'c'),
  delay(1000, 'd'),
])
.then(console.log, console.error)
// Error: bad things happened

Expand the snippet below to verify the result in your browser -

const and = (px, py) =>
  px.then(x => py.then(y => [ x, y ]))

const all = (promises = []) =>
  promises.reduce
    ( (pr, px) =>
        and(pr, px).then(([ r, x ]) => [ ...r, x ])
    , Promise.resolve([])
    )

const delay = (ms, x) =>
  new Promise(r => setTimeout(r, ms, x))

all([
  delay(100, 'a'),
  delay(200, 'b'),
  delay(50, 'c'),
  delay(1000, 'd'),
])
.then(console.log, console.error) // [ a, b, c, d ]

all([
  delay(100, 'a'),
  delay(200, 'b'),
  Promise.reject(Error('bad things happened')),
  delay(50, 'c'),
  delay(1000, 'd'),
])
.then(console.log, console.error) // Error: bad things happened

But we can do better. Putting x and y in an array [] is an extra "thing". And the operation we are using in reduce, [ ...r, x ] is also a well-known function -

const and = (px, py) =>
  px.then(x => py.then(y => [ x, y ])) // hard-coded []

const all = (promises = []) =>
  promises.reduce
    ( (pr, px) =>
        and(pr, px)
          .then(([ r, x ]) =>
            [ ...r, x ]) // <-- is known as "append"
    , Promise.resolve([])
    )

Instead of locking x and y in an array, [ x, y ], we could specify any behaviour we want, ie doSomething(x, y) -

const and = (doSomething, px, py) =>
  px.then(x => py.then(y => doSomething(x, y)))

const myFunc = (a, b) =>
  (a + b) * 100

and(myFunc, Promise.resolve(1), Promise.resolve(2))
  .then(console.log, console.error) // 300

This implementation of and is well known by another name. lift2 allows us to take an ordinary function (eg append) and use it in (ie "lift it into") the Promise context -

// complete polyfill for Promise.all

const lift2 = (f, px, py) =>
  px.then(x => py.then(y => f(x, y)))

const append = (a = [], x = null) =>
  [...a, x]

const all = (promises = []) =>
  promises.reduce
    ( (pr, px) => lift2(append, pr, px)
    , Promise.resolve([])
    )

Expand the snippet to verify the results in your browser -

const lift2 = (f, px, py) =>
  px.then(x => py.then(y => f(x, y)))

const append = (a = [], x = null) =>
  [...a, x]

const all = (promises = []) =>
  promises.reduce
    ( (pr, px) => lift2(append, pr, px)
    , Promise.resolve([])
    )

const delay = (ms, x) =>
  new Promise(r => setTimeout(r, ms, x))

all([
  delay(100, 'a'),
  delay(200, 'b'),
  delay(50, 'c'),
  delay(1000, 'd'),
])
.then(console.log, console.error) // [ a, b, c, d ]

all([
  delay(100, 'a'),
  delay(200, 'b'),
  Promise.reject(Error('bad things happened')),
  delay(50, 'c'),
  delay(1000, 'd'),
])
.then(console.log, console.error) // Error: bad things happened

why is it named lift2?

It's called lift2 because the function we lifted (append in this example) expects two arguments. In typed languages, it's common to see lift3 and lift4 variants -

const lift3 = (f, px, py, pz) => // <-- 3 promises
  px.then(x => py.then(y => pz.then(z =>
    f(x,y,z) // <-- 3 values
  )))

const lift4 = (f, pw, px, py, pz) => // <-- 4 promises
  pw.then(w => px.then(x => py.then(y => pz.then(z =>
    f(w,x,y,z) // <-- 4 values
  ))))

const p = x => // <-- helper for demo
  Promise.resolve(x)

const add = (...numbers) =>
  numbers.reduce((a, b) => a + b, 0)

lift3(add, p(1), p(2), p(3))
  .then(console.log, console.error) // 6

lift4(add, p(1), p(2), p(3), p(4))
  .then(console.log, console.error) // 10

But JavaScript is a dynamic language and so we do not need such strict implementations. We can implement liftN which accepts a variadic function -

const append = (a = [], x = null) =>
  [...a, x]

const lift2 = f =>
  (px, py) => px.then(x => py.then(y => f(x, y)))

const liftN = (f, ...promises) =>
  promises
    .reduce(lift2(append), Promise.resolve([])) // <-- !
    .then(values => f(...values))

const p = x => // <-- helper for demo
  Promise.resolve(x)

const add = (...numbers) =>
  numbers.reduce((a, b) => a + b, 0)

liftN(add, p(1))
  .then(console.log, console.error) // 1

liftN(add, p(1), p(2))
  .then(console.log, console.error) // 3

liftN(add, p(1), p(2), p(3))
  .then(console.log, console.error) // 6

liftN(add, p(1), p(2), p(3), p(4))
  .then(console.log, console.error) // 10

Notice ! above. liftN is essentially the variadic version of all. The two can be used interchangeably -

const all = (promises = []) =>
  promises.reduce(lift2(append), Promise.resolve([]))

const liftN = (f, ...promises) =>
  all(promises).then(values => f(...values))

all([ p(1), p(2), p(3), p(4) ])
  .then(nums => add(...nums))
  .then(console.log, console.error) // 10

liftN(add, p(1), p(2), p(3), p(4))
  .then(console.log, console.error) // 10

liftN
  ( console.log
  , delay(100, 'a')
  , delay(200, 'b')
  , delay(50, 'c')
  , delay(1000, 'd')
  )
  .catch(console.error) // [ a, b, c, d ]

liftN
  ( console.log
  , delay(100, 'a')
  , delay(200, 'b')
  , Promise.reject(Error('bad things happened'))
  , delay(50, 'c')
  , delay(1000, 'd')
  )
  .catch(console.error) // Error: bad things happened

Expand the snippet below to verify the results in your browser -

const append = (a = [], x = null) =>
  [...a, x]

const lift2 = f =>
  (px, py) => px.then(x => py.then(y => f(x, y)))

const all = (promises = []) =>
  promises.reduce(lift2(append), Promise.resolve([]))

const liftN = (f, ...promises) =>
  all(promises).then(values => f(...values))

const delay = (ms, x) =>
  new Promise(r => setTimeout(r, ms, x))

liftN
  ( console.log
  , delay(100, 'a')
  , delay(200, 'b')
  , delay(50, 'c')
  , delay(1000, 'd')
  )
  .catch(console.error) // [ a, b, c, d ]

liftN
( console.log
, delay(100, 'a')
, delay(200, 'b')
, Promise.reject(Error('bad things happened'))
, delay(50, 'c')
, delay(1000, 'd')
)
.catch(console.error) // Error: bad things happened


Created a basic User Profile List using an API (React) I'm relatively new when it comes to programming in React and in JavaScript, but have a surface level understanding of what things do. I've created a basic program that fetches user data from an API that generates random users, and appends the users to a list when you click a button. I was wondering if what I'm writing is good enough to be considered for a Jr. React Dev position, and if there's anything I can improve upon the code such as implementation, name convention, readability, efficiency, etc. In particular, in the UserProfile component, I'm mapping through the state that holds the JSON object that was fetched from the API to output the profile picture and name. I'm not sure if there's anyway to do it better, but it doesn't feel quite right to me. Any feed back is much appreciated.
UserProfile.jsx
import React, { useState, useEffect } from "react";

const UserProfile = () => {
  const [userData, setUserData] = useState([]);

  const fetchUserData = () => {
    fetch("https://randomuser.me/api")
      .then((resp) => resp.json())
      .then((data) => setUserData(data.results));
  };

  //Creates userProfile with picture/full name
  const userProfile = (userMeta) => {
    const {
      name: { first, last },
      picture: { medium }
    } = userMeta;

    return (
      <>

          {first}, {last}

      </>
    );
  };

  //Fetch API data on component mount
  useEffect(() => {
    fetchUserData();
  }, []);

  return (
    <>
      {userData.map((userMeta, idx) => {
        return
{userProfile(userMeta)}
;
      })}
    </>
  );
};

export default UserProfile;

UserList.jsx
import React, { useState } from "react";
import UserProfile from "./UserProfile";

const UserList = () => {
  const [userList, setUserList] = useState([]);

  //Appends UserProfile to List
  const addUser = () => {
    setUserList((prevState) => {
      return [...prevState, <UserProfile />];
    });
  };

  return (

      <button onClick={addUser}>Add User</button>
      {userList.map((user, idx) => {
        return <React.Fragment key={idx}> {user} </React.Fragment>;
      })}

  );
};

export default UserList;

App.js
import React from "react";
import UserList from "./UserList";

export default function App() {
  return (

USER LIST COMPONENT

      <UserList />

  );
}

my two cents.

if what I'm writing is good enough to be considered for a Jr. React Dev position

This answer is completely subjective and depends on the company you're applying to. Each company has different expectations.
That being said, here are a list of improvements that I think (from my own personal experience) would make your profile stand out among other candidates for a job position.

Companies want developers with an understanding on the product. Did you simply implement the feature? Or did you go out of your way to think of important situations potential clients might encounter.

i.e.:

When you make API calls, did you use a spinner to let the user know that the list of users is being fetched from you API ?
Did you plan out/work on your design? Did you follow the design system?
Did you implement a global theme?

Avoiding writing pure text in your components, think ahead and start putting them in localized files so that you're one step ahead if the website needs translation.

Don't define a component within a component unless you don't have any other choice (i.e.: UserProfile / userProfile)

UserProfile (singular) returns a list of userProfile's? Doesn't seem right.

I think I would rename UserList --> Users / UserProfile --> UserList / userProfile --> UserProfile

Did you write tests ?


Creating indexes from a theoretical decimal/int, splitting into before and after arrays I'm currently populating before and after arrays with indexes, based on a number provided. If the input going in is an int, there will be three indexes (left, middle and right). If it is a decimal, there will just be two (left, right). There will me a maximum of three indexes to sort into a maximum of two arrays (before and after). To understand, here is some expected input/output:

input: 1
output: before [] after [0, 1, 2]

input: 1.5
output: before [] after [1, 2]

input: -3.4
output: before [3, 4] after []

input: 0
output: before [1] after [0, 1]

Essentially, the input index gets floored and ceiling'd (or, in the case of a whole number, their next/previous integers are used along with the original input). If any of the resulting integers are below 0, they get put in the before array, but their index is made absolute, otherwise they go into the after array. If they do get put into the before array, then they are put in to reverse order, such that they remain sorted in numerical order.

I have the below working code. But I feel like I could do much better. How would the community go about optimising this?

var input = document.getElementById('input');
var button = document.getElementById('button');
var ouput = document.getElementById('output');

function getOutput(input) {

  var left = Math.floor(input);
  var right = Math.ceil(input);
  var middle;

  var before = [];
  var after = [];

  if (right == left)
    left--,
    middle = left + 1,
    right = left + 2;

  if (left < 0)
    before.unshift(Math.abs(left));
  else
    after.push(left);

  if (middle < 0)
    before.unshift(Math.abs(middle));
  else if (typeof middle == 'number')
    after.push(middle);

  if (right < 0)
    before.unshift(Math.abs(right));
  else
    after.push(right);

  return {before: before, after: after};

}

function buttonPressed () {

  var i = parseFloat(input.value);
  var msg = 'not a number';

  if (!isNaN(i)) {

    var o = getOutput(i);

    msg = 'before [' + o.before.toString() + '] ahead [' + o.after.toString() + ']';

  }

  ouput.innerHTML = msg;

}
<input id="input" type="text">
<input id="button" type="button" value="get" onclick="javascript:buttonPressed();">

My interest is in the getOutput method. The other stuff is for demonstration purposes.

Update

I have adjusted the expected output and the code from the original question, after being prompted to rethink from the comments.

The code in the question seems to have adequate complexity for the getOutput function. Some small hints may be given, like:

do not use comma-expressions and decrement,
initialize middle with null and check for it with !== instead of typeof,
also, I do not like comparing undefined middle with 0.

The current code is more or less readable.

As for optimizations, one observation is that apart from [-1, 1] range, all other inputs always use the same alternative, making unnecessary to check left, right and middle individually. (eg, if right < 0, then so are middle and lift). How much optimization it really brings is hard to tell. If input numbers are almost always large, then making separate branch and constructing an array directly [left, middle, right] may make the code more efficient.

The near-zero case may need more conditions, of course (or just some kind of lookup for ready arrays - as the number of cases is small).

If you want a more compact code, maybe something like this can be done:

 tmparray = ((right == left) ? [left-1, left, left+1] : [left, left+1]);

And after that push/unshift in a loop for each tmparray element. Not sure this will be faster though, but at least may be more readable. (left may be renamed to "lower", and right calculated inline only in the condition).


Building an ACL based on permission level I'm building a symfony project and at some point I've come up with a switch case to manage the acl rights. I would prefer using a dynamic access to the constant but havn't find a good solution. I've seen reflection but that doesn't seem to be the proper solution to me.

$mask = null;

switch ($participant->getRight()) {
    case 'VIEW':
        $mask = MaskBuilder::MASK_VIEW;
        break;
    case 'EDIT':
        $mask = MaskBuilder::MASK_EDIT;
        break;
    case 'OPERATOR':
        $mask = MaskBuilder::MASK_OPERATOR;
        break;
    default:
        break;
}
$this->aclManager->addObjectPermission($project, $mask, $user);

So, this switch is in a foreach, that gets the $user entity, then with the result of the list box from the view, I get the rights of the user which can be (for now) EDIT VIEW or OPERATOR. These right were chosen because they are directly related to the MaskBuilder masks.

The thing is, if there is a refactor to be done, I will need to modify this switch AND the part where the form is defined. I would like to do something like

$mask = 'MASK_' . $participant->getRight();
$this->aclManager->addObjectPermissions($project, MaskBuilder::$mask, $user);

But havn't find a way to do it.

Is my switch a good way to do it ? Or is there a proper php way to do the same, that would reduce refactor cost?

I found the solution somewhere from the documentation.  I don't know why I had not seen that before.

Symfony2 provides an easier way of doing what I was looking for:

$builder = new MaskBuilder();
$mask = $builder->add($participant->getRight())->get();


Displaying a person's personal information with formatting I am learning C# from a book, and there is an exercise as follows:

  Write a program that has a person enter full name, age, and phone
  number. Display this information with formatting. Also display the
  person's initials.

I wrote some code and I feel that it could be improved, so I ask anyone to comment on it.

using System;
using System.Collections.Generic;

internal class Person
{
    public string fname;
    public string mname;
    public string lname;
    public int age;
    public string phnumber;

    public Person()
    {
        fname = KeyInput.keyIn("\n\nPlease enter your first name");
        if (fname.ToLower() == "exit") return;

        mname = KeyInput.keyIn("\nPlease enter your middle name");
        lname = KeyInput.keyIn("\nPlease enter your last name");

        while (true)
        {
            try
            {
                age = Convert.ToInt32(KeyInput.keyIn("\nPlease enter your age"));
                break;
            }

            catch (ArgumentException)
            {
                Console.WriteLine("No value was entered");
            }

            catch (FormatException)
            {
                Console.WriteLine("You didn't entered a valid number");
            }

            catch (Exception e)
            {
                Console.WriteLine("Something went wrong with the conversion.");
            }
        }

        phnumber = KeyInput.keyIn("\nPlease enter your  phone number");
    }

    public void personDataDisplay()
    {
        Console.WriteLine("\nYour name is: " + fname + " " + mname[0] + "." + " " + lname);
        Console.WriteLine("Your initials are: " + fname[0] + mname[0] + lname[0]);
        Console.WriteLine("Your age is: " + age);
        Console.WriteLine("Your phone number is: " + phnumber + "\n");
    }
}

internal class KeyInput
{
    public static string keyIn(string scrtext)
    {
        Console.WriteLine(scrtext);
        string buffer = Console.ReadLine();
        return buffer;
    }
}

internal class MyApp
{
    public static void Main()
    {
        Console.WriteLine("Please enter data for person(s). Enter EXIT to end.");

        List<Person> list_persons = new List<Person>();

        while (true)
        {
            Person person = new Person();
            list_persons.Add(person);

            if (person.fname.ToLower() == "exit")
                break;
        }

        foreach (Person person in list_persons)
        {
            if (person.fname.ToLower() == "exit")
                break;

            person.personDataDisplay();
        }
    }
}

In designing classes we should try and confirm to the Single Responsibility Principle. Each class should only do one thing. Sometimes defining what makes a single responsibility is hard, but your Person class clearly does two things: it represents a person, and it questions the user via the console. This makes it difficult to reuse your Person class: what if you wanted to create a Person from a database, or a file or a web request? You would be better splitting this into a PersonQuestionnaire class which is responsible for creating a Person from a console input.

You should never have any significant logic in your constructor like you have here. Design your constructors to simply save any parameters and set up any default values. Any logic involving external classes, such as the Console, should definitely be placed in another method that is called once the object is constructed.

You should consider using the int.TryParse() method to read the age. Catching exceptions is slow (you will notice you ever write applications that are doing a lot of this type of thing), verbose, and sometimes unpredictable. You should definitely avoid catching the top level Exception type other than where you can do something meaningful: doing a lot of this will make your applications very difficult to debug.


Random number guessing game I'm working on a random number guessing game.  I'm done with the code and it's working, but there is a part that I want to make better.  I declared an instance of a Guess class I created, and now how to make this part more efficient.

int counter = 0;
do
{
    myGuess.UserGuess = GetUserGuess(); //read user guess
    if (myGuess.Compair() == "match")
    {
        Console.WriteLine("\n\t Correct!You WIN !");
    }

    else if (myGuess.Compair() == "high")
    {
        if (counter < 3)
            Console.WriteLine("\n\tTry a lower number,");
        else
            Console.WriteLine("\n\tSorry you LOSE !, The right number is " + myGuess.RndNum);

        counter++;
    }

    else if (myGuess.Compair() == "low")
    {
        if (counter < 3)
            Console.WriteLine("\n\tTry a higher number,");
        else
            Console.WriteLine("\n\tSorry you LOSE !, The right number is " + myGuess.RndNum);
        counter++;
    }

} while (myGuess.Compair() != "match" && counter < 4);

For example, I used the same message the same condition twice, which I think is not the best way.  Any way to loop better than that?

I think a way to make this code generally more efficient and predictable is to have your classes have a more defined role.  For example, it seems like you're defining an instance of Guess as myGuess, then externally loading in a random number (RndNum), and the current guess (UserGuess = GetUserGuess()).

You should clearly define the goals for classes, such as:

Manage the Game lifecycle.
Handle user input, ensuring the user enters only numbers and is properly prompted, etc.

In this way, your code might look more like this psuedo-code:

class GuessingGame
{
    static void Main(string[] args)
    {
        game = new GuessingGame();
        game.run();
    }

    int numGuesses;
    int correctNumber;
    UserInputHandler userInputHandler;

    GuessingGame(numGuesses = 4)
    {
        Random random = new Random();
        this.correctNumber = random.Next(10);
        this.numGuesses = numGuesses;
        this.userInputHandler = new UserInputHandler();
    }

    void run()
    {
        for (int thisGuess = 0; thisGuess < numGuesses; ++thisGuess)
        {
            Console.WriteLine(string.Format("Guess #{0}!\n", thisGuess + 1));
            guess = this.userInputHandler.getGuess();
            if (guess < this.correctNumber)
            {
                Console.WriteLine("WRONG! Try a higher number!");
            }
            else if (guess > this.correctNumber)
            {
                Console.WriteLine("WRONG! Try a lower number!");
            }
            else
            {
                win();
                return();
            }
        }
        lose();
    }

    void win()
    {
        Console.WriteLine("You WIN!  The number was indeed " + this.correctNumber);
    }

    void lose()
    {
        Console.WriteLine("You LOSE!  The number was " + this.correctNumber);
    }
}

// UserInputHandler Source
class UserInputHandler
{
    public int getGuess()
    {
        // Handle user input, and ensure the guesses are actually numbers.
    }
}

The idea is that each class is responsible for encapsulating one piece of functionality around another class.  The Main program entry point doesn't care about how the game works, the GuessingGame class doesn't care about interacting with the user at all, while the UserInputHandler class doesn't care about the correct answer, etc.

I've also wrapped the win/loss messages into a single method, so that you're not repeating the victory message.

Some benefits include:

Having a clear understanding of the role of each class.
Being able to assert that with unit tests easily.
Code reuse.  You could plug the UserInputClass into a different game that requires simple user input, for example.
Collaboration.  Properly scoped and documented code means other developers could jump into your project with less friction.

Now this example is a bit much considering that a guessing game is fairly basic, but it's good practice for tackling larger projects.


Grade of Service Probability Function Python Consider the following typical probability scenario:

I defined this function to handle that scenario, I'm curious if Python has a more efficient method to handle this, or if this is the best way:
from scipy.special import binom

def grade_of_service(n, p, c):
    prob = 0
    k = c + 1
    while k <= n:
        prob += binom(n, k)*(p**k)*((1-p)**(n-k))
        k += 1
    return prob

EDIT: Here's an example solution from the author of this example:  "If n = 100, p=0.1, and c=15, the probability of interest turns out to be 0.0399."
My probability does round to this result.  But due to the roundoff error / numerical precision, it doesn't seem to matter if k = n or k = n+1.

Pure Python

You should use a for loop rather than a while loop.
You can use a generator comprehension to build all prob's.
You can use sum to get the sum.

def grade_of_service(n, p, c):
    return sum(binom(n, k)*(p**k)*((1-p)**(n-k)) for k in range(c+1, n+1))

Numpy and friends

Use numpy.arange rather than range.
Write out the equation same as above, just not in a comprehension.
Change sum to numpy.sum.

import numpy

def grade_of_service(n, p, c):
    k = numpy.arange(c+1, n+1)
    return numpy.sum(binom(n, k)*(p**k)*((1-p)**(n-k)))

This has a problem with numbers that exceed a certain size, as numpy numbers are finite.


Programmatically close optical drive tray I really liked the close tray program that CD-ROM drivers programs used to include in MS-DOS days.
Since I live in a place where even getting an internship is impossible, I've decided to learn by myself.
I implemented this "Hello World" C# script to do that with a twist.
It will check the optical drive status and if open, it will close it, otherwise it will tell you that a disk is present.
Note, I went through Stack Overflow and couldn't find a way to programmatically find if the tray is open.
Another note, it will not close the tray on most laptops where such mechanism does not exist.
And finally, I used an online code beautifier for indentation.
using System;
using System.Runtime.InteropServices;
using System.Text;
using System.Windows.Forms;
using System.Management;

class Program {
[DllImport("winmm.dll")] protected static extern int mciSendString(string Cmd, StringBuilder StrReturn, int ReturnLength, IntPtr HwndCallback);

  static void Main(string[] args) {

  ManagementObjectSearcher searcher = new ManagementObjectSearcher("SELECT MediaLoaded FROM Win32_CDROMDrive");
  ManagementObjectCollection moc = searcher.Get();
  var enumerator = moc.GetEnumerator();
  if (!enumerator.MoveNext()) throw new Exception("No elements");
  ManagementObject obj = (ManagementObject) enumerator.Current;
  bool status = (bool) obj["MediaLoaded"];

  if (!status) {
    MessageBox.Show("The drive is either open or empty", "Optical Drive Status");
    mciSendString("set cdaudio door closed", null, 0, IntPtr.Zero);
  }
  else MessageBox.Show("The drive is closed and contains an optical media", "Optical Drive Status");
 }
}

Note that the Visual Studio C# code editor has an integrated code beautifier. Depending on your setup, it can be executed with different shortcut keys. Call it from the menu for the first time, so that you can see the active shortcut keys (it is Ctrl-E-D for me). Menu: Edit > Advanced > Format Document.
By using a technique called LINQ (Language INtegrated  Query), you can get the management object easier. It uses extension methods from the namespace System.Linq. Therefore, you must include a using System.Linq; at the top of your code. Then you can query with
ManagementObject managementObject = moc.FirstOrDefault();
if (managementObject == null) {
     // Handle error (output text or throw exception)
     Console.WriteLine("Could not find a management object!");
} else {
     ...
}

This FirstOrDefault extension method calls GetEnumerator and MoveNext internally and returns null if no object is available.
Do not give it the name obj. Almost everything is an object, so this name is not very informative.
Exceptions are a complex matter. It raises a lot of questions like "should I throw an exception or output a message to the user?", "should I create my own exception types?", "should I log the exception?" etc., etc. There is no single best answer to these questions. Therefore, I content myself with giving you a link: C# Exception Handling Best Practices.
If you intend to reuse this functionality, you could encapsulate it into your own class:
public enum OperationResult
{
    Failure,
    NoDriveFound,
    DriveIsOpenOrEmpty,
    MediaIsLoaded
}

public class OpticalDriveCloser
{
    public OperationResult CloseFirst()
    {
        ...
    }

    public OperationResult[] CloseAll()
    {
        ...
    }
}

It returns an operation status as enumeration type and lets the calling application decide how to proceed. I.e. it does neither call Console.WriteLine nor throw exceptions and focuses on pure, non-UI logic. A console application will have a different UI-logic than a windows forms application or a web application.


SQL percentage calculation I have created a query in which there is a column that calculates a percentage:

SELECT
     u.User_FirstName
    ,l.[Total Leads]
    ,SUM(s.SalesCount) AS [New Clients]
    ,c.Target
    ,CONVERT(VARCHAR, CONVERT(DECIMAL(11,0),ROUND((CONVERT(DECIMAL(11,2), SUM(s.SalesCount)) / l.[Total Leads]) * 100, 0))) + '%'

As you can see, that last column is my percentage column, which uses a lot of CONVERT.  I was wondering if there is a simpler or faster way to get a two-digit percentage in SQL Server 2008.

I feel that rofl pretty well covered how this could be done differently, but I want to point out what I feel to be an issue.

You really shouldn't be formatting the decimal results as a string. Formatting should be done in whatever application consumes the data you're sending it. As a user, I would be pretty upset to receive a "percentage" that I couldn't do any mathematical analysis on without first splitting out the "%" character, dividing by 100, and then formatting it as a percentage natively.

Select CONVERT(decimal(5,0), 1.0 * SUM([SalesCount]) / [Total Leads])


A BFS and DFS implementation I believe this code prints both the DFS and BFS of a directed graph. As with some of my previous posts, this is mainly to share my code with other people working on a similar issue but I would also appreciate any feedback.

In the code I have written, DFS and BFS use a pre-order technique, as follows:

DFS (Depth first search) starts with a given node and explores the first unexplored node it comes across before returning to itself again and exploring its remaining nodes (e.g: if the parent node 1 has 2 children 2, 3 the DFS method will explore 2 and its children nodes before exploring 3. It will print self before exploring its children (so 1->(2,3) will print 1,2,3))
BFS (Breadth first search) works down a tree in a top-to-bottom manner (e.g: a graph with parent 1 and children 2, 3 will print level 1 first (1) then level 2 (2, 3) and then level 3 (the children of nodes 2 and 3). The level of a given node is determined by the highest level it could appear on (e.g: if 2 is a child of an item on level 1 and level 4, it would be printed as if it were a level 2 item)

from collections import defaultdict

class Graph():
    def __init__(self):
        self.value = defaultdict(list)

    def addConnection(self, parent, child):
        self.value[parent].append(child)

    def DFS(self, start):
        visited = [start]
        stack = [start]
        print(start, end = " ")
        while stack:
            s = stack[-1]
            if any([item for item in self.value[s] if item not in visited]):
                for item in [item for item in self.value[s] if item not in visited]:
                        stack.append(item)
                        visited.append(item)
                        print(item, end= " ")
                        break
            else:
                stack.pop()

    def BFS(self, start):
        visited = [start]
        queue = [start]
        while queue:
            x = queue.pop(0)
            print(x, end= " ")
            for item in self.value[x]:
                if item not in visited:
                    queue.append(item)
                    visited.append(item)

#Build the graph
g=Graph()
g.addConnection(1,4)
g.addConnection(1,2)
g.addConnection(2,3)
g.addConnection(2,6)
g.addConnection(4,5)
g.addConnection(4,7)
g.addConnection(7,96)

#Explore the graph
g.DFS(1)
print("\n")
g.BFS(1)

Output is

DFS: 1 4 5 7 96 2 3 6
BFS: 1 4 2 5 7 3 6 96

Adding a (2,4) node gives

DFS: 1 4 5 7 96 2 3 6
BFS: 1 4 2 5 7 3 6 96

Clarification (OK)
After update of your question, it has gotten clear that:

you work on a DAG, which includes a Tree
both your strategies DFS and BFS are in Pre-Order

=> original answer requesting clarification from the OP
Review Definitions
Before being able to review your code, I would like to review your definitions. Perhaps you should clarify your algorithm with examples.

I believe this code prints both the DFS and BFS of a directed graph.

How do you decide the breadth level of a node when it has multiple parents? Or would you only work with Tree instead of DAG?

As a reminder of the difference:
DFS (Depth first search) starts with a given node and explores the
first unexplored node it comes across before returning to itself again
and exploring its remaining nodes (e.g: if the parent node 1 has 2
children 2, 3 the DFS method will explore 2 and its children nodes
before exploring 3

Because you say to explore remaining nodes before returning to self, you are not very clear whether the order is A or B.

(A) 1 -> 2 -> 3
(B) 2 -> 3 -> 1

BFS (Breadth first search) works down a tree in a top-to-bottom manner
(e.g: a graph with parent 1 and children 2, 3 will print level 1 first
(1) then level 2 (2, 3) and then level 3 (the children of nodes 2 and
3)

The order is independant of search strategy (DFS/BFS). BFS can be both top-to-bottom or bottom-to-top.

Terminology

DFS: process each child completely before processing the next child
BFS: process each level across childs before processing the next level
Pre-Order: process self before rest of tree
Post-Order: process rest of tree before self

In your example of parent 1 having child 2 and child 3:

DFS Pre-Order: 1 -> 2 -> 3
DFS Post-Order: 2 -> 3 -> 1
BFS Pre-Order: 1 -> 2 -> 3
BFS Post-Order: 2 -> 3 -> 1

Suppose 2 would have its own child 4:

DFS Pre-Order: 1 -> 2 -> 4 -> 3
DFS Post-Order: 4 -> 2 -> 3 -> 1
BFS Pre-Order: 1 -> 2 -> 3 -> 4
BFS Post-Order: 4 -> 2 -> 3 -> 1

You could even add a third dimension direction in which case we distinguish left-to-right and right-to-left.


Reading and printing a text file in Java I am trying to read lines from a text file in Java, but there is a lot going on. Is this the best way to do this? I am especially concerned with resource leaks.

import java.io.*;

public class Filing {
    public static void main(String[] args) throws FileNotFoundException, IOException {
        FileReader fr = new FileReader("/Users/thomaspovinelli/Desktop/apps.txt");
        BufferedReader br = new BufferedReader(fr);
        String buffer;
        String fulltext="";
        while ((buffer = br.readLine()) != null) {
            System.out.println(buffer);
            fulltext += buffer;
        }
        br.close();
        fr.close();
    }
}

import java.io.*;

Never import the full package. It is considered bad practice to do so. Instead, import what you need and only what you need.

Instead of just reading a file then printing it, why don't you create a method that reads all line of a file and returns a list of all the lines? This way, you can do more with the result:

public static List<String> getAllLines(File file) throws FileNotFoundException, IOException {
    FileReader fr = new FileReader(file);
    BufferedReader br = new BufferedReader(fr);
    String buffer;
    String fulltext="";
    while ((buffer = br.readLine()) != null) {
        System.out.println(buffer);
        fulltext += buffer;
    }
    br.close();
    fr.close();
}

  I am especially concerned with resource leaks.

Since Java 7, there is such thing as a try-with-resources statement, which automatically closes the reader when an exception occurs, which you are not doing here. Your current code has memory leaks!

With some other improvements:

public static List<String> readAllLines(File file) {
    List<String> result = new LinkedList<>();
    try (BufferedReader reader = new BufferedReader(new FileReader(file))) {
        for (String current = reader.readLine(); current != null; current = reader
                .readLine()) {
            result.add(current);
        }
    } catch (IOException e) {
        e.printStackTrace();
    }
    return result;
}

By the way, this is actually the code I use often to read all lines of a file.


Hamming distance between numbers in JavaScript In Leetcode it states that my runtime is only faster than 38% all of submitted JavaScript solutions. Is there anything I can change to make it more efficient?

/**
 * @param {number} x
 * @param {number} y
 * @return {number}
 */
var hammingDistance = function(x, y) {
    var resultStr =( x^y);
    let count =0;
    while(resultStr>0){
       resultStr =(resultStr&resultStr-1) ;
       count++;
    }
     return count;
};

A few things could be changed here, non of which I mention are optimisation however. Your solution is almost identical to the example on the wiki page where you can see hardware optimisations if supported, though the example does not apply to JavaScript.

Naming

The Str in resultStr seems to imply that it's a string but that's not the case. result may be better suited, or val which is a common choice for this.

While count is perfectly fine, distance might make it's purpose more obvious (especially since "distance" is in the function name).

Parentheses

There's a bunch of parentheses that aren't needed, adding spacing around operators makes it easier to follow.

Shorthand operators/spacing

You can take advantage of shorthand operators:

resultStr = resultStr & resultStr - 1 can be simplified to resultStr &= resultStr - 1

Another added benefit is that in this example we don't have to worry about operator precedence.

ES6

Since you're using ES6 features such as let you can take advantage additional features such as const and => (arrow functions).

ES6 - const/let

Favour let and const over var. You are already declaring count with let so it would make sense to do the same for resultStr.

If we declare const hammingDistance = ... it means that if we later try and reassign hammingDistance = ... we will get a TypeError.

I recommend using const whenever you don't need to reassign a variable. Note this does not mean the variable is immutable, just that it cannot be reassigned.

ES6 - arrow functions

I've opted to use arrow function notation over traditional syntax here. Your example does not benefit from any of the advantages such as lexical this so feel free to change back to function(x, y) as this is a personal choice.

Solution

Here's your code with the suggested changes:

const hammingDistance = (x, y) => {
  let val = x ^ y;
  let distance = 0;

  while (val > 0) {
    val &= val - 1;
    distance++;
  }

  return distance;
};

For loop solution

If you wanted, you could replace the while loop with a for loop as follows:

const hammingDistance = (x, y) => {
  let val = x ^ y;

  let distance;
  for (distance = 0; val > 0; distance++) {
    val &= val - 1;
  }

  return distance;
};


Generic methods for serializing and deserialzing xml files using streams using System.IO;
using System.Text;
using System.Xml;
using System.Xml.Serialization;

namespace Common.Extensions
{
    /// <summary>
    /// Contains the logic for streaming extensions.
    /// </summary>
    public static class StreamExtensions
    {
        /// <summary>
        /// Serialize an object.
        /// </summary>
        /// <typeparam name="T">The type of the object that gets serialized.</typeparam>
        /// <param name="stream">The stream to which the bytes will be written.</param>
        /// <param name="serializableObject">The object that gets serialized.</param>
        public static void SerializeObject<T>(this Stream stream, T serializableObject) where T : IXmlSerializable
        {
            var writerSettings = new XmlWriterSettings();

            writerSettings.Indent = true;
            writerSettings.IndentChars = "    "; // 4 spaces
            writerSettings.Encoding = Encoding.UTF8;

            using (var xmlWriter = XmlWriter.Create(stream, writerSettings))
            {
                serializableObject.WriteXml(xmlWriter);
            }

            stream.Close();
        }

        /// <summary>
        /// Deserialize a stream and return the object.
        /// </summary>
        /// <typeparam name="T">The type of the object that returns from the deserialization.</typeparam>
        /// <param name="stream">The stream which contains the bytes to deserialize.</param>
        /// <returns>The object recovered.</returns>
        public static T DeserializeObject<T>(this Stream stream) where T : IXmlSerializable
        {
            using (var xmlTextReader = XmlReader.Create(stream))
            {
                var serializer = new XmlSerializer(typeof(T));
                T result = (T)serializer.Deserialize(xmlTextReader);
                stream.Close();
                return result;
            }
        }
    }
}

I have made a class which can serialize a generic object which implements the interface IXmlSerializable and can deserialize an xml file to that same object. The idea behind this is as follows: we are using a lot of xml files to (for example) write configurations to. Since the configuration classes are often different we needed a generic function to create and read the xml files.

The stream we use (atm) is a file stream since it is currently only used for reading and writing the xml files to and from disk. Later we might use other kind of streams as well.

I am wondering if what I have written is the correct way (obviously it works but can it be made better? )

writerSettings.IndentChars = "    "; // 4 spaces

by using

writerSettings.IndentChars = string.Empty.PadRight(4, ' ');

you wouldn't need that noisy comment.

One wouldn't expect that a passed in Stream get closed inside that method. The closing of a Stream should only belong to the creator of that stream.

In extension methods you should always check the argument which is reffered by this if it is null.
This is because that method can also be called like so

StreamExtensions.SerializeObject<someType>(null, someObject);

By checking (stream == null) and throwing an ArgumentNullException no unnecessary objects like XmlWriterSettings will be created.

Otherwise than the mentioned points your code looks clean and is easy to read.


Sticky element for a sidebar widget I have this JavaScript to make a sidebar widget stick to a fixed position once the page is scrolled down, once you scroll to the bottom of the page where the footer div is I then un-sticky it so that the stickied div does not overlap the footer.

It works great but I am not very good with JavaScript, so I am hoping to clean it up because it looks sloppy.

JavaScript

$(window).load(function () {
/*-----------------------*/
 * Fixed Widget on Scroll
 *-----------------------*/

    // set some Div vars
    var stickyID = $("#sticky-scroll-wrapper"),
        footerID = $('#footer'),
        stickyDivHeight = stickyID.outerHeight(),
        stickyIDMargin = 70;

    $(window).scroll(function () {
        var currentPosition = $(document).scrollTop() + stickyIDMargin;
        var stickyIDPosition = stickyID.offset().top;

        // cut off point/un sticky when we reach this far down the page
        var limit = footerID.offset().top - stickyDivHeight;

        //determines whether sidebar should stick and applies appropriate settings to make it stick
        if (currentPosition >= stickyIDPosition && currentPosition < limit) {
            $('#sticky-elements').css({
                'position': 'fixed',
                'top': '70px'
            });
        } else {
            $('#sticky-elements').css({
                'position': 'static',
            });
        }
        // Un-stick once we reach the #fotter section of the page to prevent overlapping
        if (limit < currentPosition) {
            var diff = limit - currentPosition;
            $('#sticky-elements').css({
                'top': diff + stickyIDMargin,
                'position': 'fixed',
            });
        }

    });

});

HTML

sidebar content

I can make a small improvement in your logic structure that may make a very small improvement in performance. If limit < currentPosition, all you need to do is that code block, and you can use else if and remove testing its opposite. See code:

$(window).load(function () {
/*-----------------------*/
 * Fixed Widget on Scroll
 *-----------------------*/

    // set some Div vars
    var stickyID = $("#sticky-scroll-wrapper"),
        footerID = $('#footer'),
        stickyDivHeight = stickyID.outerHeight(),
        stickyIDMargin = 70;

    $(window).scroll(function () {
        var currentPosition = $(document).scrollTop() + stickyIDMargin;
        var stickyIDPosition = stickyID.offset().top;

        // cut off point/un sticky when we reach this far down the page
        var limit = footerID.offset().top - stickyDivHeight;

        // Un-stick once we reach the #fotter section of the page to prevent overlapping
        if (limit <= currentPosition) {
            var diff = limit - currentPosition;
            $('#sticky-elements').css({
                'top': diff + stickyIDMargin,
                'position': 'fixed',
            });
        }
        //determines whether sidebar should stick and applies appropriate settings to make it stick
        else if (currentPosition >= stickyIDPosition) {
            $('#sticky-elements').css({
                'position': 'fixed',
                'top': '70px'
            });
        }
        else {
            $('#sticky-elements').css({
                'position': 'static',
            });
        }
    });

});

Also, you might consider consolidating var diff = limit - currentPosition; with 'top': diff + stickyIDMargin if you are comfortable with that.
Otherwise, it looks great to me. :)


JavaScript Spiral Matrix Coding Challenge A whiteboarding challenge: Given a 2D array (matrix) inputMatrix of integers, create a function spiralCopy that copies inputMatrix's values into a 1D array in a clockwise spiral order. Your function then should return that array.

inputMatrix = [
  [1, 2, 3, 4, 5, 6],
  [7, 8, 9, 10, 11, 12],
  [13, 14, 15, 16, 17, 18],
  [19, 20, 21, 22, 23, 24],
  [25, 26, 27, 28, 29, 30],
  [31, 32, 33, 34, 35, 36]
]

const spiralCopy = (matrix) => {
  const spiralLength = matrix.length * matrix[0].length;
  const spiral = [];

  let iLeftRightStart = 0; // +1
  let iLeftRightEnd = matrix[0].length; // -1
  let leftRightPosition = 0; // +1
  let iTopBottomStart = 1; // +1
  let iTopBottomEnd = matrix.length; // -1
  let topBottomPosition = matrix[0].length - 1; // -1
  let iRightLeftStart = matrix[0].length - 2; // -1
  let iRightLeftEnd = 0; // +1
  let rightLeftPosition = matrix.length - 1; // -1
  let iBottomTopStart = matrix.length - 2; // -1
  let iBottomTopEnd = 1; // +1
  let bottomTopPosition = 0; // +1

  const leftToRight = (iStart, iEnd, position) => {
    for (let i = iStart; i < iEnd; i++) {
      spiral.push(matrix[position][i]);
    }
  }

  const topToBottom = (iStart, iEnd, position) => {
    for (let i = iStart; i < iEnd; i++) {
      spiral.push(matrix[i][position]);
    }
  }

  const rightToLeft = (iStart, iEnd, position) => {
    for (let i = iStart; i >= iEnd; i--) {
      spiral.push(matrix[position][i]);
    }
  }

  const bottomToTop = (iStart, iEnd, position) => {
    for (let i = iStart; i >= iEnd; i--) {
      spiral.push(matrix[i][position]);
    }
  }

  while (spiral.length < spiralLength) {
    leftToRight(iLeftRightStart, iLeftRightEnd, leftRightPosition);
    topToBottom(iTopBottomStart, iTopBottomEnd, topBottomPosition);
    rightToLeft(iRightLeftStart, iRightLeftEnd, rightLeftPosition);
    bottomToTop(iBottomTopStart, iBottomTopEnd, bottomTopPosition);
    iLeftRightStart++;
    iLeftRightEnd--;
    leftRightPosition++;
    iTopBottomStart++;
    iTopBottomEnd--;
    topBottomPosition--;
    iRightLeftStart--;
    iRightLeftEnd++;
    rightLeftPosition--;
    iBottomTopStart--;
    iBottomTopEnd++;
    bottomTopPosition++;
  }
  return spiral;
}
console.log(spiralCopy(inputMatrix));
// prints [1, 2, 3, 4, 5, 6, 12, 18, 24, 30, 36, 35, 34, 33, 32, 31, 25, 19, 13, 7, 8, 9, 10, 11, 17, 23, 29, 28, 27, 26, 20, 14, 15, 16, 22, 21]

So the function works and runs with time complexity of O(N). However, I don't like that I'm using 12 different variables which I then increment/decrement. How can this be consolidated, and is it possible to make the algorithm even faster due to the instant access of JavaScript arrays?

Travel directions
You can imagine this problem being extended to a 3D array. That would mean you have 6 directions, which using your approach would mean half as much code again, and 4D array would have 8 directions. That would double the amount of code. This becomes impractical as you get to higher dimensions.
If you break the problem into a simpler model. Move in a direction for distance collecting items as you go then turn, every second turn decrease the distance, until zero.
A direction can be represented as a vector {x,y} that is right {1,0}, down {0,1} and so on. You store them in an array of vectors in the order you need to use.
I have solved assuming that the array given is a rhombus . You have not indicated if this is true or not. However it only requires a small modification to handle non rhombus arrays.
The result is something like
function spiral(arr) {
    var count, curDir, dist, size, x = 0, y = 0, dir = 0;
    const res = [], directions = [{x: 1, y: 0}, {x: 0, y: 1}, {x: -1, y: 0}, {x: 0, y: -1}];
    curDir = directions[0];
    dist = size = arr.length;
    count = size ** 2;
    while (count--) {
        res.push(arr[y][x]);
        if (!--dist) {
            curDir = directions[++dir % 4];
            if (dir % 2) { size -- }
            dist = size;
        }
        x += curDir.x;
        y += curDir.y;
    }
    return res;
}

To test the function create some readable test arrays. The snippet has arrays that when processed will have values in order, so that it is easy to see if the result is correct.

[[
      [1, 2, 3, 4, 5, 6],
      [20, 21, 22, 23, 24, 7],
      [19, 32, 33, 34, 25, 8],
      [18, 31, 36, 35, 26, 9],
      [17, 30, 29, 28, 27, 10],
      [16, 15, 14, 13, 12, 101]
  ], [
      [1, 2, 3, 4, 5],
      [16, 17, 18, 19, 6],
      [15, 24, 25, 20, 7],
      [14, 23, 22, 21, 8],
      [13, 12, 11, 10, 9],
  ], [
      [1, 2, 3, 4],
      [12, 13, 14, 5],
      [11, 16, 15, 6],
      [10, 9, 8, 7],
  ], [
      [1, 2, 3],
      [8, 9, 4],
      [7, 6, 5],
  ], [
      [1, 2],
      [4, 3],
  ], [[1]]
].reverse().forEach(arr => console.log(spiral(arr).join(",")));

function spiral(arr) {
    var count, curDir, dist, size, x = 0, y = 0, dir = 0;
    const res = [], directions = [{x: 1, y: 0}, {x: 0, y: 1}, {x: -1, y: 0}, {x: 0, y: -1}];
    curDir = directions[0];
    dist = size = arr.length;
    count = size ** 2;
    while (count--) {
        res.push(arr[y][x]);
        if (!--dist) {
            curDir = directions[++dir % 4];
            if (dir % 2) { size -- }
            dist = size;
        }
        x += curDir.x;
        y += curDir.y;
    }
    return res;
}


Iterate over filename My program inspects filenames which are delimited via underscores. So an example may be:

  This_is_an_example_of_my_filename.tif

A counter example may be:

  This_is_a_very_bad_document.tif

The filename should always consist of 7 distinct values within the filename. Those values are used to create an instance of a class object which allows me to further validate data based on segment location in the filename.

The program should only consider *.tif files in the directory, and ignore all others.

If the filename is invalid (too few/too many fields, or field data is improperly formatted) we move it to an error bin. If it is good, I upload the data to a database and move the file on it's merry way.

The code that pulls these filenames apart looks like:

foreach(string file in Directory.GetFiles(path))
{
    FileInfo fi = new FileInfo(file)
    if(fi.Extension == ".tif")
    {
        MyDocumentObject do = new MyDocumentObject();
        string filename = fi.Name;
        string[] filenameAsArray = filename.split('_');
        string lastIndexNoExtension = (filenameAsArray[filnameAsArray.Length -1].Split('.'))[0];
        filenameAsArray.SetValue(lastIndexNoExtension, (filenameAsArray.Length - 1));
        if(filenameAsArray.Length == 6)
        {
            for(int x=0;x<=(filenameAsArray.Length - 1);x++)
            {
                switch(x)
                {
                    case 0:
                        string part1 = filenameAsArray[x];
                        do.part1 = part1;
                        break;

                    //so on for 1-6
                }
            }
        }
        bool validationResults = do.ValidateFields();
        if(validationResults)
        {
            File.Move(fi.FullName, "path\to\destination");
        }
        else
        {
            File.Move(fi.FullName, "path\to\errorbin");
        }
    }
}

My main point of focus for this review is on the method of pulling apart the filename into an array of text segments and then assigning that data to an instance object and operating on it.

In practice I'm pretty happy - it does the job and is zippy about it. But looking at it... just bugs me.

EDIT: I forgot to mention the reason I use my method of splitting to remove the file extension is due to issues that arise when a file in the directory is named something like:

  this_is_a_trick.txt.tif

First since you only care about tif extensions just pass that into the Directory.GetFiles

foreach (string file in Directory.GetFiles(path, "*.tif"))

No need for an if statement checking extension

Also you don't need to create a FileInfo object to get the name of the file use

string filename = Path.GetFileName(file);
filename = filename.Substring(0, filename.IndexOf('.'));

returns you the file name without the extension so you don't need to do the array manipulation.

If me since there is only 6 values I wouldn't do a loop statement.  You need to code each switch and set each property so the loop isn't really helping you save code nor time.  Just set them.  if they need to change in the future it's less code to change and the loop and switch would need to change as well.

string filename = Path.GetFileName(file);
filename = filename.Substring(0, filename.IndexOf('.'));
string[] filenameAsArray = filename.Split('_');
if (filenameAsArray.Length == 6)
{
    document.part1 = filenameAsArray[0];
    document.part2 = filenameAsArray[1];
    document.part3 = filenameAsArray[2];
    document.part4 = filenameAsArray[3];
    document.part5 = filenameAsArray[4];
    document.part6 = filenameAsArray[5];
}
bool validationResults = document.ValidateFields();
if (validationResults)
{
    File.Move(file, @"path\to\destination");
}
else
{
    File.Move(file, @"path\to\errorbin");
}

I also changed your object from do to document because do to most English speakers is a verb and they not going to think it stands for document object.  Again for years later whoever looks at this to make it easier to understand.


Stack with getMin method This should implement a stack that has a \$O(1)\$ getMin method. I used an auxiliary stack. Please provide comments on my solution.

import org.junit.Test;

public class Solution {

  @Test
  public void testEmptyStack(){
    StackMin<Integer> s = new StackMin<Integer>();
    System.out.println("----------------------");
    System.out.println("Empty stack");
    System.out.println(s.getMin());
  }

  @Test
  public void testRandomStack(){
    System.out.println("----------------------");
    StackMin<Integer> s = new StackMin<Integer>();
    s.push(2);
    s.push(-1);
    s.push(0);
    s.push(7);
    s.push(22);
    System.out.println("Random stack");
    s.printStack();
    System.out.println("Min element");
    System.out.println(s.getMin());
  }

  @Test
  public void testEqualities(){
    System.out.println("----------------------");
    StackMin<Integer> s = new StackMin<Integer>();
    s.push(2);
    s.push(-1);
    s.push(0);
    s.push(0);
    s.push(0);
    s.push(7);
    s.push(22);
    s.push(-1);
    s.push(-1);
    s.push(33);
    System.out.println("Stack with equalities");
    s.printStack();
    System.out.println("Min element");
    System.out.println(s.getMin());
  }

  public static void main(String[] args) {
    Solution e = new Solution();
    e.testEmptyStack();
    e.testRandomStack();
    e.testEqualities();
  }
}

class NodeV2<T> {
  private final T data;

  private final NodeV2<T> next;

  public NodeV2(T d, NodeV2<T> n) {
    data = d;
    next = n;
  }

  public NodeV2(T d) {
    this(d, null);
  }

  public T getData() {
    return data;
  }

  public NodeV2<T> getNext() {
    return next;
  }

}

class StackMin<T extends Comparable<T>> extends StackV2<T> {
  StackV2<T> auxiliaryStack = new StackV2<T>();

  void push(T element) {
    if (auxiliaryStack.isEmpty() || element.compareTo(auxiliaryStack.top.getData()) <= 0) {
      auxiliaryStack.push(element);
    }

    super.push(element);
  }

  T pop() {
    if (this.peek().compareTo(auxiliaryStack.peek()) <= 0) {
      auxiliaryStack.pop();
    }

    return super.pop();
  }

  public T getMin() {
    return auxiliaryStack.peek();
  }

}

class StackV2<T extends Comparable<T>> {
  NodeV2<T> top;

  void push(T element) {
    NodeV2<T> t = new NodeV2<T>(element, top);
    top = t;
  }

  T pop() {
    if (top != null) {
      T item = top.getData();
      top = top.getNext();
      return item;
    }
    return null;
  }

  T peek() {
    return top == null ? null : top.getData();
  }

  boolean isEmpty() {
    return top == null ? true : false;
  }

  public void printStack() {
    NodeV2<T> current = top;
    while (current != null) {
      System.out.println(current.getData());
      current = current.getNext();
    }
  }
}

Yay, you use tests!

Darn it, you don't use tests.

Confusing? Allow me to explain. While I totally approve of you using tests, there is a lot more to it than simply putting the @Test annotation above them.

A test is comprised of 3 aspects: Arrange, Act and Assert. Typically a test (whether it's unit, integration, functional, end-to-end, etc) will have these aspects in explicit or implicit form. Personally I like to explicitly separate them with comments (// Arrange).

Why do I mention this? Note the last aspect of AAA: Assert. This tells us that, as the last step in our test, we will verify the result we have and assert that it is what we expected.

Doing this allows us to run the tests and automatically compare the result of each test with the expected outcome. In your situation you have to manually verify that each test works as expected; can you see yourself doing that when you have 300 tests instead of 3?

This brings us to another point: all modern IDE's have integrated test runners. If you have tests, you can just run these using that instead of having to use a main() method as entrypoint for your tests.

By adapting these changes you also won't need those println's anymore which takes away focus from your actual content.

Junit documentation
Junit tutorial


Clear out a textbox after submitting its content I struggled to figure out how to clear a textbox after submitting it's contents via a button control.

I would like to know if my solution is an accepted practice for initializing a control after an action has been made by a user.

Is embedding event data (i.e. SendRequested) within a model an accepted practice for updating UI within Elm?

module Main exposing (..)

import Html exposing (..)
import Html.Attributes exposing (..)
import Html.Events exposing (..)
import WebSocket

main =
    program
        { init = init
        , update = update
        , view = view
        , subscriptions = subscriptions
        }

-- MODEL

type alias Model =
    { input : String
    , messages : List String
    , sendRequested : Bool
    }

init : ( Model, Cmd Msg )
init =
    ( Model "" [] False, Cmd.none )

-- UPDATE

type Msg
    = Input String
    | Send
    | NewMessage String

update : Msg -> Model -> ( Model, Cmd Msg )
update msg { input, messages, sendRequested } =
    case msg of
        Input newInput ->
            ( Model newInput messages False, Cmd.none )

        Send ->
            ( Model "" messages True, WebSocket.send "ws://echo.websocket.org" input )

        NewMessage message ->
            ( Model input (message :: messages) False, Cmd.none )

-- SUBSCRIPTIONS

subscriptions : Model -> Sub Msg
subscriptions model =
    WebSocket.listen "ws://echo.websocket.org" NewMessage

-- VIEW

view : Model -> Html Msg
view model =
    let
        inputElement =
            if not model.sendRequested then
                input [ onInput Input ] []
            else
                input [ onInput Input, value "" ] []
    in
        div []
            [ div [] (List.map viewMessage model.messages)
            , inputElement
            , button [ onClick Send ] [ text "Send" ]
            ]

viewMessage : String -> Html Msg
viewMessage message =
    div [] [ text message ]

Since you're already keeping track of the <input>'s value in your model, it would be cleaner to use that as the value in your view, too.

view : Model -> Html Msg
view model =
    div []
        [ div [] (List.map viewMessage model.messages)
        , input [ onInput Input, value model.value ] []
        , button [ onClick Send ] [ text "Send" ]
        ]

This way, your view is a pure representation of your model, and your model is kept in sync with user input.

I took the liberty of doing a quick pass over your code, additionally introducing record update syntax in your update function, as opposed to constructing a fresh record from scratch.

Adding or removing a field from a record needn't result in refactoring every single place you happen to modify the values of that record.


Strange algorithm results I came across this thread and I am working on the same problem. My best algorithm passes 6/11 cases while the algorithm in the other thread does indeed pass 8/11 cases. Running time trials with random input my algorithm is significantly quicker (8 to 12 times depending on input) then the one in the other thread. So I am pretty confused on these results.

I came across this thread and I am working on the same problem. My best algorithm passes 6/11 cases while the algorithm in the other thread does indeed pass 8/11 cases. Running time trials with random input my algorithm is significantly quicker (8 to 12 times depending on input) then the one in the other thread. So I am pretty confused on these results.

Aside from empirical testing these are the reasons I think my code should be running better than the other implementation for these reasons.

Appending to a StringBuilder and printing once should be more
efficient than multiple print statements.
Better reduction in logic for binary addition. As well doing addition
from the bit index’s least significant 0 in both BitSets, a and b,
reduces unnecessary computation.
Using BufferedReader has a lot less overhead than using Scanner.
BitSet really didn’t offer much difference in the terms of
performance than a character array but reversing the binary strings
is a waste and the StringBuilder reverse is not the best
performance.

Can anyone shed some light onto why the seemingly slower algorithm does better in the testing?

Version 2

private static String floatSum()
{
    final Scanner input = new Scanner(System.in);
    final int number_bits = input.nextInt();
    final int number_queries = input.nextInt();
    final StringBuilder builder = new StringBuilder();
    final char[] a = new StringBuilder(input.next()).reverse().toString().toCharArray();
    final char[] b = new StringBuilder(input.next()).reverse().toString().toCharArray();

    for(int queries = 0; queries < number_queries; queries++)
    {
        switch(input.next().charAt(4))
        {
            case 'a':
                a[input.nextInt()] = input.next().charAt(0);
                break;
            case 'b':
                b[input.nextInt()] = input.next().charAt(0);
                break;
            default:
                final int index = input.nextInt();

                int carry_bit = 0;

                for(int iter = index - 1; iter >= 0; iter--)
                {
                    if(a[iter] == b[iter])
                    {
                        carry_bit = a[iter] - 48;
                        break;
                    }
                }

                builder.append(index == number_bits ? carry_bit : ((a[index] - 48) + (b[index] - 48) + carry_bit) % 2);
                break;
        }

    }

    return builder.toString();
}

Consider this case:

A: 1111111111
B: 1111111111
Query: n+1

The other post terminates very quckly. It knows that since both numbers end in 1, it must carry and thus doesn't have to scan throughout the entire string. On the other hand, your method tries to find clear bits but since there aren't any, it ends up going through the whole bitstring.

Probably, yours is faster over all, but this pathological case happens to have been put first in the test cases and so your program fails.

                if(carry && a.get(iter) && b.get(iter))
                {
                    carry = true;
                    value = 1;
                }
                else if((carry && a.get(iter)) || (carry && b.get(iter))  || (a.get(iter) && b.get(iter)))
                {
                    carry = true;
                    value = 0;
                }
                else if(carry || a.get(iter) || b.get(iter))
                {
                    carry = false;
                    value = 1;
                }
                else
                {
                    carry = false;
                    value = 0;
                }

This whole bit kinda hard to follow you could instead do something like:

int bits = a.get(iter) ? 1 : 0 + b.get(iter) ? 1 : 0 + carry ? 1 : 0;
value = bits % 2;
carry = bits / 2;

I think its a clearer

                int a_index = a.nextClearBit(number_bits - index);
                int b_index = b.nextClearBit(number_bits - index);

Yeah, not gonna fly. nextClearBit has to scan through the bitset bit by bit in order to do that. You are gonna need to come up with a way to look through the bits without looking at each individual bit. I've solved this problem, and I used a specialized data structure to do it.


Alarm clock with "ringing" functionality I have a question regarding a program that is to run an alarm clock and display the time (HH:MM AM/PM) and also trigger the alarm and display "ringing" at 12:00 AM.

I am a beginner in Java and I am a little lost. I believe I have most of the skeletal work done. This is much like another question asked on here but I could not find an answer to my problems. So I apologize for the similarity. I am also to create a "dummy driver" in order to test the method as one of the requirements for this program is to be able to be read by a general driver someone else has created. Therefore, all of my variables and methods must be left the way I have them in order to be read by said driver. I wish I could be a little more specific in what I need assistance with but any feedback you can give me is much appreciated.

This is what I've got so far:

//dummy driver
package program5;

public class Program5 {

public static void main(String[] args) {
AlarmClock alarm = new AlarmClock();
System.out.println(alarm.getHour() + ":" + alarm.getMinute() + alarm.getAmOrPM());
System.out.println(alarm.getAlarmHour() + ":" + alarm.getAlarmMinute() + alarm.getAlarmAmOrPm());
System.out.println(alarm.isIsAlarmRinging());
}
}

package program5;

public class AlarmClock {

private int hour;
private int minute;
private String amOrPm;
private int alarmHour;
private int alarmMinute;
private String alarmAmOrPm;
private boolean isAlarmRinging;

//constructor:
AlarmClock() {
hour = 12;
alarmHour = 12;
minute = 0;
alarmMinute = 0;
alarmAmOrPm = new String();
amOrPm = new String();
isAlarmRinging = false;
}

//getters:
public int getHour() {
if ((hour >= 1) && (hour <= 12)) {
}
return hour;
}

public int getMinute() {
if ((minute >= 0) && (minute <= 59)) {
}
return minute;
}

public String getAmOrPM() {
return amOrPm;
}

public int getAlarmHour() {
return alarmHour;
}

public int getAlarmMinute() {
return alarmMinute;
}

public String getAlarmAmOrPm() {
return alarmAmOrPm;
}

public boolean isIsAlarmRinging() {
if (hour >= 12){
isAlarmRinging = true;
}
return isAlarmRinging;
}

//setters
public void setHour(int newHour) {
if ((newHour >= 1) && (newHour <= 12)) {
    hour = newHour;
}
}

public void setMinute(int newMinute) {
if ((newMinute >= 0) && (newMinute <= 59)) {
    minute = newMinute;
}
}

public void setAmOrPM(String newAmOrPm) {
amOrPm = newAmOrPm;
}

public void setAlarmHour(int newAlarmHour) {
if ((newAlarmHour >= 1) && (newAlarmHour <= 12)) {
    alarmHour = newAlarmHour;
}
}

public void setAlarmMinute(int newAlarmMinute) {
if ((newAlarmMinute >= 0) && (newAlarmMinute <= 59)) {
    alarmMinute = newAlarmMinute;
}
}

public void setAlarmAmOrPm(String newAlarmAmOrPm) {
alarmAmOrPm = newAlarmAmOrPm;
}

public void setIsAlarmRinging(boolean isAlarmRinging) {
if (isAlarmRinging = true) {
    System.out.println("Ringing!");
}
}

public void advanceOneMinute() {
minute++;
if (minute == 59) {
    hour++;
}
}

public void advanceMinutes(int minutesToAdvance) {

}

public void advanceOneHour() {
hour++;
if (hour > 12) {
hour = 1;
}
}

public void advanceHours(int hoursToAdvance) {

}

public void setTime(int newHour, int newMinute, String newAmOrPm) {
hour = newHour;
amOrPm = newAmOrPm;
}

public void setAlarmTime(int newAlarmHour, int newAlarmMinute, String newAlarmAmOrPm) {

if(hour >=0 && hour <= 23 && minute >=0 && minute <= 59) {
newAlarmHour = hour;
newAlarmMinute = minute;
}
}

public void turnOffAlarm() {
isAlarmRinging = false;
}

public void displayTime() {

}

public void displayAlarmTime() {

}

}

It's far from done but it's a start. If you can tell me anything I can do, I'll be very thankful. I'm not sure exactly what my question is. Just if I'm on the right track and what I'm doing wrong. Sorry it's not a short, concise, clear-cut question.

Calendar

You created an alarmclock, which is really just a time-representation. A Calendar has both a date and time component. I strongly, strongly suggest you use that so you can let your user define how he wants his date represented and let the API handle fancy stuff like timezones, summer hours, etc.

You can also look into the Java 8 time API but I think that might not be appropriate yet.

Enums

When you have a limited set of different possibilities, your mind should immediately wander to enums. This way you can represent your AM/PM by saying DayPart.AM and DayPart.PM instead of the literal string "AM" which could mistakenly be used as "aM", "am", "AX", etc. It will provide compile-time safety and is a lot more pleasant to work with.

Indentation

Stick to conventions. Some people indent a block with one tab (or 4 spaces), some with 2 spaces. This will keep the code readable for everyone.

Methods without a meaningful body

Look at your code here:

public int getHour() {
if ((hour >= 1) && (hour <= 12)) {
}
return hour;
}

With formatting this becomes this:

public int getHour() {
    if ((hour >= 1) && (hour <= 12)) {
    }

    return hour;
}

Can you tell what's wrong here? Note that there are several methods that have this construct!

Assignment vs Comparison

This is a very common mistake:

if (isAlarmRinging = true)

What you do here is set isAlarmRinging to true, you never compare it.

Keep in mind:

Comparison: ==.
Assignment: =.

Unused parameters

In your method setTime you never use the parameter newMinute.
Likewise in method setAlarmTime you never use newAlarmAmOrPm.