Simple Java animation with Swing I am learning to use Java Swing and have made a simple animation that makes a small shape bounce around the predetermined borders of a panel. The program works well, but I am looking for feedback in terms of quality and any improvements or alternative methods that could be used in such an application.

import javax.swing.*;
import java.awt.*;

final public class Tester {

    JFrame frame;
    DrawPanel drawPanel;

    private int oneX = 7;
    private int oneY = 7;

    boolean up = false;
    boolean down = true;
    boolean left = false;
    boolean right = true;

    public static void main(String[] args) {
        new Tester().go();
    }

    private void go() {
        frame = new JFrame("Test");
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

        drawPanel = new DrawPanel();

        frame.getContentPane().add(BorderLayout.CENTER, drawPanel);

        frame.setVisible(true);
        frame.setResizable(false);
        frame.setSize(300, 300);
        frame.setLocation(375, 55);
        moveIt();
    }

    class DrawPanel extends JPanel {
        public void paintComponent(Graphics g) {
            g.setColor(Color.BLUE);
            g.fillRect(0, 0, this.getWidth(), this.getHeight());
            g.setColor(Color.RED);
            g.fillRect(3, 3, this.getWidth()-6, this.getHeight()-6);
            g.setColor(Color.WHITE);
            g.fillRect(6, 6, this.getWidth()-12, this.getHeight()-12);
            g.setColor(Color.BLACK);
            g.fillRect(oneX, oneY, 6, 6);
        }
    }

    private void moveIt() {
        while(true){
            if(oneX >= 283){
                right = false;
                left = true;
            }
            if(oneX <= 7){
                right = true;
                left = false;
            }
            if(oneY >= 259){
                up = true;
                down = false;
            }
            if(oneY <= 7){
                up = false;
                down = true;
            }
            if(up){
                oneY--;
            }
            if(down){
                oneY++;
            }
            if(left){
                oneX--;
            }
            if(right){
                oneX++;
            }
            try{
                Thread.sleep(10);
            } catch (Exception exc){}
            frame.repaint();
        }
    }
}

Things I would fix:

Don't do this:

import javax.swing.*;
import java.awt.*;

It could be problematic for the compiler to import a bunch of packages at once. If two packages provide the same type, both are imported, and the type is used in the class, a compile-time error occurs. This is described in JLS 6.5.5.1:

  Otherwise, if a type of that name is declared by more than one type-import-on-demand declaration of the compilation unit, then the name is ambiguous as a type name; a compile-time error occurs.

In addition, it also saves a bit of memory.  And your IDE (if you use one), should have the ability to do this automatically.

The serializable class DrawPanel does not declare a static final serialVersionUID field of type long (thanks to @GilbertLeBlanc for the link):

private static final long serialVersionUID = 1L;

Don't ever do this:

catch (Exception exc)
{
}

Say that you do get an Exception thrown at you. You aren't handling it right now in any way.  At least print a stack trace to help you solve problems you may encounter in the future:

catch (Exception exc)
{
    exc.printStackTrace();
}

It was also mentioned in the comments to catch specific Exceptions.  Here there is no real need to do that since only an InterruptedException is being thrown.  When you do need to catch multiple Exceptions, some people might tell you to do this:

catch (IOException io)
{
    io.printStackTrace()
}
catch (InterruptedException ie)
{
    ie.printStackTrace();
}

But you could also do this:

catch (IOException | InterruptedException e)
{
    e.printStackTrace();
}

Right now there are some hard-coded values:

if(oneX >= 283)
if(oneX <= 7)
...

I would store those values in variables, and then I would make it so that it is more scalable.  For example:

int xScale = 280;
int yScale = xScale / 40;

That way, you only have to change one value in order to scale your different values.  This is just an example, and has not been implemented in my code.  I left this for you to do on your own.

Recommendations that are optional:

You have a lot of if statements with braces:

if(down)
{
    oneY++;
}

If you don't expand on them (which I don't see a need to), you can save some LOC:

if (down) oneY++;

Right now you are setting the location:

frame.setLocation(375, 55);

You don't have to do this, but I prefer the OS to set the location:

frame.setLocationByPlatform(true);

You could also set the frame to the center of the window:

frame.setLocationRelativeTo(null);

But I never liked that too much.  It makes your program seem like a pop-up, which I despise.

I always do frame.setVisible(true) after I am completely finished setting up the frame.

Right now your are doing this with your braces:

void someMethod(int someInt){
    // ...
}

