Critique of Delimited String Method I'm working my way through The Java Programming Language, Fourth Edition - The Java Series.  This is Exercise 13.3:

  As shown, the delimitedString method assumes only one such string per input string. Write a version that will pull out the delimited strings and retun an array.

The provided delimitedString method:

public static String delimitedString(String from, char start, char end) {
    int startPos = from.indexOf(start);
    int endPos = from.lastIndexOf(end);

    if (startPos == -1)
        return null;
    else if (endPos == -1)
        return null;
    else if (startPos > endPos)
        return null;
    else
        return from.substring(startPos, endPos + 1);
}

My solution. Note that regular expressions are covered in the next section of the book, so I chose not to use a split for this particular exercise:

public static String[] delimitedStrings(String from, char startDelim, char endDelim) {
    int index;
    int startPos = 0;
    ArrayList<String> list = new ArrayList<String>();

    while ((index = from.indexOf(startDelim, startPos)) != -1) {
        int endPos;
        if ((endPos = from.indexOf(endDelim, index)) != -1) {
            list.add(from.substring(index, endPos + 1));
            startPos = endPos;
        } else {
            break;
        }
    }
    return list.toArray(new String[list.size()]);
}

Supporting tests:

@Test
public void testDelimitedStringsSimple() throws Exception {
    String[] strArray = {"<Bonjour!>"};
    assertEquals(strArray, StringUtil.delimitedStrings("I say <Bonjour!>", '<', '>'));
}

@Test
public void testDelimitedStringsComplex() throws Exception {
    String[] strArray = {"<Bonjour!>","<Bonjour!>","<B<onjour!>", "<Bonjour!>"};
    assertEquals(strArray, StringUtil.delimitedStrings("I say <Bonjour!> I say <Bonjour!> and a tricky <B<onjour!> and ><Bonjour!>", '<', '>'));
}

@Test
public void testDelimitedStringsNoStart() throws Exception {
    String[] strArray = {};
    assertEquals(strArray, StringUtil.delimitedStrings("Bonjour!>", '<', '>'));
}

@Test
public void testDelimitedStringsNoEnd() throws Exception {
    String[] strArray = {};
    assertEquals(strArray, StringUtil.delimitedStrings("<Bonjour!", '<', '>'));
}

@Test
public void testDelimitedStringsReversed() throws Exception {
    String[] strArray = {};
    assertEquals(strArray, StringUtil.delimitedStrings(">Bonjour!<", '<', '>'));
}

@Test
public void testDelimitedStringsEmpty() throws Exception {
    String[] strArray = {};
    assertEquals(strArray, StringUtil.delimitedStrings("", '<', '>'));
}

If I had to put a number on your code it would be a 8 out of 10. You did good here. Since it is so good I am going to be nitpicking a few things. First is your test. You are comparing arrays but using assertEquals. Instead you need to change that to assertArrayEquals (providing you are using JUnit 4.0) The other thing I feel you need to test for is no begin and no end delimiters.

The other nitpicking thing I can find is in your split method. I know that you referred to not using the String.split method, but your method does the same thing. For this exercise there is nothing wrong with giving it the same name seeing as it does the same thing. One thing that annoys me a small amount is the making a while or if statement do more than one thing. Are you assigning or are you comparing? The convention for while and if is to compare only. So move your assignment out of those functions. I'm going to paste your code multiple times and I revise it, so here is where I am sitting now on your code.

public static String[] split(String from, char startDelimiter, char endDelimiter)
{
    int startPos = 0;
    ArrayList<String> list = new ArrayList<>();

    int index = from.indexOf(startDelimiter, startPos);

    while (index != -1)
    {
        int endPos = from.indexOf(endDelimiter, index);
        if (endPos != -1)
        {
            list.add(from.substring(index, endPos + 1));
            startPos = endPos;
        }
        else
        {
            break;
        }
        index = from.indexOf(startDelimiter, startPos);
    }
    return list.toArray(new String[list.size()]);
}

Notice that it introduces some duplication with assigning index. Since you have passing tests that means we can refactor a little bit. Now I know you are probably thinking that the reason you put the assignment in the while loop was to remove duplication. Before we address that though lets attack some of your naming. Index. Normally this is a good choice of words to use, but index usually refers to a location in an array. Although a string is an array of characters in this context it is not the exact definition of an index. So I believe a more appropriate name would be startDelimiterLocation , startLocation , or even startLookigFrom this would allow us to make a sentence out of our while and if statments by giving -1 a name such as NotFound or since it is a constant NOTFOUND.

public static String[] split(String from, char startDelimiter, char endDelimiter)
{
    int fromIndex = 0;
    ArrayList<String> list = new ArrayList<>();

    int startPosistion = from.indexOf(startDelimiter, fromIndex);

    while (startPosistion != NOTFOUND)
    {
        int endPosistion = from.indexOf(endDelimiter, startPosistion);
        if (endPosistion != NOTFOUND)
        {
            list.add(from.substring(startPosistion, endPosistion + 1));
            fromIndex = endPosistion;
        }
        else
        {
            break;
        }
        startPosistion = from.indexOf(startDelimiter, fromIndex);
    }
    return list.toArray(new String[list.size()]);
}
private static final int NOTFOUND = -1;