Since you are a beginner, I would recommend lining up your braces (I still do this, and I've been programming for a while):

void someMethod(int someInt)
{
    // ...
}

This is a matter of taste, and it is perfectly fine to stay with what you are doing right now.

You might note that this:

public static void main(String... args)

Is a bit different than your usual:

public static void main(String[] args)

They both do the same thing, but one uses variable arity parameters.

Final code:

import java.awt.BorderLayout;
import java.awt.Color;
import java.awt.Graphics;

import javax.swing.JFrame;
import javax.swing.JPanel;

final public class Test
{

    JFrame frame;
    DrawPanel drawPanel;

    private int oneX = 7;
    private int oneY = 7;

    boolean up = false;
    boolean down = true;
    boolean left = false;
    boolean right = true;

    public static void main(String... args)
    {
        new Test().go();
    }

    private void go()
    {
        frame = new JFrame("Test");
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

        drawPanel = new DrawPanel();

        frame.getContentPane().add(BorderLayout.CENTER, drawPanel);

        frame.setResizable(false);
        frame.setSize(300, 300);
        frame.setLocationByPlatform(true);
        frame.setVisible(true);
        moveIt();
    }

    class DrawPanel extends JPanel
    {
        private static final long serialVersionUID = 1L;

        public void paintComponent(Graphics g)
        {
            g.setColor(Color.BLUE);
            g.fillRect(0, 0, this.getWidth(), this.getHeight());
            g.setColor(Color.RED);
            g.fillRect(3, 3, this.getWidth() - 6, this.getHeight() - 6);
            g.setColor(Color.WHITE);
            g.fillRect(6, 6, this.getWidth() - 12, this.getHeight() - 12);
            g.setColor(Color.BLACK);
            g.fillRect(oneX, oneY, 6, 6);
        }
    }

    private void moveIt()
    {
        while (true)
        {
            if (oneX >= 283)
            {
                right = false;
                left = true;
            }
            if (oneX <= 7)
            {
                right = true;
                left = false;
            }
            if (oneY >= 259)
            {
                up = true;
                down = false;
            }
            if (oneY <= 7)
            {
                up = false;
                down = true;
            }
            if (up) oneY--;
            if (down) oneY++;
            if (left) oneX--;
            if (right) oneX++;
            try
            {
                Thread.sleep(10);
            }
            catch (Exception e)
            {
                e.printStackTrace();
            }
            frame.repaint();
        }
    }
}


Key and value mapping from one object to another I've been trying to figure out a clean way of managing mappings between two objects. In the case of this example, it's two hashes. This spec should illustrate the problem at hand:

describe 'key mapping' do
  let(:have) { { data1: 'foo', data2: 'bar', data3: 'baz' } }
  let(:want) { { LINE1: 'foo', LINE2: 'bar', LINE3: 'baz' } }
  let(:result) { {} }

  after(:each) do
    expect(result).to eq want
  end

  it 'by assigning manually' do
    result[:LINE1] = have[:data1]
    result[:LINE2] = have[:data2]
    result[:LINE3] = have[:data3]
  end

  it 'by reading keys from a hash' do
    mappings = { data1: :LINE1, data2: :LINE2, data3: :LINE3 }
    mappings.each do |k, v|
      result[v] = have[k]
    end
  end
end

The notable thing here is that the source and destination can be mapped by known keys and unlike in the example there is no numeric correlation between the source and destination (example lists numbered keys just to make it easier to read)

The number of keys could be rather high, so the latter example will make code more readable, but is there an even better way of handling this?

If there's no "logical" correlation between the key you have, and the key you want (i.e. no consistent way to rewrite them) then you basically have to use a lookup of some sort to do the translation.

So your "reading keys from a hash" strategy is your best bet, but it can be improved a little.

You can be more functional and just use map:

lookup = { data1: :LINE1, data2: :LINE2, data3: :LINE3 }

result = Hash[ lookup.map { |in, out| [out, have[in]] } ]

Or you can just go through the ones in the input hash, and translate the ones that are actually there, discarding ones without a translation:

result = Hash[ have.map { |k, v| [lookup[k], v] if lookup[k] }.compact ]

And for either of those, if you ever need to translate the other way, you can just use lookup.invert to flip the keys/values around.

Lastly, for you spec, don't put an expectation in a after(:each) block. Put the expectation in the spec itself.


C# asynchronous tasks training (turn-based simulation) I know embarassingly little about asynchronous programming in C#, so decided to start catching up. It would help me a lot if anyone checked this basic example I've created.
Assumption: we're creating a simulation where the time passes turn by turn with predefined minimal interval, e.g. 1 second. However, if any important calculations during the turn would take longer than this interval, next turn should not begin until they're done.
I would be grateful if you could tell me if there any visible issues, code smells, or if it can be optimized. I've read bad things about using Task.Run, but I don't really know how to tackle it better.
public static class Program
{
    static void Main()
    {
        var gameTurnsController = new GameTurnsController();
        var cts = new CancellationTokenSource();

        try
        {
            // Asynchronously wait for input to send cancellation token
            Task.Run(() =>
            {
                Console.ReadKey(true);
                cts.Cancel();
            });

            gameTurnsController.PassTurns(TimeSpan.FromSeconds(1), cts.Token).Wait(cts.Token);
        }
        // It smells of controlling the execution flow through exception. Isn't there a better way to handle cancellation?
        catch (OperationCanceledException)
        {
            Console.WriteLine(Environment.NewLine + "Cancelled.");
            Environment.ExitCode = 0;
        }

        Console.WriteLine("Execution finished.");
    }
}

public class GameTurnsController
{
    public async Task PassTurns(TimeSpan interval, CancellationToken cancellationToken)
    {
        while (true)
        {
            if (cancellationToken.IsCancellationRequested)
            {
                return;
            }

            // Wait until both delay and calculation is finished
            await Task.WhenAll(Task.Delay(interval, cancellationToken), LongRunningOperation(cancellationToken));
        }
    }

    // expensive method example
    private async Task<long> LongRunningOperation(CancellationToken cancellationToken)
    {
        return await Task.Run(() =>
        {
            long result;
            // ...
            // expensive math operation that returns long value
            // ...
            return result;
        }, cancellationToken);
    }
}

In the above code I've removed most of the logs and stopwatches for clarity.

The Wait call is tricky.

It throws by default AggregateException not OperationCanceledException.
If you provide the cancellationToken to the Wait then it will throw an OperationCanceledException.
The biggest problem with this approach is that it cancels only the Wait not the underlying task.

Please read this article for more details.

So, if we switch to async main then the application will not stop after user cancellation:
static async Task Main()
{
    var gameTurnsController = new GameTurnsController();
    var userRequestedCancellation = new CancellationTokenSource();

    try
    {
        _ = Task.Run(() =>
        {
            Console.WriteLine("Please press any key to cancel");
            Console.ReadKey(true);
            userRequestedCancellation.Cancel();
        });

        await gameTurnsController.PassTurns(TimeSpan.FromSeconds(1), userRequestedCancellation.Token);
    }
    catch (OperationCanceledException)
    {
        Console.WriteLine(Environment.NewLine + "Cancelled.");
        Environment.ExitCode = 0;
    }

    Console.WriteLine("Execution finished.");
}

Because the underlying LongRunningOperation will not terminate.

(Let me use Thread.Sleep(5000) to simulate complex CPU bound operation)

private async Task<long> LongRunningOperation(CancellationToken cancellationToken)
{
    return await Task.Run(() =>
    {
        long result;
        Thread.Sleep(5000);
        return result;
    }, cancellationToken);
}

If I run this app and I press any key then it will not cancel the LongRunningOperation method that's why the output will be the Execution finished..

How to fix it?
By passing all the way long the cancellationToken:
private async Task<long> LongRunningOperation(CancellationToken cancellationToken)
{
    var job = Task.Run(() => LongRunningOperationInternal(cancellationToken), cancellationToken);
    return await job;
}

private long LongRunningOperationInternal(CancellationToken cancellationToken)
{
    long result = 1;
    for (int i = 0; i < 5; i++)
    {
        cancellationToken.ThrowIfCancellationRequested();
        Thread.Sleep(1000);
    }

    return result;
}

If need to do graceful cancellation then consider to use the following pattern:
if(cancellationToken.IsCancellationRequested)
{
  //TODO: clean up whatever is needed

  cancellationToken.ThrowIfCancellationRequested();
}

TL;DR: You need to check the cancellationToken at milestones/checkpoints to be able to truly cancel the whole application.

Might be an interesting read: How to can non-cancelable async operations


Monte Carlo simulation of amoeba population I wrote a simple Python simulation to answer the "Amoeba" population question posed here:

  A population of amoebas starts with 1. After 1 period that amoeba can divide into 1, 2, 3, or 0 (it can die) with equal probability. What is the probability that the entire population dies out eventually?

The script seems to work fine, but then I started playing with it to investigate curious effects.

If the number of trial generations is too large (num_generations_per_trial), I have problems with performance - the population size gets huge, and the simulation either runs slow or I encounter OverflowError on my brute force FOR loops.

I would appreciate feedback on efficiency options, and also on general code improvements.  I know the runs are independent and could be run in parallel. But that is still sort of brute force. I am curious more about making a single thread approach faster.

from __future__ import division
import random
import math
import time

def run_trial(max_split, num_generations):
    population = 1
    for generation in xrange(num_generations):
        for amoeba in xrange(population):
            amoeba_split = random.randint(0, max_split)
            population -= 1  # remove current amoeba (she will split or die)
            population += amoeba_split

        if population == 0:
            break
    return population

def main():
    extinct_counter = 0
    trials = 10000

    max_split_per_amoeba = 3
    num_generations_per_trial = 20 # populations can get *massive* as generations increase (memory / overflow errors at 100)

    print '***starting simulation***'
    print 'num trials: %i' % (trials)
    print 'max_split_per_amoeba: %i' % (max_split_per_amoeba)
    print 'num_generations_per_trial: %i' % (num_generations_per_trial)

    for trial in xrange(trials):
        outcome_population = run_trial(max_split_per_amoeba, num_generations_per_trial)
        if outcome_population == 0:
            extinct_counter += 1

        if divmod(trial+1, max(1,int(trials/20)))[1] == 0:
            print 'progress: %i trials complete | %i extinction counter | %.4f extinction probability' %  (trial+1, extinct_counter, extinct_counter/(trial+1))

    print 'extinct outcomes: %i' % (extinct_counter)
    print 'total trials: %i' % (trials)

    extinction_probability = extinct_counter / trials
    print 'extinction probability: %.4f' % (extinction_probability)

    expected_answer = math.sqrt(2)- 1
    print 'expected probability: %.4f' % (expected_answer)
    print 'delta from answer: %.4f' % (extinction_probability - expected_answer)

if __name__ == '__main__':
    start = time.clock()
    main()
    print 'runtime: %.3f s' % (time.clock() - start)
    print 'done'

Performance

Limiting each trial to 20 generations is not really helpful.  Let's put it this way: the population is expected to increase exponentially with each generation.  Once the population reaches, say, 10 amoebas, it's pretty safe to declare that infection has taken hold and the population will never dwindle to 0.

If you run a trial for 20 generations, you can easily get populations in the thousands.  The problem with letting populations grow so large is that you have to decide the fate of each individual amoeba at each time step, and that ends up being a lot of work.

If you change num_generations_per_trial to 10, you'll get identical results, and finish much more quickly.  But how do you decide what an appropriately low num_generations_per_trial number is?  It would be much more intuitive to impose a cap on the population size than on the number of generations.

Expressiveness

I suggest writing a generator for the population count at each time step.  You can easily count elements using the sum() function.

from __future__ import division
from random import randint

def amoeba_generations(max_split=3, population=1):
    yield population
    while population > 0:
        population = sum(randint(0, max_split) for _ in range(population))
        yield population

def trial(generator, success_pred, failure_pred):
    for g in generator:
        if success_pred(g):
            return True
        if failure_pred(g):
            return False
    return False

def main():
    num_trials = 10000
    max_split_per_amoeba = 3
    num_dead_populations = sum(
        trial(
            amoeba_generations(max_split_per_amoeba),
            lambda n: n == 0,
            lambda n: n >= 10     # Assume a population of >= 10 will never die
        )
        for _ in range(num_trials)
    )
    print("""***starting simulation***
num trials: {num_trials}
max_split_per_amoeba: {max_split_per_amoeba}
extinct outcomes: {num_dead_populations}
extinction probability: {extinction_probability:.4f}
expected probability: {expected_probability:.4f}
delta from answer: {delta:.4f}""".format(
        extinction_probability=num_dead_populations/num_trials,
        expected_probability=2**.5 - 1,
        delta=(num_dead_populations/num_trials) - (2**.5 - 1),
        **locals()
    ))

if __name__ == '__main__':
    main()


Method Of Retrieving Random Value Depending on Type I was wondering what would be the most appealing way of achieving this goal.

I have a blackbox. I give it a supported type and it returns a random value base off of the type.

For my supported types, I have an enum defined as follows:

public enum Types
{
    INTEGER,
    DOUBLE,
    BIGINTEGER,
    BIGDECIMAL,
    STRING,
    BOOLEAN,
    CHAR
}

I thought the easiest way would be to create a static function which returns an Object based off of the Type

public static Object getRandomValue(Types type)
{
    switch(type)
    {
        case INTEGER:
            return randomInt();
        case DOUBLE:
            return randomDouble();
        case BIGINTEGER:
            return randomBigInt();
        case BIGDECIMAL:
            return randomBigDecimal();
        case STRING:
            return randomString();
        case BOOLEAN:
            return randomBool();
        case CHAR:
            return randomChar();
    }
}

The problem is that an additional cast would have to be made each time that I want to retrieve a random value based off of this method.

I've looked into the design patterns abstract factory and factory, and I can't decide if there's an advantage into implementing either of those. Or if there's a different design pattern that seems to be more appropriate.

Assume that all my random methods are defined.

1) I don't think you need to define an enum since you can use Boolean.class, Integer.class, etc.

2)

public static <T> T getRandom(Class<T> classs) {
  if (classs.equals(Boolean.class)) {
      return (T) randomBool();
  } else if (classs.equals(Integer.class) {
      return (T) randomInt();
  // etc.
  } else
      // Edit: original "else" was: "return null;" changed to:
      throw new IllegalStateException("Cannot generate a random " + classs.toString());
}

EDIT
There is also the solution below, but that would still require casting:

   public enum Type {
    INTEGER {
        @Override public Integer getRandom() {
            return new randomInt();
        }
    }; // BOOLEAN, etc.

    public abstract Object getRandom();
}

That is basically the same as the original OP code, but where the random method call is within the enum instead of being in a separate method with a switch-statement.


TimeZone computing method I feel like there is a code smell in this. Can someone help me to refactor/optimize it?

protected TimeZone resolveTimeZone() {
    IUserPreferences preferences = userProfileService.getCurrentUserPreferences();
    if (preferences != null) {
        String timeZone = preferences.getPreferredTimeZone();
        if (timeZone != null && !"".equals(timeZone))
            return TimeZone.getTimeZone(timeZone);
        else return defaultTimeZone();
    }
    else return defaultTimeZone();

}

public TimeZone defaultTimeZone() {
    Locale defaultLocale = new Locale(appConfiguration.getDefaultLocale());
    Calendar calendar = Calendar.getInstance(defaultLocale);
    return calendar.getTimeZone();
}

Some immediate improvement ideas are possible at first glance:

Avoid duplicated code: defaultTimeZone() appears twice. It's easy to factor that out by changing the branching logic: keep checking the requirements (preferences != null, the preference value not null, and so on) until you find a usable value and return. To handle all other (failure) execution paths, use a single return at the end of the method
Use the .isEmpty() method to check if a string is empty. This is better than !"".equals(value), which means literally "is this value the same as """. Using isEmpty is natural.
The variable named timeZone suggests it's an instance of TimeZone. That's misleading, I would rename it to something else to clarify.

Applying the above:

protected TimeZone resolveTimeZone() {
    IUserPreferences preferences = userProfileService.getCurrentUserPreferences();
    if (preferences != null) {
        String timeZoneText = preferences.getPreferredTimeZone();
        if (timeZoneText != null && !timeZoneText.isEmpty()) {
            return TimeZone.getTimeZone(timeZoneText);
        }
    }
    return defaultTimeZone();
}

On closer look, there are a few more things I don't really like:

The defaultTimeZone method sticks out a little from other getter calls in the posted code: getCurrentUserPreferences, getPreferredTimeZone, getDefaultLocale, ... Perhaps it would be better to be consistent with those and rename the method to getDefaultTimeZone
Calling Calendar.getInstance is costly. If the default timezone will not change during the lifetime of the application, then it would be better to initialize it once during startup, and reuse it afterwards.
The name resolveTimeZone is a bit vague. What does it mean, "resolve", really? Looking at the implementation, it's not as clear as it can be.

I would recommend to split the method like this:

protected TimeZone getTimeZone() {
    TimeZone preferredTimeZone = getPreferredTimeZone();
    if (preferredTimeZone != null) {
        return preferredTimeZone;
    }
    return defaultTimeZone();
}

private TimeZone getPreferredTimeZone() {
    IUserPreferences preferences = userProfileService.getCurrentUserPreferences();
    if (preferences != null) {
        String timeZoneText = preferences.getPreferredTimeZone();
        if (timeZoneText != null && !timeZoneText.isEmpty()) {
            return TimeZone.getTimeZone(timeZoneText);
        }
    }
    return null;
}

Here, the names and implementations of the methods are more natural:

getTimeZone: looking at the implementation it's perfectly clear what's going on: use the preferred timezone if possible, otherwise the default. Nice and simple.
getPreferredTimeZone: from the method name, it comes natural that the method tries to figure out a timezone from the user configuration and return null otherwise.

Alas, this does have the drawback of an extra if condition compared to the original. However, the cost of the extra call should be negligible in practice. This may be a matter of taste. I think the value of clarity in the design outweighs the cost of that extra if (unless performance is absolutely critical, of course).


My own function for copying memory in C I have written a function that copies memory from one destination to another. I would like you to view my code and tell me if I have to fix something. That's the code:
void* memCopy(void* destination, const void* source, size_t size)
{
    double *a = (double*)source, *b = (double*)destination;
    void* end = a + (size / sizeof(double));
    while ((void*)a < end)
    {
        *b = *a;
        ++a;
        ++b;
    }
    char* a2 = (char*)a;
    char* b2 = (char*)b;
    end = a2 + (size % sizeof(double));
    while (a2 < end)
    {
        *b2 = *a2;
        ++a2;
        ++b2;
    }
    return destination;
}

I have used double* to make the function faster.

Your code doesn't compile cleanly with GCC:
move.c: In function 'memCopy':
move.c:16:15: warning: comparison of distinct pointer types lacks a cast
     while (a2 < end)
               ^

Also 2 brief points:

Why reinvent the wheel? Most implementations of memcpy, memmove etc in the standard libraries will be correctly and efficiently implemented, in many cases not using pure C.
Your code assumes that both source and data are correctly aligned to be accessed as double. If they are not, depending on the platform the code may a) perform badly, b) give incorrect results or c) simply crash, unless I'm much mistaken. It happens to run (apparently correctly) on my PC, but that's no guarantee of portability.