This gives us the ability to read the code outloud if we wanted (while startPosistion is not not found) (yes i know it's a double negative, but well.. this isn't English. Well that gives me an idea.. Lets make that into an actual sentance, and have it return a boolean.

private static boolean stringHasStringBetweenTwoCharacters(String from, char startDelimiter, char endDelimiter)
{
    int startPosistion = from.indexOf(startDelimiter);
    if (startPosistion == NOTFOUND) return false;

    int endPosistion = from.indexOf(endDelimiter, startPosistion);
    return (startPosistion != NOTFOUND && endPosistion != NOTFOUND);
}

this allows me to rewrite my while method to this. while (stringHasStringBetweenTwoCharacters(from, startDelimiter, endDelimiter)) but this makes the tests fail because my method is not looking at where we left off in our string. I removed it on purpose because I wanted this method to only find the first occurrence of a string in between 2 characters. So this means I have to change the logic slightly. Instead of keeping track of where we are in the string, lets just remove up to the endPosistion and leave the rest.

public static String[] split(String from, char startDelimiter, char endDelimiter)
{
    ArrayList<String> list = new ArrayList<>();

    while (stringHasStringBetweenTwoCharacters(from, startDelimiter, endDelimiter))
    {
        int startPosistion = from.indexOf(startDelimiter);
        int endPosistion = from.indexOf(endDelimiter, startPosistion);
        if (endPosistion != NOTFOUND)
        {
            list.add(from.substring(startPosistion, endPosistion + 1));
            from = from.substring(endPosistion + 1);
        }
        else
        {
            break;
        }
    }
    return list.toArray(new String[list.size()]);
}
private static boolean stringHasStringBetweenTwoCharacters(String from, char startDelimiter, char endDelimiter)
{
    int startPosistion = from.indexOf(startDelimiter);
    if (startPosistion == NOTFOUND) return false;

    int endPosistion = from.indexOf(endDelimiter, startPosistion);
    return (startPosistion != NOTFOUND && endPosistion != NOTFOUND);
}
private static final int NOTFOUND = -1;

so now my tests pass again. Oh but I have duplication again, but no longer in the start position, now it is in the end position. Since I know it is there I can remove the check in my while loop and make a slight adjustment in my duplicate change in index. My final outcome is something like this.

public static String[] split(String from, char startDelimiter, char endDelimiter)
{
    ArrayList<String> list = new ArrayList<>();

    while (stringHasStringBetweenTwoCharacters(from, startDelimiter, endDelimiter))
    {
        int startPosistion = from.indexOf(startDelimiter);
        int endPosistion = from.indexOf(endDelimiter, startPosistion) + 1;
        list.add(from.substring(startPosistion, endPosistion));
        from = from.substring(endPosistion);
    }
    return list.toArray(new String[list.size()]);
}
private static boolean stringHasStringBetweenTwoCharacters(String from, char startDelimiter, char endDelimiter)
{
    int startPosistion = from.indexOf(startDelimiter);
    if (startPosistion == NOTFOUND) return false;

    int endPosistion = from.indexOf(endDelimiter, startPosistion);
    return (startPosistion != NOTFOUND && endPosistion != NOTFOUND);
}
private static final int NOTFOUND = -1;

Now my methods only do one thing, and it does them well. I moved the logic of how to check to see if my string still contains a token into its own method which leave only the logic of adding my tokens to my arrayList. It may seem a little more verbose, but in my opinion is easier to read


Haskell Ackermann function I had a challenge a while back from a friend to try and produce the fastest possible program to compute A(3,16), where A is the Ackermann function.

He wrote it in Java, and it took ~4.4 seconds. The below (very ugly) Haskell program that I wrote took ~1.7 seconds:

{-# LANGUAGE BangPatterns #-}

import           Data.Map.Strict (Map)
import qualified Data.Map.Strict as Map

-- Main method:

main :: IO ()
main = print $ fst $ ack (I 3 16) Map.empty

-- Definitions:

data I = I {-# UNPACK #-} !Int {-# UNPACK #-} !Int
    deriving (Eq, Ord)

ack :: I -> Map I Int -> (Int, Map I Int)
ack   (I 0 n) r  = (n + 1, r)

ack i@(I m 0) r1 = maybe bak fun (Map.lookup i r1)
    where
        (!val, !r2) = ack (I (m-1) 1) r1
        bak       = (val, Map.insert i val r2)
        fun v     = (v, r1)

ack i@(I m n) r1 = maybe bak2 fun2 (Map.lookup call2 r2)
    where
        call1       = (I m (n - 1))
        (!val1, !re1) = ack call1 r1
        bak1        = (val1, Map.insert call1 val1 re1)
        fun1 v      = (v, r1)
        (!v1, !r2)    = maybe bak1 fun1 (Map.lookup call1 r1)
        call2       = (I (m - 1) v1)
        (!val2, !re2) = ack call2 r2
        bak2        = (val2, Map.insert call2 val2 re2)
        fun2 v      = (v, r2)

This is used on Ints specifically because A(3,16) is 524285, which within Ints range.

How can I make this function more memory efficient, and faster for greater Ackermann calls?

Also, the syntax is disgusting. How can I make this more readable (maybe using monads?) without having horrible performance consequences?

Using Data.Memocombinators

Have a look at Data.Memocombinators module. It offers combinators for memoizing functions which is essentially what you are doing with the Map.

Here is the example from the documentation on how to use it to create a memoizing fibonacci function:

import Data.MemoCombinators

fib = Memo.integral fib'
   where
     fib' 0 = 0
     fib' 1 = 1
     fib' x = fib (x-1) + fib (x-2)
              ^^^         ^^^

Things to note:

fib is the memoized version of fib' which is just a helper function
fib' calls fib for any recursive calls

The other thing which will help is to keep in mind that the type Memo a represents a a combinator which is able to memoize a function whose argument type is a.

So:

integral is a combinator which can memoize functions taking an Int
pair integral integral is a combinator which memoize functions taking an (Int,Int)

And thus to memoize the Ackerman function:

ack = (pair integral integral) ack'
  where ack' (0,n) = n+1
        ack' (m,0) = ack (m-1,1)
        ack' (m,n) = ack (m-1, ack (m, n-1))

Again, note how ack is defined as the memoized version of ack' and ack' calls ack for recursive cases.

Using Array memoization

Faster results can be obtained by using arrays to memoize the functions ack(1,.), ack(2,.) and ack(3,.):

import Data.Array
import Data.Ix

-- memoize the function f using arrays
arrayMemo bnds f = g
  where g i = arr ! i
        arr = array bnds [ (i,f arr i) | i <- range bnds ]

a0 n = n+1

a1 = arrayMemo (0,530000) f
  where f arr 0 = a0 1
        f arr n = a0 (arr ! (n-1))

a2 = arrayMemo (0,270000) f
  where f arr 0 = a1 1
        f arr n = a1 (arr ! (n-1))

a3 = arrayMemo (0,16) f
  where f arr 0 = a2 1
        f arr n = a2 (arr ! (n-1))

The only drawback is that explicit bounds have to be determined for each level function. This approach computes a3 16 in about half a second.


Simplifying Counter of Checked Checkboxes Is the following code a proper solution to my problem? I have a feeling it can be simplified quite a bit.

  If one of the first four checkboxes is checked the sum is 1; if two 1.5; if three 2; if four 2.5. If the bar5 is checked then it's 1 + calculated value of first four (if any is checked). Afterwards, multiply by 100.

function checkboxCount() {
 var form = document.forms["calculator"];
 var bar5 = form.elements["bar5"];
 var foo = document.querySelectorAll("input[name=foo]");
 count = 0;

 for (var i = 0; i < foo.length; i++) {
  if (foo[i] && foo[i].checked == true) {
   count++;
  }
 }
 if (count == 1 && bar5.checked == true) {
  count = 1;
 } else if (count == 1) {
  count = 1;
 }
 if (count == 2 && bar5.checked == true) {
  count = 2;
 } else if (count == 2) {
  count = 1.5;
 }
 if (count == 3 && bar5.checked == true) {
  count = 2.5;
 } else if (count == 3) {
  count = 2;
 }
 if (count == 4 && bar5.checked == true) {
  count = 3;
 } else if (count == 4) {
  count = 2.5;
 }
 if (count == 5 && bar5.checked == true) {
  count = 3.5;
 }
 return count;
}

function calculateTotalCost() {
 var totalCost = 0;
 var printCost = document.getElementById('calculatedCost');
 totalCost = 100 * checkboxCount();

 printCost.innerHTML = "$" + totalCost;
}
<form id="calculator">

<fieldset>
<input type="checkbox" name="foo" id="bar1" onclick="calculateTotalCost()">Bar 1
<input type="checkbox" name="foo" id="bar2" onclick="calculateTotalCost()">Bar 2
<input type="checkbox" name="foo" id="bar3" onclick="calculateTotalCost()">Bar 3
<input type="checkbox" name="foo" id="bar4" onclick="calculateTotalCost()">Bar 4
<input type="checkbox" name="foo" id="bar5" onclick="calculateTotalCost()">Bar 5
</fieldset>

</form>

The formula can be simplified:

for the first four checkboxes, if any of the items are checked, multiple the number checked by 0.5 and add 0.5
Then add 1 if the fifth checkbox is checked.

So it might be simpler to just have a reference to the first for checkboxes and then the 5th checkbox. For the first for, one can use document.getElementsByName('foo') to get a NodeList, and then put those nodes in an array using Array.from() and use Array.slice() to get the first four checkboxes.

var firstFourBars = Array.from(document.getElementsByName('foo')).slice(0, 4);

For the record - I did consider using a CSS selector identical to that of the answer by @200_success (i.e. input[name=foo]:not(#bar5):checked) but because the NodeList returned by document.querySelectorAll() is not live, that DOM query would need to be run each time. See point #3 for more details.
Event delegation can be used to avoid adding a click handler to each checkbox in the HTML. The code below utilizes document.addEventListener() to call the calculateTotalCost whenever a click occurs. Because of this change, that function receives a reference to the event as the first argument. This allows changes based on any click anywhere but conditional logic dependent on event.target could be added.
DOM references can be stored in variables at the start of the script, so as to avoid multiple lookups (e.g. var printCost)
One other simplification:

var totalCost = 0;
var printCost = document.getElementById('calculatedCost');
totalCost = 100 * checkboxCount();

There is little point in setting totalCost to 0 and then assigning it 100 * checkboxCount(). Just assign it the latter product:

var totalCost = 100 * checkboxCount();

document.addEventListener('DOMContentLoaded', function() { //Wait for DOM to be ready
  //cache DOM references
  var printCost = document.getElementById('calculatedCost');
  var firstFourBars = Array.from(document.getElementsByName('foo')).slice(0, 4);
  var bar5 = document.forms["calculator"].elements["bar5"];

  document.body.addEventListener('click', calculateTotalCost);
  // helper function for filter
  function isChecked(checkbox) {
    return checkbox.checked;
  }

  function checkboxCount() {
    var count = firstFourBars.filter(isChecked).length;
    if (count) {
      count = count * 0.5 + 0.5;
    }
    if (bar5.checked) {
      count += 1;
    }
    return count;
  }

  function calculateTotalCost(event) {
    //could conditionally execute based on event.target
    var totalCost = 100 * checkboxCount();
    printCost.innerHTML = "$" + totalCost;
  }

});
<form id="calculator">

  <fieldset>
    <input type="checkbox" name="foo" id="bar1">Bar 1
    <input type="checkbox" name="foo" id="bar2">Bar 2
    <input type="checkbox" name="foo" id="bar3">Bar 3
    <input type="checkbox" name="foo" id="bar4">Bar 4
    <input type="checkbox" name="foo" id="bar5">Bar 5
  </fieldset>

</form>

Edit

I forgot to mention that in the modified code above, it waits for the DOM to be ready before before querying it (notice the first line document.addEventListener('DOMContentLoaded', function() { //Wait for DOM to be ready). Not only does it prevent attempts to access DOM elements before they exist, but the variables are scoped to the callback function instead of the global namespace.


Speeding Up BufferedWriter Are there any possible speed improvements I can make to the following method. It takes a JDBC ResultSet and breaks it into several CSV files:

public static void convertToCSV(final ResultSet rs) throws SQLException, IOException {
    final DateFormat format = new SimpleDateFormat("yyyyMMdd,HH:mm:ss");
    format.setTimeZone(TimeZone.getTimeZone("Etc/UTC"));
    final StringBuilder sb = new StringBuilder();
    while (rs.next()) {
        final File file = new File(sb.append(rs.getString("FIELD1")).append(".csv").toString());
        if (!file.exists()) {
            file.createNewFile();
        }
        final BufferedWriter fw = new BufferedWriter(new FileWriter(file.getAbsoluteFile(), true));
        fw.write(rs.getString("FIELD2"));
        fw.write(',');
        fw.write(rs.getString("FIELD3"));
        fw.write(',');
        final String clobValue = rs.getString("FIELD4");
        if(clobValue==null)
            fw.write("null,");
        else{
            fw.write('\"');
            fw.write(clobValue);
            fw.write("\",");
        }
        final Date date = new Date(rs.getLong("FIELD5"));
        fw.write(format.format(date));
        fw.write('\n');
        fw.close();
        sb.setLength(0);
    }
    rs.close();
}

You may see a performance increase by adding multithreading support. For each row, create an instance of a custom Runable that takes the five values and add it to a ThreadPoolExecutor. You can futz around with the size of the pool. As with all performance enhancements, you'll need to test, test, test to make sure you're seeing a real benefit, and you'll really need to test on a production clone for it to have any meaning. It may not make a difference on your local machine but be a big help on production, or vice versa.

Your use of the StringBuilder as opposed to just concatenation is probably not doing you any good. Compilers will do that work, and the code is harder to read as-is. There are other things to pick at, but none of them are performance-related.


Function to format the local time This is a statement that I use a lot in my code base:

 (LocalDateTime.now()).format(DateTimeFormatter.ofPattern("ssmmHHddMMMuu"))

Should I create a static method for this or is it overkill?

public static String getTodaysDateInFormat(String format){
  return (LocalDateTime.now()).format(DateTimeFormatter.ofPattern(format));
}

First of all, the parenthesis around LocalDateTime.now() are useless.

Now, let's see what we're really doing here:

return LocalDateTime.now().format(DateTimeFormatter.ofPattern(format));

We're creating a unique LocalDateTime each time, but I am assuming that you always use the same format, and therefore the DateTimeFormatter.ofPattern(format) object will always be the same.

I would recommend not creating a method for this and instead extract a constant for the format:

private static final DateTimeFormatter FORMAT = DateTimeFormatter.ofPattern("ssmmHHddMMMuu");

And then each time you need it, you just do

LocalDateTime.now().format(FORMAT);

A method named getTodaysDateInFormat provides exactly the same information as the above piece of code, therefore the method does not really provide any additional value.

I don't consider LocalDateTime.now().format(FORMAT) as duplicated code. Or rather: I'm considering just as much code duplication as a call to getTodaysDateInFormat().


Overriding default animation parameters with custom values Using JavaScript ES5, I would like to know if you could suggest me a more "elegant" and succinct way to overriding defaults variable when User pass custom values.

In the following example, an animation is executed using _defaultOptions, a developer can change the default behavior passing only some properties to a function, example:

animate(dom, {duration: 2000}); // overriding only duration, rest as default

Notes:

I need also to support IE 11.
optionsArg is optional, so animate(dom) is allowed

var dom = document.getElementById('logo');

var _defaultOptions = {
  duration: 1000,
  delay: 0,
  iterations: 1,
  direction: 'normal',
  fill: 'both'
};

var animate = function(dom, optionsArg) {
  // code to be improved here
  var options = {
    duration: optionsArg && 'duration' in optionsArg ? optionsArg.duration : _defaultOptions.duration,
    delay: optionsArg && 'delay' in optionsArg ? optionsArg.delay : _defaultOptions.delay,
    iterations: optionsArg && 'iterations' in optionsArg ? optionsArg.iterations : _defaultOptions.iterations,
    direction: optionsArg && 'direction' in optionsArg ? optionsArg.direction : _defaultOptions.direction,
    fill: optionsArg && 'fill' in optionsArg ? optionsArg.fill : _defaultOptions.fill,
  };
  dom.animate([{
    opacity: 1
  }, {
    opacity: 0
  }], options);
};

animate(dom, {duration: 2000});
<img id="logo" src="https://upload.wikimedia.org/wikipedia/commons/e/e5/NASA_logo.svg">

Object.assign(_defaultOptions, optionsArg )

But this will have side effects as it changes _defaultOptions object itself.


Parsing a URL from a document and matching it from an array I need help in improving this script that I have written to parse a URL and check it from an array.

This is what I have done till now: (fiddle)

This is working fine. Was this done correctly? What exactly I need is to search URLs in a list (ul) (which will be a domain specific), and then match if the URL is found in an array (array will have 500+ domains). If it is found then I need to append some text.

Is this function fine to use if there is a long list of domain names in a document and an array has 100+ domains?

Should I use regex to get the domain or is finding it from a CSS selector a good idea?

            Link 1

            Link 2

            Link 3

            Link 4

            Link 5

            Link 6

            Link 7

            Link 8

var arr = ['http://www.azaz.com', 'http://www.123.com'];
        $("#links li").each(function () {
        var aa = $(this).find('a').attr('href');
        var found = $.inArray(aa, arr) > -1;
        if(found === true){
            $(this).append("true");
        }
});

Your code is simple and works, so I don't see any need to over-complicate things. It ofcourse depends on the sizes of the lists/data. But 500 doesn't seem so bad, performance wise.

It's good that you decided not to use regular expressions ([source])1
Using a CSS selector seems more appropriate. However it's prone to end up in errors if the site that provides the list changes its css.

In addition, I believe you have a small mistake with your indentation. Perhaps it's just a copy/paste error.

var arr = ['http://www.azaz.com', 'http://www.123.com'];
$("#links li").each(function () {
        var aa = $(this).find('a').attr('href');
        var found = $.inArray(aa, arr) > -1;
        if(found === true){
            $(this).append("true");
        }
});

instead of

var arr = ['http://www.azaz.com', 'http://www.123.com'];
        $("#links li").each(function () {
        var aa = $(this).find('a').attr('href');
        var found = $.inArray(aa, arr) > -1;
        if(found === true){
            $(this).append("true");
        }
});


Find the next multiple of N that is a perfect square number Given a number \$t\$, (\$1 \leq t \leq 1000\$) that represent
  testcases and \$t\$ numbers \$n\$ (\$1 \leq n \leq 10^9\$). Show the
  next multiple of \$n\$ that is a perfect square number.

  Example of input:

5
5 9 10 12 13

  Example of output:

Case #1: 25
Case #2: 9
Case #3: 100
Case #4: 36
Case #5: 169

My solution iterates over all the next perfect squares of \$n\$ (using this formula \$ \left(\lfloor \sqrt{x} \rfloor + 1\right) ^ 2 \$ until it's a multiple of \$n\$.

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

unsigned long long myPow(unsigned long long x){
    return x*x;
}

int main(){

    unsigned int j;

    unsigned int t;
    scanf("%u",&t);

    for(j = 1; j <= t; j++){
        unsigned long long n;
        scanf("%llu",&n);
        double sqrtn = sqrt(n);
        if(sqrtn == (unsigned long long) sqrtn) //test if N is a perfect square
            printf("Case #%d: %llu\n",j,n);
        else{
            unsigned long long i = n;
            while((i = myPow(floor(sqrt(i))+1)) % n != 0);//find the next perfect square multipe of n
            printf("Case #%d: %llu\n",j,i);
        }
    }
}

This solution encounters "Time limit exceeded". Is there a faster way of finding the solution?

I'm afraid we need to use… math.

Take any square number and look at its prime factorization. For example, \$ 100 = 2^2·5^2 \$; \$ 144 = 2^4·3^2 \$; \$ 729 = 3^6 \$. The thing that's common to all of these examples is that the exponent of each prime in the factorization is even. And that's because if you take any number \$ n \$, with factorization $$ n = 2^a·3^b·5^c\dotsm $$ then its square is $$ n^2 = 2^{2a}·3^{2b}·5^{2c}\dotsm $$ where all the exponent are even.

So if you want to find the smallest multiple of \$ m \$ that's a square, then you can factorize $$ m = 2^a·3^b·5^c\dotsm $$ find the odd exponents among \$ a, b, c, \dotsc \$, and then multiply by the appropriate primes to make all the exponents even.

Let's take the number \$ 12 \$ as an example. First, factorize \$ 12 = 2^2·3^1 \$. Then note that \$ 3 \$ is raised to an odd exponent in the factorization. So we need to multiply by \$ 3 \$ to get \$ 2^2·3^2 = 36 \$ which is \$ 6^2 \$.

A bigger example: \$ 24696 \$. First, factorize \$ 24696 = 2^3·3^2·7^3 \$. Then note that both \$ 2 \$ and \$ 7 \$ are raised to odd exponents in the factorization. So we need to multiply by \$ 2·7 \$ to get \$ 2^4·3^2·7^4 = 345744 \$ which is \$ 588^2 \$.


The sorting in the app with fragments This phonebook application has a menu, which sorts by name or category. I do it like this in MainActivity:

Bundle bundle = new Bundle();
Fragment lvFragSortBy = new ListViewFragment();

switch (item.getItemId()) {

    case R.id.menuSortOrderName:
        item.setChecked(true);

        bundle.putString("sortBy", "name");

        lvFragSortBy.setArguments(bundle);
        getFragmentManager()
        .beginTransaction()
        .replace(R.id.fragContainer, lvFragSortBy)
        .commit();
        return true;

    case R.id.menuSortOrderCategory:
        item.setChecked(true);

        bundle.putString("sortBy", "category");
        lvFragSortBy.setArguments(bundle);
        getFragmentManager()
        .beginTransaction()
        .replace(R.id.fragContainer, lvFragSortBy)
        .commit();

        return true;

In ListViewFragment:

Bundle sortBundle = getArguments();
if (sortBundle != null) {
    orderBy = sortBundle.getString("sortBy");
}

And then:

protected ArrayList<Contact> doInBackground(Void... params) {
    try {
        db = new DbWorker(ctx);
        contacts = (ArrayList<Contact>) db.selectAllContacts(orderBy);
    } catch (Exception e) {
        Log.e(TAG, "Exception in AsyncTask.doInBackground");
    }
    return contacts;
}

This code works fine but I would like to know if it can be done better.

You should use newInstance() pattern, described in this link

The profit of using this pattern is, developer cannot instantiate Fragment without providing arguments. There will be no chances that you will get default value when you try to get that value. One more thing is, whatever key is required for a fragment should be defined in public static final String so that no chances for string mismatch in key.


Bowling scoring kata I've been writing basic Python scripts for a while now to help process data or automate some task but I've decided I should start picking up unit testing and objective orientated programming (the vast majority of my scripts so far have been procedural).

As a starter I decided to follow along with Uncle Bob's bowling scoring kata to try and get my mind around TDD and the idea of writing the absolute minimal code at every step to either make the test go red or green (plus any refactoring steps).

As it's a bare bones example of following TDD the main program doesn't actually have an entry point other than via the tests.

Things that stand out to my beginner's eye:

There are a lot of selfs which look like a lot of visual clutter when I read through the code. Is there a better way of doing this? I think it's the density of them that really gets me so I wasn't sure if I could abstract some of them somehow?
unittest seems to have a lot of boilerplate. I had a play with nose a while back which seemed to strip a lot of that out but I thought it might be a good idea to start properly with unittest until I have a use case for anything that nose (or any other library) offers.

bowling_game.py

#!/usr/bin/env python

class Game:

  _rolls = [0] * 21
  _current_roll = 0

  def roll(self, pins):
    self._rolls[self._current_roll] = pins
    self._current_roll += 1

  def score(self):
    score = 0
    frame_index = 0
    for frame in range(0, 10):
      if self._is_strike(frame_index):
        score += 10 + self._strike_bonus(frame_index)
        frame_index += 1
      elif self._is_spare(frame_index):
        score += 10 + self._spare_bonus(frame_index)
        frame_index += 2
      else:
        score += self._rolls[frame_index] + self._rolls[frame_index + 1]
        frame_index += 2
    return score

  def _sum_of_balls_in_frame(self, frame_index):
    return self._rolls[frame_index] + self._rolls[frame_index + 1]

  def _spare_bonus(self, frame_index):
    return self._rolls[frame_index + 2]

  def _strike_bonus(self, frame_index):
    return self._rolls[frame_index + 1] + self._rolls[frame_index + 2]

  def _is_spare(self, frame_index):
    return self._rolls[frame_index] + self._rolls[frame_index + 1] == 10

  def _is_strike(self, frame_index):
    return self._rolls[frame_index] == 10

bowling_game_test.py

#!/usr/bin/env python

import unittest

from bowling_game import Game

class BowlingGameTest(unittest.TestCase):

  def setUp(self):
    self.g = Game()

  def roll_many(self, rolls, pins):
    for roll in range(0, rolls):
      self.g.roll(pins)

  def roll_spare(self):
    self.g.roll(5)
    self.g.roll(5)

  def roll_strike(self):
    self.g.roll(10)

  def test_gutter_game(self):
    rolls = 20
    pins = 0
    self.roll_many(rolls, pins)
    self.assertEquals(self.g.score(),0)

  def test_all_ones(self):
    rolls = 20
    pins = 1
    self.roll_many(rolls, pins)
    self.assertEquals(self.g.score(),20)

  def test_one_spare(self):
    self.roll_spare()
    self.g.roll(3)
    self.roll_many(17, 0)
    self.assertEquals(self.g.score(),16)

  def test_one_strike(self):
    self.roll_strike()
    self.g.roll(3)
    self.g.roll(4)
    self.roll_many(16, 0)
    self.assertEquals(self.g.score(),24)

  def test_perfect_game(self):
    self.roll_many(12, 10)
    self.assertEquals(self.g.score(),300)

if __name__ == '__main__':
    unittest.main()

The commit history is available on GitHub if anyone fancies taking a look at that to see where I might have gone better with the red -> green -> refactor cycle.

It doesn't look like too many selfs to me.

class Game:

  _rolls = [0] * 21
  _current_roll = 0

  ...

3 things right off the bat:

What's with the blank line between class Game: and _rolls = [0] * 21?
You are using 2 spaces to indent. Use 4 spaces.
Shouldn't this be in the __init__ function?:

_rolls = [0] * 21
_current_roll = 0

So it would look like:

class Game:
    def __init__(self):
        self._rolls = [0] * 21
        self._current_roll = 0

    def roll(self, pins):
        self._rolls[self._current_roll] = pins
        self._current_roll += 1

    ...


String encryption using keys I wrote a program that encrypts a users string by increasing every char's position by a random number. I'm looking for any and all criticism on it.

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

namespace Encryption2
{
 class Program
 {
    public static void Main()
    {
        Console.Clear();

        bool valid = true;

        while (valid) //while loop is here for error handling in case user inputs something other than
        {
            Console.WriteLine("Enter d for decrypt and e for encrypt");

            string uInput = Console.ReadLine().ToLower();

            if (uInput == "e")
            {
                encrypt();
                valid = false;
            }
            else if (uInput == "d")
            {
                decrypt();
                valid = false;
            }
            else
            {
                Console.Clear();
            }
        }
    }

    public static void encrypt()
    {
        Console.Clear();

        Console.WriteLine("Enter a string for encryption");

        string input = Console.ReadLine(); //string that will be encrypted

        int[] keyVals = new int[input.Length]; //decryption key

        Random generateKeyVals = new Random();

        for (int i = 0; i < input.Length; i++) //generates input.Length amount of numbers between 1 and 9 that are used to increase
        {                                       //each char in in the input string and then decrease each char when you decrypt
            keyVals[i] = generateKeyVals.Next(1, 9);

            Console.Write(keyVals[i]);
        }

        Console.WriteLine(" : This is your decryption key");

        for (int i = 0; i < input.Length; i++) //encrypts string by adding first char in the encrypted string with the first value in the key and so on
        {
            char encrypted = Convert.ToChar(input[i] + keyVals[i]);

            Console.Write(encrypted);
        }

        Console.WriteLine(" : This is your encrypted string");

        Console.ReadKey();

        Main();
    }

    public static void decrypt() //decrypts an encrypted message using a decryption key
    {
        Console.Clear();

        Console.WriteLine("Enter a string for decryption");

        string input = Console.ReadLine(); //user inputs the encrypted message

        int[] keyVals = new int[input.Length]; //stores all values entered from the for loop below. This is the decryption key used to decrypt the string

        for (int i = 0; i < input.Length; i++) //using a for loop to get all numbers from the key one at a time.
        {
            Console.WriteLine("Enter the #" + (i + 1) + " of the key");
            keyVals[i] = Convert.ToInt32(Console.ReadLine());
        }

        for (int i = 0; i < input.Length; i++) //decrypts string by subtracting first char in the encrypted string by the first value in the key and so on
        {
            char decrypted = Convert.ToChar(input[i] - keyVals[i]);

            Console.Write(decrypted);
        }

        Console.WriteLine(System.Environment.NewLine);

        Console.WriteLine("Press any key to return to the main menu");

        Console.ReadKey();

        Main();
    }
}

}

I know it's obvious but I really have to say this: outside a programming exercise do not write your own encryption function.

Let's now talk about code...

The first thing to do is to separate responsibilities. Encrypt() and Decrypt() methods (note PascalCase, you should follow C# naming guidelines) should do only one thing: encrypt and decrypt a string. To get user input and to provide an output is something else responsibility.

If you strip out all the console I/O your Encrypt() will be much simpler and you may note you can make it even simpler (to read):

static (IEnumerable<int> key, string encrypted) Encrypt(string text)
{
    var key = GenerateEncryptionKey(text.Length);
    return (key, new String(text.Zip(key, (v, k) => (char)(v + k))));
}

static IEnumerable<int> GenerateEncryptionKey(int length)
    => new Enumerable.Range(0, length).Select(x => _rnd.Next(1, 9));

static _rnd = new Random();

What I did:

I moved the generation of the encryption key to a separate function. In real world it will be a service you set separately. This gives you the ability to test the Encrypt() code with edge-cases using well-known and repeatable encryption keys (what will happen if v + k - which is Int32 - is higher than UInt16.MaxValue when casting back to char? Try and see).
I used Enumerable.Zip() function instead of manually looping through the input + key. Whenever possible you should use available functions to perform a task because they make your code faster to write, easier to read and...less buggy (they already been tested!) Take the time to explore LINQ functions, when working with collections they provide many things you may need.
I moved everything not directly related to encryption out of this function. Now I can test encryption in isolation!

It's an exercise for the reader to do the same thing for the Decrypt() function and to move magic values (like 0 and 9) to appropriate const int fields. Also, as exercise, think that adding a random value to a character does not necessarily produce another valid character (Unicode surrogates) and the string resulting from concatenation may not be a valid or representable string (you'd better encode the IEnumerable<int> produced by encryption instead of simply creating a string out of it).

Your logic for looping in the Main() method is little bit weird. You have a loop, you exit the loop and you recursively (from the encryption/decryption functions...) call Main() again. I will skip the StackOverflowException because you probably won't ever use this app long enough but it makes the program flow really hard to follow.

There are many ways to implement this but let's start with something easy:

static void Main()
{
    while (true)
    {
        var actionToPerform = Prompt("(E)ncrypt, (D)ecrypt or (Q)uit?");
        if (actionToPerform.Equals("e", StringComparison.CurrentCultureIgnoreCase))
        {
            ReadTextAndEncrypt();
        }
        else if (actionToPerform.Equals("d", StringComparison.CurrentCultureIgnoreCase))
        {
            ReadTextAndDecrypt();
        }
        else if (actionToPerform.Equals("q", StringComparison.CurrentCultureIgnoreCase))
        {
            return;
        }
        else
        {
            Console.WriteLine("Unknown option. Try again.");
        }
    }
}

The same can be done using a dictionary (<string, Action>) but for now let's keep it simple. For string comparison I do not use ToLower() but the appropriate overload of String.Equals() where I can specify a StringComparison value. The schoolbook example about case insensitive comparison is the i in Turkish (== performs comparison using current culture) where "i" != "I".ToLower() (and similarly "i".ToUpper() != "I").

Note the Prompt() function which is also reused elsewhere:

static string Prompt(string prompt)
{
    Console.Write(prompt + ' ');
    return Console.ReadLine();
}

We can now write our ReadTextAndEncrypt() function putting the things together:

static void ReadTextAndEncrypt()
{
    (string key, string encrypted) result = Encrypt(Prompt("Enter the text to encrypt:"));

    var key = new String(result.key.Select(x => (char)('0' + x)));
    Console.WriteLine("Encryption key is: {0}, key);
    Console.WriteLine("Encrypted string is: {0}, result.encrypted);
}

That's all, now, as exercise, write ReadTextAndDecrypt() with the same logic. You may also add Console.Clear() and Console.ReadKey() as appropriate (also that conversion for result.key isn't something to be proud of...)

In this example where I wrote (T1 key, T2 encrypted) I used value tuples (a C# 7 feature), it's similar to Tuple<T1, T2> but with an attached compile-time name. Function may be well rewritten as:

static Tuple<IEnumerable<int>, string> Encrypt(text)
{
    var key = GenerateEncryptionKey(text.Length);
    var encryptedText = new String(text.Zip(key, (v, k) => (char)(v + k)));

    return Tuple.Create(key, encryptedText);
}


Canvas performance and stable frames per second I'm sure this has probably been asked before (although I couldn't find a question akin to this one).

I have a canvas "app" that is refreshing persistently, but performance is awful on mobile devices. I just wondered if there was a nice alternative to requestAnimFrame() or the like.

Here is my "app":
http://www.barriereader.co.uk/Principality/

Here is the main rendering methods (I've removed the actual rendering - you can view that here http://www.barriereader.co.uk/Principality/System/Engine.js)

var fps = 0;
var oldtime = +new Date();
var time;

var Engine = {
    /** FOR DEBUGGING **/
    FPS: true,
    /*******************/

    Canvas : null,
    CTX : null,
    GameLooper : null,
    NodeSize : 10,
    MapSize : 32,
    Init : function (w, h) {
        Engine.Canvas = document.createElement('canvas');
        document.body.appendChild(Engine.Canvas);
        Engine.CTX = Engine.Canvas.getContext('2d');
        Engine.Canvas.width = w;
        Engine.Canvas.height = h;
        Engine.GameLoop();
    },
    Update : function (time) {
        fps = 1000/(time-oldtime)
        oldtime = time;
        Engine.Draw();
    },
    Draw : function () {
        Engine.CTX.save();
        Engine.CTX.setTransform(1, 0, 0, 1, 0, 0);
        Engine.CTX.clearRect(0, 0, Engine.Canvas.width, Engine.Canvas.height);
        Engine.CTX.restore();

        /**************DRAW_SCENE_HERE*****************/
            //A BUNCH OF RENDERING GOES HERE...
            //Performance is bad on mobiles :(
        /**********************************************/

        Engine.GameLoop();
    },
    GameLoop : function () {
        Engine.GameLooper = setTimeout(function () {
            requestAnimFrame(Engine.Update, Engine.Canvas);
        }, 10);
    },
};

window.requestAnimFrame = (function () {
    return window.requestAnimationFrame ||
    window.webkitRequestAnimationFrame ||
    window.mozRequestAnimationFrame ||
    window.oRequestAnimationFrame ||
    window.msRequestAnimationFrame ||
    function (callback, element) {
        window.setTimeout(callback(+new Date), 1000 / 60);
    };
}());

window.onload = function () {
    Engine.Init(320, 320);
};

As far I understand your code, here's are some bumps:

Cache your values

Property access in objects causes a very minor overhead. A very common example of this case is a normal loop. In this version of the loop, you'd be accessing length i times during the course of the loop.

for(i = 0; i < someArray.length; ++i){

It may not matter in normal array use, where array length is less than a hundred records at best. But when you are aiming for at least 60fps and doing pixel operations in between, these minor optimizations do give you that edge.

Just to point you to the right perspective, a canvas of 1024x768 has 3,145,728 entries in the image data array! Imagine doing a loop at that scale, manipulating pixels on every iteration, do this on every frame for 60fps. I tell you, my CPU (fan) screamed.

So instead of the code above, the following code solves the problem by caching the length, and in effect, only accessing length on the object once.

var arrayLength = someArray.length;
for(i = 0; i < arrayLength; ++i){

Use a closure

Sure, cache the variables, but where? I assume you made the Engine object as your namespace to avoid globals. But if property access is an overhead, where should we place your code?

Well, you should create a closure for your code. That way, you have a scope to play with, where:

You can place your code freely without using a namespace (thus avoiding property access).
At the same time not polluting the global space.

Consider this:

(function(ns){

  var Update = function(){/*Engine.Update code*/}
  var Canvas = function(){/*Engine.Canvas code*/}
  var GameLooper;

  function GameLoop(){
    requestAnimationFrame(GameLoop);
    Update();
    Canvas();
  }

  //Expose init to our namespace
  var Init = ns.Init = function(){/*Init code*/}

}(this.Engine = this.Engine || {}));

window.onload = function(){
  Engine.Init(320, 320);
}

As you can see, you don't need to call Engine.something(). You can directly do something() directly. This solves your property access and namespacing problems, two birds with one stone.

rAF

requestAnimationFrame, often abbreviated as rAF, is a more optimized timer compared to setTimeout and setInterval. It's basically built with animations in mind, and is optimized to give you the best framerates as possible. It's also an efficient timer since it slows down if the tab is set to the background, thus not eating up processing time.

However, I notice that you are hampering the rAF call with a setTimeout.

GameLoop : function () {
  Engine.GameLooper = setTimeout(function () {
    requestAnimFrame(Engine.Update, Engine.Canvas);
  }, 10);
},

This code has several bad effects as well, aside from those mentioned earlier:

Your game loop runs in intervals of ~10ms. You are not reaping the benefits of rAF at all. You are basically slowing down the game loop.
Also, timer delays don't really reflect the actual time they execute, thus in this code, the delay isn't 10ms. It could be greater, depending on what JS is doing at the moment.
You are creating a timer and an anonymous function on every iteration. setInterval would have been a better option (but not better that rAF) because it creates only one timer and one anonymous function.

With rAF, try this instead:

// Aapplying our no-property access policy
var GameLoop = function () {

  // Schedule the next frame ASAP
  requestAnimFrame(GameLoop);

  // In the meantime, call the functions for this frame
  Update();
  Canvas();
}

Notice that the scheduled call is GameLoop. This way, your functions Update and Canvas don't need to call back GameLoop to run the next frame. The next frame is already scheduled with this call.

Date....now()!

Yes, in recent browsers, there's Date.now() which is an optimized substitute to +new Date() and new Date().getTime() to get the current timestamp.

In classical OOP terms, it's a "static" member, which means you don't need to create an instance of Date to use it. This way, you are not generating a new object every time just to get the current time, thus saving you memory.


Calculating angles and distances I am running a simulation with 250 interacting agents and have a few functions that are called over and over again. Even with precomputing all distances between agents before the N2 (250x250) interaction loop, my simulation is still very slow. Are there any C++ optimization tricks that I could use to speed these up?

This is the most-used function in my simulation. It calculates the distance2 between two agents in a continuous space. I have a feeling there isn't much that can be done to further optimize this, but you guys have surprised me with some tricks before:

double tGame::calcDistanceSquared(double fromX, double fromY, double toX, double toY)
{
    double diffX = fromX - toX;
    double diffY = fromY - toY;

    return ( diffX * diffX ) + ( diffY * diffY );
}

Here's another expensive function in my simulation. It calculates the angle from one agent to another agent relative to the 'from' agent's heading. As you can see, I already did a little precomputing with the atan2() function (and that DOES speed things up a bit, despite what I've read in other posts).

double tGame::calcAngle(double fromX, double fromY, double fromAngle, double toX, double toY)
{
    double Ux = 0.0, Uy = 0.0, Vx = 0.0, Vy = 0.0;

    Ux = (toX - fromX);
    Uy = (toY - fromY);

    Vx = cosLookup[(int)fromAngle];
    Vy = sinLookup[(int)fromAngle];

    int firstTerm = (int)((Ux * Vy) - (Uy * Vx));
    int secondTerm = (int)((Ux * Vx) + (Uy * Vy));

    if (fabs(firstTerm) < 1000 && fabs(secondTerm) < 1000)
    {
        return atan2Lookup[firstTerm + 1000][secondTerm + 1000];
    }

    else
    {
        return atan2(firstTerm, secondTerm) * 180.0 / cPI;
    }
}

Finally, here's the monster function that uses the calcDistanceSquared() function so much. This is run every simulation time step, and there's 2,000 time steps per simulation (and MANY simulations). The most expensive part is the calcDistanceSquared() in the N2 loop.

void tGame::recalcPredAndPreyDistTable(double preyX[], double preyY[], bool preyDead[],
                                       double predX, double predY,
                                       double predDists[250], double preyDists[250][250])
{
    for (int i = 0; i < 250; ++i)
    {
        if (!preyDead[i])
        {
            predDists[i] = calcDistanceSquared(predX, predY, preyX[i], preyY[i]);
            preyDists[i][i] = 0.0;

            for (int j = i + 1; j < 250; ++j)
            {
                if (!preyDead[j])
                {
                    preyDists[i][j] = preyDists[j][i] = calcDistanceSquared(preyX[i], preyY[i], preyX[j], preyY[j]);
                }
            }
        }
    }
}

It looks good to me. I would change

    if (!preyDead[i])
    {

To

    if (preyDead[i])
            continue;

Just so its less nested. Same with preyDead[j]. But everything looks fine to me.
A tip I once saw on SO is, if your list order doesn't matter you can sort the preyDead so all the dead would be at the beginning or end and that will help branch prediction. However thats assuming its not really expensive to sort it and that its a very erratic bool and it isnt true/false 90% of the time already.

Thats a uber optimize that may not help, it shouldn't be done less you really really want to


Categorizing gene sequences read from a CSV file I am relatively new to programming and would love to get some feedback on the following section of my code.

class Gene:
    def __init__(self, gene_symbol, gene_id):
        # gene_symbol represents the abbreviated name of the gene (string)
        # Example: 'RHO' (short for 'Rhodopsin')
        self.gene_symbol = gene_symbol

        # gene_id represents an accession ID for the gene (string)
        # Example: 'NM_000539.3'
        self.gene_id = gene_id

        # is_valid_ functions check if the type of the gene_id is valid
        # and returns True or False
        # Assume that the types of the gene_id are mutually exclusive
        if is_valid_refseq(gene_id):
            # Example of a valid refseq: 'NM_000539.3'
            self.gene_id_type = REFSEQ

        elif is_valid_ensembl_gene(gene_id):
            # Example of a valid ensembl_gene: 'ENSG00000163914'
            self.gene_id_type = ENSEMBL_GENE

        elif is_valid_ensembl_transcript(gene_id):
            # Example of a valid ensembl_transcript: 'ENST00000296271'
            self.gene_id_type = ENSEMBL_TRANSCRIPT

        else:
            raise InvalidGeneIDError("Invalid gene_id: {}".format(gene_id))

refseqs = []
ensembl_genes = []
ensembl_transcripts = []

with open(csv_gene_list, 'r', newline='') as csv_input:

    reader = csv.reader(csv_input, delimiter=',')
    next(reader)
    for row in reader:
        row_gene_symbol = row[0]
        row_gene_id = row[1]

        row_gene = Gene(row_gene_symbol, row_gene_id)

        if row_gene.gene_id_type == REFSEQ:
            refseqs.append(row_gene)

        elif row_gene.gene_id_type == ENSEMBL_GENE:
            ensembl_genes.append(row_gene)

        elif row_gene.gene_id_type == ENSEMBL_TRANSCRIPT:
            ensembl_transcripts.append(row_gene)

        else:
            # What do I do here?
            raise AssertionError('Unrecognized gene_id_type: {}'.format(
                    row_gene.gene_id_type))

Assume gene_symbol and gene_id are strings and the is_valid_ functions are implemented elsewhere in my code.

I would like to ask if implementing these multiple if-else statements is the right approach. I feel like my code might get cluttered if I add more of these conditional expressions down the line.

Another question is on the very last else statement. What do I do here in a case like this? If the code works as intended, then it shouldn't reach the last else statement because an InvalidGeneIDError should've been raised when initializing the Gene object. One solution is to omit this part and change elif row_gene.gene_id_type == ENSEMBL_TRANSCRIPT: to else:, but I would much rather explicitly state the specific check to help convey the intentions of my code.

My main priority as I move forward is to focus on developing good coding style/habits and to write readable code. I would also like to learn/discover new tools/tricks that Python has to offer. Any suggestions and help would be greatly appreciated.

Yeah, the problem is the switch: you have the same set of  if-elif-else conditions duplicated in two places. Chances are that as your program grows, you will add more replicas of these chains. The problem with these chains is that if later you need to add one more case in the middle, you will need to modify all replicas accordingly. This is error prone, as you might not remember all the places where you replicated the chains.

How can we deal with that better? Can we replace these chains with something else, in a way that if we add a new case in the Gene constructor, we can be free from worrying about the rest of the program?

At least in this example, there is a workable solution. Consider these 3 lists:

refseqs = []
ensembl_genes = []
ensembl_transcripts = []

These lists are an important part of the problem. They mirror the currently supported 3 types, and when you put items in these lists, you need to refer to these lists by these names exactly.

Instead of these lists, you could use a dictionary of lists, where the keys are the gene types, and the values are lists. Something like this:

genes = dict()

with open(csv_gene_list, 'r', newline='') as csv_input:

    reader = csv.reader(csv_input, delimiter=',')
    next(reader)
    for row in reader:
        row_gene_symbol = row[0]
        row_gene_id = row[1]

        row_gene = Gene(row_gene_symbol, row_gene_id)

        if row_gene.row_gene_id not in genes:
            genes[row_gene.row_gene_id] = []

        genes[row_gene.row_gene_id].append(row_gene)
        # ...

In this form the chain is gone, and the second part of your question is naturally gone too.

As @jaime pointed out in a comment, this can be greatly simplified:

if row_gene.row_gene_id not in genes:
    genes[row_gene.row_gene_id] = []

genes[row_gene.row_gene_id].append(row_gene)

Using setdefault like this:

genes.setdefault(row_gene.row_gene_id, []).append(row_gene)


List User Online Status I'm using Rails with MongoDB for database. In my apps I have 1 action for displaying list of users. User has name, image, score and is_online attributes. The list of users is displayed after ordering with multiple fields (score and is_online) and paging them.

I should now get all users and order by is_online and score and then paging them:

User.where(role_id: Core::Role.where(name: 'member').first.id).order_by([:is_online, :desc], [:score, :desc]).page(params[:page]).per(10)

The problem are:

I think using pagination will ruin the list of users like a duplicate user in some page if the status is online or the score is changed.
If I remove pagination it will overload.

Should I using cache system like redis or else?

You have a few challenges here.

As you say, pagination will be wonky if a user's status/score changes between page requests. You could use fragment caching to cache the rendered pages, and serve the same content to all users for a fixed period of time. But this has a different drawback: Rendering is lazy, so page 2 (or 3, or N) will only be cached if someone requests it. So that's the same problem as before; sorting may change. Also, if the cache updates between page views, it doesn't matter that you're using a cache, since it's not that same cache version as was used in the previous request.

So what you'll want to do is run something that periodically rebuilds/-queries the entire list, a multi-version cache, and a way to lock a user into a specific version for his/her subsequent page views.

The tradeoff (besides more work) is that the list will be stale after a while.

An example of this is stackexchange's rep leagues tables which, while based on score, lags a bit behind actual score. You can decide whether that's worth the tradeoff.

Now, for querying periodically, I'd recommend

using an ActiveJob task to do the querying at fixed intervals
storing multiple versions of the query results as a hash in Rails.cache, using a timestamp as key for simplicity
a way of cleaning out old results, so stuff doesn't keep accumulating
adding a param to page requests to identify the active cache version/timestamp (if blank, serve the latest)
some means of telling the user "we're serving you a snapshot of this list as of [time and date]" - and a way of saying "the snapshot you requested is too old, and we've thrown it away"

In the end, it's simpler to only show unpaginated (but limited to top-N) lists with no caching. Or show lists that use a (more) stable sorting parameter (e.g. user name).


Matching parenthesis This is a simple implementation of a parenthesis-matcher. Given an expression, I want to find out if it is balanced.For example, the expression '{{[[()]]}}' is balanced while '{]{{}}]]' is not. I will output a simple 'NO' is the expression is not balanced and 'YES' if it is.

The algorithm is to push every opening parenthesis into a stack and match it against every closing when when pop-ed.

#include <iostream>
#include <vector>
#include <set>
#include<stack>
#include <utility>
#include <string>

bool contains(std::set<char> & inset, char elem){
  return inset.find(elem) != inset.end();
}

bool charmatch(char a,char b){
  return ((a == '(' && b == ')') ||
          (a == '{' && b == '}') ||
          (a == '[' && b == ']'));
}

void process(std::string inputArg){
  std::vector<char> input(inputArg.begin(), inputArg.end());

  std::set<char> openParen;
  openParen.insert('(');openParen.insert('{');openParen.insert('[');

  std::set<char> closeParen;
  closeParen.insert(')');closeParen.insert('}');closeParen.insert(']');

  std::stack<char> exprStack;
  for (auto i : input){
    char inputChar = i;
    if (contains(openParen, inputChar))
      {
        exprStack.push(inputChar);
        continue;
      }

    else if (exprStack.empty() || !charmatch(exprStack.top(),inputChar)){
      std::cout << "NO" <<std::endl;
      return;
    }

    exprStack.pop();

  }
  if (exprStack.empty())  {std::cout << "YES" << std::endl; return;}
  else {std::cout << "NO" << std::endl; return;}
}

int main()
{
  std::vector<char> input;
  int num;std::string inputString;
  std::cin >> num;
  for (int i = 0; i < num; i++){
      std::cin >> inputString;
        process(inputString);
    }
  return 0;
}

I'm practicing Data Structures questions on HackerRank, any suggestions to improve code quality appreciated!

Let's go top down:

charmatch is a bad name for the function. parenMatch would make much more sense.
Judging by the one range-based for loop, you're using C++11 - so might as well use it to the full extend:

You can use range-based for loops directly on a std::string. There is no need to copy it into a vector. So the input vector can be removed.
You can initialize a std::set with an initializer list:

std::set<char> openParen = { '(', '[', '{' };

Not sure why the range based loop variable is called i just to be copied into a loop-local variable called inputChar. You might as well name the loop variable that way.

for (auto inputChar : inputArg) {
    ....

closeParen is never used and can be removed.
exprStack is not really the best name - parenStack would be more appropriate.
You should use spaces more consistently. Your indent is off here and there and you often omit a space before the opening brace which doesn't help readability. Also a lot of people prefer 4 spaces indent but YMMV.
Use a consistent bracing style. Sometimes you put the opening brace on it's own line and sometimes it's on the previous one and sometimes the entire block is on one line.
I'd return a bool from process instead of writing directly to std::out - let the caller deal with the output concern (Single Responsibility Principle).
input in main is never used and should be removed.
You don't have to explicitly return 0 from main.

Update

Regarding your question of making charmatch more efficient: charmatch is already pretty efficient - at most 8 compares and 3 ANDs. You could turn it into a switch:

bool parenMatch(char left, char right)
{
    switch (left)
    {
        case '(': return right == ')';
        case '[': return right == ']';
        case '{': return right == '}';
    }
    return false;
}

However this is a micro-optimization which will probably have close to no effect on the execution speed, in fact I wouldn't be surprised if the generated code is almost the same.


Storing a message from a queue I am doing a code review and I have encountered a class in an application that is throwing an exception in the constructor:

class QueueMessage
{
private:
    std::string m_bucketName;
    std::string m_objectName;

public:
    QueueMessage(const std::string& messageIn)
    {
        std::stringstream ss;
        ss << messageIn;
        PTree pt;
        json::read_json(ss, pt);
        m_bucketName = pt.get<std::string>("bucket");
        m_objectName = pt.get<std::string>("path");
        if (m_bucketName.empty() || m_objectName.empty())
        {
            throw QueueMessageException("Empty fields in queue message");
        }
    }

    std::string getBucketName() const { return m_bucketName; }
    std::string getObjectName() const { return m_objectName; }
};

The class is used to store the message from the queue (that is a JSON-like text) in its members.  Is it ok to throw an exception in the constructor if the members of the class are string, int, vector, smart pointers, etc.?

Throwing exceptions from a constructor is not only a perfectly legitimate pattern to use. It is also necessary to correctly implement Resource Acquisition Is Initialization (RAII) for some classes (this appears to be the case in your code). And trust me, you really do want RAII :)

However, please be aware of the caveat that the destructor of the object will not be executed if the constructor throws. But the individual members of the class will be destructed.

On a related note, it is also okay for the constructor to perform work if the work is necessary for RAII. Again under the caveat that you adhere to Single Responsibility Principle (SRP) and Dependency Injection (DI). See this question: https://stackoverflow.com/questions/7048515/is-doing-a-lot-in-constructors-bad


Check whether given a pair of arrays are permutation of each other Does this implementation guarantee execution of \$O(n)\$ time?  What is the additional space utilization for this implementation by excluding the original array size? Is it \$O(1)\$?

#include<iostream>
#include<set>
#include<algorithm>

bool ArraysPermute(int  array1[],int size1, int  array2[], int size2)
{
       if( size1  != size2)
           return false;
       else
       {
           std::set<int> first_set(array1, array1+size1);
           std::set<int> second_set(array2, array2+size2);

           std::pair<std::set<int>::iterator,std::set<int>::iterator> myPair=std::mismatch(first_set.begin(),first_set.end(),second_set.begin());

           if( *myPair.first != *myPair.second)

                return false;
            else
                return true;
       }
}

int main()
{
    int array1[] ={1,2,3,5};
    int array2[]={1,2,4,3};

    if(ArraysPermute(array1,sizeof(array1)/sizeof(int), array2, sizeof(array2)/sizeof(int)))
        std::cout<< " Arrays are permutation of each other\n";
    else
        std::cout<< " Arrays are not permutation of each other\n";

    return 0;
}

Complexity

You suggest that the performance is \$O(n)\$, but it is not. It is \$O(n \log{n})\$ because the set constructor is a binary-tree building algorithm that has an additional log(n) operations for each value.

Style

While your code is all neat, and well laid out, the variable names are good, etc., there are two issues I see with unnecessary else-condiditions on if-statements, and un-braced 1-liners. Additionally, white-space around punctuation and operators is useful for readability. This code:

  if( size1  != size2)
       return false;
   else
   {
       std::set<int> first_set(array1, array1+size1);
       std::set<int> second_set(array2, array2+size2);

       std::pair<std::set<int>::iterator,std::set<int>::iterator> myPair=std::mismatch(first_set.begin(),first_set.end(),second_set.begin());

       if( *myPair.first != *myPair.second)

            return false;
        else
            return true;
   }

should be written:

if( size1 != size2)
{
    return false;
}
std::set<int> first_set(array1, array1 + size1);
std::set<int> second_set(array2, array2 + size2);

std::pair<std::set<int>::iterator, std::set<int>::iterator> myPair
   = std::mismatch(first_set.begin(), first_set.end(), second_set.begin());

return *myPair.first == *myPair.second;

Bugs

The set construct keeps just a single entry for all instances of a given value. Thus, for example, given the input [1,2,3,4,4,4,4] it will have just 4 entries.

This leads to bugs in your code when there are duplicates. For example, your code will identify the following two input arrays as being permutations of each other:

[1,2,3,4,5,5,5]
[1,1,1,2,3,4,5]


Generic solution to Hibernate's CriteriaQuery I've created a little class which helps me to generalize many query cases. I have a DbService and couple methods in it. Can you please take a look at it and share some criticism? The usage of that tool is really easy to me and I do wanna know how to make it safer and optimized.
 public class DbServiceImpl implements DbService {
 private final EntityManager entityManager;

 @Inject
 DbServiceImpl(EntityManager entityManager) {
    this.entityManager = entityManager;
 }

 @Override
 public <T, V> List<V> createQuery(Query<T, V> query) {
    try {
        if (query != null && query.entityType.getAnnotation(Entity.class) != null) {
            CriteriaBuilder criteriaBuilder = entityManager.getCriteriaBuilder();

            CriteriaQuery<V> criteriaQuery = criteriaBuilder.createQuery(query.returnType);

            query.root = criteriaQuery.from(query.entityType);
            query.builder = criteriaBuilder;

            Selection<? extends V> selection = query.select(); //invokes on the call.
            Predicate predicate = query.where();

            if (predicate != null) {
                criteriaQuery.where(predicate); //null of a simple select without where clause.
            }

            criteriaQuery.select(selection); //null is allowed, it will just select * from the entity

            return entityManager.createQuery(criteriaQuery).getResultList();
        }
      } catch (Exception exception) {
        exception.printStackTrace();
      }
    return new ArrayList<>(); //a simple check of .isEmpty() covers that case.
   }

 @SuppressWarnings("unchecked")
 public abstract static class Query<T, V> {
    private final Class<T> entityType;
    private final Class<V> returnType;

    //accessible only by outer class's methods.
    private Root<T> root;
    private CriteriaBuilder builder;

    public Query() {
        this.entityType=(Class<T>) ((ParameterizedType) getClass()
                .getGenericSuperclass()).getActualTypeArguments()[0];
        this.returnType=(Class<V>) ((ParameterizedType) getClass()
                .getGenericSuperclass()).getActualTypeArguments()[0];
    }

    protected abstract Selection<? extends V> select();

    protected abstract Predicate where();

    protected Root<T> root() {
        return root;
    }

    protected CriteriaBuilder builder() {
        return builder;
    }
  }
}

And here are some use cases. Passed types are as follows - entity & return (User,Long)
  List<Long> data = dbService.createQuery(new DbServiceImpl.Query<User, Long>() {
        @Override
        protected Selection<? extends Long> select() {
            return builder().count(root());
        }
        @Override
        protected Predicate where() {
            return root();
        }
    });

This allows me to find the count of a table so understandable, without a line of pure HQL or any SQL code. One more example:
  List<User> users = dbService.createQuery(new DbServiceImpl.Query<User, User>() {
        @Override
        protected Selection<? extends User> select() {
            return root();
        }

        @Override
        protected Predicate where() {
            return builder().equal(root().get("username"), username);
        }
    });

One with a join:
    List<Recipe> recipeList= dbService.createQuery(new DbServiceImpl.Query<Recipe, Recipe>() {
        @Override
        protected Selection<? extends Recipe> select() {
            return null;
        }

        @Override
        protected Predicate where() {
            root().fetch("author", JoinType.LEFT);
            return builder().equal(root().get("author").get("id"),user.getId());
        }
    });

The code structure remains the same but the query is different. The access modifiers I used allowed me to encapsulate the behaviour on the use cases. What do you think? How broken or useful is that?

Welcome to Code Review, I have seen your code and I have seen you encapsulated your createQuery method like below:
try { body method } catch (Exception exception) { exception.printStackTrace(); }

You are basically catching all NullPointerException cases and other runtime exceptions you can meet within your method with the Exception class, this is a sign something should be redesigned to avoid these situations.
About the paradigm you are following , you are implementing a personal version of the repository pattern, referring to structures like the jpa repository present for example in the spring framework. You can check while you are creating different queries worrying about the construction of a valid query (and this is the reason why you had to encapsulate your method in the try catch construct), it should be better using an already built class to do the same things with much less effort.


Remove blank entries from row I wrote a sub to remove the blank entries in a row without shifting the cells around but it seems unnecessarily clunky and I'd like to get some advice on how to improve it.

Public Sub removeBlankEntriesFromRow(inputRow As Range, pasteLocation As String)
    'Removes blank entries from inputRow and pastes the result into a row starting at cell pasteLocation

    Dim oldArray, newArray, tempArray
    Dim j As Integer
    Dim i As Integer

    'dump range into temp array
    tempArray = inputRow.Value
    'redim the 1d array
    ReDim oldArray(1 To UBound(tempArray, 2))
    'convert from 2d to 1d
    For i = 1 To UBound(oldArray, 1)
        oldArray(i) = tempArray(1, i)
    Next
    'redim the newArray
    ReDim newArray(LBound(oldArray) To UBound(oldArray))
    'for each not blank in oldarray, fill into newArray
    For i = LBound(oldArray) To UBound(oldArray)
        If oldArray(i) <> "" Then
            j = j + 1
            newArray(j) = oldArray(i)
        End If
    Next
    'Catch Error
    If j <> 0 Then
        'redim the newarray to the correct size.
        ReDim Preserve newArray(LBound(oldArray) To j)
        'clear the old row
        inputRow.ClearContents
        'paste the array into a row starting at pasteLocation
        Range(pasteLocation).Resize(1, j - LBound(newArray) + 1) = (newArray)
    End If
End Sub

Let's start with the procedure's signature:

Public Sub removeBlankEntriesFromRow(inputRow As Range, pasteLocation As String)

I like that you're explicitly making it Public - VBA procedures are Public by default, but in most other languages members are private if an access modifier isn't specified, which can make reading VBA code a bit confusing if you don't know this and you're switching languages (VB.NET, C#, etc.). Even if the only language you code in is VBA, being explicit about access modifiers is a very good practice, keep it! :)

I also like that your procedure has a meaningful name that's starting with a verb - procedures do something, and they should be doing something very specific. Giving a specific name to your procedures makes it easier to see what's doing what. By convention, removeBlankEntriesFromRow should be RemoveBlankEntriesFromRow though, as PascalCase is the recommended casing style for pretty much everything except local variables and parameters.

Parameters can be passed by value (i.e. method receives its own copy) or by reference (i.e. reassigning a parameter will affect the caller's copy too) - in other languages parameters are often passed by value by default, but VBA passes them ByRef unless you explicitly specify they're ByVal.

Hence, if you don't intend to reassign a parameter and have the calling code "see" that new value, you should always pass parameters by value, like this:

ByVal inputRow As Range, ByVal pasteLocation As String

Dim oldArray, newArray, tempArray

For better readability and maintainability, it's best to declare the 3 variables on 3 separate instructions:

Dim oldArray
Dim newArray
Dim tempArray

Now, these 3 variables are all implicitly Variant. If you intend them to be Variant, it's best to say so - and since a Variant can be just about anything in VBA, it will happily hold an array of variants - better declare them as their exact type - Variant():

Dim oldArray As Variant()
Dim newArray As Variant()
Dim tempArray As Variant()

And now we know what we're up against! :)

Dim j As Integer
Dim i As Integer

Typical loop counters, everybody calls 'em that. But it's probably better to name them after their usage, too - and Integer will probably fit most usages, but the day you need to run this on a range that has more than 32,767 cells it will overflow the 16-bit Integer type; I'd use a 32-bit Long here, just to be safe:

Dim oldIndex As Long
Dim newIndex As Long

If oldArray(i) <> "" Then

This is a minor point, but I have to mention it: the hard-coded "" string literal may look like it's just an empty string, but it's actually allocated a memory space of its own. Nothing to worry about, but it's usually better to use the built-in constant vbNullString instead.

Try this in the immediate pane (Ctrl+G):

?StrPtr(""), StrPtr(vbNullString)

You should see a bunch of numbers (a memory address) for the StrPtr("") call, and a 0 for the StrPtr(vbNullString) call: vbNullString is completely zero-footprint, give it the love it deserves!

'Catch Error

This comment is misleading: you're not handling any runtime errors that may occur here. To handle errors, your procedure body should be templated something like this:

Public Sub DoSomething()
    On Error GoTo CleanFail

    'method body goes here

CleanExit:
    'cleanup code goes here
    Exit Sub

CleanFail:
    'error-handling code goes here
    Resume CleanExit
End Sub

That said, I would remove all but the first comment. Comments shouldn't say what the code is doing - that's the code's job (and that's why code readability matters). Good comments say why the code is doing something.

Wherever you have a comment that says "this chunk of code does X", then you might have an opportunity to name that chunk of code, by extracting it into its own function. That can be done manually of course, but the Rubberduck VBE add-in I've written with @RubberDuck makes it easier than ever: just select a block of code, and then refactor / extract method and you're done!


Dividing a ThreadPool class into component classes to reduce/isolate complexity I have a ThreadPool class which has grown to a point where it is becoming difficult to keep all of this unit of work in mind. It is also becoming harder to read. For this reason I would like to break this class into component classes.

First of all here is the class header with comments:

//ThreadPool.h

#ifndef THREADPOOL_H
#define THREADPOOL_H

#define doNothing()

#include <iostream>
#include <chrono>
#include <queue>
#include <vector>
#include <functional>
#include <thread>
#include <mutex>
#include <condition_variable>

typedef std::function< void() > voidFunctionType;

class ThreadPool{

 public:
  ThreadPool(unsigned int desiredThreadCount);
  ~ThreadPool();

  //Functions Affecting ThreadPool
  //Function used by threads to submit work to ThreadPool
  void workSubmit(voidFunctionType functionPointer);
  //Function used by threads to take work from ThreadPool
  voidFunctionType takeTask(void);
  //Function which returns the number of work in the queue
  unsigned int workCount(void);
  //Function which
  bool isWorkAvailable(void);

  //Functions Affecting Thread Availability.
  //Increments availableThreads variable which indicates number of threads available
  void incThreadAvailability(void);
  //Decrements availableThreads variable which indicates number of treads available
  void decThreadAvailability(void);
  //Returns availableThreads variable which indicates number of threads available
  unsigned int getThreadAvailability(void);
  //Function returns total number of threads generated
  unsigned int getThreadNum(void);

  //Functions Affecting Threads
  //Checks to see if work is available, if work is not available thread is put to sleep
  //If work is available thread processes work.
  void threadSleepUntillWorkAvail(void);

 private:
  // Number of threads generated
  unsigned int numberOfThreads;

  //Vector of threads
  std::vector<std::thread> workers;

  // Number of threads available to process work
  unsigned int availableThreads;
  // Mutex for the availableThreads variable
  std::mutex threadAvailabilityLock;
  std::unique_lock<std::mutex> threadAvailabilityLockUnique;

  //Workqueue to track work
  std::queue< std::function< void()> > workQueue;
  //WorkSubmissionLock mutex is used to lock submission of work only
  std::mutex workSubmissionLock;
  std::unique_lock<std::mutex> workSubmissionLockUnique;
  std::condition_variable workSubmissionCondition;
  //WorkQueueLock mutex is used to lock all access points to the queue it self
  std::mutex workQueueLock;
  std::unique_lock<std::mutex> workQueueLockUnique;
  std::condition_variable workQueueCondition;

  //Private Member Functions
  //workAccept is used within workSubmit. This function is used to load the workQueue with the
  //Accepted work.
  void workAccept(voidFunctionType functionPointer);

  //Operators
  //Function executed by threads.
  void operator()(void);
};
#endif

I have tried creating a thread component class, however it was highly coupled with the ThreadPool class and the only value it provided was a single routine which was the main routine the threads executed. So I replaced this class with the function method operator within the threadpool class.

Other than the thread class component I am unsure if I can extract out any other single responsibility from this class.

Note:

I am aware of a resource deadlock problem when 3 or more threads are created, I do not want this issue to be the focal point.
I do not think the implementation is relevant to the question, however, it can be viewed here.

There are way too many member variables:

unsigned int numberOfThreads;
std::vector<std::thread> workers;
unsigned int availableThreads;

Can you not find the number of threads with workers.size(). What's the difference between number of Threads and Available Threads and more importantly why do you need to know!

You have three mutex's and two condition variables! Why? Also why are you putting the unique_lock as a member of the class! That would make them hard to use in a shared context.

Normally I would expect to see:

//In class:
    std::mutex               lock;
    std::condition_variable  sync;

//Adding Stuff Function:
    std::lock_guard   guard(lock);
    queue.push(item);
    sync.notify();

//Taking stuff from Queue
    std::lock_guard   guard(lock);
    sync.wait(guard, [queue](){!queue.empty();});
    if (!queue.empty()) {
        result = queue.pop();
    }

The only method that should be public on that class is adding a new job. All other methods are private members.

This is what mine would look like this:

class WorkQueue
{
    private:
        std::vector<std::thread>            threads;
        std::deque<std::function<void()>>   queue;
        std::mutex                          lock;
        std::condition_variable             sync;
        bool                                finished;

        std::function<void()> getWork();
        void work();
    public:
        WorkQueue(int threadCount);
        ~WorkQueue();
        template<typename F>
        void addWork(F&& func);
};


Best practice to load settings to the web application I have to load settings to an app when it's opened (It's an Electron application and I'm loading settings from the local json file)

I'm loading settings right after the page is loaded:

document.onreadystatechange = function(){
    if (document.readyState === 'complete'){
        loadSettings();
    }
}

but I think this is not the right way to do it because at first, the page sets inputs, like checkboxes, as they are in HTML file and only after that, changes their values again according to settings.
I want to know what is the best practice to load the settings to a web app.

I can't claim to be familiar with Electron, but after looking at the documentation and an example on github, it appears that an electron convention is to use app.on/app.once

var electron = require('electron') // http://electron.atom.io/docs/api
var path = require('path')         // https://nodejs.org/api/path.html
var url = require('url')           // https://nodejs.org/api/url.html
var window = null
// Wait until the app is ready
electron.app.once('ready', function () {
  // Create a new window
});

In general, Javascript developers can subscribe to the readystatechange event as you have done, as well as use document.addEventListener() to subscribe to the DOMContentLoaded event:

document.onreadystatechange = function(){
  console.log('document readystate: ',document.readyState);
}
document.addEventListener('DOMContentLoaded',function() {
  console.log('DOM Load: document readystate: ',document.readyState);
})

You might find this SO post interesting...


Two solutions with min max values - which should be slower? While practicing at codinbat.com on the assignment:

  Given an array length 1 or more of ints, return the difference between the largest and smallest values in the array. Note: the built-in Math.min(v1, v2) and Math.max(v1, v2) methods return the smaller or larger of two values.

I wrote a solution (1):

public int bigDiff(int[] nums)
{
    int min = nums[0];
    int max = nums[0];

    for (int i = 1 ; i < nums.length; i++)
    {
        if (nums[i] < min)
        {
            min = nums[i];
        }
        if (nums[i] > max)
        {
            max = nums[i];
        }
    }

    return max - min;
}

You can see it has no Math.min or Math.max calls. So then I rewrote the solution to see how it looks with Math.min and Math.max:

public int bigDiff(int[] nums)
{
    int min = nums[0];
    int max = nums[0];

    for (int i = 1 ; i < nums.length; i++)
    {
        min = Math.min(nums[i],min);
        max = Math.max(nums[i],max);
    }

    return max - min;
}

The solution became a little shorter, but I think it makes unnecessary assignments when going through the array.

Is solution (2) in general slower than solution (1)?

To answer your question: those solutions are identical performance-wise. There is no "magic" in Min or Max methods: they do the same thing you do in your first solution - compare the two values using if operator. There is no significant performance cost for a method call or for an extra assignment - those operations are so cheap, that you won't solve any real-life performance issue with this kind of optimization. If you will become obsessed with those small things - it will have a negative effect on your code quality. So, instead of asking, which is faster, you should ask yourself which is more clean, readable and OOP-ish. I would go with second solution.

If you want to really speed things up - you should first identify the real problem. In your example - it is the number of iterations, not the number of assignments. So the faster solution would probably involve multi-threading and iterating through different parts of your array simultaneously.


Still learning, looking for general advice about a 4-function calculator I programmed using java here's some code I wrote in java SE to get use to if statements and scanning. just looking for suggestions that will improve this program or/and teach me techniques that I should get use to using.
import java.util.Scanner;

public class Calculator {
   public static void main(String[] args) {
      System.out.print("Enter the first value ");
      Scanner scan = new Scanner(System.in);
      int x = scan.nextInt();
      System.out.print("Enter the next value ");
      int y = scan.nextInt();
      System.out.println("Choose an operation: 1-multiply 2-divide 3-add 4-subtract");
      int z = scan.nextInt();
      if (z == 1) {
         int a = x * y;
         System.out.println("Result: " + a);
      }//end of if multiply
      else if (z == 2) {
         int a = x / y;
         System.out.println("Result: " + a);
      }//end of if divide
      else if (z == 3) {
         int a = x + y;
         System.out.println("Result: " + a);
      }// end of if add
      else {
         int a = x-y;
         System.out.println("Result: " + a);
      } //end of is subtract
   }// End of main
}// End of public class

here's my new program, still not perfect but the class i'm taken hasn't even gone over data types and classes yet so i'm already ahead lol
Thank you every one for your help!
import java.util.Scanner;

 public class Calculatortwo {
   public static void main(String[] args) {
   //get imput values from user
      System.out.print("Enter the first value ");
      Scanner scan = new Scanner(System.in);
      double x = scan.nextDouble();
      System.out.print("Enter the next value ");
      double y = scan.nextDouble();
      System.out.println("Choose an operation: 1-multiply 2-divide 3-add 4-subtract");
      int operation = scan.nextInt();
    //calculator logic
      double result = 0;
      switch (operation) {
       case 1:
         result = x * y;
         break;
       case 2:
         if(y != 0) {
         result = x / y;
        }
        else {
        System.out.println("Error: Divide by 0");
        }
         break;
       case 3:
         result = x + y;
         break;
       case 4:
         result = x - y;
         break;
         default:
            System.out.println("Error: operation not found");
      }
            System.out.println("Result: " + result);
   }
}

It would be best to codify the various operations in a separate function. This limits the opportunity for error and eliminates the redundancy of the System.out.println("Result: " + a); statements.
   private static int applyOperation(int x, int y, int op) {
      switch(op) {
         case 1: return x*y;
         case 2: return x/y;
         case 3: return x+y;
         case 4: default: return x-y;
      }
   }
   public static void main(String[] args) {
      System.out.print("Enter the first value ");
      Scanner scan = new Scanner(System.in);
      int x = scan.nextInt();
      System.out.print("Enter the next value ");
      int y = scan.nextInt();
      System.out.println("Choose an operation: 1-multiply 2-divide 3-add 4-subtract");
      int op = scan.nextInt();
      int ans = applyOperation((x), y, op);
      System.out.println("Result: " + ans);
   }


MaxSum sub matrix within a matrix Code review for best practices, optimizations, code cleanup etc. Also requesting verification of the complexity: O(row*row*col).

/**
 * Contains coordinates of the
 * topMost left, of matrix, obtained by getRowOne and colRowOne
 * bottomMost right, of matrix, obtained by getRowTwo and getColTwo
 *
 * @author javadeveloper
 */
final class RectangleCoord {
    private final int rowOne;
    private final int colOne;
    private final int rowTwo;
    private final int colTwo;

    public RectangleCoord(int rowOne, int colOne, int rowTwo, int colTwo) {
        this.rowOne = rowOne;
        this.colOne = colOne;
        this.rowTwo = rowTwo;
        this.colTwo = colTwo;
    }

    public int getRowOne() {
        return rowOne;
    }

    public int getColOne() {
        return colOne;
    }

    public int getRowTwo() {
        return rowTwo;
    }

    public int getColTwo() {
        return colTwo;
    }
}

/**
 * Returns the maximum rectangle in the matrix
 *
 * Complexity:
 *  O(row2 * col)
 *
 * @author javadeveloper
 */
public final class MaximumSumRectangeInMatrix {

    // made this utility class non-instantiable
    private MaximumSumRectangeInMatrix() { }

    private static class MaxSubArray {
        int start;
        int end;
        int maxSum;

        public MaxSubArray(int start, int end, int maxSum) {
            this.start = start;
            this.end = end;
            this.maxSum = maxSum;
        }
    }

    private static MaxSubArray maxSubArraySum(int[] a) {
        assert a != null && a.length > 0;

        int max = a[0];
        int currSum = a[0];

        int start = 0;
        int end = 0;

        for (int i = 1; i < a.length; i++) {
            currSum = currSum + a[i];
            if (a[i] > currSum) {
                start = i;
                currSum = a[i];
            }
            if (currSum > max) {
                max = currSum;
                end = i;
            }
        }
        return new MaxSubArray(start, end, max);
    }

    /**
     * Returns the RectangleCoord object containing the maximum
     * rectangle in the matrix.
     *
     * @param  The input matrix.
     * @return The rectanglecoor object containing the coordinates of the maximum sub matrix
     */
    public static RectangleCoord calMaxRectangle(int[][] m) {
        if (m.length == 0 || m[0].length == 0) {
            throw new IllegalArgumentException("The matrix should be non empty");
        }

        int max = Integer.MIN_VALUE;
        int rowOne = 0;
        int colOne = 0;
        int rowTwo = 0;
        int colTwo = 0;

        for (int fromRow = 0; fromRow < m.length; fromRow++) {
            int[] temp = new int[m[0].length];
            for (int toRow = fromRow; toRow < m.length; toRow++) {
                for (int col = 0; col < m[0].length; col++) {
                    temp[col] = temp[col] + m[toRow][col];
                }

                MaxSubArray maxSubArrayCoord = maxSubArraySum(temp);

                if (maxSubArrayCoord.maxSum > max) {
                    max = maxSubArrayCoord.maxSum;
                    rowOne = fromRow;
                    colOne = maxSubArrayCoord.start;
                    rowTwo = toRow;
                    colTwo = maxSubArrayCoord.end;
                }
            }
        }

        return new RectangleCoord(rowOne, colOne, rowTwo, colTwo);
    }

    public static void main(String[] args) {

        int[][] m = {  {-20, -30, -40, -50},
                       { 10,  20,  30,  50},
                       { 50,  60,  70,  80},
                       {-10, -20, -30, -40},
                    };

        RectangleCoord r = calMaxRectangle(m);
        System.out.println(r.getRowOne() + " : " + r.getColOne());
        System.out.println(r.getRowTwo() + " : " + r.getColTwo());

    }

}

The begin returned by maxSubArraySub can be wrong as we update start after we've found a good candidate for the maximal sum. This can lead to potential bugs (I am way too lazy to find an actual case where it makes a difference but that should be possible). The solution from http://en.wikipedia.org/wiki/Maximum_subarray_problem introduces another variable to have a temporary starting point and one for the actual starting point of the maximum found so far.

You can make things much simpler by defining (very simple) classes for 3-tuple and 4-tuple. Make the members private and final and you won't need getters. As the point is just to return multiple values from a function, I don't think there a point in making things too specific and I'd rather have the class and the members have simple names.

You have a typo in your class name and it's especially annoying in Java where the file name has to match ;-)

You could make your code a bit more robust by asserting that all rows have same length.

In your code, the comment about complexity is wrong. The one is your stackoverflow question is right, it is indeed N*N*M :-)

Here's what I've done :

final class Tuple4 {
    public final int a;
    public final int b;
    public final int c;
    public final int d;

    public Tuple4(int a, int b, int c, int d) {
        this.a = a;
        this.b = b;
        this.c = c;
        this.d = d;
    }
}

final class Tuple3 {
    public final int a;
    public final int b;
    public final int c;

    public Tuple3(int a, int b, int c) {
        this.a = a;
        this.b = b;
        this.c = c;
    }
}

/**
 * Returns the maximum rectangle in the matrix
 *
 * @author javadeveloper
 */
public final class MaximumSumRectangleInMatrix {

    // made this utility class non-instantiable
    private MaximumSumRectangleInMatrix() { }

    private static Tuple3 maxSubArraySum(int[] a) {
        assert a != null && a.length > 0;

        int max = a[0];
        int currSum = a[0];

        int start = 0;
        int start_tmp = 0;
        int end = 0;

        for (int i = 1; i < a.length; i++) {
            if (currSum < 0) {
                start_tmp = i;
                currSum = a[i];
            } else {
                currSum += a[i];
            }
            if (currSum > max) {
                max = currSum;
                start = start_tmp;
                end = i;
            }
        }
        return new Tuple3(start, end, max);
    }

    /**
     * Returns the Tuple4 object containing the maximum
     * rectangle in the matrix.
     *
     * @param  The input matrix.
     * @return The rectanglecoor object containing the coordinates of the maximum sub matrix
     */
    public static Tuple4 calMaxRectangle(int[][] m) {
        if (m.length == 0 || m[0].length == 0) {
            throw new IllegalArgumentException("The matrix should be non empty");
        }

        int max = Integer.MIN_VALUE;
        int a = 0;
        int b = 0;
        int c = 0;
        int d = 0;

        int lengthRow = m[0].length;

        for (int i1 = 0; i1 < m.length; i1++)
        {
            if (lengthRow != m[i1].length)
                throw new IllegalArgumentException("Matrix is not square");

            int[] temp = new int[lengthRow];
            for (int i2 = i1; i2 < m.length; i2++)
            {
                for (int j = 0; j < lengthRow; j++)
                    temp[j] += m[i2][j];

                Tuple3 maxSubArrayCoord = maxSubArraySum(temp);

                if (maxSubArrayCoord.c > max) {
                    max = maxSubArrayCoord.c;
                    a = i1;
                    b = maxSubArrayCoord.a;
                    c = i2;
                    d = maxSubArrayCoord.b;
                }
            }
        }
        return new Tuple4(a,b,c,d);
    }

    public static void main(String[] args) {
        int[][] m = {  {-20, -30, -40, -50,  50},
                       { 10,  20,  30,  50, -10},
                       { 50,  60,  70,  80,  30},
                       {-10, -20, -30, -40,  60},
                    };

        Tuple4 r = calMaxRectangle(m);
        System.out.println("(" + r.a + "," + r.b + ") ; (" + r.c + "," + r.d + ")");
    }
}