The FreeBSD implementation shows how this can be done, assuming that that the mapping of pointers to uintptr_t gives values which can be masked appropriately - this is true of the relevant targets of the code, but may not be universally so.


Add a class to an element when it's loaded I want to add a class to only those <p> elements that have an <img>.
So far this is what I have, it works, but I don't think I'm doing it right.

$(".entry-content p img").addClass(function(){
    $(this).parent("p").addClass("no-orphans-ctnr");
});

Here's a Demo/fiddle.

As you can see, I used the .addClass() method twice. I'm not a jQuery/JavaScript developer but that doesn't look right to me.
Please review and help me improve it.

To select the p elements that have an img as child element,
you can use the p:has(> img) selector, like this:

$(".entry-content p:has(> img)").addClass("no-orphans-ctnr");


my implementation of conway's game of life in C++ I found Conway's game of life. I had heard of it, of course, but didn't think it would be so easy and fun to implement. Anyway, here's my code which I know needs a lot of polish:
#include <iostream>
#include <vector>
#include <windows.h>

#define SIZE 10

using std::vector;

struct Pos {
    int x;
    int y;

    Pos(int ix, int iy) : x(ix), y(iy) {}
};

void printGrid();

int getNeighborCount(Pos cellPos);

void next(int (&universe)[SIZE][SIZE]);

void killOrBirthCells(int (&universe)[SIZE][SIZE], vector<Pos> &toKill, vector<Pos> &toBirth);

void cellFate(int &cell, Pos cellPos, int neighborCount, vector<Pos> &toKill, vector<Pos> &toBirth);

void printUniverse(int (&universe)[SIZE][SIZE]);

int main() {
    int universe[SIZE][SIZE] = {0};

    // glider
    universe[0][2] = 1;
    universe[1][2] = 1;
    universe[2][2] = 1;
    universe[2][1] = 1;
    universe[1][0] = 1;

    while (1) {
        system("cls");
        next(universe);
        printUniverse(universe);
        Sleep(500);
    }

    return 0;
}

int getNeighborCount(int (&universe)[SIZE][SIZE], Pos cellPos) {
    int count = 0;
    for (int i = 0; i < 3; i++) {
        for (int j = 0; j < 3; j++) {
            int x = cellPos.x - 1 + j;
            int y = cellPos.y - 1 + i;
            // handle cells on the edge
            if (x >= 0 && y >= 0 && x < SIZE && y < SIZE
                && universe[cellPos.y - 1 + i][cellPos.x - 1 + j] == 1) {
                count++;
            }
        }
    }
    // exclude itself
    if (universe[cellPos.y][cellPos.x]) {
        return count - 1;
    }
    return count;
}

void next(int (&universe)[SIZE][SIZE]) {
    std::vector<Pos> toKill;
    std::vector<Pos> toBirth;

    for (int i = 0; i < SIZE; i++) {
        for (int j = 0; j < SIZE; j++) {
            Pos cellPos{j, i};
            int neighborCount = getNeighborCount(universe, cellPos);
            cellFate(universe[i][j], cellPos, neighborCount, toKill, toBirth);
        }
    }

    killOrBirthCells(universe, toKill, toBirth);
}

// populate toKill and toBirth vectors
void cellFate(int &cell, Pos cellPos, int neighborCount, vector<Pos> &toKill, vector<Pos> &toBirth) {
    if (cell == 1) {
        // lonely
        if (neighborCount == 0 || neighborCount == 1) {
            toKill.push_back(cellPos);
        // overcrowded
        } else if (neighborCount >= 4 && neighborCount <= 8) {
            toKill.push_back(cellPos);
        }
    } else {
        if (neighborCount == 3) {
            toBirth.push_back(cellPos);
        }
    }
}

void killOrBirthCells(int (&universe)[SIZE][SIZE], vector<Pos> &toKill, vector<Pos> &toBirth) {
    for (Pos cellPos : toKill) {
        universe[cellPos.y][cellPos.x] = 0;
    }
    for (Pos cellPos : toBirth) {
        universe[cellPos.y][cellPos.x] = 1;
    }
    toKill.clear();
    toBirth.clear();
}

void printUniverse(int (&universe)[SIZE][SIZE]) {
    for (int i = 0; i < SIZE; i++) {
        for (int j = 0; j < SIZE; j++) {
            if (universe[i][j] == 1) {
                std::cout << 'x';
            } else {
                std::cout << '.';
            }
        }
        std::cout << '\n';
    }
}

If you're going to use C++, make full use of the language. Create a class to hold your universe, then all of your non-main functions can be members. You also won't have to pass around a reference to the universe array as it would be a member of the class.
One alternative to the toKill and toBirth vectors is to have two "universe" arrays. You'd use one as the source, and copy over and update the values into the second, then trade off which array you modify on the next pass thru. cellFate would return the new (updated) value for that cell, and killOrBirthCells would be eliminated.
getNeighborCount is only using i and j as offsets to the x and y values. You can redo the loops to directly update the x and y values, and also handle the edge clipping by proper tracking of the start and end values for the loops. It would also be easy to exclude the cell itself.
Combining all that we get a new getNeighborCount (that would be a member of a class):
int Universe::getNeighborCount(Pos cellPos) const {
    int count = 0;
    int sx = std::max(cellPos.x - 1, 0);
    int ex = std::min(cellPos.x + 1, SIZE - 1);
    int sy = std::max(cellPos.y - 1, 0);
    int ey - std::min(cellPos.y + 1, SIZE - 1);
    for (int y = sy; y <= ey; ++y) {
        for (int x = sx; x <= ex; ++x) {
            if (y != cellPos.y || x != cellPos.x) {
                if (universe[y][x] == 1)
                    count++;
            }
        }
    }
    return count;
}

You may be able to reduce flickering by moving the system("cls"); call to immediately before the call to printUniverse. This would reduce the amount of time that you have an empty screen.


Merge k Sorted Arrays Please review my answer for this interview question:
Merge k sorted arrays, using min heap.

import java.util.*;
public class MinHeap{
 public static class HeapItem implements Comparable<HeapItem>{
      int[] array;
      int index;        // the index of current element
      public HeapItem(int[] arr, int index) {
           array = arr;
           index = 0;
      }
      @Override
      public int compareTo(HeapItem h){
          if(array[index] > h.array[h.index]){
              return 1;
          }else if(array[index] < h.array[h.index]){
              return -1;
          }else{
              return 0;
          }
      }
 }
 public static List<Integer> mergeArrays(int[][] arrays) {
      if (arrays == null || arrays.length == 0) {
          throw new IllegalArgumentException("Invalid input!");
      }
      // priority queue is heap in Java
      PriorityQueue<HeapItem> pq = new PriorityQueue<HeapItem>();
      // add arrays to the heap
      for (int i = 0; i < arrays.length; i++) {
          pq.add(new HeapItem(arrays[i], 0));
      }
      List<Integer> result = new ArrayList<Integer> ();
      while (!pq.isEmpty()) {
          HeapItem current = pq.remove();
          result.add(current.array[current.index]);
          if (current.index < current.array.length-1) {
             pq.add(new HeapItem(current.array, current.index+1));
          }
     }
     return result;
 }
 public static void main(String[] args){
     int[] arr1 = {1, 3, 5, 7};
     int[] arr2 = {2, 4, 6, 8};
     int[] arr3 = {0, 9, 10, 11};
     System.out.println(mergeArrays(new int[][] {arr1, arr2, arr3}));
 }
}

Bug

This might be a typo, but when I ran your program it infinitely looped.  The problem was in your constructor:

  public HeapItem(int[] arr, int index) {
       array = arr;
       index = 0;
  }

Here, you are setting index to 0 when you should be setting it to the input argument, like this:

  public HeapItem(int[] arr, int index) {
       array = arr;
       this.index = index;
  }

Reuse HeapItem

Your code all seemed very reasonable to me.  There is just one small improvement I would make here:

      if (current.index < current.array.length-1) {
         pq.add(new HeapItem(current.array, current.index+1));
      }

I would just reuse current instead of creating a new HeapItem, like this:

      if (current.index < current.array.length-1) {
         current.index++;
         pq.add(current);
      }

Create result list with exact size

One other small thing you could do is compute the output list size, when you are iterating through the arrays.  That way, you could create result with the correct size instead of having it autoexpand as you add elements to it.

  int outputSize = 0;
  for (int i = 0; i < arrays.length; i++) {
      pq.add(new HeapItem(arrays[i], 0));
      outputSize += arrays[i].length;
  }
  List<Integer> result = new ArrayList<Integer> (outputSize);


Algorithm that generates orthogonal vectors: C++ implementation Recently I asked the similar question, but the algorithm was implemented in Python. Now I've tried to implement the same algorithm, but in C++ (I'm very new to it):

#include<iostream>
#include<cstdlib>
#include<ctime>
#include<cmath>
#include <tgmath.h>
using namespace std;

const int MAXRANGE = 1000;

int randint(int low, int high){
    return rand() % (high - low) + low;

    }

void generate_vector(float vec[],int dimension){
    for (int i = 0; i < dimension; i++){
        vec[i] = randint(-MAXRANGE,MAXRANGE);
        }

    // We need to ensure that the last entry doesn't equal to zero

    if (vec[dimension-1] == 0){
        while (vec[dimension-1] == 0){
            vec[dimension-1] = randint(-MAXRANGE,MAXRANGE);

        }
    }

}

void generate_orthogonal (float *vector, float orthogonal_vector[], int dimension){
    float last_entry;
    float dot_product = 0;
    for (int i = 0; i < dimension -1; i++){
        orthogonal_vector[i] = randint(-MAXRANGE,MAXRANGE);
    }
    for (int i = 0; i < dimension -1; i++){
        dot_product = dot_product + (vector[i] * orthogonal_vector[i]);
    }
    last_entry = -(dot_product/vector[dimension-1]);
    orthogonal_vector[dimension-1] = last_entry;
}

float dot_product(float *vector1, float vector2[],int dimension){
    float sum = 0;
    for (int i = 0; i < dimension; i++){
        sum += (vector1[i])*(vector2[i]);
    }
    return sum;
}

void print_vector(float *A,int dim){
    cout << "(";
    for (int i = 0; i < dim; i++){
        if (i == dim -1){
            cout << A[i];
        }
        else{
            cout << A[i] << ",";}
    }
    cout << ")^T" << endl;
}
int main(){
    srand(time(0));
    int dimension;
    cout << "Choose dimension for the vector: ";
    cin >> dimension;
    float arbitrary_vector[dimension], orthogonal[dimension];
    generate_vector(arbitrary_vector,dimension);
    generate_orthogonal(arbitrary_vector,orthogonal,dimension);
    cout << "First vector: ";
    print_vector(arbitrary_vector,dimension);
    cout << "Orthogonal vector: ";
    print_vector(orthogonal,dimension);
    cout << "Dot product of the vectors: ";
    cout << dot_product(arbitrary_vector,orthogonal,dimension) << endl;

}

What can be improved?

As some people previously pointed out, the algorithm would throw exception if the last entry of the first vector was \$0\$. I made an a slight change to ensure that generate_vector function won't generate vector where the last element is zero.

Welcome to C++.

It seems that you are more or less trying to write C code rather than C++ though. Lets have a look at what you can improve:

Never do using namespace std; It is a terrible practise that gets you into trouble really fast. std:: is not that hard to type so get in the habit early.
If you have compile time constants use them via constexpr
Your dot_product function is already implemented in the standard library. It is called inner_product. So the following code can be replaced

float dot_product(float *vector1, float vector2[],int dimension){
    float sum = 0;
    for (int i = 0; i < dimension; i++){
        sum += (vector1[i])*(vector2[i]);
    }
    return sum;
}

by the following code

std::inner_product(a.cbegin(), a.cend(), b.cbegin(), float(0.0));

Similarly there is a generate function that fills a vector with a generator. Note that C++ has a variety of good random number generators (rand() is not one of them) in the <random> header. I don't know why you use integers for the float vectors but you can adapt that easily.

EDIT: Incorporated problems found by @L.F.

#include <algorithm>
#include <random>

class randomStreamUniformInt {
public:
    explicit randomStreamUniformInt(int lower_bound, int upper_bound)
    : mt(std::random_device{}()), uniform_dist(lower_bound, upper_bound) {}
    explicit randomStreamUniformInt(int lower_bound, int upper_bound, double seed)
    : mt(seed), uniform_dist(lower_bound, upper_bound) {}

    int operator() () { return uniform_dist(mt); }
private:
    std::mt19937_64                     mt;
    std::uniform_int_distribution<>     uniform_dist;
};

static randomStreamUniformInt rng(-MAXRANGE, MAXRANGE);

std::vector<float> generate random(const std::size_t numElements) {
    std::vector<float> res(numElements);
    std::generate(res.begin(), res.end(), rng);
    return res;
}

It would be even better to pass the random number generator as an argument rather than a global variable. I leave that as an exercise.
You should use standard facilities for arrays such as std::vector or std::array depending on whether you know the size at compile time. So the following code:

float arbitrary_vector[dimension], orthogonal[dimension]

Should be better written as

std::vector<float> arbitrary_vector = generate_random(dimension);

Note that it is always better to put each declaration into a single line.
Your method generate_orthogonal can be improved accordingly.

std::vector<float> generate_orthogonal(const std::vector<float>& a) {
    // get some random data
    std::vector<float> b = generate_random(a.size());

    // find the last non zero entry in a
    // We have to turn the reverse iterator into an iterator via std::prev(rit.base())
    auto IsZero = [] (const float f) -> bool { return f == float(0.0);};
    auto end = std::prev(std::find_if_not(a.crbegin(), a.crend(), IsZero).base());

    // determine the dot product up to end
    float dot_product = std::inner_product(a.cbegin(), end, b.cbegin(), float(0.0));

    // set the value of b so that the inner product is zero
    b[std::distance(a.cbegin(), end)] = - dot_product / (*end);

    return b;
}

So put together it would look something like this:

#include <algorithm>
#include <random>
#include <vector>

constexpr int MAXRANGE = 1000;

class randomStreamUniformInt {
public:
    explicit randomStreamUniformInt(int lower_bound, int upper_bound)
    : mt(std::random_device{}()), uniform_dist(lower_bound, upper_bound) {}
    explicit randomStreamUniformInt(int lower_bound, int upper_bound, double seed)
    : mt(seed), uniform_dist(lower_bound, upper_bound) {}

    int operator() () { return uniform_dist(mt); }
private:
    std::mt19937_64                     mt;
    std::uniform_int_distribution<>     uniform_dist;
};

static randomStreamUniformInt rng(-MAXRANGE, MAXRANGE);

std::vector<float> generate random(const std::size_t numElements) {
    std::vector<float> res(numElements);
    std::generate(res.begin(), res.end(), rng);
    return res;
}

std::vector<float> generate_orthogonal(const std::vector<float>& a) {
    // get some random data
    std::vector<float> b = generate_random(a.size());

    // find the last non zero entry in a
    // We have to turn the reverse iterator into an iterator via std::prev(rit.base())
    auto IsZero = [] (const float f) -> bool { return f == float(0.0);};
    auto end = std::prev(std::find_if_not(a.crbegin(), a.crend(), IsZero).base());

    // determine the dot product up to end
    float dot_product = std::inner_product(a.cbegin(), end, b.cbegin(), float(0.0));

    // set the value of b so that the inner product is zero
    b[std::distance(a.cbegin(), end)] = - dot_product / (*end);

    return b;
}

int main() {
    std::size_t dimension = 20;

    std::vector<float> a = generate_random(dimension);
    std::vector<float> b = generate_orthogonal(a);
}


Create an array with random numbers private static byte[] GenerateData(long size)
{
    var result = new byte[size];
    new Random().NextBytes(result);

    return result;
}

Is it good, robust and efficient solution?

I use my method like this

[TestMethod]
public void WriteAndRead16MBbyZeroOffsetTest()
{
     var length = 1024 * 1024 * 16; // 16mb
     WriteAndReadTest(length);
}

private void WriteAndReadTest(long length, long offset = 0L)
{
    byte[] generatedData = GenerateData(length);
    byte[] expectedData = null;

    StorageException resultException = null;
    try
    {
         _blobReaderWriterLazy.Value.Write(offset, generatedData);
         _blobReaderWriterLazy.Value.Read(offset, length, out expectedData);
    }
    catch (StorageException exception)
    {
        resultException = exception;
    }

    Assert.IsNull(resultException);
    Assert.IsTrue(generatedData.Length == expectedData.Length);
    Assert.IsTrue(generatedData.SequenceEqual(expectedData));
}

I will use this method inside my tests class

Your plan is to generate random data to test your thing. Suppose a test fails.  How are you planning on debugging it with the data that caused it to fail?

Don't randomize tests.  Generate the random test data once, and save it, and then use it over and over again.


Handling rejected HTTP responses with status 401 I'm kinda new to javascript and have question about it. Now my practical project is growing a bit bigger. I got the following if else statement. Is it possible to refactor this ? Should i use an use case or are there other options or will this decrease the loading speed of the application.

(function () {
    'use strict';

    angular
        .module('app')
        .factory('HttpResponseInterceptor', HttpResponseInterceptor);

    HttpResponseInterceptor.$inject = ['$q','$location'];
    function HttpResponseInterceptor($q,$location) {

        return {
            response: function(response){
                if (response.status === 401) {

                }
                return response || $q.when(response);
            },
            responseError: function(rejection) {

                if(rejection.status ===401 && rejection.config.url === "user") {
                    console.log("Check")
                }
                else if (rejection.status ===401 &&  rejection.config.url == "l18n/fr_FR.json")
                {
                    console.log("Check2")
                }
                else if (rejection.status ===401 &&  rejection.config.url == "l18n/nl_NL.json")
                {
                    console.log("Check2")
                }
                else if (rejection.status ===401 &&  rejection.config.url == "l18n/de_DE.json")
                {
                    console.log("Check2")
                }
                else if (rejection.status ===401 &&  rejection.config.url == "l18n/es_ES.json")
                {
                    console.log("Check2")
                }
                else if (rejection.status ===401 &&  rejection.config.url == "l18n/en_US.json")
                {
                    console.log("Check2")
                }
                else
                {
                    $location.path('/login');
                }

               /* if (rejection.status === 401 && rejection.config.url !== "user") {

                    $location.path('/login');
                }*/
                return $q.reject(rejection);
            }
        };

    }

})();

I'd actually go a bit further than janos. He is perfectly right in that by repeatedly checking rejection.status, you're repeating yourself in code. Repeatedly checking the variable against multiple values makes this a perfect candidate for a switch statement:

responseError: function(rejection) {
    if (rejection.status === 401) {
        switch (rejection.config.url) {
            case "user":
                console.log("Check");
                break;
            case "l18n/fr_FR.json":
            case "l18n/nl_NL.json":
            case "l18n/de_DE.json":
            case "l18n/es_ES.json":
            case "l18n/en_US.json":
                console.log("Check2");
                break;
            default:
                $location.path('/login');
                break;
        }
    }

    return $q.reject(rejection);
}


Branding Roses & Tulips The logic is like this:

All brands have 2 products (Rose and Tulip)
Each brand names their product name differently (such as Brand 1 names their rose as No. 1 Rose and Brand 2 names their rose as No. 2 Rose )
All brands don't have products from all countries (Such as Brand 1 has Roses from USA and US whereas Brand 2 has Roses from Canada and China)

This is the working prototype of this function given below (Or, the fiddle):

$(document).ready(function() {

    $(document).on("input", "#intBrand, #intProcudt", function() {

        var intBrand = $('#intBrand').val();
        var intProcudt = $('#intProcudt').val();

        if (intBrand == "bAll") {
            if (intProcudt == "rose") {
                product = 'Rose';
            } else if (intProcudt == "tulip") {
                product = 'Tulip';
            }
        } else if (intBrand == "b1") {
            if (intProcudt == "rose") {
                product = 'No. 1 Rose';
            } else if (intProcudt == "tulip") {
                product = 'No. 1 Tulip';
            }
        } else if (intBrand == "b2") {
            if (intProcudt == "rose") {
                product = 'No. 2 Rose';
            } else if (intProcudt == "tulip") {
                product = 'No. 2 Tulip';
            }
        }

        var item1 = product + ' from US(1)
';
        var item2 = product + ' form USA(2)
';
        var item3 = product + ' from Canada(3)
';
        var item4 = product + ' from China(4)
';

        if (intBrand == "bAll") {
            roseP = item1 + item2 + item3 + item4;
            tulipP = item1 + item2 + item4;
        } else if (intBrand == "b1") {
            roseP = item1 + item2;
            tulipP = item1;
        } else if (intBrand == "b2") {
            roseP = item3 + item4;
            tulipP = item3 + item4;
        }

        $('#allRoseItems').html(roseP);
        $('#allTulipItems').html(tulipP);

    });

    $("#intBrand").trigger("input");
});
<link href="https://stackpath.bootstrapcdn.com/bootstrap/4.1.1/css/bootstrap.min.css" rel="stylesheet">

      Select Brand:

      <select id="intBrand" name="intBrand" required>
         <option value="bAll" >All items</option>
         <option value="b1" selected>Brand 1</option>
         <option value="b2" >Brand 2</option>
      </select>

      Select Product:

      <select id="intProcudt" name="intProcudt" required>
         <option value="rose" >Rose</option>
         <option value="tulip">Tulip</option>
      </select>

My question is just keeping the following section untouched, could it be built in a better way?

var item1 = product + ' from US(1)
';
var item2 = product + ' form USA(2)
';
var item3 = product + ' from Canada(3)
';
var item4 = product + ' from China(4)
';

I'd really appreciate your suggestions on how it could be written in a more efficient way with fewer lines of code.

I was in the process of answering your question on StackOverflow but it was moved to Code Review -- so I'll answer it here.

You might be able to avoid using switch or multiple if conditions if you build a data structure that fits your needs.

$(function() {

  // data structure
  var products = {
    bAll: {
      rose: {
        name: 'Rose',
        countries: ['US(1)', 'USA(2)', 'Canada(3)', 'China(4)']
      },
      tulip: {
        name: 'Tulip',
        countries: ['US(1)', 'USA(2)', 'China(4)']
      }
    },
    b1: {
      rose: {
        name: 'No. 1 Rose',
        countries: ['US(1)', 'USA(2)']
      },
      tulip: {
        name: 'No. 1 Tulip',
        countries: ['US(1)']
      }
    },
    b2: {
      rose: {
        name: 'No. 2 Rose',
        countries: ['USA(3)', 'China(4)']
      },
      tulip: {
        name: 'No. 2 Tulip',
        countries: ['USA(3)', 'China(4)']
      }
    }
  };

  $(document).on("input", "#intBrand, #intProcudt", function() {

    var intBrand = $('#intBrand').val();
    var intProcudt = $('#intProcudt').val();

    // using the data structure, it makes it much easier to get information

    // get product
    var product = products[intBrand][intProcudt]['name'];

    // get rose countries
    var roseCountries = products[intBrand]['rose']['countries'];

    // get tulip countries
    var tulipCountries = products[intBrand]['tulip']['countries'];

    // build HTML string using product's rose countries
    var roseP = roseCountries.map(function(country) {
      return product + ' from ' + country;
    }).join('
');

    // build HTML string using product's tulip countries
    var tulipP = tulipCountries.map(function(country) {
      return product + ' from ' + country;
    }).join('
');

    $('#allRoseItems').html(roseP);
    $('#allTulipItems').html(tulipP);
  });

  $("#intBrand").trigger("input");

  // alternate way is to get the data from a JSON file
  // instead of having the large data in our file;

  // you would need to wait for the response from the $.getJSON
  // before triggering the initial load automatically

  /*
  var products;
  $.getJSON('path/products.json', function (res) {
      // store the data, so that we can use it
      products = res;
      // trigger initial oninput
      $("#intBrand").trigger("input");
  });
  */
});
<link href="https://stackpath.bootstrapcdn.com/bootstrap/4.1.1/css/bootstrap.min.css" rel="stylesheet">

    Select Brand:

    <select id="intBrand" name="intBrand" required>
      <option value="bAll">All items</option>
      <option value="b1" selected>Brand 1</option>
      <option value="b2">Brand 2</option>
    </select>

    Select Product:

    <select id="intProcudt" name="intProcudt" required>
      <option value="rose">Rose</option>
      <option value="tulip">Tulip</option>
    </select>

You should also cache your jQuery objects if you are going to be re-using them. There's no need to search the DOM every time.


Binary search tree insertion in Racket I am learning Racket and implemented a BST insert function (insert tree n) where the format for a BST node is (left-tree value right-tree).

Example

'((() 2 ()) 3 ()) is isomorphic to:

      (3)
  (2)    ()
()  ()

Therefore (insert '((() 2 ()) 3 ()) 4) yields ((() 2 ()) 3 (() 4 ())):

      (3)
  (2)    (4)
()  ()  () ()

Implementation

My implementation feels overly complicated.  For example, I used append with cons so that the right-tree for 3, represented by (), would properly become (() 4 ()) after inserting 4.

How can I approach this problem more elegantly or more functionally?

#lang racket

(define (left tree)
  (list-ref tree 0)
  )
(define (value tree)
  (list-ref tree 1)
  )
(define (right tree)
  (list-ref tree 2)
  )
(define (merge left value right)
  (append (cons left '()) (append (cons value '()) (cons right '())))
  )
(define (insert tree n)
  (cond
    [(empty? tree) (append '(()) (cons n '()) '(()))] ; leaf
    [(> (value tree) n) (merge (insert (left tree) n) (value tree)(right tree))] ; internal - go left
    [(< (value tree) n) (merge (left tree) (value tree) (insert (right tree) n))] ; internal - go right
    [(eq? (value tree) n) tree] ; internal - n already in tree
    )
  )

Update

Given the answer to the question, I updated my code:

(define (insert BST n)
  (cond
    [(empty? BST) ; leaf
     (list empty n empty)]
    [(< n (cadr BST)) ; internal - go left
     (list (insert (car BST) n) (cadr BST) (caddr BST))]
    [(> n (cadr BST)) ; internal - go right
     (list (car BST) (cadr BST) (insert (caddr BST) n))]
    [else
     BST]
    )
  )

As you suspected, you don't need append for this problem. The trick is to notice that if, for example, your goal is to create the list '(1 2 3), then writing (list 1 2 3) is more straight-forward and more efficient than writing (append '(1) '(2) '(3)).

With that in mind, consider the following insertion function:

(define (insert BST n)
  (cond
    ((null? BST)
     (list empty n empty))
    ((< n (cadr BST))
     (list (insert (car BST) n) (cadr BST) (caddr BST)))
    ((> n (cadr BST))
     (list (car BST) (cadr BST) (insert (caddr BST) n)))
    (else BST)))

To run a small test, suppose you have the list '((() 3 ()) 5 (() 8 (() 10 ()))), which looks as follows:

then inserting 7 to it, ie. (insert '((() 3 ()) 5 (() 8 (() 10 ()))) 7) will produce '((() 3 ()) 5 ((() 7 ()) 8 (() 10 ()))), which looks as follows:


Designing an EventHandler in C++ I am writing a simple event handling class in C++, to avoid having delegate registration and calls cluttering the rest of the code. I have a design that seems to do the job, but it has a couple of drawbacks.

So far it looks like this:

template<typename T>
struct IDelegate { virtual void operator ()(const T & args) const = 0; };

template<typename T>
class EventHandler
{
public:
    void Trigger(const T & args) const;
    void AddListener(const IDelegate<T> & delegate);
    void RemoveListener(const IDelegate<T> & delegate);
};

Which in practice would look like this (some code elided for clarity):

class Foo
{
public:
    struct CustomEventArgs { int i; };
    EventHandler<CustomEventArgs> * SomeHandler() const { return m_handler; }

    void SomeMethod()
    {
        // Some code
        CustomEventArgs someArgs = { 42 };
        SomeHandler()->Trigger(someArgs);
    }

private:
    EventHandler<CustomEventArgs> * m_handler;
};

class Bar : public IDelegate<Foo::CustomEventArgs>
{
public:
    void operator ()(const Foo::CustomEventArgs & args) const
    {
        std::cout << args.i << " bottles of beer" << std::endl;
    }

    void StartListening(const Foo & foo)
    {
        foo.SomeHandler()->AddListener(*this);
    }
};

This code would basically do what I want, but there are still several things I don't quite like.

EventHandler::Trigger() is public

Anyone can call it, which is a serious flaw. Only the owner should be able to call that method. I can see two ways of ensuring this, but I am not happy with any of them:

Making the EventHandler(s) hidden from outside Foo. This defeats the purpose though, since it would then require to add as many AddListener/RemoveListener methods.
Making Trigger() protected, deriving from EventHandler, then declaring Foo as a friend. This would require to declare a specific class for every single pair of event-handler/class-using-that-event-handler, which is less than ideal.

SomeHandler() returns a pointer

If possible, I think it'd be better to make m_handler an actual member instead of having it somewhere (else) on the heap, and return a reference to it. But I don;t see how to do that without getting plagued with non-const-ness: I want StartListening() to take a const reference.

The question[s]

What would you recommend regarding these two issues?
Do you see other flaws in this design?

Been a while since I've done much in the way of heavy C++ & I'm sure others will weigh in with their own suggestions, but my two cents:

I think the best solution is the first one you rejected: add Foo::{Add,Remove}Listener. Yep, it's more code but (a) it solves both the issues you observe with the code & (b) the implementation of these methods will be trivial. It's a compromise: better encapsulation in exchange for a little more code.

If you're really set on your approach, I'm pretty sure you could work around issue #1 by separating EventHandler into two classes (e.g. EventHandler & EventTrigger) & using friends to expose the dirty details, but IMO that's comparatively ugly. This stuff doesn't deserve that kind of hack.

Along that line of thinking but maybe a little cleaner: have EventTrigger derive from EventHandler (the former providing ::Trigger, the latter ::{Add,Remove}Listener), making m_handler an EventTrigger & having Foo::SomeHandler return a pointer to an EventListener -- effectively "hiding" the Trigger method from the outside world. You're still "leaking" here because you'll need to expose a protected instance variable or method (the list of listeners), but this stuff is simple enough that it shouldn't be completely abhorrent.

Depends what you consider a "flaw" :) Implementors of IDelegate might find the fact the method is const to be annoying (but then you might be making assumptions about implementations of IDelegate that I'm not aware of). Other than that & the issues you observed, things look fine to me.


Merge sort implementation in C++ I was wondering if this implementation of merge sort could be improved. Are there any things that I have done wrong?

void merge(int *a, int *l, int leftCount, int *r, int rightCount) {
int i = 0, j = 0, k = 0;

// Compare l and r and add it to a
while (i < leftCount && j < rightCount) {
    if (l[i] < r[j]) {
        a[k++] = l[i++];
    }
    else {
        a[k++] = r[j++];
    }
}

// Add the leftovers
while (i < leftCount) {
    a[k++] = l[i++];
}
while (j < rightCount) {
    a[k++] = r[j++];
}}

void mergeSort(int *a, int n) {
if (n >= 2) {
    int mid, i, *l, *r;
    mid = n / 2;

    /* Create left and right subarrays */
    // Mid elements from index 0 till mid - 1
    l = (int*)malloc(mid * sizeof(int));
    // From n - mid till n - 1
    r = (int*)malloc((n - mid) * sizeof(int));

    // Fill left subarray
    for (i = 0; i < mid; i++) {
        l[i] = a[i];
    }
    // Fill right subarray
    for (i = mid; i < n; i++) {
        r[i - mid] = a[i];
    }

    // Sort left subarray
    mergeSort(l, mid);
    // Sort right subarray
    mergeSort(r, n - mid);

    // Merge l and r in a
    merge(a, l, mid, r, n-mid);

    // Free the memory
    free(l);
    free(r);
}}

Algorithm

mergesort allocates a new buffer of scratch memory for each recursive call. This means sorting an array has to use \$O(n \log n)\$ allocations total, with \$2 * n\$ peak memory usage. This is double the amount of memory usually required, and if mallocs runtime complexity is worse than \$O(1)\$, the whole mergesort operation will perform worse than \$O(n \log n)\$.

Also, every call to mergesort unnecessarily copies all of the elements of the previous buffer into the new ones. These copies can be completely skipped with appropriate management of the already existing buffers.

The merge operation isn't stable, though it can trivially be made so by changing l[i] < r[i] to l[i] <= r[i]. Stability is usually a major factor for choosing merge sort!

Implementation

Unnecessary restriction on int - this algorithm could be used for any comparable type with templates.
malloc/free or new/delete should usually be used sparingly. In your case, what would speak against the use of either a std::vector<int> or (preferably) gsl::span<int>, or at the very least a std::unique_ptr<int[]>? Both release their memory just fine in case of exceptions (which the current code doesn't even begin to check: results of malloc are never tested!), and freeing the memory will be done automatically once the objects leave the scope.
Naming: Many variables have short, cryptic names that do not express their intentions clearly. What reads easier, l[i] = a[i]; or left_buffer[index] = original[index];?

Fixes

Trying to keep faithful to the original, but including the improvements suggested:

template<typename Element>
void merge(Element *target, Element *source, size_t left_start, size_t right_start, size_t right_end) {
    auto target_index = left_start, left_end = right_start;
    auto left_index = left_start, right_index = right_start;

    while(left_index < left_end && right_index < right_end) {
        if(source[left_index] <= source[right_index]) {
            target[target_index++] = source[left_index++];
        } else {
            target[target_index++] = source[right_index++];
        }
    }

    while(left_index < left_end) {
        target[target_index++] = source[left_index++];
    }

    while(right_index < right_end) {
        target[target_index++] = source[right_index++];
    }
}

template<typename Element>
void merge_sort(Element *original, Element *buffer, size_t start, size_t end) {
    auto distance = end - start;
    if(distance < 2) return;
    auto mid = start + distance / 2;

    merge_sort(buffer, original, start, mid);
    merge_sort(buffer, original, mid, end);

    merge(original, buffer, start, mid, end);
}

template<typename Element>
void merge_sort(Element *original, size_t size) {
    auto buffer = std::make_unique<Element[]>(size);

    for(auto index = 0u; index < size; ++index) {
        buffer[index] = original[index];
    }

    merge_sort(original, buffer.get(), 0, size);
}

  merge basically just gets a slightly different signature (as I'm passing around indices to indicate different buffers instead of pointers).
  merge_sort got split into two functions: One to do the actual sorting, and one to setup the extra buffer. Note the reuse of the original array as buffer!
  Also note that the elements get copied into the buffer beforehand. This is done so the cascading between buffers works in all cases (\$\lceil\log_2 n\rceil\$ might be odd = the lowest merge would start copying from the wrong buffer, or \$n\$ is not a power of 2 = recursion depth might differ by 1 for different parts of the array). This copy step could conceivably be moved elsewhere (or, under certain circumstances, be skipped entirely), but this way the implementation is much clearer about when the copy is done.

Further improvements

merge_sort could use a different sorting algorithm once the number of elements in a part goes below a threshold in order to improve overall performance.
merge_sort could be extended to support iterators, instead of just plain arrays.


Directly initialize variable from std::istream For some time it has been bothering me that there is apparently no way to directly initialize variables from input streams (something like int i << std::cin;)

My solution is this:

#include <iostream>
#include <iomanip>
#include <string>

template<class T>
T get(std::istream& in){
    T tmp;
    in >> tmp;
    return tmp;
}

template<class T,class A, class... ARGS>
T get(std::istream& in, A& arg, ARGS&... args){
    in >> arg;
    return get<T>(in, args...);
}

int main() {
    int i = get<int>(std::cin);
    std::cout << i << std::endl;

    std::string s = get<std::string>(std::cin , std::setw(5));
    std::cout << s << std::endl;
}

I'd like to know if

I can improve something on the design
There are serious performance implications
There are corner cases where this will not work or even worse produce silent errors.
I've overlooked any standard functions/classes that already implement that functionality.

There are corner cases where this will not work or even worse produce silent errors.

The function as it is, depends on the client to check for errors on the stream (either by calling std::cin.exceptions(std::ifstream::failbit); or by checking the stream state, after the call to get).

Ideally, I would want to write:

if(auto a = get<int>(std::cin))
    // use a here

Maybe you could add something like this to your solution:

template<class T>
boost::optional<T> get_optional(std::istream& in){
    T tmp;
    in >> tmp;
    return (in) ?
        boost::optional<T>{ std::move(tmp) } :
        boost::optional<T>{};
}


Python OOP - Web App (Flask) - User registration class? I am working on a simple CRUD app as a personal project using Flask. I am currently working on the user blueprint and trying to use as less as libraries as I could and no ORM (for learning purposes). On my blueprints, I have multiple folders including a user folder. I wrote 2 classes in blueprints/user/models.py:
User: represent a User entity
UserService : register and login (login is not present yet)

Does it make sense to have2 separate classes? I am actually thinking to move all of my User methods to the UserService class and just have a UserService class with setters and getters.

I am confused about is where and when to interact with the database. Should I do it directly on my route.py vs create a new class?

For instance, to add a new User to the database, my SQL should look more or less like this:
sql = "INSERT INTO users (email, password) VALUES (%s, %s)"
cursor = conn.cursor()
cursor.execute(sql, (email, password))
cursor.fetchall()
conn.commit()

blueprints/user/models.py
class UserService():
    def register_user(self,
                      email,
                      password,
                      registration_date,
                      active,
                      sign_in_count,
                      current_sign_in_on,
                      last_sign_in_on):
        new_user = User(email, password, registration_date, active, sign_in_count, current_sign_in_on, last_sign_in_on)
        return new_user.__str__()

class User():
    def __init__(self, email, password, registration_date, active, sign_in_count, current_sign_in_on, last_sign_in_on ):
        self.email = email
        self.password = password
        self.registration_date = registration_date
        self.active = active

        # Activity tracking
        self.sign_in_count = sign_in_count
        self.current_sign_in_on = current_sign_in_on
        self.last_sign_in_on = last_sign_in_on

    def desactivate_user(self):
        if self.active == False:
            print(f"User {self.email} is already inactive")
        self.active = False

    def reactive_user(self):
        if self.active == True:
            print(f"User {self.email} is already active")
        self.active = True

    def is_active(self):
        return self.active

    def update_activity_tracking(self, ip_address):
        self.sign_in_count += 1
        self.last_sign_in_on = self.current_sign_in_on
        self.current_sign_in_on = datetime.datetime.now()

    def update_password(self, new_password):
        self.password = get_hashed_password(new_password)

    def __str__(self):
        user_attributes = vars(self)
        return (', '.join("%s: %s" % item for item in user_attributes.items()))

blueprints/user/views.py
from flask import Blueprint, render_template, request, jsonify
from prepsmarter.extensions import conn #database connection variable

user = Blueprint('user', __name__, template_folder='templates')

@user.route('/register')
def login():
    return render_template('register.html')

@user.route('/new-user',methods = ['POST'])
def register_user():
   # where I am going to register a new user

extensions/Database.py
import pymysql

class Database:
    instance = None

    def __init__(self, host, user, password, db):
        self.host = host
        self.user = user
        self. password = password
        self.db = db

    def connect(self):
        try:
            conn = pymysql.connect(
                host = self.host,
                user = self.user,
                passwd = self.password,
                db = self.db
            )
            return conn
        except Exception as e:
            print(e)

In general, your code seems to be in pretty good shape if you're hoping to move towards domain driven design, notably seen from your "entity-like" User object and the UserService.

Does it make sense to have2 separate classes? I am actually thinking to move all of my User methods to the UserService class and just have a UserService class with setters and getters.

First off, if you're anticipating your application to grow, a clear separation of concerns is what you'd want to achieve and to further achieve this, the two objects you have are fine. That is, one to represent a definition of what a user is and another to determine what you can do with a user object or related objects which in this case is the UserService. This naturally means that the methods on the User class should now be part of the UserService, considering them business logic of the application. I would use the words model to describe the User class and service to describe the UserService to define the types of components for the application.

I am confused about is where and when to interact with the database. Should I do it directly on my route.py vs create a new class?

One thing that you can also consider from domain driven design is also to include a repository object when your application becomes large enough. Repositories typically carry operations that might interact with a database, replacing what may be seen like an ORM that you have mentioned. Otherwise, if you're still small and nimble and don't have too many complex relationships in your database, you can keep it simple and keep the database queries in your model for simplicity for the time being. Both allows the user to be created from anywhere if needed and enables code re-use more than including them in the route. Designing from an interface perspective consider the following.
user_repository = UserRepository(db_conn)
user_service = UserService()
foo_user = user_service.register_user(...)

// using the repository
repository.save(foo_user)

// using the model
foo_user.save()

I'll end this with, it depends how large your application is growing, the larger it is, the more you want to re-evaluate your design so to better organize groups of functionality to better extend from and if possible re-use code. That said, what you may have could be good in the long run if your application doesn't really grow beyond those needs.


Parsing a complicated CSV generated on election nights I have a Python script which parses a complicated CSV generated on election nights. Each row of the CSV represents a race. As I loop through the races, I store the candidates for each race into a list called cnds. The other variable to note is called num_win, and it holds the number of people who will be elected for that particular race. Usually it's just 1, but in cases like school boards, it can be much higher.

For illustration purposes, here's some sample data we might process:

num_win = 6

cnds = [
    { 'cnd' : 'Christine Matthews', 'votes' : 200, 'winner': False },
    { 'cnd' : 'Dexter Holmes', 'votes' : 123, 'winner': False },
    { 'cnd' : 'Gerald Wheeler', 'votes' : 123, 'winner': False },
    { 'cnd' : 'Timothy Hunter', 'votes' : 100, 'winner': False },
    { 'cnd' : 'Sheila Murray', 'votes' : 94, 'winner': False },
    { 'cnd' : 'Elisa Banks', 'votes' : 88, 'winner': False },
    { 'cnd' : 'John Park', 'votes' : 88, 'winner': False },
    { 'cnd' : 'Guadalupe Bates', 'votes' : 76, 'winner': False },
    { 'cnd' : 'Lynne Austin', 'votes' : 66, 'winner': False }
]

First attempt:

My initial version was pretty straightforward. Make a copy of cnds, sort it in order of vote count, and limit to all but the num_win number of candidates. These are the winners. Then loop through cnds and mark the winners.

winners = sorted(cnds, key=lambda k: int(k['votes']), reverse=True)[0:num_win]
for cnd in cnds:
    for winner in winners:
        if cnd['cnd'] == winner['cnd']:
            cnd['winner'] = True

This works great -- except I realized later that it doesn't account for ties.

Since this script is for election night when the results are unofficial, I only want to mark as winners the candidates I am sure are winners. In the data above, the clear winners would be: Christine Matthews, Dexter Holmes, Gerald Wheeler, Timothy Hunter, and Sheila Murray. There is a tie for the sixth spot. Depending on the type of race, etc, it might be settled later by a runoff or some other mechanism. So, on election night, I simply wouldn't mark anyone else after those 5 as being a winner.

Here's the new code I've written, which accounts for tie situations:

# Make list of unique vote totals, with number of candidates who had those vote totals
# This code uses collections.Counter to make the list of uniques.
# http://stackoverflow.com/a/15816111/566307
uniques = Counter(cnd['votes'] for cnd in cnds).iteritems()

# Now convert the Counter() output into a sorted list of tuples.
uniquesCount = sorted( uniques, reverse=True )[0:num_win]

# How many candidates are there in this list?
# http://stackoverflow.com/a/14180875/566307
cndsInUniques = map(sum,zip(*uniquesCount))[1]

# There's too many candidates. Must be one or more ties
if cndsInUniques > num_win:
    adjusted_num_win = num_win

    # We need to remove items from the uniques list until we get the
    # num of candidates below or equal to the num_win threshold.
    while len(uniquesCount) > 0:
        # delete last item
        del uniquesCount[-1]
        cndsInUniques = map(sum,zip(*uniquesCount))[1]
        if cndsInUniques <= num_win:
            adjusted_num_win = cndsInUniques
            break
    winners = sorted(cnds, key=lambda k: int(k['votes']), reverse=True)[0:adjusted_num_win]

# Right number of candidates means no ties. Proceed as normal.
else:
    # Make list of candidates, sorted by vote totals
    winners = sorted(cnds, key=lambda k: int(k['votes']), reverse=True)[0:num_win]

# loop through all candidates and mark the ones who are winners
for cnd in cnds:
    for winner in winners:
        if cnd['cnd'] == winner['cnd']:
            cnd['winner'] = True

This code is working for me, but I feel like it's a lot of work to reach the adjusted_num_win number that I need. Can anyone suggest an alternative, or ways I might simplify this?

# Make one more candidate than necessary into winners list
winners = sorted(cnds, key=lambda k: int(k['votes'], reverse=True)[0:num_win + 1]

# A tie to be resolved happens when two last candidates have equal vote count.
# if so, go backwards removing everybody with the same vote count.
# Binary search would work faster, of course. If a standard library
# provides something similar to std::lower_bound from STL -  its even better.

index = num_win
while index > 0 and winners[index - 1]['votes'] == winners[num_win]['votes']:
    index -= 1

# Finally, adjust your list
winners = winners[0:index]

PS: One more thing to mention. The final nested loop is not really the best approach. You should decorate the original list with the sequential numbers (or use some other method to remember initial ordering), sort it, mark the winners which are at the beginning of the list, and sort it by sequence numbers back to the original state.


Twisted Internationalization I'm not particularly Python savy and recently wrote a LocalizationEngine for Twisted.

The strategy I implemented was to make a dictionary from base.json and then merge with any existing files set in the Accept-Language set in the URL (or cookie). For instance the Accept-Language of en-US,en;q=0.8,es;q=0.6 would merge base.json, en.json, en-US.json in that order. The folder layout looks like this:

from twisted.web import resource, server, static, error as http_error
import json
import os

## load the JSON resource from the path and return the parse obj (dict in this case)
def getJSON(path):
  file = open(path)
  data = json.load(file)
  file.close()
  return data

class LocalizationEngine(resource.Resource):
  isLeaf = True
  localePath = "static/lang/"

  def __init__(self, prefix):
    pass

  def render_GET(self, request):
    return self.getLocale(request)

  def getLocale(self, request):
    """
    Request a localization and respond with json object of appropriate locale
    """
    setLocales = request.getCookie("i18n") #preferred locale (todo implement)?
    locales = []
    if setLocales:
      locales = json.load(setLocales)
    else:
      lang_header = request.headers.get("Accept-Language", "en") # for example en-US,en;q=0.8,es;q=0.6
      locales = [locale.split(';')[0] for locale in lang_header.split(',')]

    basePath = self.localePath + (request.args.get("path", [""])[0])
    if not basePath.endswith("/"):
      basePath += "/"

    if not os.path.exists(basePath):
      raise http_error.NoResource().render(request)

    lang = getJSON(basePath + "base.json")

    # reverse so specificity overrides
    for locale in reversed(locales):
      path = basePath + locale + ".json"
      if os.path.exists(path):
        lang.update(getJSON(path))

    cache(request)
    request.write(json.dumps(lang))
    request.finish()
    return True

cache.py

from wsgiref.handlers import format_date_time as format_date
from datetime import date, timedelta
from time import mktime

'''
   Sets the cache headers for a (static resource) request
'''
def cache(request, expires=30, public=True):
    #set expires header
    expiry = (date.today() + timedelta(expires)).timetuple()
    request.responseHeaders.setRawHeaders("expires" , [format_date(mktime(expiry))])

    #set cache control
    cache_control = "max-age=" + str(int(60*60*24*expires))
    if public:
        cache_control += ", public"
    else:
        cache_control += ", private"
    request.responseHeaders.setRawHeaders("cache-control", [cache_control])

    return request

A code review of the above implementation or suggesting an existing Twisted/Python Internationalization implementation (couldn't find one) would be great answers :)

I haven't worked with Twisted,
and also cannot recommend existing internationalization implementations,
but I can code review in terms of Python in general.

You are not following PEP8,
the official coding style guide of Python.
The coding style violations are not so bad but they are noticeable.
I won't comment on those one by one,
I suggest to give the doc a good read and follow it.

It's nice that you documented this method with a comment:

## load the JSON resource from the path and return the parse obj (dict in this case)
def getJSON(path):
  file = open(path)
  data = json.load(file)
  file.close()
  return data

Why not go all the way and document with a proper docstring?
Also, closing files manually is old-fashioned.
The recommend way is simpler and better:

def getJSON(path):
    """
    load the JSON resource from the path and return the parse obj (dict in this case)

    @param path: JSON file to parse
    @return: parsed object as a dict
    """
    with open(path) as fh:
        return json.load(fh)

In this code, the initialization of locales = [] is unnecessary,
because you assign to the variable in both branches of the if, so you can remove it:

    locales = []
    if setLocales:
      locales = json.load(setLocales)
    else:
      lang_header = request.headers.get("Accept-Language", "en") # for example en-US,en;q=0.8,es;q=0.6
      locales = [locale.split(';')[0] for locale in lang_header.split(',')]


Caching 1600 integers and searching from treeSet takes a lot of time What should I use to cache the result so that it takes less time to search?  It currently takes a large number of minutes to complete 10000 test cases with range 1-99999999999999 (18 times 9 - the worst case), even though the search values have been hard-coded for testing purposes (1600, 1501).

Set<Integer> set = new TreeSet<Integer>();
//some logic to populate 1600 numbers
for (int cases = 0; cases < totalCases; cases++) {
    String[] str = br.readLine().split(" ");
    long startRange = Long.parseLong(str[0]);
    long endRange = Long.parseLong(str[1]);
    int luckyCount = 0;
    for (long num = startRange; num <= endRange; num++) {
        int[] longArray = { 1600, 1501 };
        if (set.contains(longArray[0]) && set.contains(longArray[1])) {
            luckyCount++;
        }

    }
    System.out.println(luckyCount);
}

It actually is a problem to find a lucky number - those numbers whose sum of digits and sum of square of digits are prime. I have implemented the Sieve of Eratosthenes for this.  To optimize it further, I commented my getDigitSum method, and I thought was heavy, so I replaced it with two hard-coded values, but it still takes minutes to solve one test case. Here is a reference to actual problem asked.

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.util.Set;
import java.util.TreeSet;

public class Solution {

    private static int[] getDigitSum(long num) {

        long sum = 0;
        long squareSum = 0;
        for (long tempNum = num; tempNum > 0; tempNum = tempNum / 10) {
            if (tempNum < 0) {
                sum = sum + tempNum;
                squareSum = squareSum + (tempNum * tempNum);
            } else {
                long temp = tempNum % 10;
                sum = sum + temp;
                squareSum = squareSum + (temp * temp);

            }
        }
        int[] twosums = new int[2];
        twosums[0] = Integer.parseInt(sum+"");
        twosums[1] = Integer.parseInt(squareSum+"");
        // System.out.println("sum Of digits: " + twoDoubles[0]);
        // System.out.println("squareSum Of digits: " + twoDoubles[1]);
        return twosums;
    }

    public static Set<Integer> getPrimeSet(int maxValue) {
        boolean[] primeArray = new boolean[maxValue + 1];
        for (int i = 2; i < primeArray.length; i++) {
            primeArray[i] = true;
        }
        Set<Integer> primeSet = new TreeSet<Integer>();
        for (int i = 2; i < maxValue; i++) {
            if (primeArray[i]) {
                primeSet.add(i);
                markMutiplesAsComposite(primeArray, i);
            }
        }

        return primeSet;
    }

    public static void markMutiplesAsComposite(boolean[] primeArray, int value) {
        for (int i = 2; i*value < primeArray.length; i++) {
            primeArray[i * value] = false;

        }
    }

    public static void main(String args[]) throws NumberFormatException,
            IOException {
        // getDigitSum(80001001000l);
        //System.out.println(getPrimeSet(1600));
        Set set = getPrimeSet(1600);
        BufferedReader br = new BufferedReader(new InputStreamReader(System.in));
        int totalCases = Integer.parseInt(br.readLine());
        for (int cases = 0; cases < totalCases; cases++) {
            String[] str = br.readLine().split(" ");
            long startRange = Long.parseLong(str[0]);
            long endRange = Long.parseLong(str[1]);
            int luckyCount = 0;
            for (long num = startRange; num <= endRange; num++) {
                int[] longArray = getDigitSum(num);
                if(set.contains(longArray[0]) && set.contains(longArray[1])){
                    luckyCount++;
                }
            }
            System.out.println(luckyCount);
        }
    }
}

You claim the lookup in the TreeSet takes a long time, why don't you use a HashSet which guarantees constant time instead of the log(n) time of TreeSet.

Also did you profile your code to verify that it is the lookup that is taking so much time? And that it isn't something else?


Time Limit Exceeds [HS11DIVS] - How to make it faster? Here is the question. From SPOJ [HS11DIVS]. Now , the problem is that my code works perfectly for small numbers. But when it comes to Large numbers , the time limit exceeds.
I am not getting any hint on how to make it more efficient. The code is simple enough to understand. I mean its a pretty basic question but difficult to make it efficient.

  For given integers a and b we need to find how many integers in the range [a,b] are divisible by a number x, and have the additional property that the sum of their digits lies in the ,range [l,r] for given l,r.

  Input

  In the first line you're given a and b ( 1 <= a <= b < 10^100 ). In
  the second line you're given three positive integers x ( 1 <= x <= 10
  ), l, r ( 1 <= l <= r <= 1,000 ).

  Output

  In the first and only line output the result modulo 1,000,000,007.

And this is my attempt to the question.

UPDATED :

#include <stdio.h>

int sumOfDigits(int number)
{
  int sum=0;
  while(number!=0)
  {
     sum = sum + (number%10);
     number = number/10;
  }
  return sum;
}

/* UPDATED SEGMENT */

unsigned long long int first(unsigned long long int a,unsigned long long int b,int x)
{
  unsigned long long int m = a%x;
      a = a - m + x;
      return a;
}

int main (void)
{
  unsigned long long int a, b;
  int x, l, r;
  int counter=0,sum;
  scanf("%llu %llu", &a, &b);
  scanf("%d %d %d", &x, &l, &r);
  a = first(a,b,x);   // gives the first number divisible by x.
  while(a<=b)
  {
    sum = sumOfDigits(a);
    if(l<=sum && r>=sum) // check if sum of digits lies b/w l&r.
    {
        counter+=1;
    }
    a+=x;     // increment a by multiples of x.
  }
  printf("%d", counter);
  return 0;
}

Where am i being inefficient? I even calculated the first number divisible by x , and then assign it to a , and then increase a by multiples of x ( a+=x; ).

Here is a sample output :

  Input:
      1 100 5 10 50

  Output:   5

First you need to correct first()

unsigned long long int first(unsigned long long int a,unsigned long long int b,int x)
{
  // Removed b
  return a;
}       ^^^ otherwise the main loop will run through everything in the main loop (from 0).

Though first() looks efficient because you are doing it only once. The questioner may carefully craft a,b,x to take a long time. So you can optimize its evaluation:

unsigned long long int m = a%x;
a = a - m + x;  // a now first valid value.

Your summing of the digits does a lot of extra work.

99   = 18
199  = 19 1 + sum_of_digits(99)

You can use this to cache results and re-use them in later calculations.

Pre compute the following array and compile it into your application.

sum_cache[10000]   cache  = {
               /* Generated code goes here */

                            };

int sumOfDigits(int number)
{
     int sum=sum_cache[number % 10000];
     for(number = number/10000;number != 0; number/= 10)
     {
         sum    += (number%10);
     }
     return sum;
}

Edit (based on comments below)

int sumOfDigits(int number)
{
  int sum=0;
  while(number!=0)
  {
     sum = sum + (number%10);
     number = number/10;
  }
  return sum;
}
int main()
{
    std::ofstream data("data.d");

    data << 0;
    for(int loop=1; loop < 10000; ++loop)
    {
        data << ", " << sumOfDigits(loop);
    }
}

Then change the code above too:

sum_cache[]   cache  = {
               /* Generated code goes here */
 #include "data.d"
                       };

Edit 2:

Improved: (I was hopping you would see the simple optimization)
Does this help?

int sumOfDigits(int number)
{
     int sum=0;
     for(;number != 0; number/= 10000)
     {
         sum    += sum_cache[number % 10000];
     }
     return sum;
}

Max Loop:

Original Version (your Code)       100      = 100
First Cache Version                100-4    = 96
Second Cache Version               100/4    = 25


URL percent encoding function I wrote a custom URL encoding function and I'd like to run it past a few other experienced C developers. I have tested it on a few strings, and it has worked on all of them.

This is to be run on iOS devices, so memory and processor use are potentially a small concern.

Do you see any potential problems - UB, wrong return, excessive memory or processor usage, difficulty reading?

Any suggestions for improvement?

NSString *urlEncode(NSString *orig)
{
    static const BOOL safe[0x100] = {
        ['a'...'z'] = YES,
        ['A'...'Z'] = YES,
        ['0'...'9'] = YES,
        ['-'] = YES,
        ['_'] = YES,
        ['.'] = YES,
        ['~'] = YES
    };
    const char *digits = "0123456789ABCDEF";
    const char *cstr = orig.UTF8String;
    int clen = orig.length;
    int nlen = 3*clen;
    char newstr[nlen + 1];

    const char *o = cstr   + clen;
    char *p = newstr + nlen;
    *p = 0;
    while(o > cstr) {
        unsigned char c = *--o;
        if(safe[c]) {
            *--p = c;
        } else {
            *--p = digits[c&0xf];
            *--p = digits[c>>4];
            *--p = '%';
        }
    }
    NSString *str = [NSString stringWithUTF8String:p];
    return str;
}

Yes, newStr could get long, but given that a URL > 2kB is unreliable, I think it's unlikely to be a problem in reality.

First of all, URL encoding is more nuanced than you might think.  Be cautious when applying encoding.

As for your code, the only blatant bug I see is that it can segfault when encoding a non-ASCII string.  The -length method returns the number of Unicode characters in the string.  Since you are working one char at a time, you need the number of bytes instead.

NSUInteger clen = [orig lengthOfBytesUsingEncoding:NSUTF8StringEncoding];
NSUInteger nlen = 3 * clen;

It would be better to change the method signature to:

NSString *urlEncode(const NSString *orig)

I think that using a for loop would improve readability.  Currently, you declare/initialize *o, test it, and decrement it on three separate lines.  A for loop would provide an instantly recognizable structure to make it obvious how those three lines of code are related:

for (const char *o = cstr + clen - 1; o >= cstr; o--) {
    unsigned char c = *o;
    if (safe[c]) {
        *--p = c;
    } else {
        *--p = digits[c & 0x0f];
        *--p = digits[c >> 4];
        *--p = '%';
    }
}

The null char would be better written as '\0'.

Personally, I would choose to iterate forward rather than backward through the string, as it simplifies the loop by a few instructions, and is easier to understand.  It also gives you the option of calling -maximumLengthOfBytesUsingEncoding (which runs in O(1) time) instead of -lengthOfBytesUsingEncoding.

unsigned char c;
char *p = newstr;
for (const char *o = cstr; (c = *o); o++) {
    ...
}


Converting a list to a CSV string I want to be able to convert a list of objects into a string csv format.  I've written this extension method below but have a feeling I'm missing something as this seems like potentially a common thing to want to do.

private static readonly char[] csvChars = new[] { ',', '"', ' ', '\n', '\r' };

public static string ToCsv<T>(this IEnumerable<T> source, Func<T, object> getItem)
{
    if ((source == null) || (getItem == null))
    {
        return string.Empty;
    }

    var builder = new StringBuilder();
    var items = from item in source.Select(getItem)
                where item != null
                select item.ToString();

    foreach (var str in items)
    {
        if (str.IndexOfAny(csvChars) > 0)
        {
            builder.Append("\"").Append(str).Append("\"").Append(", ");
        }
        else
        {
            builder.Append(str).Append(", ");
        }
    }

    var csv = builder.ToString();

    return csv.Length > 0 ? csv.TrimEnd(", ".ToCharArray()) : csv;
}

Is there anything I can do to improve this or refactor to a more elegant or working solution.  Or even an existing method out there already that I may have missed.

UPDATE:  Updated to take into account quotations as per Jesse comments below.

If your items contain a comma, carriage return or other special CSV character, you must delimit it with quotation marks.

namespace CsvStuff
{
    using System;
    using System.Collections.Generic;
    using System.Linq;
    using System.Text;

    internal static class CsvConstants
    {
        public static char[] TrimEnd { get; } = { ' ', ',' };

        public static char[] CsvChars { get; } = { ',', '"', ' ', '\n', '\r' };
    }

    public abstract class CsvBase<T>
    {
        private readonly IEnumerable<T> values;

        private readonly Func<T, object> getItem;

        protected CsvBase(IEnumerable<T> values, Func<T, object> getItem)
        {
            this.values = values;
            this.getItem = getItem;
        }

        public override string ToString()
        {
            var builder = new StringBuilder();

            foreach (var item in
                from element in this.values.Select(this.getItem)
                where element != null
                select element.ToString())
            {
                this.Build(builder, item).Append(", ");
            }

            return builder.ToString().TrimEnd(CsvConstants.TrimEnd);
        }

        protected abstract StringBuilder Build(StringBuilder builder, string item);
    }

    public class CsvBare<T> : CsvBase<T>
    {
        public CsvBare(IEnumerable<T> values, Func<T, object> getItem) : base(values, getItem)
        {
        }

        protected override StringBuilder Build(StringBuilder builder, string item)
        {
            return builder.Append(item);
        }
    }

    public sealed class CsvTrimBare<T> : CsvBare<T>
    {
        public CsvTrimBare(IEnumerable<T> values, Func<T, object> getItem) : base(values, getItem)
        {
        }

        protected override StringBuilder Build(StringBuilder builder, string item)
        {
            return base.Build(builder, item.Trim());
        }
    }

    public class CsvRfc4180<T> : CsvBase<T>
    {
        public CsvRfc4180(IEnumerable<T> values, Func<T, object> getItem) : base(values, getItem)
        {
        }

        protected override StringBuilder Build(StringBuilder builder, string item)
        {
            item = item.Replace("\"", "\"\"");
            return item.IndexOfAny(CsvConstants.CsvChars) >= 0
                ? builder.Append("\"").Append(item).Append("\"")
                : builder.Append(item);
        }
    }

    public sealed class CsvTrimRfc4180<T> : CsvRfc4180<T>
    {
        public CsvTrimRfc4180(IEnumerable<T> values, Func<T, object> getItem) : base(values, getItem)
        {
        }

        protected override StringBuilder Build(StringBuilder builder, string item)
        {
            return base.Build(builder, item.Trim());
        }
    }

    public class CsvAlwaysQuote<T> : CsvBare<T>
    {
        public CsvAlwaysQuote(IEnumerable<T> values, Func<T, object> getItem) : base(values, getItem)
        {
        }

        protected override StringBuilder Build(StringBuilder builder, string item)
        {
            return builder.Append("\"").Append(item.Replace("\"", "\"\"")).Append("\"");
        }
    }

    public sealed class CsvTrimAlwaysQuote<T> : CsvAlwaysQuote<T>
    {
        public CsvTrimAlwaysQuote(IEnumerable<T> values, Func<T, object> getItem) : base(values, getItem)
        {
        }

        protected override StringBuilder Build(StringBuilder builder, string item)
        {
            return base.Build(builder, item.Trim());
        }
    }

    public static class CsvExtensions
    {
        public static string ToCsv<T>(this IEnumerable<T> source, Func<T, object> getItem, Type csvProcessorType)
        {
            if ((source == null)
                || (getItem == null)
                || (csvProcessorType == null)
                || !csvProcessorType.IsSubclassOf(typeof(CsvBase<T>)))
            {
                return string.Empty;
            }

            return csvProcessorType
                .GetConstructor(new[] { source.GetType(), getItem.GetType() })
                ?.Invoke(new object[] { source, getItem })
                .ToString();
        }

        private static void Main()
        {
            var words = new[] { ",this", "   is   ", "a", "test", "Super, \"luxurious\" truck" };

            Console.WriteLine(words.ToCsv(word => word, typeof(CsvAlwaysQuote<string>)));
            Console.WriteLine(words.ToCsv(word => word, typeof(CsvRfc4180<string>)));
            Console.WriteLine(words.ToCsv(word => word, typeof(CsvBare<string>)));
            Console.WriteLine(words.ToCsv(word => word, typeof(CsvTrimAlwaysQuote<string>)));
            Console.WriteLine(words.ToCsv(word => word, typeof(CsvTrimRfc4180<string>)));
            Console.WriteLine(words.ToCsv(word => word, typeof(CsvTrimBare<string>)));
            Console.ReadLine();
        }
    }
}